From: Chuck Lever Date: Tue, 4 Jan 2011 19:47:55 +0000 (-0500) Subject: Authunix_create_default() should truncate at 16 groups instead of failing X-Git-Tag: libtirpc-0-2-2-rc4 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e45f1768323a5c43d9002131dc180fbfd1dcece5;p=libtirpc Authunix_create_default() should truncate at 16 groups instead of failing Instead of failing if the calling process has more than 16 supplemen- tal groups, authunix_create_default() should simply truncate the group list to 16 entries. This is what the legacy RPC implementation in glibc does, what the Linux kernel's RPC implementation does (see net/sunrpc/auth_unix.c:unx_create_cred()), and what libtirpc on Solaris does. RFC 5531, Appendix A does not provide any guidance about AUTH_SYS behavior when a calling process is a member of more than 16 groups. Thus we cannot follow a specification, but must instead be guided by the conventional precedent set by existing implementations. Sun doc 816-1435, "ONC+ Developer's Guide," p. 148, refers to authunix_create_default() as one of several functions that are supported for backwards compatibility. Therefore, our libtirpc implementation should behave just like the glibc implementation it is replacing, so that applications can easily migrate from glibc's legacy RPC implementation to the newer libtirpc implementation. The upshot is that glibc's authunix_create_default() is a formal legacy API. The rules about changing formal APIs trump the idea that existing APIs should be changed to fail if they can't ideally fulfill an application's request. Related commit: 89323aaf BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=565507 Signed-off-by: Chuck Lever Signed-off-by: Steve Dickson --- diff --git a/src/auth_unix.c b/src/auth_unix.c index ddd89cc..c2469da 100644 --- a/src/auth_unix.c +++ b/src/auth_unix.c @@ -186,27 +186,64 @@ authunix_create_default() int len; char machname[MAXHOSTNAMELEN + 1]; uid_t uid; - gid_t gid; - gid_t gids[NGRPS]; + gid_t gid, *gids; + AUTH *result; memset(&rpc_createerr, 0, sizeof(rpc_createerr)); if (gethostname(machname, sizeof machname) == -1) { - rpc_createerr.cf_stat = RPC_SYSTEMERROR; rpc_createerr.cf_error.re_errno = errno; - return NULL; + goto out_err; } machname[sizeof(machname) - 1] = 0; uid = geteuid(); gid = getegid(); - len = getgroups(NGRPS, gids); - if (len < 0) { - rpc_createerr.cf_stat = RPC_SYSTEMERROR; + + /* According to glibc comments, an intervening setgroups(2) + * call can increase the number of supplemental groups between + * these two getgroups(2) calls. */ +retry: + len = getgroups(0, NULL); + if (len == -1) { + rpc_createerr.cf_error.re_errno = errno; + goto out_err; + } + + /* Bump allocation size. A zero allocation size may result in a + * NULL calloc(3) result, which is not reliably distinguishable + * from a memory allocation error. */ + gids = calloc(len + 1, sizeof(gid_t)); + if (gids == NULL) { + rpc_createerr.cf_error.re_errno = ENOMEM; + goto out_err; + } + + len = getgroups(len, gids); + if (len == -1) { rpc_createerr.cf_error.re_errno = errno; - return NULL; + free(gids); + if (rpc_createerr.cf_error.re_errno == EINVAL) { + rpc_createerr.cf_error.re_errno = 0; + goto retry; + } + goto out_err; } + + /* + * AUTH_UNIX sends on the wire only the first NGRPS groups in the + * supplemental groups list. + */ + if (len > NGRPS) + len = NGRPS; + /* XXX: interface problem; those should all have been unsigned */ - return (authunix_create(machname, uid, gid, len, gids)); + result = authunix_create(machname, uid, gid, len, gids); + free(gids); + return result; + +out_err: + rpc_createerr.cf_stat = RPC_SYSTEMERROR; + return NULL; } /*