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

please support less restricted root backend permissions #2935

Closed
michaelrsweet opened this issue Sep 6, 2008 · 7 comments
Closed

please support less restricted root backend permissions #2935

michaelrsweet opened this issue Sep 6, 2008 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@michaelrsweet
Copy link
Collaborator

Version: 2.0-feature
CUPS.org User: martin.pitt.canonical

At the moment, backends which run as root need to be 0700, i. e. not have any privileges for group or others. This is very rigid and e. g. prevents system integrity checkers, bug report scripts, and other tools from verifying the contents of those backends (and also violates the Debian Policy). Distribution packages should not ship binaries which are not world readable, since anyone can just download the package and get it from there.

Would you consider relaxing the check in scheduler/job.c a bit?

 backroot = !(backinfo.st_mode & (S_IRWXG | S_IRWXO));

this could become

backroot = !(backinfo.st_mode & (S_IXGRP | S_IXOTH));

so that it is possible to install those backends with 744 permissions.

Preferably the backends should installed with 744 mode as well, but if you don't like that, upstream could stay with installing them as 700 (distros can easily adapt the permissions in their build scripts without patching the source).

Thanks for considering,

Martin

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

First, this should be an enhancement request, not a bug report.

Second, I don't know whether we will make this change since it has serious security issues. IMHO any integrity checker needs to run as root as well.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: martin.pitt.canonical

I am curious, what are those "serious security issues"? Built binaries from open source software are hardly that secret? (And if you do local s3kr1t modifications, nobody stops you from chmoding them to 700).

Feel free to drop the Makefile part of the patch. The more important issue is to run backends with 744 permisssions as root as well.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

The main security issue is that some system files/directories are restricted to provide necessary security, but a non-root tool will not be able to validate them - you would be missing a fairly significant portion of sensitive system/log files...

Also, it is fairly easy to accidentally install with partial execute permissions, which would lead to accidental running of the backends as root with your patch.

Anyways, don't expect any change (if we do decide to make a change) until CUPS 1.5 at the earliest.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: martin.pitt.canonical

Posted updated patch which also checks for group/other writeability.

Also, it is fairly easy to accidentally install with partial execute
permissions, which would lead to accidental running of the backends as
root with your patch.

But in principle that is the same in the current version as well, just the particularly tested privileges are a bit different (if you install backends with umask 077, you'd get backends installed as root as well). That's why we are using distro packages, or at least "make install" instead of setting up files by hand, right? :-)

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: martin.pitt.canonical

Updated patch, missed the identical test in deviced.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Fixed in Subversion repository.

The final patch allows group read and execute, but not group write.

@michaelrsweet
Copy link
Collaborator Author

"str2935.patch":

Index: scheduler/cups-deviced.c

--- scheduler/cups-deviced.c (revision 11775)
+++ scheduler/cups-deviced.c (working copy)
@@ -270,8 +270,7 @@
* all others run as the unprivileged user...
*/

  • start_backend(dent->filename,
  •              !(dent->fileinfo.st_mode & (S_IRWXG | S_IRWXO)));
    
  • start_backend(dent->filename, !(dent->fileinfo.st_mode & (S_IWGRP | S_IRWXO)));
    }

cupsDirClose(dir);

Index: scheduler/job.c

--- scheduler/job.c (revision 11775)
+++ scheduler/job.c (working copy)
@@ -1233,7 +1233,7 @@
else if (stat(command, &backinfo))
backroot = 0;
else

  •    backroot = !(backinfo.st_mode & (S_IRWXG | S_IRWXO));
    
  •    backroot = !(backinfo.st_mode & (S_IWGRP | S_IRWXO));
    

    argv[0] = job->printer->sanitized_device_uri;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant