Re: removal of NGROUPS_MAX dependancy from base
On 22.02-16:28, Brooks Davis wrote: On Sun, Feb 22, 2009 at 11:07:19AM +, ttw+...@cobbled.net wrote: On 21.02-22:49, Julian Elischer wrote: [ ... ] this patch should remove the dependancy on the definition of NGROUPS_MAX as a static constant and implement it as a writable sysconf variable of the same. it should also make the necessary changes to the codebase to support those. [ ... ] What do you do about NFS? I seem to remember that NFWS had a maximum number of groups supported.. NFS will be supported by mapping 16 groups into the auth_unix structure dynamically. my intention is to try and make this transparent by allocating moving the 'most used' groups into that mapping as user processes check them, however, this is very conceptual at the moment and needs more thought as well as validation from others with more experience. I think this behavior will probably need to be configurable by the administrator because some sites are probably using groups to supply negative permissions. It's quite reasionable to argue that's a bad idea, but we need to take some care since people do occationally use that feature. agree. i'm hoping to make the rpc group allocations dynamic and thus, mostly transparent, but would suggest the only consistent way administrators to set permissions (when NFS is required) is to restrict NGROUP_MAX to 16 or less. i intend this to be the default, changed by sysctl/sysconf. my current primary concern is with software that defines it static arrays with a length of NGROUPS_MAX and then forgets to sanitize 'ngroups' count against that maximum but no idea how to catch those except too say that is 'broken'. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: removal of NGROUPS_MAX dependancy from base
On 21.02-22:49, Julian Elischer wrote: [ ... ] this patch should remove the dependancy on the definition of NGROUPS_MAX as a static constant and implement it as a writable sysconf variable of the same. it should also make the necessary changes to the codebase to support those. [ ... ] What do you do about NFS? I seem to remember that NFWS had a maximum number of groups supported.. NFS will be supported by mapping 16 groups into the auth_unix structure dynamically. my intention is to try and make this transparent by allocating moving the 'most used' groups into that mapping as user processes check them, however, this is very conceptual at the moment and needs more thought as well as validation from others with more experience. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: removal of NGROUPS_MAX dependancy from base
ttw+...@cobbled.net wrote: attached is the first in a series of patches that is intended to remove the current limitation on group membership. this patch should remove the dependancy on the definition of NGROUPS_MAX as a static constant and implement it as a writable sysconf variable of the same. it should also make the necessary changes to the codebase to support those. i need some guidance as to what i should re-define NGROUPS_MAX to be (so that code that depends on it can continue to operate, i'm thinking just make it 16 but perhaps it would be worth extending the default while we're at it to something like 64??). i also need feedback on any braindamage in the current changes. the next step will be to extend the kernel groups and map them back to the user structs / calls. finally i'll extend the user groups and implement those calls. nb: not tested the code (it builds) ... was intending to test it on my XEN box but only just realised that Xen on amd64 isn't working. :-( happy for any questions that may help guide the process. What do you do about NFS? I seem to remember that NFWS had a maximum number of groups supported.. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
removal of NGROUPS_MAX dependancy from base
attached is the first in a series of patches that is intended to
remove the current limitation on group membership.
this patch should remove the dependancy on the definition of
NGROUPS_MAX as a static constant and implement it as a writable
sysconf variable of the same. it should also make the necessary
changes to the codebase to support those.
i need some guidance as to what i should re-define NGROUPS_MAX to be
(so that code that depends on it can continue to operate, i'm thinking
just make it 16 but perhaps it would be worth extending the default
while we're at it to something like 64??). i also need feedback on
any braindamage in the current changes.
the next step will be to extend the kernel groups and map them back
to the user structs / calls. finally i'll extend the user groups
and implement those calls.
nb: not tested the code (it builds) ... was intending to test it on
my XEN box but only just realised that Xen on amd64 isn't working.
:-(
happy for any questions that may help guide the process.
Index: contrib/openpam/lib/openpam_borrow_cred.c
===
RCS file:
/home/__orole/dev/cabinet/zeeNi/ai/freebsd/src/contrib/openpam/lib/openpam_borrow_cred.c,v
retrieving revision 1.1.1.9
diff -b -u -r1.1.1.9 openpam_borrow_cred.c
--- contrib/openpam/lib/openpam_borrow_cred.c 21 Dec 2007 11:49:29 -
1.1.1.9
+++ contrib/openpam/lib/openpam_borrow_cred.c 4 Feb 2009 16:38:46 -
@@ -60,6 +60,7 @@
struct pam_saved_cred *scred;
const void *scredp;
int r;
+ int ngroups ;
ENTERI(pwd-pw_uid);
r = pam_get_data(pamh, PAM_SAVED_CRED, scredp);
@@ -73,26 +74,55 @@
(int)geteuid());
RETURNC(PAM_PERM_DENIED);
}
- scred = calloc(1, sizeof *scred);
- if (scred == NULL)
- RETURNC(PAM_BUF_ERR);
- scred-euid = geteuid();
- scred-egid = getegid();
- r = getgroups(NGROUPS_MAX, scred-groups);
- if (r 0) {
- FREE(scred);
- RETURNC(PAM_SYSTEM_ERR);
- }
- scred-ngroups = r;
+/* get the maximum number of system groups */
+#if _POSIX_VERSION 199212
+ ngroups = sysconf( _SC_NGROUPS_MAX ) ;
+#elif defined(NGROUPS_MAX)
+ ngroups = NGROUPS_MAX ;
+#else
+ ngroups = _NGROUPS_COMPAT ;
+#endif
+/* initally allocate enough memory for max_groups */
+ scred = malloc( sizeof(struct pam_saved_cred) +
+ ngroups*sizeof(gid_t) ) ;
+ if( scred == NULL )
+ RETURNC( PAM_BUF_ERR ) ;
+/* set the save values */
+ scred-euid = geteuid() ;
+ scred-egid = getegid() ;
+/* save groups into our (probably) oversized memory allocation */
+ r = getgroups( ngroups, scred-groups ) ;
+ if( r 0 ) {
+ FREE( scred ) ; /* call PAM's free macro */
+ RETURNC( PAM_SYSTEM_ERR ) ;
+ } ;
+ scred-ngroups = r ;
+ ngroups = r ngroups ? r : ngroups ; /* choose the smallest */
+ /* ... number of groups to allocate */
+ ngroups = ngroups _NGROUPS_COMPAT ? ngroups : _NGROUPS_COMPAT ;
+ /* but keep it within expected minimum value */
+ /* XXX: we don't really want this but
until we get
+* educated on the implications this is
probably safe
+* and certainaly compatible */
+/* realloc, releasing unneeded memory */
+ scred = realloc( (void*)scred,
+ sizeof(struct pam_saved_cred)+ngroups*sizeof(gid_t) ) ;
+ /* nb: we ignore failure and try to store the
larger
+* ... structure as initially requested.
catching the
+* ... error in 'pam_set_data' if neccessary. */
+/* save the credentials to PAM user data area */
r = pam_set_data(pamh, PAM_SAVED_CRED, scred, openpam_free_data);
if (r != PAM_SUCCESS) {
FREE(scred);
RETURNC(r);
}
+/* set the new credentials */
if (geteuid() == pwd-pw_uid)
RETURNC(PAM_SUCCESS);
if (initgroups(pwd-pw_name, pwd-pw_gid) 0 ||
- setegid(pwd-pw_gid) 0 || seteuid(pwd-pw_uid) 0) {
+ setegid(pwd-pw_gid) 0 || seteuid(pwd-pw_uid) 0)
+ {
+ /* if any of the set calls failed, then restore and fail */
openpam_restore_cred(pamh);
RETURNC(PAM_SYSTEM_ERR);
}
Index: contrib/openpam/lib/openpam_impl.h
===
RCS file:
/home/__orole/dev/cabinet/zeeNi/ai/freebsd/src/contrib/openpam/lib/openpam_impl.h,v
retrieving revision 1.1.1.17
diff -b -u -r1.1.1.17 openpam_impl.h
--- contrib/openpam/lib/openpam_impl.h 21 Dec 2007 11:49:29 - 1.1.1.17
+++
