Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log/conf files not accessible via web interface when not world-readable #4461

Closed
michaelrsweet opened this issue Aug 3, 2014 · 15 comments

Comments

@michaelrsweet
Copy link
Collaborator

commented Aug 3, 2014

Version: 1.7.5
CUPS.org User: twaugh.redhat

Attempting GET for /admin/log/error_log or /admin/conf/cupsd.conf fails when either is not world-readable, due to the fix for STR #4455.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 11, 2014

CUPS.org User: twaugh.redhat

Note that this is the case for authenticated requests.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 11, 2014

CUPS.org User: twaugh.redhat

Here's a patch that works for me.

Now 'cupsctl -U root' works, and 'View Error Log' in the web interface works when logged in.

diff -up cups-1.7.4/scheduler/client.c.str4461 cups-1.7.4/scheduler/client.c
--- cups-1.7.4/scheduler/client.c.str4461 2014-08-11 16:30:04.695889827 +0100
+++ cups-1.7.4/scheduler/client.c 2014-08-11 16:30:04.697889838 +0100
@@ -3360,8 +3360,18 @@ get_file(cupsd_client_t con, / I - C

if (!status && !(filestats->st_mode & S_IROTH))
{

  • cupsdLogMessage(CUPSD_LOG_INFO, "[Client %d] Files/directories such as "%s" must be world-readable.", con->http.fd, filename);
  • return (NULL);
  • /*
  • * The exception is for cupsd.conf and log files for
  • * authenticated access.
  • */
  • if ((strncmp(con->uri, "/admin/conf/cupsd.conf", 22) &&
  • strncmp(con->uri, "/admin/log/", 11)) ||
  • cupsdIsAuthorized(con, NULL) != HTTP_OK)
  • {
  •  cupsdLogMessage(CUPSD_LOG_INFO, "[Client %d] Files/directories such as \"%s\" must be world-readable.", con->http.fd, filename);
    
  •  return (NULL);
    
  • }
    }

/*

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 13, 2014

CUPS.org User: odyx

Indeed, that's been reported in Debian as https://bugs.debian.org/757964 . Mike: can you confirm inclusion of this patch?

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 13, 2014

CUPS.org User: mike

Dider,

No, at the moment I am hip deep in PWG stuff (face-to-face meeting with OpenPrinting so you should expect that I will not be doing anything with this bug until next week at the earliest.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 13, 2014

CUPS.org User: mike

P3 because this doesn't happen with the default settings we ship with.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 13, 2014

CUPS.org User: odyx

Sure, no problem. I'll ship it as-is for Debian then.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 25, 2014

CUPS.org User: mike

Fixed in Subversion repository.

The changes whitelist any cupsd-generated file, limit /admin/conf access to cupsd.conf, and change the copy_model function to treat PPD files as config files (so they now use the ConfigFilePerm setting).

Please let me know if you see any problems with the change - the patch applies cleanly to 1.7.x but we currently do not plan on releasing another 1.7.x with the change...

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 27, 2014

CUPS.org User: twaugh.redhat

Is there a reason rss files aren't permitted? It means RSS feeds can't be accessed.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 27, 2014

CUPS.org User: twaugh.redhat

How about something like this for checking /rss/ resources?

  • cupsd_subscription_t *sub;
  • char rssscheme[32];
  • char rssusername[256];
  • char rsshost[1024];
  • char rssresource[1024];
  • int rssport;
  • char *rssquery;
    [...]
    snprintf(filename, len, "%s/rss/%s", CacheDir, con->uri + 5);
  • /*
  • * Is this a file we created?
  • */
  • for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
  • sub;
  • sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
  • {
  •  if (httpSeparateURI(HTTP_URI_CODING_ALL, sub->recipient,
    
  •         rssscheme, sizeof(rssscheme),
    
  •         rssusername, sizeof(rssusername),
    
  •         rsshost, sizeof(rsshost),
    
  •         &rssport,
    
  •         rssresource, sizeof(rssresource)) < HTTP_URI_OK)
    
  • continue;
  •  rssquery = strchr (rssresource, '?');
    
  •  if (rssquery)
    
  • *rssquery = '\0';
  •  if (!strcmp (rssresource, con->uri + 4))
    
  •   /*
    
  • * Yes, it is. No need for permissions check.
  • */
  • perm_check = 0;
  • }

Also, should the /icons/ check be stricter? It looks like you might get free access to CacheDir that way.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 28, 2014

CUPS.org User: mike

Tim, the correct fix for RSS would be to make them world-readable.

Second patch for that is attached.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 29, 2014

CUPS.org User: twaugh.redhat

Oh. Doesn't that potential expose job-private information? I'm thinking of sites that have access control on /rss/.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 29, 2014

CUPS.org User: mike

None of the RSS readers I've used support authentication, but for that we can just make CacheDir mode 770 so that only cupsd and the filters can read it directly.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 8, 2014

"str4461.patch":

Index: scheduler/client.c

--- scheduler/client.c (revision 12112)
+++ scheduler/client.c (working copy)
@@ -2912,6 +2912,7 @@
char ptr; / Pointer info filename /
size_t plen; /
Remaining length after pointer /
char language[7]; /
Language subdirectory, if any */

  • int perm_check = 1; /* Do permissions check? */

/*
@@ -2921,17 +2922,27 @@
language[0] = '\0';

if (!strncmp(con->uri, "/ppd/", 5) && !strchr(con->uri + 5, '/'))

  • {
    snprintf(filename, len, "%s%s", ServerRoot, con->uri);
  • perm_check = 0;
  • }
    else if (!strncmp(con->uri, "/icons/", 7) && !strchr(con->uri + 7, '/'))
    {
    snprintf(filename, len, "%s/%s", CacheDir, con->uri + 7);
    if (access(filename, F_OK) < 0)
    snprintf(filename, len, "%s/images/generic.png", DocumentRoot);
  • perm_check = 0;
    }
    else if (!strncmp(con->uri, "/rss/", 5) && !strchr(con->uri + 5, '/'))
    snprintf(filename, len, "%s/rss/%s", CacheDir, con->uri + 5);
  • else if (!strncmp(con->uri, "/admin/conf/", 12))
  • snprintf(filename, len, "%s%s", ServerRoot, con->uri + 11);
  • else if (!strcmp(con->uri, "/admin/conf/cupsd.conf"))
  • {
  • strlcpy(filename, ConfigurationFile, len);
  • perm_check = 0;
  • }
    else if (!strncmp(con->uri, "/admin/log/", 11))
    {
    if (!strncmp(con->uri + 11, "access_log", 10) && AccessLog[0] == '/')
    @@ -2942,6 +2953,8 @@
    strlcpy(filename, PageLog, len);
    else
    return (NULL);
  • perm_check = 0;
    }
    else if (con->language)
    {
    @@ -3007,7 +3020,7 @@
  • not allow access...
    */
  • if (!status && !(filestats->st_mode & S_IROTH))
  • if (!status && perm_check && !(filestats->st_mode & S_IROTH))
    {
    cupsdLogClient(con, CUPSD_LOG_INFO, "Files/directories such as "%s" must be world-readable.", filename);
    return (NULL);
    @@ -3115,7 +3128,7 @@
  • not allow access...
    */
  • if (!status && !(filestats->st_mode & S_IROTH))
  • if (!status && perm_check && !(filestats->st_mode & S_IROTH))
    {
    cupsdLogClient(con, CUPSD_LOG_INFO, "Files/directories such as "%s" must be world-readable.", filename);
    return (NULL);

Index: scheduler/ipp.c

--- scheduler/ipp.c (revision 12112)
+++ scheduler/ipp.c (working copy)
@@ -2715,7 +2715,6 @@

 cupsdLogMessage(CUPSD_LOG_DEBUG,
        "Copied PPD file successfully");
  •  chmod(dstfile, 0644);
    
    }
    }

@@ -4623,7 +4622,7 @@

  • Open the destination file for a copy...
    */
  • if ((dst = cupsFileOpen(to, "wb")) == NULL)
  • if ((dst = cupsdCreateConfFile(to, ConfigFilePerm)) == NULL)
    {
    cupsFreeOptions(num_defaults, defaults);
    cupsFileClose(src);
    @@ -4678,7 +4677,7 @@

unlink(tempfile);

  • return (cupsFileClose(dst));
  • return (cupsdCloseCreatedConfFile(dst, to));
    }
@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 8, 2014

"str4461p2.patch":

Index: notifier/rss.c

--- notifier/rss.c (revision 12126)
+++ notifier/rss.c (working copy)
@@ -1,27 +1,16 @@
/*

  • "$Id$"
    *

    • * RSS notifier for CUPS.
    • * RSS notifier for CUPS.
      *
    • * Copyright 2007-2012 by Apple Inc.
    • * Copyright 2007 by Easy Software Products.
    • * Copyright 2007-2014 by Apple Inc.
    • * Copyright 2007 by Easy Software Products.
      *
    • * These coded instructions, statements, and computer programs are the
    • * property of Apple Inc. and are protected by Federal copyright
    • * law. Distribution and use rights are outlined in the file "LICENSE.txt"
    • * which should have been included with this file. If this file is
    • * file is missing or damaged, see the license at "http://www.cups.org/".
    • * Contents:
    • * main() - Main entry for the test notifier.
    • * compare_rss() - Compare two messages.
    • * delete_message() - Free all memory used by a message.
    • * load_rss() - Load an existing RSS feed file.
    • * new_message() - Create a new RSS message.
    • * password_cb() - Return the cached password.
    • * save_rss() - Save messages to a RSS file.
    • * xml_escape() - Copy a string, escaping &, <, and > as needed.
    • * These coded instructions, statements, and computer programs are the
    • * property of Apple Inc. and are protected by Federal copyright
    • * law. Distribution and use rights are outlined in the file "LICENSE.txt"
    • * which should have been included with this file. If this file is
    • * file is missing or damaged, see the license at "http://www.cups.org/".
      */

    /*
    @@ -29,6 +18,7 @@
    */

    #include <cups/cups.h>
    +#include <sys/stat.h>
    #include <cups/language.h>
    #include <cups/string-private.h>
    #include <cups/array.h>
    @@ -629,6 +619,8 @@
    return (0);
    }

  • fchmod(fileno(fp), 0644);

