[Pkg-cups-devel] Ubuntu patches for 1.2 cups

Martin Pitt mpitt at debian.org
Tue Nov 29 09:25:52 UTC 2005


Hi CUPS developers!

I recently upgraded Ubuntu to Cups 1.2beta, based on the current
experimental package. There are some Ubuntu specific changes (we run
cups as normal user by default for security reasons), but most of the
fixes apply to Debian, too, so I would like to discuss them here
before committing to the SVN.

There are quite a bunch, so let's walk through the debdiff (Ubuntu
specific changes are already removed). Where available, I also copy
part of the changelog.

I would appreciate comments, especially if somebody thinks that a
particular change should not be made.

| diff -u cupsys-1.1.99.b1.r4841/debian/local/enable_browsing cupsys-1.1.99.b1.r4841/debian/local/enable_browsing
| --- cupsys-1.1.99.b1.r4841/debian/local/enable_browsing
| +++ cupsys-1.1.99.b1.r4841/debian/local/enable_browsing
| @@ -13,7 +13,7 @@
|  # 1: enabled browsing
|  # Return 0 on success, or 1 on failure (prints error to stderr)
|  
| -CONF=/etc/cups/cupsd-browsing.conf
| +CONF=/etc/cups/cups.d/browse.conf
|  STATUS_SCRIPT=/usr/share/cups/browsing_status
|  
|  [ -x $STATUS_SCRIPT ] || {
| @@ -54,7 +54,3 @@
|  if [ -x "/etc/init.d/cupsys" ]; then
| -    if [ -x /usr/sbin/invoke-rc.d ]; then
| -	invoke-rc.d cupsys restart || exit 0
| -    else
| -	/etc/init.d/cupsys restart || exit 0
| -    fi
| +    /etc/init.d/cupsys restart || exit 0

Makes more sense for me to not use invoke-rc.d.

|  fi
| diff -u cupsys-1.1.99.b1.r4841/debian/local/browsing_status cupsys-1.1.99.b1.r4841/debian/local/browsing_status
| --- cupsys-1.1.99.b1.r4841/debian/local/browsing_status
| +++ cupsys-1.1.99.b1.r4841/debian/local/browsing_status
| @@ -11,7 +11,7 @@
|  #    be used
|  
|  MAINCONF=/etc/cups/cupsd.conf
| -BROWSECONF=/etc/cups/cupsd-browsing.conf
| +BROWSECONF=/etc/cups/cups.d/browse.conf
|  
|  [ -f $MAINCONF -a -f $BROWSECONF ] || exit 2
|  

Both scripts: Adaptions to the new configuration file format; trivial,
seems ok to me to commit to unbreak them.

| diff -u cupsys-1.1.99.b1.r4841/debian/control cupsys-1.1.99.b1.r4841/debian/control
| --- cupsys-1.1.99.b1.r4841/debian/control
| +++ cupsys-1.1.99.b1.r4841/debian/control
| @@ -10,7 +10,7 @@
|  Priority: optional
|  Section: net
|  Architecture: any
| -Depends: ${shlibs:Depends}, adduser (>= 3.12), debconf (>= 1.2.9) | debconf-2.0, patch, xpdf-utils, perl-modules, procps, gs-esp
| +Depends: ${shlibs:Depends}, adduser (>= 3.12), debconf (>= 1.2.9) | debconf-2.0, patch, poppler-utils, perl-modules, procps, gs-esp, lsb-base (>= 3)
|  Replaces: cupsys-pstoraster
|  Conflicts: cupsys-pstoraster (<< 2)
|  Recommends: cupsys-client, smbclient (>= 3.0.9), foomatic-filters
| diff -u cupsys-1.1.99.b1.r4841/debian/cupsys.templates cupsys-1.1.99.b1.r4841/debian/cupsys.templates
| --- cupsys-1.1.99.b1.r4841/debian/cupsys.init.d
| +++ cupsys-1.1.99.b1.r4841/debian/cupsys.init.d
| @@ -18,6 +18,8 @@
|  
|  set -e
|  
| +. /lib/lsb/init-functions
| +
|  # Get the timezone set.
|  if [ -e /etc/timezone ]; then
|      TZ=`cat /etc/timezone`
| @@ -26,28 +28,23 @@
|  
|  case "$1" in
|    start)
| -	echo -n "Starting $DESC: $NAME"
| +	log_begin_msg "Starting $DESC: $NAME"
|  	chown root:lpadmin /usr/share/cups/model 2>/dev/null || true
|  	chmod 3775 /usr/share/cups/model 2>/dev/null || true
|  	start-stop-daemon --start --quiet --pidfile "$PIDFILE" --exec $DAEMON
| -	echo "."
| +	log_end_msg $?
|  	;;
|    stop)
| -	echo -n "Stopping $DESC: $NAME"
| +	log_begin_msg "Stopping $DESC: $NAME"
|  	start-stop-daemon --stop --quiet --user root --retry TERM/10 --oknodo --pidfile $PIDFILE --name $NAME
| -	echo "."
| -	;;
| -  reload)
| -	echo -n "Reloading $DESC: $NAME"
| -	start-stop-daemon --stop --quiet --pidfile $PIDFILE --name $NAME --signal 1
| -	echo "."
| +	log_end_msg $?
|  	;;
|    restart|force-reload)
| -	echo -n "Restarting $DESC: $NAME"
| +	log_begin_msg "Restarting $DESC: $NAME"
|  	if start-stop-daemon --stop --quiet --user root --retry TERM/10 --oknodo --pidfile $PIDFILE --name $NAME; then
|  		start-stop-daemon --start --quiet --pidfile "$PIDFILE" --exec $DAEMON
|  	fi
| -	echo "."
| +	log_end_msg $?
|  	;;
|    status)
|  	echo -n "Status of $DESC: "
| unchanged:

