Another attempt at supporting Ambient vector setting from pam_cap.so.

While the session idea worked with contrib/sucap/su.c, it failed on
more traditional PAM apps. For a second (likely last) attempt to find a
path, I've deleted the session support and now attempt to do the setting
via a PAM data item cleanup() callback. In the contrib/sucap/su.c code,
evolved from the original SimplePAMApps 'su', there is a

   pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT)

from within the fork()d launcher code, so I hope this convention is
standard for all the PAM apps that came after.

The suggested config for this module for an app, that wants to support
the Ambient vector, is thus now:

    #%PAM-1.0
    auth            required pam_cap.so keepcaps defer
    auth            required pam_unix.so
    account         required pam_unix.so
    password        required pam_unix.so
    session         required pam_unix.so

This is all part of an effort to address:

   https://bugzilla.kernel.org/show_bug.cgi?id=214377

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

3 files changed, 55 insertions, 60 deletions

diff --git a/pam_cap/execable.c b/pam_cap/execable.c
index 8e5028d..f826a57 100644
--- a/pam_cap/execable.c
+++ b/pam_cap/execable.c
@@ -49,6 +49,6 @@ SO_MAIN(int argc, char **argv)
 	   "keepcaps      - workaround for apps that setuid without this\n"
 	   "autoauth      - pam_cap.so to always succeed for the 'auth' phase\n"
 	   "default=<iab> - fallback IAB value if there is no '*' rule\n"
-	   "use_session   - apply IAB value during session open, not setcred\n",
+	   "defer         - apply IAB value at pam_exit (not via setcred)\n",
 	cmd);
 }
diff --git a/pam_cap/pam_cap.c b/pam_cap/pam_cap.c
index 80eb40b..2538b35 100644
--- a/pam_cap/pam_cap.c
+++ b/pam_cap/pam_cap.c
@@ -39,17 +39,14 @@ struct pam_cap_s {
     int debug;
     int keepcaps;
     int autoauth;
-    int session;
+    int defer;
     const char *user;
     const char *conf_filename;
     const char *fallback;
+    pam_handle_t *pamh;
 };
 
 /*
- * pam_cap_iab_s is used to capture any cap_iab_t value if it needs to be applied during pam_sm_
- */
-
-/*
  * load_groups obtains the list all of the groups associated with the
  * requested user: gid & supplemental groups.
  */
@@ -190,6 +187,33 @@ defer:
 }
 
 /*
+ * This is the "defer" cleanup function that actually applies the IAB
+ * tuple. This happens really late in the PAM session, hopefully after
+ * the application has performed its setuid() function.
+ */
+static void iab_apply(pam_handle_t *pamh, void *data, int error_status)
+{
+    cap_iab_t iab = data;
+    int retval = error_status & ~(PAM_DATA_REPLACE|PAM_DATA_SILENT);
+
+    data = NULL;
+    if (error_status & PAM_DATA_REPLACE) {
+	goto done;
+    }
+
+    if (retval != PAM_SUCCESS || !(error_status & PAM_DATA_SILENT)) {
+	goto done;
+    }
+
+    if (cap_iab_set_proc(iab) != 0) {
+	D(("IAB setting failed"));
+    }
+
+done:
+    cap_free(iab);
+}
+
+/*
  * Set capabilities for current process to match the current
  * permitted+executable sets combined with the configured inheritable
  * set.
@@ -246,7 +270,11 @@ static int set_capabilities(struct pam_cap_s *cs)
 	goto cleanup_conf;
     }
 
-    if (!cap_iab_set_proc(iab)) {
+    if (cs->defer) {
+	D(("configured to delay applying IAB"));
+	pam_set_data(cs->pamh, "pam_cap_iab", iab, iab_apply);
+	iab = NULL;
+    } else if (!cap_iab_set_proc(iab)) {
 	D(("able to set the IAB [%s] value", conf_caps));
 	ok = 1;
     }
@@ -260,9 +288,7 @@ static int set_capabilities(struct pam_cap_s *cs)
 	 *
 	 * It isn't needed unless you want to support Ambient vector
 	 * values in the IAB. In this case, it will likely also
-	 * require you use the "use_session" module argument and
-	 * include a "session" line in your PAM config that points to
-	 * pam_cap.so.
+	 * require you use the "defer" module argument.
 	 */
 	D(("setting keepcaps"));
 	(void) cap_prctlw(PR_SET_KEEPCAPS, 1, 0, 0, 0, 0);
