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

adminutil functions break browsing #1993

Closed
michaelrsweet opened this issue Sep 28, 2006 · 13 comments
Closed

adminutil functions break browsing #1993

michaelrsweet opened this issue Sep 28, 2006 · 13 comments
Milestone

Comments

@michaelrsweet
Copy link
Collaborator

Version: 1.2.4
CUPS.org User: twaugh.redhat

STR #1992 probably wouldn't be such a problem if the configuration could be adjusted without having 'BrowseOrder' reset to 'allow,deny' whenever the adminutil functions are used.

Start with a cupsd.conf file with 'BrowseOrder deny,allow' and 'BrowseAllow @Local' in.
Visit http://localhost:631/admin
Toggle 'Save debugging information for troubleshooting'
Click 'Change Settings'

cupsd.conf now has 'BrowseOrder allow,deny'.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Not sure there is anything we can do about this; the convenience functions specifically override the BrowseAllow/BrowseDeny/BrowseOrder directives on purpose.

FWIW, the following:

BrowseOrder allow,deny
BrowseAllow @LOCAL

is the same as:

BrowseOrder deny,allow
BrowseDeny all
BrowseAllow @LOCAL

If you can supply a complete example cupsd.conf file, we'll look at at least making sure that the rewritten cupsd.conf is still usable.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: twaugh.redhat

Attached.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Tim, is that file the "before" or "after" version?

The BrowseOrder line is "deny.allow", which isn't technically valid because you have a period (.) instead of a comma (,) separating the words.

Anyways, cupsd will still understand it but the end result is the same as "allow all" since there are no deny lines and the default for "deny,allow" is to allow, then process deny lines, and lastly process allow lines.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: twaugh.redhat

The before version, and I must have accidentally changed the ',' to a '.' (sorry).

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: twaugh.redhat

This problem boils down to the fact that there is no way to alter only one of the advertised server settings -- instead, all must be set.

So if I want to diagnose a user problem, I can't ask the user to go to the web interface and turn on server debug logging (CUPS_SERVER_DEBUG_LOGGING) because that will destroy the rest of the config file tweaks they may have made.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

I'll see if we can do optimizations, at least for the debug toggle. However, any changes will strip the comments from the config file, even if the "content" lines aren't changed...

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

The attached patch seems to provide the functionality you want, looking at the existing cupsd.conf file to determine whether a change has occurred and assuming no change for options that are omitted from the array you pass in...

Let me know if you have any issues with this change...

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: twaugh.redhat

It's good, and definitely the right approach. I think there might be one or two bugs in there though.

I started with:

  • show printers shared by other systems
  • share published printers connected to this system
  • allow remote administration
  • don't allow users to cancel any job
  • don't save debugging information

I set 'save debugging information', and ended up with these changes:

--- /etc/cups/cupsd.conf.orig 2007-02-09 18:07:32.000000000 +0000
+++ /etc/cups/cupsd.conf 2007-02-09 18:08:02.000000000 +0000
@@ -1,34 +1,22 @@
MaxLogSize 2000000000
-# Show general information in error_log.
-LogLevel info
+# Show troubleshooting information in error_log.
+LogLevel debug
SystemGroup sys root
-# Allow remote access
Port 631
Listen /var/run/cups/cups.sock
-# Enable printer sharing and shared printers.
Browsing On
BrowseOrder allow,deny
-# (Change '@Local' to 'ALL' if using directed broadcasts from another subnet.)
BrowseAllow @Local
BrowseAddress @Local
DefaultAuthType Basic

  • Allow shared printing and remote administration...

  • Order allow,deny
  • Allow @Local
Encryption Required - # Allow remote administration... - Order allow,deny - Allow @Local AuthType Basic Require user @System - # Allow remote access to the configuration files... - Order allow,deny - Allow @Local @@ -44,7 +32,6 @@ Require user @owner @System Order deny,allow - # Only the owner or an administrator can cancel a job... Order deny,allow Require user @owner @System