LSBification of init script. Debian slowly adapts these as well, since
they allow for a great deal of flexibility (customized logging,
graphical boot, etc). I'd like to commit that, fine for everybody?

| --- cupsys-1.1.99.b1.r4841/debian/cupsys.templates
| +++ cupsys-1.1.99.b1.r4841/debian/cupsys.templates
| @@ -53,7 +53,7 @@
|  
|  Template: cupsys/browse
|  Type: boolean
| -Default: yes
| +Default: true
|  _Description: Do you want to broadcast and/or listen for CUPS printer information on the network?
|   CUPS daemon can broadcast printer information for clients on the network,
|   and detect printers on the network automatically.

  * debian/cupsys.templates: Fix default value for cupsys/browse: 'yes' is an
    invalid bool option, change to true.

Pretty important to commit, since this breaks when the question is not displayed.

| diff -u cupsys-1.1.99.b1.r4841/debian/cupsys.postinst cupsys-1.1.99.b1.r4841/debian/cupsys.postinst
| --- cupsys-1.1.99.b1.r4841/debian/cupsys.postinst
| +++ cupsys-1.1.99.b1.r4841/debian/cupsys.postinst
| @@ -43,6 +43,7 @@
|              if [ -n "`pidof /usr/sbin/cupsd`" ]; then
|   	      echo "Killing cupsys..."
|  	      kill -9 `pidof /usr/sbin/cupsd` || true
| +              sleep 1
|  	    fi
|  	  fi
|  	  if [ -n "`ps aux | grep /usr/sbin/cupsd | grep -v grep`" ]; then

  * debian/cupsys.postinst: Wait a second between kill -9'ing cupsys and
    checking if the process still exists to avoid false positives and upgrade
    failures.

Without that, restart often failed for me.

