apple / cups

Apple CUPS Sources
https://www.cups.org
Apache License 2.0
1.91k stars 461 forks source link

setgroups fails preventing jobs from printing #213

Closed michaelrsweet closed 21 years ago

michaelrsweet commented 21 years ago

Version: 1.1-current CUPS.org User: jlovell

The recent changes in cvs using setgroups fails on MacOSX Jaguar and other BSDs (according to the xinetd readme). This will work in Panther but to maintain compatibility a change may be warranted. For reference, I applied the attached patch for the time being. It has the small benefit of validating a user ID specified in cupsd.conf.

michaelrsweet commented 21 years ago

CUPS.org User: mike

Jim,

Is the issue that setgroups() removes all group membership on OSX, or that you need additional groups for that user?

Also, it looks like there is an additional fix in StartJob() WRT the whitespace stuff? I'm not sure the optptr++ is valid (strlcat could conceivably fail if we miscalculated the size of the buffer...)

michaelrsweet commented 21 years ago

CUPS.org User: jlovell

The problem is setgroups returns EINVAL for ngroups less than 1 or greater than MAX_GROUPS. My patch had the benefit of allowing an administrator greater control via group membership for the cups_user (as I believe was mentioned in the STR that prompted the original change). An alternate patch might be something like "setgroups(1, &Group)".

The other patch is to log job options individually. This change went in earlier when long log entries were being truncated. It allows for more readable entries like:

D [ ] StartJob: option = "AP_D_InputSlot=" D [ ] StartJob: option = "nocollate" D [ ] StartJob: option = "media=Letter" D [ ] StartJob: option = "com.apple.print.PrinterInfo.PMColorDeviceID..n.=1678338968" D [ ] StartJob: option = "com.apple.print.PrintSettings.PMPrimaryPaperFeed..a.0=PageSize"

instead having them all listed on one line. It's not strictly required anymore (you can ignore it) but I believe it's a little easier for people to read.

Thanks!

michaelrsweet commented 21 years ago

CUPS.org User: mike

At the moment I'm leaning towards using setgroups(1,&Group) (or the equivalent, since the group list needs to use gid_t... :) since setgroups() is more portable and has no side-effects (initgroups could give group perms that you don't expect...)

As for the option logging patch, we'll stick with the magical expanding line buffer trick... :)

michaelrsweet commented 21 years ago

CUPS.org User: mike

OK, I've commited the necessary changes to call setgroups with 1 group (see attached patch).

michaelrsweet commented 21 years ago

CUPS.org User: jlovell

Looks good -- works on Jaguar. Thanks!

michaelrsweet commented 21 years ago

"cups_setgroups.patch":

Index: scheduler/client.c

RCS file: /home/anoncvs/cups/scheduler/client.c,v retrieving revision 1.168 diff -u -d -b -w -r1.168 client.c --- scheduler/client.c 20 Jul 2003 15:15:07 -0000 1.168 +++ scheduler/client.c 25 Jul 2003 01:24:02 -0000 @@ -53,6 +53,9 @@

include "cupsd.h"

include

+#ifdef APPLE

+#ifdef APPLE

+#ifdef APPLE

@@ -1363,21 +1369,35 @@ * User ID to run as... */

+#ifdef APPLE

+#ifdef APPLE

+#ifdef APPLE

+#ifdef APPLE

michaelrsweet commented 21 years ago

"str213.patch":

Index: client.c

RCS file: /development/cvs/cups/scheduler/client.c,v retrieving revision 1.168 diff -u -r1.168 client.c --- client.c 2003/07/20 15:15:07 1.168 +++ client.c 2003/08/01 19:58:50 @@ -3171,7 +3171,7 @@ if (setgid(Group)) exit(errno);