Add group, ambient and bound setting support to pam_cap.

Rewrote the pam_cap config file parsing to support:

  - @group syntax for identifying groups of users
  - ^cap_foo support for raising both inheritable and ambient caps
  - !cap_bar support for dropping bounding capabilities

Updated documentation for pre-existing libcap's ambient support.

This pam_cap feature upgrade was done in collaboration with
Knut Omang and Christoph Lameter.

Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

10 files changed, 556 insertions, 113 deletions

diff --git a/Makefile b/Makefile
index b52ae03..14e29ef 100644
--- a/Makefile
+++ b/Makefile
@@ -35,18 +35,24 @@ release: distclean
 	cd .. && ln -s libcap libcap-$(VERSION).$(MINOR) && tar cvf libcap-$(VERSION).$(MINOR).tar --exclude patches libcap-$(VERSION).$(MINOR)/* && rm libcap-$(VERSION).$(MINOR)
 
 test: all
-	make -C tests test
+	make -C tests $@
+ifneq ($(PAM_CAP),no)
+	$(MAKE) -C pam_cap $@
+endif
 ifeq ($(GOLANG),yes)
-	make -C go test
+	make -C go $@
 endif
-	make -C progs test
+	make -C progs $@
 
 sudotest: all
-	make -C tests sudotest
+	make -C tests $@
+ifneq ($(PAM_CAP),no)
+	$(MAKE) -C pam_cap $@
+endif
 ifeq ($(GOLANG),yes)
-	make -C go sudotest
+	make -C go $@
 endif
-	make -C progs sudotest
+	make -C progs $@
 
 morganrelease: distclean
 	@echo "sign the tag twice: older DSA key; and newer RSA kernel.org key"
diff --git a/doc/cap_get_ambient.3 b/doc/cap_get_ambient.3
new file mode 100644
index 0000000..65ea3e4
--- /dev/null
+++ b/doc/cap_get_ambient.3
@@ -0,0 +1 @@
+.so man3/cap_get_proc.3
diff --git a/doc/cap_get_proc.3 b/doc/cap_get_proc.3
index ed87fb7..712b3ff 100644
--- a/doc/cap_get_proc.3
+++ b/doc/cap_get_proc.3
@@ -1,6 +1,7 @@
-.TH CAP_GET_PROC 3 "2019-12-14" "" "Linux Programmer's Manual"
+.TH CAP_GET_PROC 3 "2019-12-21" "" "Linux Programmer's Manual"
 .SH NAME
 cap_get_proc, cap_set_proc, capgetp, cap_get_bound, cap_drop_bound, \
+cap_get_ambient, cap_set_ambient, cap_reset_ambient, \
 cap_get_secbits, cap_set_secbits, cap_get_mode, cap_set_mode, \
 cap_mode_name, cap_get_pid, cap_setuid, cap_setgroups \
 \- capability manipulation on processes
@@ -17,6 +18,14 @@ cap_mode_name, cap_get_pid, cap_setuid, cap_setgroups \
 .sp
 .BI "int cap_drop_bound(cap_value_t " cap );
 .sp
+.BI "int cap_get_ambient(cap_value_t " cap );
+.sp
+.BI "int cap_set_ambient(cap_value_t " cap ", cap_flag_value_t " value );
+.sp
+.B int cap_reset_ambient(void);
+.sp
+.BI CAP_AMBIENT_SUPPORTED();
+.sp
 .B "unsigned cap_get_secbits(void);"
 .sp
 .BI "int cap_set_secbits(unsigned " bits );
@@ -89,16 +98,43 @@ If the system does not support the capability, this function returns
 .PP
 .BR cap_drop_bound ()
 can be used to lower the specified bounding set capability,
-.BR cap ,
+.BR cap .
 To complete successfully, the prevailing
 .I effective
 capability set must have a raised
 .BR CAP_SETPCAP .
 .PP
+.BR cap_get_ambient ()
+returns the prevailing value of the specified ambient capability, or
+-1 if the capability is not supported by the running kernel.  A macro
+.BR CAP_AMBIENT_SUPPORTED ()
+uses this function to determine if ambient capabilities are supported
+by the kernel.
+.PP
+.BR cap_set_ambient ()
+sets the specified ambient capability to a specific value. To complete
+successfully, the prevailing
+.I effective
+capability set must have a raised
+.BR CAP_SETPCAP .
+.PP
+.BR cap_reset_ambient ()
+resets all of the ambient capabilities for the current process to
+their lowered value. To complete successfully, the prevailing
+.I effective
+capability set must have a raised
+.BR CAP_SETPCAP .
+Note, the ambient set is intended to operate in a legacy environment
+where the application has limited awareness of capabilities in
+general. Executing a file with associated filesystem capabilities, the
+kernel will implicitly reset the ambient set of the process. Also,
+changes to the inheritable set by the program code without explicitly
+fixing up the ambient set can also drop ambient bits.
+.PP
 .BR cap_get_secbits ()
-returns the securebits of the current process. These bits affect the way
-in which the current process implements things like setuid-root fixup and
-ambient capabilities.
+returns the securebits of the current process. These bits affect the
+way in which the current process implements things like setuid-root
+fixup and ambient capabilities.
 .PP
 .BR cap_set_secbits ()
 attempts to modify the securebits of the current process. Note
diff --git a/doc/cap_reset_ambient.3 b/doc/cap_reset_ambient.3
new file mode 100644
index 0000000..65ea3e4
--- /dev/null
+++ b/doc/cap_reset_ambient.3
@@ -0,0 +1 @@
+.so man3/cap_get_proc.3
diff --git a/doc/cap_set_ambient.3 b/doc/cap_set_ambient.3
new file mode 100644
index 0000000..65ea3e4
--- /dev/null
+++ b/doc/cap_set_ambient.3
@@ -0,0 +1 @@
+.so man3/cap_get_proc.3
diff --git a/pam_cap/.gitignore b/pam_cap/.gitignore
index 11806f5..05e9bbf 100644
--- a/pam_cap/.gitignore
+++ b/pam_cap/.gitignore
@@ -1,2 +1,3 @@
 pam_cap.so
-testcompile
+testlink
+test_pam_cap
diff --git a/pam_cap/Makefile b/pam_cap/Makefile
index 22f0f81..56604fd 100644
--- a/pam_cap/Makefile
+++ b/pam_cap/Makefile
@@ -4,7 +4,7 @@ topdir=$(shell pwd)/..
 include ../Make.Rules
 
 all: pam_cap.so
-	$(MAKE) testcompile
+	$(MAKE) testlink
 
 install: all
 	mkdir -p -m 0755 $(FAKEROOT)$(LIBDIR)/security
@@ -21,8 +21,23 @@ pam_cap.so: pam_cap.o
 pam_cap.o: pam_cap.c
 	$(CC) $(CFLAGS) $(IPATH) -c $< -o $@
 
-testcompile: test.c pam_cap.o
+test_pam_cap: test_pam_cap.c pam_cap.c
+	$(CC) $(CFLAGS) $(IPATH) -o $@ test_pam_cap.c $(LIBCAPLIB) $(LDFLAGS) --static
+
+testlink: test.c pam_cap.o
 	$(CC) $(CFLAGS) -o $@ $+ -lpam -ldl $(LIBCAPLIB) $(LDFLAGS)
 
+test: pam_cap.so
+	make testlink
+
+sudotest: test test_pam_cap
+	sudo ./test_pam_cap root 0x0 0x0 0x0 config=./capability.conf
+	sudo ./test_pam_cap root 0x0 0x0 0x0 config=./sudotest.conf
+	sudo ./test_pam_cap alpha 0x0 0x0 0x0 config=./capability.conf
+	sudo ./test_pam_cap alpha 0x0 0x1 0x80 config=./sudotest.conf
+	sudo ./test_pam_cap beta 0x0 0x1 0x0 config=./sudotest.conf
+	sudo ./test_pam_cap gamma 0x0 0x0 0x81 config=./sudotest.conf
+	sudo ./test_pam_cap delta 0x41 0x80 0x41 config=./sudotest.conf
+
 clean:
-	rm -f *.o *.so testcompile *~
+	rm -f *.o *.so testlink test_pam_cap *~
diff --git a/pam_cap/pam_cap.c b/pam_cap/pam_cap.c
index b1cc5cb..58ffe4a 100644
--- a/pam_cap/pam_cap.c
+++ b/pam_cap/pam_cap.c
@@ -1,20 +1,23 @@
 /*
- * Copyright (c) 1999,2007 Andrew G. Morgan <morgan@kernel.org>
+ * Copyright (c) 1999,2007,2019 Andrew G. Morgan <morgan@kernel.org>
  *
- * The purpose of this module is to enforce inheritable capability sets
- * for a specified user.
+ * The purpose of this module is to enforce inheritable, bounding and
+ * ambient capability sets for a specified user.
  */
 
 /* #define DEBUG */
 
