| Summary: | Support for users in more than 16 groups on Mac OS X. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Disco Vince Giffin <vgiffin> | ||||||
| Component: | sshd | Assignee: | Assigned to nobody <unassigned-bugs> | ||||||
| Status: | CLOSED FIXED | ||||||||
| Severity: | normal | CC: | djm, dtucker | ||||||
| Priority: | P2 | ||||||||
| Version: | 4.7p1 | ||||||||
| Hardware: | Other | ||||||||
| OS: | Mac OS X | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 1481 | ||||||||
| Attachments: |
|
||||||||
Comment on attachment 1407 [details] Fixes issue with users in more than 16 groups. >+ if (initgroups(pw->pw_name, pw->pw_gid) < 0) Why is this necessary? There's an initgroups() call in session.c. (and yeah, I know that #ifdef hairball in session.c makes it hard to follow, but I have plans to tidy that up.) (In reply to comment #1) > Why is this necessary? There's an initgroups() call in session.c. Good question. I'll have to do a little more research to answer that. I did run a couple of tests and confirmed that this patch is required for scp to honor groups past the 16th (for a given user) for OpenSSH 4.7p1 on Leopard (Mac OS X 10.5.1). Comment on attachment 1407 [details]
Fixes issue with users in more than 16 groups.
Seems reasonable to me. Darren, can you see any problems with this?
(In reply to comment #3) > (From update of attachment 1407 [details]) > Seems reasonable to me. Darren, can you see any problems with this? I'd like to understand why it's needed first, given that there's already an initgroups() call in session.c. This patch should probably be within the #else portion of the above #ifdef and/or within an #ifdef __APPLE__. (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 1407 [details] [details]) > > Seems reasonable to me. Darren, can you see any problems with this? > > I'd like to understand why it's needed first, given that there's > already an initgroups() call in session.c. Our (Apple's) implementation of initgroups() opts you in to using memberd (which gives you the ability to be in more than 16 groups). For conformance and compatibility reasons, certain calls will opt you out of this behavior. The setgid() call just above this patch opts out of using more than 16 groups. The patch to call initgroups() again is required to opt back in. Basically, initgroups has to be called after any setgroups(), setgid(), etc., if you want to use more than 16 groups. Won't a similar call to initgroups() need to be in permanently_drop_suid() and restore_uid() too? (In reply to comment #7) > Won't a similar call to initgroups() need to be in > permanently_drop_suid() and restore_uid() too? No, because setgid() isn't called from permanently_drop_suid() and restore_uid() is restoring the original (privileged) uid. Sorry, I meant temporarily_use_uid(), not permanently_drop_suid() temporarily_use_uid() calls setgroups(), does that opt out of memberd? (In reply to comment #9) > Sorry, I meant temporarily_use_uid(), not permanently_drop_suid() > > temporarily_use_uid() calls setgroups(), does that opt out of memberd? No. The call to setgroups() in temporarily_use_uid() should be fine. Created attachment 1592 [details]
wrap in __APPLE__ and comment
This is the diff I'd like to commit then.
fix applied - will be in openssh-5.2. Thanks! Close bugs fixed/reviewed for openssh-5.2 release |
Created attachment 1407 [details] Fixes issue with users in more than 16 groups. Attached is a patch for building OpenSSH 4.7p1 on Mac OS X. This patch corrects functionality for users in more than 16 groups.