@@ -310,8 +336,8 @@ static void parse_args(int argc, const char **argv, struct pam_cap_s *pcs)
 	    pcs->autoauth = 1;
 	} else if (!strncmp(*argv, "default=", 8)) {
 	    pcs->fallback = 8 + *argv;
-	} else if (!strcmp(*argv, "use_session")) {
-	    pcs->session = 1;
+	} else if (!strcmp(*argv, "defer")) {
+	    pcs->defer = 1;
 	} else {
 	    _pam_log(LOG_ERR, "unknown option; %s", *argv);
 	}
@@ -380,9 +406,8 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 /*
  * pam_sm_setcred optionally applies inheritable capabilities loaded
  * by the pam_sm_authenticate pass for the user. If it doesn't apply
- * them directly (because of the "use_session" module argument), it
- * caches the cap_iab_t value for later use during the
- * pam_sm_open_session() call.
+ * them directly (because of the "defer" module argument), it caches
+ * the cap_iab_t value for later use during the pam_end() call.
  */
 int pam_sm_setcred(pam_handle_t *pamh, int flags,
 		   int argc, const char **argv)
@@ -403,50 +428,9 @@ int pam_sm_setcred(pam_handle_t *pamh, int flags,
 	return PAM_AUTH_ERR;
     }
 
-    if (!pcs.session) {
-	retval = set_capabilities(&pcs);
-	memset(&pcs, 0, sizeof(pcs));
-    }
-
-    return (retval ? PAM_SUCCESS:PAM_IGNORE);
-}
-
-/*
- * pam_sm_open_session supports applying the configured cap_iab_t
- * tuple after the user credentials have been fully applied. This is
- * for programs where pam_cap.so's auth configuration includes the
- * "use_session" and "keepcaps" module arguments. Typically, the
- * latter is needed to coax the application into persisting the
- * ability to apply the IAB value after a setuid() call by the
- * application.
- *
- * Note, this is only needed in cases where the local system needs to
- * support adding Ambient capability vector values. For Inheritable
- * capabilities which survive setuid() in all modes, the existing
- * module works fine.
- */
-int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc,
-			const char **argv)
-{
-    int retval;
-    struct pam_cap_s 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_USER_UNKNOWN;
-    }
-
-    set_capabilities(&pcs);
+    pcs.pamh = pamh;
+    retval = set_capabilities(&pcs);
     memset(&pcs, 0, sizeof(pcs));
-    return PAM_SUCCESS;
-}
 
-/* pam_sm_close_session is mostly a no-op; it always returns PAM_SUCCESS. */
-int pam_sm_close_session(pam_handle_t *pamh, int flags, int argc,
-			 const char **argv)
-{
-    return PAM_SUCCESS;
+    return (retval ? PAM_SUCCESS:PAM_IGNORE);
 }
diff --git a/pam_cap/test_pam_cap.c b/pam_cap/test_pam_cap.c
index 0a58da6..886888e 100644
--- a/pam_cap/test_pam_cap.c
+++ b/pam_cap/test_pam_cap.c
@@ -51,6 +51,17 @@ int pam_get_item(const pam_handle_t *pamh, int item_type, const void **item) {
     return 0;
 }
 
+int pam_set_data(pam_handle_t *pamh, const char *module_data_name, void *data,
+		 void (*cleanup)(pam_handle_t *pamh, void *data,
+				 int error_status)) {
+    if (cleanup != iab_apply) {
+	errno = EINVAL;
+	return -1;
+    }
+    cap_free(data);
+    return -1;
+}
+
 int getgrouplist(const char *user, gid_t group, gid_t *groups, int *ngroups) {
     int i,j;
     for (i = 0; i < n_users; i++) {
@@ -159,7 +170,7 @@ static int test_arg_parsing(void) {
 	},
 	{
 	    { 0, 0, 0, 1, NULL, NULL, NULL },
-	    { "use_session", NULL }
+	    { "defer", NULL }
 	},
 	{
 	    { 0, 0, 0, 0, NULL, NULL, NULL },