| @@ -191,15 +210,6 @@
|  	else
|  	  echo "Browsing off" > /etc/cups/cups.d/browse.conf
|  	fi
| -
| -	grep -v "^\(Browsing\|Port\|Listen\)[[:space:]]" /etc/cups/cupsd.conf > /etc/cups/cupsd.conf.$$
| -	if [ -z "$(grep -e "^Include[[:space:]]\+/etc/cups/cups.d/ports.conf" /etc/cups/cupsd.conf.$$)" ]; then
| -	  echo "Include /etc/cups/cups.d/ports.conf" >> /etc/cups/cupsd.conf.$$
| -	fi
| -	if [ -z "$(grep -e "^Include[[:space:]]\+/etc/cups/cups.d/browse.conf" /etc/cups/cupsd.conf.$$)" ]; then
| -	  echo "Include /etc/cups/cups.d/browse.conf" >> /etc/cups/cupsd.conf.$$
| -	fi
| -	mv /etc/cups/cupsd.conf.$$ /etc/cups/cupsd.conf
|      ;;
|  
|      abort-upgrade|abort-remove|abort-deconfigure)
| --- cupsys-1.1.99.b1.r4841.orig/debian/patches/ubuntu-include-conf.d.dpatch
| +++ cupsys-1.1.99.b1.r4841/debian/patches/ubuntu-include-conf.d.dpatch
| @@ -0,0 +1,21 @@
| +#! /bin/sh /usr/share/dpatch/dpatch-run
| +## ubuntu-include-conf.d.dpatch by  <martin.pitt at ubuntu.com>
| +##
| +## All lines beginning with `## DP:' are a description of the patch.
| +## DP: No description.
| +
| + at DPATCH@
| +diff -urNad cupsys-1.1.99.b1.r4841~/conf/cupsd.conf.in cupsys-1.1.99.b1.r4841/conf/cupsd.conf.in
| +--- cupsys-1.1.99.b1.r4841~/conf/cupsd.conf.in	2005-11-22 16:05:48.000000000 +0100
| ++++ cupsys-1.1.99.b1.r4841/conf/cupsd.conf.in	2005-11-22 16:06:31.000000000 +0100
| +@@ -71,6 +71,10 @@
| +   </Limit>
| + </Policy>
| + 
| ++# Include files in /etc/cups/conf.d
| ++Include /etc/cups/cups.d/ports.conf
| ++Include /etc/cups/cups.d/browse.conf
| ++
| + #
| + # End of "$Id: cupsd.conf.in 4817 2005-11-04 16:21:01Z mike $".
| + #

  * Sanitize support for /etc/cups/conf.d:
    - Add debian/patches/ubuntu-include-conf.d.dpatch: Add include commands to
      default cupsd.conf file.
    - debian/cupsys.postinst: Remove fiddling with cupsd.conf.
    - This will ensure that cupsd.conf will remain an unchanged conffile.

This is *really* *utterly* important. Automatically touching conffiles
is eeeevil. Besides, it is much more robust that way and allows me to
do a reasonably sane transition from Ubuntu's different configuration file
layout.

| --- cupsys-1.1.99.b1.r4841/debian/patched/02_addpie.dpatch
| +++ cupsys-1.1.99.b1.r4841.orig/debian/patched/02_addpie.dpatch
| @@ -1,2 +0,0 @@
| -patching file cups/Makefile
| -Hunk #1 succeeded at 163 (offset 2 lines).

That just seems to be garbage. debian/patched should not be in a clean
source tree.

