[sudo-workers] Sudo 1.8.6p3 on an AD domain: excessive group data requests

[Todd C. Miller]

Starting with Sudo 1.8.2, groups are matched by name instead of
group ID.  This was done to reduce the load on network-based group
sources like AD.  By using the system_group plugin, you are reverting
to the old behavior which results in a lot more group lookups.

> The code has set_perms() called from several places.  The routine
> calls getgrouplist() which uses the system routine getgrent() to
> get successive group names.

The only actual call to getgrouplist() should be when sudo enumerates
the list of groups the runas user belongs to.  Group list lookups
in sudo are cached so it will only happen once.

> This causes a long delay before sudo finally gets around to using
> the system_group plugin to validate the group and the command
> argument is run.  (The man page for getgrent() says its use is
> discouraged as it is inefficient and not supported by all DB sources.)

Unfortunately, it is the only public way to enumerate the list of
all groups a user has access to without actually changing the
process's group list.  Sudo used to call initgroups() and then swap
things back with setgroups().  That may be better on systems like
Solaris where initgroups() has knowledge of the nss internals.
Removing the group setting will result in the command being run
with the invoking user's group list, not the runas user's.

Sudo could use the same internal function on Solaris that the system
initgroups function does.  Can you try the following patch?  It is
also available as:
    ftp://ftp.sudo.ws/pub/millert/sudo/solaris_getgrouplist.patch:

 - todd

diff -r c947aaef4880 compat/getgrouplist.c
--- a/compat/getgrouplist.c	Tue Jan 15 10:03:01 2013 -0500
+++ b/compat/getgrouplist.c	Fri Jan 18 14:13:44 2013 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 Todd C. Miller <Todd.Miller at courtesan.com>
+ * Copyright (c) 2010, 2011, 2013 Todd C. Miller <Todd.Miller at courtesan.com>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -36,7 +36,7 @@
 
 #include "missing.h"
 
-#ifdef HAVE_GETGRSET
+#if defined(HAVE_GETGRSET)
 /*
  * BSD-compatible getgrouplist(3) using getgrset(3)
  */
@@ -79,7 +79,32 @@
     return rval;
 }
 
-#else /* HAVE_GETGRSET */
+#elif defined(HAVE__GETGROUPSBYMEMBER)
+
+/*
+ * BSD-compatible getgrouplist(3) using _getgroupsbymember(3)
+ */
+int
+getgrouplist(const char *name, gid_t basegid, gid_t *groups, int *ngroupsp)
+{
+    int ngroups, grpsize = *ngroupsp;
+    int rval = -1;
+
+    if (grpsize > 0) {
+	/* We support BSD semantics where the first element is the base gid */
+	groups[0] = basegid;
+
+	/* The last arg is 1 because we already filled in the base gid. */
+	ngroups = _getgroupsbymember(name, groups, grpsize, 1);
+	if (ngroups != -1) {
+	    rval = 0;
+	    *ngroupsp = ngroups;
+	}
+    }
+    return rval;
+}
+
+#else /* !HAVE_GETGRSET && !HAVE__GETGROUPSBYMEMBER */
 
 /*
  * BSD-compatible getgrouplist(3) using getgrent(3)
@@ -128,4 +153,4 @@
 
     return rval;
 }
-#endif /* HAVE_GETGRSET */
+#endif /* !HAVE_GETGRSET && !HAVE__GETGROUPSBYMEMBER */
diff -r c947aaef4880 config.h.in
--- a/config.h.in	Tue Jan 15 10:03:01 2013 -0500
+++ b/config.h.in	Fri Jan 18 14:13:44 2013 -0500
@@ -700,6 +700,9 @@
 /* Define to 1 if the system has the type `_Bool'. */
 #undef HAVE__BOOL
 
+/* Define to 1 if you have the `_getgroupsbymember' function. */
+#undef HAVE__GETGROUPSBYMEMBER
+
 /* Define to 1 if you have the `_getpty' function. */
 #undef HAVE__GETPTY
 
diff -r c947aaef4880 configure
--- a/configure	Tue Jan 15 10:03:01 2013 -0500
+++ b/configure	Fri Jan 18 14:13:44 2013 -0500
@@ -13902,6 +13902,19 @@
 		# LD_PRELOAD is space-delimited
 		RTLD_PRELOAD_DELIM=" "
 
+		# For implementing getgrouplist()
+		for ac_func in _getgroupsbymember
+do :
+  ac_fn_c_check_func "$LINENO" "_getgroupsbymember" "ac_cv_func__getgroupsbymember"
+if test "x$ac_cv_func__getgroupsbymember" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE__GETGROUPSBYMEMBER 1
+_ACEOF
+
+fi
+done
+
+
 		# To get the crypt(3) prototype (so we pass -Wall)
 		OSDEFS="${OSDEFS} -D__EXTENSIONS__"
 		# AFS support needs -lucb
diff -r c947aaef4880 configure.in
--- a/configure.in	Tue Jan 15 10:03:01 2013 -0500
+++ b/configure.in	Fri Jan 18 14:13:44 2013 -0500
@@ -1551,6 +1551,9 @@
 		# LD_PRELOAD is space-delimited
 		RTLD_PRELOAD_DELIM=" "
 
+		# For implementing getgrouplist()
+		AC_CHECK_FUNCS(_getgroupsbymember)
+
 		# To get the crypt(3) prototype (so we pass -Wall)
 		OSDEFS="${OSDEFS} -D__EXTENSIONS__"
 		# AFS support needs -lucb