-#include <stdio.h>
-#include <string.h>
 #include <errno.h>
+#include <grp.h>
+#include <limits.h>
+#include <pwd.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
 #include <syslog.h>
-
 #include <sys/capability.h>
+#include <sys/types.h>
 
 #include <security/pam_modules.h>
 #include <security/_pam_macros.h>
@@ -22,8 +25,6 @@
 #define USER_CAP_FILE           "/etc/security/capability.conf"
 #define CAP_FILE_BUFFER_SIZE    4096
 #define CAP_FILE_DELIMITERS     " \t\n"
-#define CAP_COMBINED_FORMAT     "%s all-i %s+i"
-#define CAP_DROP_ALL            "%s all-i"
 
 struct pam_cap_s {
     int debug;
@@ -31,25 +32,71 @@ struct pam_cap_s {
     const char *conf_filename;
 };
 
+/*
+ * load_groups obtains the list all of the groups associated with the
+ * requested user: gid & supplemental groups.
+ */
+static int load_groups(const char *user, char ***groups, int *groups_n) {
+    struct passwd *pwd;
+    gid_t grps[NGROUPS_MAX];
+    int ngrps = NGROUPS_MAX;
+
+    *groups = NULL;
+    *groups_n = 0;
+
+    pwd = getpwnam(user);
+    if (pwd == NULL) {
+	return -1;
+    }
+
+    /* must include at least pwd->pw_gid, hence < 1 test. */
+    if (getgrouplist(user, pwd->pw_gid, grps, &ngrps) < 1) {
+	return -1;
+    }
+
+    *groups = calloc(ngrps, sizeof(char *));
+    int g_n = 0;
+    for (int i = 0; i < ngrps; i++) {
+	const struct group *g = getgrgid(grps[i]);
+	if (g == NULL) {
+	    continue;
+	}
+	D(("noting [%s] is a member of [%s]", user, g->gr_name));
+	(*groups)[g_n++] = strdup(g->gr_name);
+    }
+
+    *groups_n = g_n;
+    return 0;
+}
+
 /* obtain the inheritable capabilities for the current user */
 
 static char *read_capabilities_for_user(const char *user, const char *source)
 {
     char *cap_string = NULL;
     char buffer[CAP_FILE_BUFFER_SIZE], *line;
+    char **groups;
+    int groups_n;
     FILE *cap_file;
 
+    if (load_groups(user, &groups, &groups_n)) {
+	D(("unknown user [%s]", user));
+	return NULL;
+    }
+
     cap_file = fopen(source, "r");
     if (cap_file == NULL) {
 	D(("failed to open capability file"));
-	return NULL;
+	goto defer;
     }
 
-    while ((line = fgets(buffer, CAP_FILE_BUFFER_SIZE, cap_file))) {
-	int found_one = 0;
+    int found_one = 0;
+    while (!found_one &&
+	   (line = fgets(buffer, CAP_FILE_BUFFER_SIZE, cap_file))) {
 	const char *cap_text;
 
-	cap_text = strtok(line, CAP_FILE_DELIMITERS);
+	char *next = NULL;
+	cap_text = strtok_r(line, CAP_FILE_DELIMITERS, &next);
 
 	if (cap_text == NULL) {
 	    D(("empty line"));
@@ -60,38 +107,63 @@ static char *read_capabilities_for_user(const char *user, const char *source)
 	    continue;
 	}
 
-	while ((line = strtok(NULL, CAP_FILE_DELIMITERS))) {
-
+	/*
+	 * Explore whether any of the ids are a match for the current
+	 * user.
+	 */
+	while ((line = strtok_r(next, CAP_FILE_DELIMITERS, &next))) {
 	    if (strcmp("*", line) == 0) {
 		D(("wildcard matched"));
 		found_one = 1;
-		cap_string = strdup(cap_text);
 		break;
 	    }
 
 	    if (strcmp(user, line) == 0) {
 		D(("exact match for user"));
 		found_one = 1;
-		cap_string = strdup(cap_text);
 		break;
 	    }
 
-	    D(("user is not [%s] - skipping", line));
-	}
+	    if (line[0] != '@') {
+		D(("user [%s] is not [%s] - skipping", user, line));
+	    }
 
-	cap_text = NULL;
-	line = NULL;
+	    for (int i=0; i < groups_n; i++) {
+		if (!strcmp(groups[i], line+1)) {
+		    D(("user group matched [%s]", line));
+		    found_one = 1;
+		    break;
+		}
+	    }
+	    if (found_one) {
+		break;
+	    }
+	}
 
 	if (found_one) {
+	    cap_string = strdup(cap_text);
 	    D(("user [%s] matched - caps are [%s]", user, cap_string));
-	    break;
 	}
+
+	cap_text = NULL;
+	line = NULL;
     }
 
     fclose(cap_file);
 
+defer:
     memset(buffer, 0, CAP_FILE_BUFFER_SIZE);
 
+    for (int i = 0; i < groups_n; i++) {
+	char *g = groups[i];
+	_pam_overwrite(g);
+	_pam_drop(g);
+    }
+    if (groups != NULL) {
+	memset(groups, 0, groups_n * sizeof(char *));
+	_pam_drop(groups);
+    }
+
     return cap_string;
 }
 
@@ -100,15 +172,16 @@ static char *read_capabilities_for_user(const char *user, const char *source)
  * permitted+executable sets combined with the configured inheritable
  * set.
  */
-
 static int set_capabilities(struct pam_cap_s *cs)
 {
     cap_t cap_s;
-    ssize_t length = 0;
-    char *conf_icaps;
-    char *proc_epcaps;
-    char *combined_caps;
+    char *conf_caps;
     int ok = 0;
+    int has_ambient = 0, has_bound = 0;
+    int *bound = NULL, *ambient = NULL;
+    cap_flag_value_t had_setpcap = 0;
+    cap_value_t max_caps = 0;
+    const cap_value_t wanted_caps[] = { CAP_SETPCAP };
 
     cap_s = cap_get_proc();
     if (cap_s == NULL) {
@@ -116,82 +189,170 @@ static int set_capabilities(struct pam_cap_s *cs)
 	   strerror(errno)));
 	return 0;
     }
+    if (cap_get_flag(cap_s, CAP_SETPCAP, CAP_EFFECTIVE, &had_setpcap)) {
+	D(("failed to read a e capability: %s", strerror(errno)));
+	goto cleanup_cap_s;
+    }
+    if (cap_set_flag(cap_s, CAP_EFFECTIVE, 1, wanted_caps, CAP_SET) != 0) {
+	D(("unable to raise CAP_SETPCAP: %s", strerrno(errno)));
+	goto cleanup_cap_s;
+    }
 
-    conf_icaps =
-	read_capabilities_for_user(cs->user,
-				   cs->conf_filename
-				   ? cs->conf_filename:USER_CAP_FILE );
-    if (conf_icaps == NULL) {
+    conf_caps =	read_capabilities_for_user(cs->user,
+					   cs->conf_filename
+					   ? cs->conf_filename:USER_CAP_FILE );
+    if (conf_caps == NULL) {
 	D(("no capabilities found for user [%s]", cs->user));
 	goto cleanup_cap_s;
     }
 
-    proc_epcaps = cap_to_text(cap_s, &length);
-    if (proc_epcaps == NULL) {
-	D(("unable to convert process capabilities to text"));
-	goto cleanup_icaps;
+    ssize_t conf_caps_length = strlen(conf_caps);
+    if (!strcmp(conf_caps, "all")) {
+	/*
+	 * all here is interpreted as no change/pass through, which is
+	 * likely to be the same as none for sensible system defaults.
+	 */
+	ok = 1;
+	goto cleanup_caps;
     }
 
-    /*
-     * This is a pretty inefficient way to combine
-     * capabilities. However, it seems to be the most straightforward
-     * one, given the limitations of the POSIX.1e draft spec. The spec
-     * is optimized for applications that know the capabilities they
-     * want to manipulate at compile time.
-     */
-
-    combined_caps = malloc(1+strlen(CAP_COMBINED_FORMAT)
-			   +strlen(proc_epcaps)+strlen(conf_icaps));
-    if (combined_caps == NULL) {
-	D(("unable to combine capabilities into one string - no memory"));
-	goto cleanup_epcaps;
+    if (cap_set_proc(cap_s) != 0) {
+	D(("unable to use CAP_SETPCAP: %s", strerrno(errno)));
+	goto cleanup_caps;
+    }
+    if (cap_reset_ambient() == 0) {
+	// Ambient set fully declared by this config.
+	has_ambient = 1;
     }
 
-    if (!strcmp(conf_icaps, "none")) {
-	sprintf(combined_caps, CAP_DROP_ALL, proc_epcaps);
-    } else if (!strcmp(conf_icaps, "all")) {
-	/* no change */
-	sprintf(combined_caps, "%s", proc_epcaps);
+    if (!strcmp(conf_caps, "none")) {
+	/* clearing CAP_INHERITABLE will also clear the ambient caps. */
+	cap_clear_flag(cap_s, CAP_INHERITABLE);
     } else {
-	sprintf(combined_caps, CAP_COMBINED_FORMAT, proc_epcaps, conf_icaps);
-    }
-    D(("combined_caps=[%s]", combined_caps));
+	/*
+	 * we know we have to perform some capability operations and
+	 * we need to know how many capabilities there are to do it
+	 * successfully.
+	 */
+	while (cap_get_bound(max_caps) >= 0) {
+	    max_caps++;
+	}
+	has_bound = (max_caps != 0);
+	if (has_bound) {
+	    bound = calloc(max_caps, sizeof(int));
+	    if (has_ambient) {
+		// In kernel lineage, bound came first.
+		ambient = calloc(max_caps, sizeof(int));
+	    }
+	}
+
+	/*
+	 * Scan the configured capability string for:
+	 *
+	 *   cap_name: add to cap_s' inheritable vector
+	 *   ^cap_name: add to cap_s' inheritable vector and ambient set
+	 *   !cap_name: drop from bounding set
+	 *
+	 * Setting ambient capabilities requires that we first enable
+	 * the corresponding inheritable capability to set them. So,
+	 * there is an order we use: parse the config line, building
+	 * the inheritable, ambient and bounding sets in three separate
+	 * arrays. Then, set I set A set B. Finally, at the end, we
+	 * restore the E value for CAP_SETPCAP.
+	 */
+	char *token = NULL;
+	char *next = conf_caps;
+	while ((token = strtok_r(next, ",", &next))) {
+	    if (strlen(token) < 4) {
+		D(("bogus cap: [%s] - ignored\n", token));
+		goto cleanup_caps;
+	    }
+	    int is_a = 0, is_b = 0;
+	    if (*token == '^') {
+		if (!has_ambient) {
+		    D(("want ambient [%s] but kernel has no support", token));
+		    goto cleanup_caps;
+		}
+		is_a = 1;
+		token++;
+	    } else if (*token == '!') {
+		if (!has_bound) {
+		    D(("want bound [%s] dropped - no kernel support", token));
+		}
+		is_b = 1;
+		token++;
+	    }
+
+	    cap_value_t c;
+	    if (cap_from_name(token, &c) != 0) {
+		D(("unrecognized name [%s]: %s - ignored", token,
+		   strerror(errno)));
+		goto cleanup_caps;
+	    }
 
-    cap_free(cap_s);
-    cap_s = cap_from_text(combined_caps);
-    _pam_overwrite(combined_caps);
-    _pam_drop(combined_caps);
+	    if (is_b) {
+		bound[c] = 1;
+	    } else {
+		if (cap_set_flag(cap_s, CAP_INHERITABLE, 1, &c, CAP_SET)) {
+		    D(("failed to raise inheritable [%s]: %s", token,
+		       strerror(errno)));
+		    goto cleanup_caps;
+		}
+		if (is_a) {
+		    ambient[c] = 1;
+		}
+	    }
+	}
 
 #ifdef DEBUG
-    {
-        char *temp = cap_to_text(cap_s, NULL);
-	D(("abbreviated caps for process will be [%s]", temp));
-	cap_free(temp);
-    }
+	{
+	    char *temp = cap_to_text(cap_s, NULL);
+	    D(("abbreviated caps for process will be [%s]", temp));
+	    cap_free(temp);
+	}
 #endif /* DEBUG */
+    }
 
-    if (cap_s == NULL) {
-	D(("no capabilies to set"));
-    } else if (cap_set_proc(cap_s) == 0) {
-	D(("capabilities were set correctly"));
-	ok = 1;
-    } else {
+    if (cap_set_proc(cap_s)) {
 	D(("failed to set specified capabilities: %s", strerror(errno)));
+    } else {
+	for (cap_value_t c = 0; c < max_caps; c++) {
+	    if (ambient != NULL && ambient[c]) {
+		cap_set_ambient(c, CAP_SET);
+	    }
+	    if (bound != NULL && bound[c]) {
+		cap_drop_bound(c);
+	    }
+	}
+	ok = 1;
     }
 
-cleanup_epcaps:
-    cap_free(proc_epcaps);
-
-cleanup_icaps:
-    _pam_overwrite(conf_icaps);
-    _pam_drop(conf_icaps);
+cleanup_caps:
+    if (has_ambient) {
+	memset(ambient, 0, max_caps * sizeof(*ambient));
+	_pam_drop(ambient);
+	ambient = NULL;
+    }
+    if (has_bound) {
+	memset(bound, 0, max_caps * sizeof(*bound));
+	_pam_drop(bound);
+	bound = NULL;
+    }
+    memset(conf_caps, 0, conf_caps_length);
+    _pam_drop(conf_caps);
 
 cleanup_cap_s:
+    if (!had_setpcap) {
+	/* Only need to lower if it wasn't raised by caller */
+	if (!cap_set_flag(cap_s, CAP_EFFECTIVE, 1, wanted_caps,
+			  CAP_CLEAR)) {
+	    cap_set_proc(cap_s);
+	}
+    }
     if (cap_s) {
 	cap_free(cap_s);
 	cap_s = NULL;
     }
-
     return ok;
 }
 
@@ -210,11 +371,8 @@ static void _pam_log(int err, const char *format, ...)
 
 static void parse_args(int argc, const char **argv, struct pam_cap_s *pcs)
 {
-    int ctrl=0;
-
     /* step through arguments */
-    for (ctrl=0; argc-- > 0; ++argv) {
-
+    for (; argc-- > 0; ++argv) {
 	if (!strcmp(*argv, "debug")) {
 	    pcs->debug = 1;
 	} else if (!strncmp(*argv, "config=", 7)) {
@@ -222,23 +380,25 @@ static void parse_args(int argc, const char **argv, struct pam_cap_s *pcs)
 	} else {
 	    _pam_log(LOG_ERR, "unknown option; %s", *argv);
 	}
-
     }
 }
 
+/*
+ * pam_sm_authenticate parses the config file with respect to the user
+ * being authenticated and determines if they are covered by any
+ * capability inheritance rules.
+ */
 int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 			int argc, const char **argv)
 {
     int retval;
     struct pam_cap_s pcs;
-    char *conf_icaps;
+    char *conf_caps;
 
     memset(&pcs, 0, sizeof(pcs));
-
     parse_args(argc, argv, &pcs);
 
     retval = pam_get_user(pamh, &pcs.user, NULL);
-
     if (retval == PAM_CONV_AGAIN) {
 	D(("user conversation is not available yet"));
 	memset(&pcs, 0, sizeof(pcs));
@@ -251,24 +411,22 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	return PAM_AUTH_ERR;
     }
 
-    conf_icaps =
-	read_capabilities_for_user(pcs.user,
-				   pcs.conf_filename
-				   ? pcs.conf_filename:USER_CAP_FILE );
-
+    conf_caps =	read_capabilities_for_user(pcs.user,
+					   pcs.conf_filename
+					   ? pcs.conf_filename:USER_CAP_FILE );
     memset(&pcs, 0, sizeof(pcs));
 
-    if (conf_icaps) {
+    if (conf_caps) {
 	D(("it appears that there are capabilities for this user [%s]",
-	   conf_icaps));
+	   conf_caps));
 
 	/* We could also store this as a pam_[gs]et_data item for use
 	   by the setcred call to follow. As it is, there is a small
 	   race associated with a redundant read. Oh well, if you
 	   care, send me a patch.. */
 
-	_pam_overwrite(conf_icaps);
-	_pam_drop(conf_icaps);
+	_pam_overwrite(conf_caps);
+	_pam_drop(conf_caps);
 
 	return PAM_SUCCESS;
 
@@ -280,6 +438,10 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
     }
 }
 
+/*
+ * pam_sm_setcred applies inheritable capabilities loaded by the
+ * pam_sm_authenticate pass for the user.
+ */
 int pam_sm_setcred(pam_handle_t *pamh, int flags,
 		   int argc, const char **argv)
 {
@@ -292,18 +454,15 @@ int pam_sm_setcred(pam_handle_t *pamh, int flags,
     }
 
     memset(&pcs, 0, sizeof(pcs));
-
     parse_args(argc, argv, &pcs);
 
     retval = pam_get_item(pamh, PAM_USER, (const void **)&pcs.user);
     if ((retval != PAM_SUCCESS) || (pcs.user == NULL) || !(pcs.user[0])) {
-
 	D(("user's name is not set"));
 	return PAM_AUTH_ERR;
     }
 
     retval = set_capabilities(&pcs);
-
     memset(&pcs, 0, sizeof(pcs));
 
     return (retval ? PAM_SUCCESS:PAM_IGNORE );
diff --git a/pam_cap/sudotest.conf b/pam_cap/sudotest.conf
new file mode 100644
index 0000000..ff528ce
--- /dev/null
+++ b/pam_cap/sudotest.conf
@@ -0,0 +1,23 @@
+# only root
+all                                root
+
+# this should fire for beta only
+!cap_chown                         beta
+
+# the next one should snag gamma since beta done
+cap_setuid,cap_chown               @three
+
+# neither of these should fire
+cap_chown                          beta gamma
+
+# just alpha
+!cap_chown,cap_setuid              @one
+
+# not this one
+^cap_setuid                        alpha
+
+# this should fire
+^cap_chown,^cap_setgid,!cap_setuid delta
+
+# not this one
+cap_setuid                         @four
diff --git a/pam_cap/test_pam_cap.c b/pam_cap/test_pam_cap.c
new file mode 100644
index 0000000..2f519f1
--- /dev/null
+++ b/pam_cap/test_pam_cap.c
@@ -0,0 +1,200 @@
+/*
+ * Copyright (c) 2019 Andrew G. Morgan <morgan@kernel.org>
+ *
+ * This test inlines the pam_cap module and runs test vectors against
+ * it.
+ */
+
+#include "./pam_cap.c"
+
+const char *test_groups[] = {
+    "root", "one", "two", "three", "four", "five", "six", "seven"
+};
+#define n_groups sizeof(test_groups)/sizeof(*test_groups)
+
+const char *test_users[] = {
+    "root", "alpha", "beta", "gamma", "delta"
+};
+#define n_users sizeof(test_users)/sizeof(*test_users)
+
+// Note about memberships:
+//
+//   user gid   suppl groups
+//  root  root
+//  alpha one   two
+//  beta  two   three four
+//  gamma three four five six
+//  delta four  five six seven [eight]
+//
+
+static char *test_user;
+
+int pam_get_user(pam_handle_t *pamh, const char **user, const char *prompt) {
+    *user = test_user;
+    if (*user == NULL) {
+	return PAM_CONV_AGAIN;
+    }
+    return PAM_SUCCESS;
+}
+
+int pam_get_item(const pam_handle_t *pamh, int item_type, const void **item) {
+    if (item_type != PAM_USER) {
+	errno = EINVAL;
+	return -1;
+    }
+    *item = test_user;
+    return 0;
+}
+
+int getgrouplist(const char *user, gid_t group, gid_t *groups, int *ngroups) {
+    int i,j;
+    for (i = 0; i < n_users; i++) {
+	if (strcmp(user, test_users[i]) == 0) {
+	    *ngroups = i+1;
+	    break;
+	}
+    }
+    if (i == n_users) {
+	return -1;
+    }
+    groups[0] = i;
+    for (j = 1; j < *ngroups; j++) {
+	groups[j] = i+j;
+    }
+    return *ngroups;
+}
+
+static struct group gr;
+struct group *getgrgid(gid_t gid) {
+    if (gid >= n_groups) {
+	errno = EINVAL;
+	return NULL;
+    }
+    gr.gr_name = strdup(test_groups[gid]);
+    return &gr;
+}
+
+static struct passwd pw;
+struct passwd *getpwnam(const char *name) {
+    for (int i = 0; i < n_users; i++) {
+	if (strcmp(name, test_users[i]) == 0) {
+	    pw.pw_gid = i;
+	    return &pw;
+	}
+    }
+    return NULL;
+}
+
+/* we'll use these to keep track of the three vectors - only use
+   lowest 64 bits */
+
+#define A 0
+#define B 1
+#define I 2
+
+/*
+ * load_vectors caches a copy of the lowest 64 bits of the inheritable
+ * cap vectors
+ */
+static void load_vectors(unsigned long int bits[3]) {
+    memset(bits, 0, 3*sizeof(unsigned long int));
+    cap_t prev = cap_get_proc();
+    for (int i = 0; i < 64; i++) {
+	unsigned long int mask = (1ULL << i);
+	int v = cap_get_bound(i);
+	if (v < 0) {
+	    break;
+	}
+	bits[B] |= v ? mask : 0;
+	cap_flag_value_t u;
+	if (cap_get_flag(prev, i, CAP_INHERITABLE, &u) != 0) {
+	    break;
+	}
+	bits[I] |= u ? mask : 0;
+	v = cap_get_ambient(i);
+	if (v > 0) {
+	    bits[A] |= mask;
+	}
+    }
+    cap_free(prev);
+}
+
+/*
+ * args: user a b i config-args...
+ */
+int main(int argc, char *argv[]) {
+    unsigned long int before[3], change[3], after[3];
+
+    /*
+     * Start out with a cleared inheritable set.
+     */
+    cap_t orig = cap_get_proc();
+    cap_clear_flag(orig, CAP_INHERITABLE);
+    cap_set_proc(orig);
+
+    change[A] = strtoul(argv[2], NULL, 0);
+    change[B] = strtoul(argv[3], NULL, 0);
+    change[I] = strtoul(argv[4], NULL, 0);
+
+    void* args_for_pam = argv+4;
+
+    int status = pam_sm_authenticate(NULL, 0, argc-4,
+				     (const char **) args_for_pam);
+    if (status != PAM_INCOMPLETE) {
+	printf("failed to recognize no username\n");
+	exit(1);
+    }
+
+    test_user = argv[1];
+
+    status = pam_sm_authenticate(NULL, 0, argc-4, (const char **) args_for_pam);
+    if (status == PAM_IGNORE) {
+	if (strcmp(test_user, "root") == 0) {
+	    exit(0);
+	}
+	printf("unconfigured non-root user: %s\n", test_user);
+	exit(1);
+    }
+    if (status != PAM_SUCCESS) {
+	printf("failed to recognize username\n");
+	exit(1);
+    }
+
+    // Now it is time to execute the credential setting
+    load_vectors(before);
+
+    status = pam_sm_setcred(NULL, PAM_ESTABLISH_CRED, argc-4,
+			    (const char **) args_for_pam);
+
+    load_vectors(after);
+
+    printf("before: A=0x%016lx B=0x%016lx I=0x%016lx\n",
+	   before[A], before[B], before[I]);
+
+    long unsigned int dA = before[A] ^ after[A];
+    long unsigned int dB = before[B] ^ after[B];
+    long unsigned int dI = before[I] ^ after[I];
+
+    printf("diff  : A=0x%016lx B=0x%016lx I=0x%016lx\n", dA, dB, dI);
+    printf("after : A=0x%016lx B=0x%016lx I=0x%016lx\n",
+	   after[A], after[B], after[I]);
+
+    int failure = 0;
+    if (after[A] != change[A]) {
+	printf("Ambient set error: got=0x%016lx, want=0x%016lx\n",
+	       after[A], change[A]);
+	failure = 1;
+    }
+    if (dB != change[B]) {
+	printf("Bounding set error: got=0x%016lx, want=0x%016lx\n",
+	       after[B], before[B] ^ change[B]);
+	failure = 1;
+    }
+    if (after[I] != change[I]) {
+	printf("Inheritable set error: got=0x%016lx, want=0x%016lx\n",
+	       after[I], change[I]);
+	failure = 1;
+    }
+
+    exit(failure);
+}