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 <chuck.lever@oracle.com>
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/src/auth_unix.c b/src/auth_unix.c
index ddd89cc649d6c5de7760e663e3722659c17557dd..c2469daff8d6476ee68e35622c1b211d4ce8e726 100644 (file)
--- 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;
 }
 
 /*