| --- cupsys-1.1.99.b1.r4841.orig/debian/patches/ubuntu-localports.dpatch
| +++ cupsys-1.1.99.b1.r4841/debian/patches/ubuntu-localports.dpatch
| @@ -0,0 +1,21 @@
| +#! /bin/sh /usr/share/dpatch/dpatch-run
| +## ubuntu-localports.dpatch by  <martin.pitt at ubuntu.com>
| +##
| +## All lines beginning with `## DP:' are a description of the patch.
| +## DP: No description.
| +
| + at DPATCH@
| +diff -urNad cupsys-1.1.99.b1.r4841~/conf/cupsd.conf.in cupsys-1.1.99.b1.r4841/conf/cupsd.conf.in
| +--- cupsys-1.1.99.b1.r4841~/conf/cupsd.conf.in	2005-11-22 15:01:06.000000000 +0100
| ++++ cupsys-1.1.99.b1.r4841/conf/cupsd.conf.in	2005-11-22 15:01:41.000000000 +0100
| +@@ -14,8 +14,8 @@
| + SystemGroup @CUPS_GROUP@
| + 
| + # Only listen for connections from the local machine.
| +-#Listen localhost:@DEFAULT_IPP_PORT@
| +- at CUPS_LISTEN_DOMAINSOCKET@
| ++Listen localhost:@DEFAULT_IPP_PORT@
| ++#@CUPS_LISTEN_DOMAINSOCKET@
| + 
| + # Show shared printers on the local network.
| + #Browsing On

  * debian/patches/ubuntu-localports.dpatch: Listen to localhost:631 instead
    of only to Unix socket.

... to actually enable the web interface and unbreak client libraries.
Not sure whether it should listen to both by default?

| --- cupsys-1.1.99.b1.r4841.orig/debian/patches/ubuntu-sanitize-conffile-handling.dpatch
| +++ cupsys-1.1.99.b1.r4841/debian/patches/ubuntu-sanitize-conffile-handling.dpatch
| @@ -0,0 +1,58 @@
| +#! /bin/sh /usr/share/dpatch/dpatch-run
| +## ubuntu-disable-conffile-chmod.dpatch by  <martin.pitt at ubuntu.com>
| +##
| +## All lines beginning with `## DP:' are a description of the patch.
| +## DP: No description.
| +
| + at DPATCH@
| +diff -urNad cupsys-1.1.99.b1.r4841~/scheduler/conf.c cupsys-1.1.99.b1.r4841/scheduler/conf.c
| +--- cupsys-1.1.99.b1.r4841~/scheduler/conf.c	2005-11-23 11:44:07.000000000 +0100
| ++++ cupsys-1.1.99.b1.r4841/scheduler/conf.c	2005-11-23 11:45:40.000000000 +0100
| +@@ -371,7 +371,7 @@
| +   * Find the default group (nobody)...
| +   */
| + 
| +-  group = getgrnam("nobody");
| ++  group = getgrnam("root");
| +   endgrent();
| + 
| +   if (group != NULL)
| +@@ -383,7 +383,7 @@
| +     * complement number...)
| +     */
| + 
| +-    Group = 65534;
| ++    Group = 0;
| +   }
| + 
| +  /*
| +@@ -517,7 +517,7 @@
| + 
| +       cupsdLogMessage(CUPSD_LOG_NOTICE,
| +                       "Group and SystemGroup cannot use the same groups!");
| +-      cupsdLogMessage(CUPSD_LOG_INFO, "Resetting Group to \"nobody\"...");
| ++      cupsdLogMessage(CUPSD_LOG_INFO, "Resetting Group to \"root\"...");
| + 
| +       group = getgrnam("nobody");
| +       endgrent();
| +@@ -531,7 +531,7 @@
| + 	* complement number...)
| + 	*/
| + 
| +-	Group = 65534;
| ++	Group = 0;
| +       }
| +     }
| +   }

So far: do not create config files with group nogroup/user nobody,
this is evil. root is much better for that and more compliant with the
Debian policy.

| +@@ -645,9 +645,11 @@
| +   chown(temp, RunUser, Group);
| +   chmod(temp, 0700);
| + 
| ++  /*
| +   snprintf(temp, sizeof(temp), "%s/cupsd.conf", ServerRoot);
| +   chown(temp, RunUser, Group);
| +   chmod(temp, ConfigFilePerm);
| ++  */
| + 
| +   snprintf(temp, sizeof(temp), "%s/classes.conf", ServerRoot);
| +   chown(temp, RunUser, Group);