I think it's because 'remote_admin' gets set to -1 meaning 'no change', but the 'Location' processing code only compares 'remote_admin > 0':

  •   if (share_printers || remote_admin)
    
  •   if (share_printers > 0 || remote_admin > 0)
    {
      cupsFilePuts(temp, "# Allow remote access\n");
    

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Attached is a supplemental patch that seems to fix the loss of the Order, Allow, and Deny directives.

Let me know if you see any other problems.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: twaugh.redhat

Works very well here. Thanks!

@michaelrsweet
Copy link
Collaborator Author

"cupsd.conf":

MaxLogSize 2000000000

Show general information in error_log.

LogLevel info
SystemGroup sys root

Allow remote access

Port 631

Enable printer sharing and shared printers.

Browsing On
BrowseOrder deny.allow
BrowseAllow @Local
BrowseAddress @Local
DefaultAuthType Basic

Allow shared printing and remote administration...

Order allow,deny
Allow @Local

<Location /admin>
Encryption Required

Allow remote administration...

Order allow,deny
Allow @Local

<Location /admin/conf>
AuthType Basic
Require user @System

Allow remote access to the configuration files...

Order allow,deny
Allow @Local



Require user @owner @System
Order deny,allow


AuthType Basic
Require user @System
Order deny,allow


Require user @owner @System
Order deny,allow


Order deny,allow

@michaelrsweet
Copy link
Collaborator Author

"str1993.patch":

Index: adminutil.c

--- adminutil.c (revision 6237)
+++ adminutil.c (working copy)
@@ -1003,6 +1003,11 @@
wrote_root_location; /* Wrote the / location? /
int indent; /
Indentation /
int cupsd_num_settings; /
New number of settings */

  • int old_remote_printers, /* Show remote printers */

  •   old_share_printers, /\* Share local printers */
    
  •   old_remote_admin,   /\* Remote administration allowed? */
    
  •   old_user_cancel_any,    /\* Cancel-job policy set? */
    
  •   old_debug_logging;  /* LogLevel debug set? */
    

    cups_option_t cupsd_settings, / New settings /
    *setting; /
    Current setting _/
    _cups_globals_t *cg = cupsGlobals(); / Global data */
    @@ -1036,38 +1041,133 @@
    return (0);

    /*

  • * Get current settings...

  • */

  • if (!_cupsAdminGetServerSettings(http, &cupsd_num_settings,
  •                               &cupsd_settings))
    
  • return (0);
  • if ((val = cupsGetOption(CUPS_SERVER_DEBUG_LOGGING, cupsd_num_settings,
  •                       cupsd_settings)) != NULL)
    
  • old_debug_logging = atoi(val);
  • else
  • old_debug_logging = 0;
  • if ((val = cupsGetOption(CUPS_SERVER_REMOTE_ADMIN, cupsd_num_settings,
  •                       cupsd_settings)) != NULL)
    
  • old_remote_admin = atoi(val);
  • else
  • old_remote_admin = 0;
  • if ((val = cupsGetOption(CUPS_SERVER_REMOTE_PRINTERS, cupsd_num_settings,
  •                       cupsd_settings)) != NULL)
    
  • old_remote_printers = atoi(val);
  • else
  • old_remote_printers = 1;
  • if ((val = cupsGetOption(CUPS_SERVER_SHARE_PRINTERS, cupsd_num_settings,
  •                       cupsd_settings)) != NULL)
    
  • old_share_printers = atoi(val);
  • else
  • old_share_printers = 0;
  • if ((val = cupsGetOption(CUPS_SERVER_USER_CANCEL_ANY, cupsd_num_settings,
  •                       cupsd_settings)) != NULL)
    
  • old_user_cancel_any = atoi(val);
  • else
  • old_user_cancel_any = 0;
  • cupsFreeOptions(cupsd_num_settings, cupsd_settings);
  • /*

    • Get basic settings...
      */

    if ((val = cupsGetOption(CUPS_SERVER_DEBUG_LOGGING, num_settings,
    settings)) != NULL)

  • {
    debug_logging = atoi(val);

  • if (debug_logging == old_debug_logging)
  • {
  • /*
    
  •  \* No change to this setting...
    
  •  */
    
  •  debug_logging = -1;
    
  • }
  • }
    else
  • debug_logging = 0;
  • debug_logging = -1;

if ((val = cupsGetOption(CUPS_SERVER_REMOTE_ADMIN, num_settings,
settings)) != NULL)

  • {
    remote_admin = atoi(val);
  • if (remote_admin == old_remote_admin)
  • {
  • /*
    
  •  \* No change to this setting...
    
  •  */
    
  •  remote_admin = -1;
    
  • }
  • }
    else
  • remote_admin = 0;
  • remote_admin = -1;

if ((val = cupsGetOption(CUPS_SERVER_REMOTE_PRINTERS, num_settings,
settings)) != NULL)

  • {
    remote_printers = atoi(val);
  • if (remote_printers == old_remote_printers)
  • {
  • /*
    
  •  \* No change to this setting...
    
  •  */
    
  •  remote_printers = -1;
    
  • }
  • }
    else
  • remote_printers = 1;
  • remote_printers = -1;

if ((val = cupsGetOption(CUPS_SERVER_SHARE_PRINTERS, num_settings,
settings)) != NULL)

  • {
    share_printers = atoi(val);
  • if (share_printers == old_share_printers)
  • {
  • /*
    
  •  \* No change to this setting...
    
  •  */
    
  •  share_printers = -1;
    
  • }
  • }
    else
  • share_printers = 0;
  • share_printers = -1;

if ((val = cupsGetOption(CUPS_SERVER_USER_CANCEL_ANY, num_settings,
settings)) != NULL)

  • {
    user_cancel_any = atoi(val);
  • if (user_cancel_any == old_user_cancel_any)
  • {
  • /*
    
  •  \* No change to this setting...
    
  •  */
    
  •  user_cancel_any = -1;
    
  • }
  • }
    else
  • user_cancel_any = 0;
  • user_cancel_any = -1;

/*

  • Create a temporary file for the new cupsd.conf file...
    @@ -1119,13 +1219,14 @@

while (cupsFileGetConf(cupsd, line, sizeof(line), &value, &linenum))
{

  • if (!strcasecmp(line, "Port") || !strcasecmp(line, "Listen"))

  • if ((!strcasecmp(line, "Port") || !strcasecmp(line, "Listen")) &&

  •    (share_printers >= 0 || remote_admin >= 0))
    

    {
    if (!wrote_port_listen)
    {
    wrote_port_listen = 1;

  • if (share_printers || remote_admin)

  • if (share_printers > 0 || remote_admin > 0)
    {
    cupsFilePuts(temp, "# Allow remote access\n");
    cupsFilePrintf(temp, "Port %d\n", server_port);
    @@ -1150,22 +1251,23 @@
    )
    cupsFilePrintf(temp, "Listen %s\n", value);
    }

  • else if (!strcasecmp(line, "Browsing") ||

  •         !strcasecmp(line, "BrowseAddress") ||
    
  •         !strcasecmp(line, "BrowseAllow") ||
    
  •         !strcasecmp(line, "BrowseDeny") ||
    
  •         !strcasecmp(line, "BrowseOrder"))
    
  • else if ((!strcasecmp(line, "Browsing") ||

  •          !strcasecmp(line, "BrowseAddress") ||
    
  •          !strcasecmp(line, "BrowseAllow") ||
    
  •          !strcasecmp(line, "BrowseDeny") ||
    
  •          !strcasecmp(line, "BrowseOrder")) &&
    
  •    (remote_printers >= 0 || share_printers >= 0))
    

    {
    if (!wrote_browsing)
    {
    wrote_browsing = 1;

  •    if (remote_printers || share_printers)
    
  •    if (remote_printers > 0 || share_printers > 0)
    

    {

  • if (remote_printers && share_printers)
    
  • if (remote_printers > 0 && share_printers > 0)
    cupsFilePuts(temp,
                 "# Enable printer sharing and shared printers.\n");
    
  • else if (remote_printers)
    
  • else if (remote_printers > 0)
    cupsFilePuts(temp,
                 "# Show shared printers on the local network.\n");
    

    else
    @@ -1175,10 +1277,10 @@
    cupsFilePuts(temp, "Browsing On\n");
    cupsFilePuts(temp, "BrowseOrder allow,deny\n");

  • if (remote_printers)
    
  • if (remote_printers > 0)
    cupsFilePuts(temp, "BrowseAllow @LOCAL\n");
    
  • if (share_printers)
    
  • if (share_printers > 0)
    cupsFilePuts(temp, "BrowseAddress @LOCAL\n");
     }
    

    else
    @@ -1189,7 +1291,7 @@
    }
    }
    }

  • else if (!strcasecmp(line, "LogLevel"))

  • else if (!strcasecmp(line, "LogLevel") && debug_logging >= 0)
    {
    wrote_loglevel = 1;

@@ -1252,13 +1354,13 @@
{
in_location = 0;
indent -= 2;

  •  if (in_admin_location)
    
  •  if (in_admin_location && remote_admin >= 0)
    

    {
    wrote_admin_location = 1;

    if (remote_admin)
    cupsFilePuts(temp, " # Allow remote administration...\n");

  • else

  • else if (remote_admin == 0)
    cupsFilePuts(temp, " # Restrict access to the admin pages...\n");

 cupsFilePuts(temp, "  Order allow,deny\n");

@@ -1268,7 +1370,7 @@
else
cupsFilePuts(temp, " Allow localhost\n");
}

  •  else if (in_conf_location)
    
  •  else if (in_conf_location && remote_admin >= 0)
    
    {
    wrote_conf_location = 1;

@@ -1286,23 +1388,23 @@
else
cupsFilePuts(temp, " Allow localhost\n");
}

  •  else if (in_root_location)
    
  •  else if (in_root_location && (remote_admin >= 0 || share_printers >= 0))
    

    {
    wrote_root_location = 1;

  • if (remote_admin && share_printers)

  • if (remote_admin > 0 && share_printers > 0)
    cupsFilePuts(temp, " # Allow shared printing and remote "
    "administration...\n");

  • else if (remote_admin)

  • else if (remote_admin > 0)
    cupsFilePuts(temp, " # Allow remote administration...\n");

  • else if (share_printers)

  • else if (share_printers > 0)
    cupsFilePuts(temp, " # Allow shared printing...\n");
    else
    cupsFilePuts(temp, " # Restrict access to the server...\n");

 cupsFilePuts(temp, "  Order allow,deny\n");
  • if (remote_admin || share_printers)

  • if (remote_admin > 0 || share_printers > 0)
    cupsFilePuts(temp, " Allow @Local\n");
    else
    cupsFilePuts(temp, " Allow localhost\n");
    @@ -1325,7 +1427,7 @@

    indent += 2;

  •  if (!strcasecmp(value, "cancel-job"))
    
  •  if (!strcasecmp(value, "cancel-job") && user_cancel_any >= 0)
    

    {
    /*

    • Don't write anything for this limit section...
      @@ -1344,7 +1446,7 @@
      if (*valptr)
      *valptr++ = '\0';
  •      if (!strcasecmp(value, "cancel-job"))
    
  •      if (!strcasecmp(value, "cancel-job") && user_cancel_any >= 0)
    

    {
    /*
    * Write everything except for this definition...
    @@ -1445,13 +1547,13 @@

    • Write any missing info...
      */
  • if (!wrote_browsing)

  • if (!wrote_browsing && (remote_printers >= 0 || share_printers >= 0))
    {

  • if (remote_printers || share_printers)

  • if (remote_printers > 0 || share_printers > 0)
    {

  •  if (remote_printers && share_printers)
    
  •  if (remote_printers > 0 && share_printers > 0)
    

    cupsFilePuts(temp, "# Enable printer sharing and shared printers.\n");

  •  else if (remote_printers)
    
  •  else if (remote_printers > 0)
    

    cupsFilePuts(temp, "# Show shared printers on the local network.\n");
    else
    cupsFilePuts(temp, "# Share local printers on the local network.\n");
    @@ -1459,10 +1561,10 @@
    cupsFilePuts(temp, "Browsing On\n");
    cupsFilePuts(temp, "BrowseOrder allow,deny\n");

  •  if (remote_printers)
    
  •  if (remote_printers > 0)
    

    cupsFilePuts(temp, "BrowseAllow @Local\n");

  •  if (share_printers)
    
  •  if (share_printers > 0)
    

    cupsFilePuts(temp, "BrowseAddress @Local\n");
    }
    else
    @@ -1472,7 +1574,7 @@
    }
    }

  • if (!wrote_loglevel)

  • if (!wrote_loglevel && debug_logging >= 0)
    {
    if (debug_logging)
    {
    @@ -1486,9 +1588,9 @@
    }
    }

  • if (!wrote_port_listen)

  • if (!wrote_port_listen && (share_printers >= 0 || remote_admin >= 0))
    {

  • if (share_printers || remote_admin)

  • if (share_printers > 0 || remote_admin > 0)
    {
    cupsFilePuts(temp, "# Allow remote access\n");
    cupsFilePrintf(temp, "Port %d\n", ippPort());
    @@ -1506,14 +1608,14 @@
    #endif /* CUPS_DEFAULT_DOMAINSOCKET */
    }

  • if (!wrote_root_location)

  • if (!wrote_root_location && (remote_admin >= 0 || share_printers >= 0))
    {

  • if (remote_admin && share_printers)

  • if (remote_admin > 0 && share_printers > 0)
    cupsFilePuts(temp,
    "# Allow shared printing and remote administration...\n");

  • else if (remote_admin)

  • else if (remote_admin > 0)
    cupsFilePuts(temp, "# Allow remote administration...\n");

  • else if (share_printers)

  • else if (share_printers > 0)
    cupsFilePuts(temp, "# Allow shared printing...\n");
    else
    cupsFilePuts(temp, "# Restrict access to the server...\n");
    @@ -1521,7 +1623,7 @@
    cupsFilePuts(temp, "\n"
    " Order allow,deny\n");

  • if (remote_admin || share_printers)

  • if (remote_admin > 0 || share_printers > 0)
    cupsFilePuts(temp, " Allow @Local\n");
    else
    cupsFilePuts(temp, " Allow localhost\n");
    @@ -1529,7 +1631,7 @@
    cupsFilePuts(temp, "\n");
    }

  • if (!wrote_admin_location)

  • if (!wrote_admin_location && remote_admin >= 0)
    {
    if (remote_admin)
    cupsFilePuts(temp, "# Allow remote administration...\n");
    @@ -1547,7 +1649,7 @@
    cupsFilePuts(temp, "\n");
    }

  • if (!wrote_conf_location)

  • if (!wrote_conf_location && remote_admin >= 0)
    {
    if (remote_admin)
    cupsFilePuts(temp,
    @@ -1568,7 +1670,7 @@
    cupsFilePuts(temp, "\n");
    }

  • if (!wrote_policy)

  • if (!wrote_policy && user_cancel_any >= 0)
    {
    cupsFilePuts(temp, "\n"
    " # Job-related operations must be done by the owner "
    @@ -1647,22 +1749,51 @@

  • Updated OK, add the basic settings...
    */

  • cupsd_num_settings = cupsAddOption(CUPS_SERVER_DEBUG_LOGGING,

  •                                   debug_logging ? "1" : "0",
    
  •                  cupsd_num_settings, &cupsd_settings);
    
  • cupsd_num_settings = cupsAddOption(CUPS_SERVER_REMOTE_ADMIN,

  •                                   remote_admin ? "1" : "0",
    
  •                  cupsd_num_settings, &cupsd_settings);
    
  • cupsd_num_settings = cupsAddOption(CUPS_SERVER_REMOTE_PRINTERS,

  •                                   remote_printers ? "1" : "0",
    
  •                  cupsd_num_settings, &cupsd_settings);
    
  • cupsd_num_settings = cupsAddOption(CUPS_SERVER_SHARE_PRINTERS,

  •                                   share_printers ? "1" : "0",
    
  •                  cupsd_num_settings, &cupsd_settings);
    
  • cupsd_num_settings = cupsAddOption(CUPS_SERVER_USER_CANCEL_ANY,

  •                                   user_cancel_any ? "1" : "0",
    
  •                  cupsd_num_settings, &cupsd_settings);
    
  • if (debug_logging >= 0)

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_DEBUG_LOGGING,
    
  •                                debug_logging ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • else

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_DEBUG_LOGGING,
    
  •                                old_debug_logging ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • if (remote_admin >= 0)

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_REMOTE_ADMIN,
    
  •                                remote_admin ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • else

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_REMOTE_ADMIN,
    
  •                                old_remote_admin ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • if (remote_printers >= 0)

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_REMOTE_PRINTERS,
    
  •                                remote_printers ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • else

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_REMOTE_PRINTERS,
    
  •                                old_remote_printers ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • if (share_printers >= 0)

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_SHARE_PRINTERS,
    
  •                                share_printers ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • else

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_SHARE_PRINTERS,
    
  •                                old_share_printers ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • if (user_cancel_any >= 0)

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_USER_CANCEL_ANY,
    
  •                                user_cancel_any ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    
  • else

  •  cupsd_num_settings = cupsAddOption(CUPS_SERVER_USER_CANCEL_ANY,
    
  •                                old_user_cancel_any ? "1" : "0",
    
  •                cupsd_num_settings, &cupsd_settings);
    

    /*

    • Save the new values...
      */

@michaelrsweet
Copy link
Collaborator Author

"str1993p2.patch":

Index: adminutil.c

--- adminutil.c (revision 6253)
+++ adminutil.c (working copy)
@@ -1482,7 +1482,9 @@

   in_cancel_job = 0;
 }
  • else if ((in_admin_location || in_conf_location || in_root_location) &&
  • else if ((((in_admin_location || in_conf_location || in_root_location) &&
  •           remote_admin >= 0) ||
    
  •          (in_root_location && share_printers >= 0)) &&
          (!strcasecmp(line, "Allow") || !strcasecmp(line, "Deny") ||
      !strcasecmp(line, "Order")))
    
    continue;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant