diod: improve supplementary group warning + comments

Problem: the code for setting supplemental groups under Linux seems
fragile at first glance.

It was carefully designed, but the code comments have not aged well.
Also, when the runtime check fails, the warning message is not very helpful.

Improve the warning message.
Improve the code comments.

src/cmd/diod.c

@@ -501,9 +501,15 @@ _service_sigsetup (void)
 }
 
 #if USE_IMPERSONATION_LINUX
-/* Test whether setgroups applies to thread or process.
- * On RHEL6 (2.6.32 based) it applies to threads and we can use it.
- * On Ubuntu 11 (2.6.38 based) it applies to the whole process and we can't.
+/* POSIX setgroups(2) is per process but in Linux the underlying system call
+ * is per-thread and the per-process bit is handled in glibc, so we can use
+ * SYS_setgroups directly in the server thread pool when switching users.
+ * This assumption is tenuous though, so we should quickly check it during
+ * server startup, in case a future kernel update invalidates it.
+ *
+ * If we can't use it, a warning is issued and life goes on with group access
+ * checks based on the user's primary group (e.g. the login one or the one
+ * selected by newgrp(1)).
  */
 void *
 _test_setgroups_thread (void *arg)
@@ -626,8 +632,10 @@ _service_run (srvmode_t mode, int rfdno, int wfdno)
 #if USE_IMPERSONATION_LINUX
         if (_test_setgroups ())
             flags |= SRV_FLAGS_SETGROUPS;
-        else
-            msg ("test_setgroups: groups are per-process (disabling)");
+        else {
+            msg ("warning: supplemental group membership will be ignored."
+                "  Some accesses might be inappropriately denied.");
+        }
 #elif USE_IMPERSONATION_GANESHA
         if (init_ganesha_syscalls() < 0)
             msg ("nfs-ganesha-kmod not loaded: changing user/group will fail");

src/libnpfs/user-linux.c

@@ -160,11 +160,23 @@ np_setfsid (Npreq *req, Npuser *u, u32 gid_override)
 			else if (wt->fsuid == 0)
 				wt->privcap = 0; /* trans from 0 clears caps */
 
-			/* Suppl groups need to be part of cred for NFS
-			 * forwarding even with DAC_BYPASS.  However only
-			 * do it if kernel treats sg's per-thread not process.
-			 * Addendum: late model glibc attempts to make this
-			 * per-process, so for now bypass glibc. See issue 53.
+			/* Set the user's supplementary groups (as read from the host
+			 * system's group configuration), but only if it can be
+			 * done safely.
+			 *
+			 * setgroups(2) is per-process (POSIX), which is incompatible
+			 * with the npfs thread pool model.  SYS_setgroups, the
+			 * direct system call as opposed to the libc wrapper, is
+			 * per-thread, but relying on that is fragile.  Therefore,
+			 * require the server to attest that SYS_setgroups is per-
+			 * thread on the server host kernel by setting a flag, e.g.
+			 * based on a test at startup.  If the flag is unset, access
+			 * is determined solely by the user's uid and primary gid.
+			 *
+			 * Even with DAC_BYPASS, the supplementary groups might
+			 * into play if the host file system is NFS, since the
+			 * supplementary groups are still checked on the NFS server.
+			 * Unlike 9P, NFS transmits them over the wire.
 			 */
 			if ((srv->flags & SRV_FLAGS_SETGROUPS)) {
 				if (syscall(SYS_setgroups, u->nsg, u->sg) < 0) {