We never ever want to touch conffile permissions, so I disabled that.
OK for Debian, too?

| --- cupsys-1.1.99.b1.r4841.orig/debian/patches/ubuntu-runasuser.dpatch
| +++ cupsys-1.1.99.b1.r4841/debian/patches/ubuntu-runasuser.dpatch
| @@ -0,0 +1,60 @@
| +#! /bin/sh /usr/share/dpatch/dpatch-run
| +## ubuntu-runasuser.dpatch by Martin Pitt <martin.pitt at ubuntu.com>
| +##
| +## All lines beginning with `## DP:' are a description of the patch.
| +## DP: Enable RunAsUser and document that option.
| +
| + at DPATCH@
| +--- cupsys-1.1.99.b1.r4841~/scheduler/conf.c	2005-11-23 18:38:38.000000000 +0100
| ++++ cupsys-1.1.99.b1.r4841/scheduler/conf.c	2005-11-23 18:39:14.000000000 +0100
| +@@ -371,7 +371,7 @@
| +   * Find the default group (nobody)...
| +   */
| + 
| +-  group = getgrnam("root");
| ++  group = getgrnam(CUPS_DEFAULT_GROUP);
| +   endgrent();
| + 
| +   if (group != NULL)
| +diff -urNad cupsys-1.1.99.b1.r4841~/scheduler/main.c cupsys-1.1.99.b1.r4841/scheduler/main.c
| +--- cupsys-1.1.99.b1.r4841~/scheduler/main.c	2005-11-23 18:38:38.000000000 +0100
| ++++ cupsys-1.1.99.b1.r4841/scheduler/main.c	2005-11-23 18:38:43.000000000 +0100
| +@@ -52,6 +52,7 @@
| + #include <sys/resource.h>
| + #include <syslog.h>
| + #include <grp.h>
| ++#include <pwd.h>
| + 
| + #if defined(HAVE_MALLOC_H) && defined(HAVE_MALLINFO)
| + #  include <malloc.h>
| +@@ -440,7 +441,7 @@
| +   if (RunAsUser)
| +   {
| +     setgid(Group);
| +-    setgroups(1, &Group);
| ++    initgroups(getpwuid(User)->pw_name, Group);
| +     setuid(User);
| +   }
| + 
| +@@ -818,8 +819,7 @@
| +     * Update the root certificate once every 5 minutes...
| +     */
| + 
| +-    if ((current_time - RootCertTime) >= RootCertDuration && RootCertDuration &&
| +-        !RunUser)
| ++    if ((current_time - RootCertTime) >= RootCertDuration && RootCertDuration)
| +     {
| +      /*
| +       * Update the root certificate...

  * debian/patches/ubuntu-runasuser.dpatch:
    - scheduler/main.c: Generate a certificate even when running as user. What
      a braindead upstream change.
    - scheduler/conf.c: Use CUPS_DEFAULT_GROUP instead of "root" for Group,
      just as in 1.1.x.
    - Merge debian/patches/ubuntu-auxgroups.dpatch into this patch.


These are various upstream fixes when enabling the RunAsUser feature
(as Ubuntu does by default). I submitted them to upstream, too:
http://www.cups.org/str.php?L1349+P0+S-2+C0+I0+E0+Q

For now I'd like to see them in Debian, so that Debian users can get
the benefit of improved security, too.

Thanks in advance for any feedback!

Martin
-- 
Martin Pitt        http://www.piware.de
Ubuntu Developer   http://www.ubuntu.com
Debian Developer   http://www.debian.org

In a world without walls and fences, who needs Windows and Gates?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.alioth.debian.org/pipermail/pkg-cups-devel/attachments/20051129/0870496d/attachment.pgp


More information about the Pkg-cups-devel mailing list