fputs("\n", fp);
fputs("<rss version="2.0">\n", fp);
fputs(" \n", fp);

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 8, 2014

"str4461p3.patch":

Index: scheduler/Makefile

--- scheduler/Makefile (revision 12131)
+++ scheduler/Makefile (working copy)
@@ -171,7 +171,7 @@
echo Creating $(REQUESTS)/tmp...
$(INSTALL_DIR) -m 1770 -g $(CUPS_GROUP) $(REQUESTS)/tmp
echo Creating $(CACHEDIR)...

  • $(INSTALL_DIR) -m 775 -g $(CUPS_GROUP) $(CACHEDIR)
  • $(INSTALL_DIR) -m 770 -g $(CUPS_GROUP) $(CACHEDIR)
    if test "x$(INITDIR)" != x; then
    echo Installing init scripts...;
    $(INSTALL_DIR) -m 755 $(BUILDROOT)$(INITDIR)/init.d; \

Index: scheduler/conf.c

--- scheduler/conf.c (revision 12131)
+++ scheduler/conf.c (working copy)
@@ -1076,7 +1076,7 @@

if ((cupsdCheckPermissions(RequestRoot, NULL, 0710, RunUser,
Group, 1, 1) < 0 ||

  •   cupsdCheckPermissions(CacheDir, NULL, 0775, RunUser,
    
  •   cupsdCheckPermissions(CacheDir, NULL, 0770, RunUser,
             Group, 1, 1) < 0 ||
    cupsdCheckPermissions(temp, NULL, 0775, RunUser,
             Group, 1, 1) < 0 ||
    

@michaelrsweet michaelrsweet added this to the Stable milestone Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.