[cups.bugs] Fixing local job user name verification

[Martin Pitt]

Hi Cups developers!

For a fair while already (also in 1.2 snapshot 4892) cupsd does not
check the request user against the job owner, i. e. any user can e. g.
cancel jobs from other users. This is not desirable, and the code even
tries to check that a user can only remove his own job. (At least
that's the default setting in cupsd.conf).

I tried to track this down a little:

 - cancel_job() calls validate_user().
 - in validate_user(), con->username is empty since it's a local
   connection. so it determines the request user name with
   ippFindAttribute(con->request, "requesting-user-name", but doesn't
   copy it to con->username.
 - validate_user() calls cupsdCheckPolicy(), which calls
   cupsdIsAuthorized().
 - cupsdIsAuthorized() executes this bit of code which is totally
   unclear to me:

|  if (best->type == AUTH_NONE && best->limit == AUTH_LIMIT_IPP)
|  {
|   /*
|    * Check for unauthenticated username...
|    */
|
|    ipp_attribute_t     *attr;          /* requesting-user-name attribute */
|
|
|    attr = ippFindAttribute(con->request, "requesting-user-name", IPP_TAG_NAME);
|    if (attr)
|    {
|      cupsdLogMessage(CUPSD_LOG_DEBUG2,
|                      "cupsdIsAuthorized: requesting-user-name=\"%s\"",
|                      attr->values[0].string.text);
|      return (HTTP_OK);
|    }
|  }

  This is the reason why every user can cancel other people's jobs:
  the connection user is determined, but never checked against the
  job's owner.

 - If this part of the code is removed, then this code makes *every*
   local request (even valid ones) fail, since con->username is empty:

|  if (!con->username[0])
|  {
|    if (best->satisfy == AUTH_SATISFY_ALL || auth == AUTH_DENY)
|      return (HTTP_UNAUTHORIZED);       /* Non-anonymous needs user/pass */
|    else
|      return (HTTP_OK);                 /* unless overridden with Satisfy */
|  }

So I created a crappy band-aid patch which fixes this for now: It
disables the first quoted code part, and validate_user() copies the
determined connection user into con->username, so that
cupsdIsAuthorized() has a chance to compare it against the job owner.
It's a minimally invasive patch, but not a really clean solution.

What do you think about this issue?

Thanks,

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 --------------
#! /bin/sh /usr/share/dpatch/dpatch-run
## 50_local_username_check.dpatch by  <martin.pitt at ubuntu.com>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.

@DPATCH@
diff -urNad cupsys-1.1.99.b1.r4892~/scheduler/auth.c cupsys-1.1.99.b1.r4892/scheduler/auth.c
--- cupsys-1.1.99.b1.r4892~/scheduler/auth.c	2006-01-13 13:16:40.000000000 +0100
+++ cupsys-1.1.99.b1.r4892/scheduler/auth.c	2006-01-13 15:27:33.000000000 +0100
@@ -1662,6 +1662,7 @@
       (best->type == AUTH_NONE && best->num_names == 0))
     return (HTTP_OK);
 
+#if 0
   if (best->type == AUTH_NONE && best->limit == AUTH_LIMIT_IPP)
   {
    /*
@@ -1680,6 +1681,7 @@
       return (HTTP_OK);
     }
   }
+#endif
 
   cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdIsAuthorized: username=\"%s\"",
 	          con->username);
diff -urNad cupsys-1.1.99.b1.r4892~/scheduler/ipp.c cupsys-1.1.99.b1.r4892/scheduler/ipp.c
--- cupsys-1.1.99.b1.r4892~/scheduler/ipp.c	2006-01-07 05:50:14.000000000 +0100
+++ cupsys-1.1.99.b1.r4892/scheduler/ipp.c	2006-01-13 15:26:41.000000000 +0100
@@ -8921,6 +8921,14 @@
   else
     strlcpy(username, "anonymous", userlen);
 
+  if (!con->username[0]) {
+    /*
+     * Copy the username to con so that cupsdCheckPolicy() can compare it against
+     * the job owner.
+     */
+     strlcpy(con->username, username, sizeof(con->username));
+  }
+
  /*
   * Check the username against the owner...
   */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <https://lists.cups.org/pipermail/cups/attachments/20060113/bfcff0c0/attachment.bin>