Merge pull request #8 from grondo/issue#7

support `allow-guest-user` option

.github/workflows/main.yml

@@ -31,8 +31,8 @@ jobs:
           image: "el8"
           env:
             chain_lint: t
-        - name: "fedora38"
-          image: "fedora38"
+        - name: "fedora40"
+          image: "fedora40"
           env: {}
       fail-fast: false
     name: ${{ matrix.name }}

config/ax_flux_core.m4

@@ -49,7 +49,7 @@ AC_DEFUN([AX_FLUX_CORE], [
   PKG_CONFIG_PATH=${prefix}/lib/pkgconfig:${PKG_CONFIG_PATH}
   export PKG_CONFIG_PATH
 
-  PKG_CHECK_MODULES([FLUX_CORE], [flux-core > 0.29.0],
+  PKG_CHECK_MODULES([FLUX_CORE], [flux-core > 0.64.0],
     [
       FLUX_PREFIX=`pkg-config --variable=prefix flux-core`
       LIBFLUX_VERSION=`pkg-config --modversion flux-core`

src/pam/pam_flux.c

@@ -36,7 +36,16 @@
 
 #define PAM_SM_ACCOUNT
 #include <security/pam_modules.h>
-#include <security/_pam_macros.h>
+
+struct options {
+    /*  If set, permit access to all users if the specified user has
+     *  a job in RUN state on this host, this is rank 0 of that job,
+     *  the job is an instance of Flux, and has access.allow-guest-user
+     *  configured.
+     *  (Allows guests to access multi-user instance jobs via ssh connector)
+     */
+    bool allow_guest_user;
+};
 
 /*
  *  Write message described by the 'format' string to syslog.
@@ -53,20 +62,154 @@ static void log_msg (int level, const char *format, ...)
     return;
 }
 
-/*  Return 1 if uid has the local rank currently allocated to an active
- *   Flux job.
+static char *uri_to_local (const char *uri)
+{
+    char *local_uri = NULL;
+    char *p;
+
+    /* Ensure this uri starts with `ssh://`
+     */
+    if (!uri || strncmp (uri, "ssh://", 6) != 0)
+        return NULL;
+
+    /* Skip to next '/' after ssh:// part
+     */
+    if (!(p = strchr (uri+6, '/')))
+        return NULL;
+
+    /* Construct local uri from remainder (path)
+     */
+    if (asprintf (&local_uri, "local:///%s", p) < 0)
+        return NULL;
+    return local_uri;
+}
+
+/* Return 1 if local instance at uri has access.allow-guest-user=true.
+ * Return 0 otherwise.
  */
-static int flux_check_user (uid_t uid)
+static int check_guest_allowed (const char *uri)
+{
+    int allowed = 0;
+    flux_t *h = NULL;
+    flux_future_t *f = NULL;
+    char *local_uri = NULL;
+
+    if (!uri)
+        goto out;
+
+    if (!(local_uri = uri_to_local (uri))) {
+        log_msg (LOG_ERR, "failed to transform %s into local uri", uri);
+        goto out;
+    }
+    if (!(h = flux_open (local_uri, 0))) {
+        log_msg (LOG_ERR, "flux_open (%s): %m", local_uri);
+        goto out;
+    }
+    if (!(f = flux_rpc (h, "config.get", NULL, FLUX_NODEID_ANY, 0))
+        || flux_rpc_get_unpack (f,
+                                "{s?{s?b}}",
+                                "access",
+                                 "allow-guest-user", &allowed) < 0) {
+        log_msg (LOG_ERR, "failed to get config: %m");
+        goto out;
+    }
+    if (!allowed)
+        log_msg (LOG_INFO, "access.allow-guest-user not enabled in child");
+out:
+    flux_close (h);
+    free (local_uri);
+    return allowed;
+}
+
+/* Loop over jobs in json array 'jobs'.
+ * - If any job owner is uid, permit.
+ * - If any job owner is allow_if_user and rank == rank 0 of the job,
+ *   permit if the job is an instance (has a uri) and acesss.allow-guest-user
+ *   is true.
+ */
+static int check_jobs_array (json_t *jobs,
+                             unsigned int rank,
+                             uid_t uid,
+                             uid_t allow_if_user)
 {
-    int authorized = 0;
     size_t index;
-    json_t *value;
+    json_t *entry;
+
+    json_array_foreach (jobs, index, entry) {
+        const char *job_ranks;
+        const char *uri = NULL;
+        int job_uid;
+
+        if (json_unpack (entry,
+                         "{s:i s:s s?{s?{s?s}}}",
+                         "userid", &job_uid,
+                         "ranks", &job_ranks,
+                         "annotations",
+                          "user",
+                           "uri", &uri) < 0) {
+            log_msg (LOG_ERR, "failed to unpack userid, ranks for job");
+            return 0;
+        }
+        if (job_uid == uid)
+            return 1;
+        else if (job_uid == allow_if_user) {
+            struct idset *ranks;
+            if ((ranks = idset_decode (job_ranks))) {
+                int allowed = 0;
+                /* Only if this rank is rank 0 of the job, check that
+                 * access.allow-guest-user is enabled in the job instance:
+                 */
+                if (rank == idset_first (ranks))
+                    allowed = check_guest_allowed (uri);
+                idset_destroy (ranks);
+                if (allowed)
+                    return 1;
+            }
+        }
+    }
+    return 0;
+}
+
+/* Fetch an attribute and return its value as uid_t.
+ */
+static uid_t attr_get_uid (flux_t *h, const char *name)
+{
+    const char *s;
+    char *endptr;
+    long i;
+
+    if (!(s = flux_attr_get (h, name))) {
+        log_msg (LOG_ERR, "flux_attr_get (%s): %m", name);
+        return (uid_t) -1;
+    }
+    errno = 0;
+    i = strtol (s, &endptr, 10);
+    if (errno != 0 || *endptr != '\0') {
+        log_msg (LOG_ERR, "error converting %s to uid: %m", name);
+        return (uid_t) -1;
+    }
+    return (uid_t) i;
+}
+
+
+/*  get jobs in RUN state on this node for user(s) of interest:
+ */
+static int flux_check_user (struct options *opts, uid_t uid)
+{
+    int authorized = 0;
     json_t *jobs = NULL;
     flux_t *h = NULL;
     unsigned int rank = -1;
-    flux_job_state_t state = FLUX_JOB_STATE_NEW;
+    char rankstr[16];
     flux_future_t *f = NULL;
 
+    /* allow_if_user MAY be set to the instance owner to allow guest
+     * access for uid to this node in the case of a multi-user subinstance.
+     * However, initialize it to uid so it can unconditionally be used below
+     * in the RPC to job-list, which greatly simplifies code.
+     */
+    uid_t allow_if_user = uid;
+
     if (!(h = flux_open (NULL, 0))) {
         log_msg (LOG_ERR, "Unable to connect to Flux: %m");
         return 0;
@@ -75,45 +218,43 @@ static int flux_check_user (uid_t uid)
         log_msg (LOG_ERR, "Failed to get current broker rank: %m");
         goto out;
     }
+    if (opts->allow_guest_user) {
+        uid_t owner = attr_get_uid (h, "security.owner");
+        if (owner != (uid_t) -1)
+            allow_if_user = owner;
+        else
+            log_msg (LOG_ERR,
+                     "Failed to get security.owner, can't allow guest access");
+    }
+    if (snprintf (rankstr,
+                  sizeof (rankstr),
+                  "%u",
+                  rank) >= sizeof (rankstr)) {
+        log_msg (LOG_ERR, "Failed to encode broker rank as string: %m");
+        goto out;
+    }
 
+    /* Query jobs in RUN state on current rank using RFC 43 constraint object
+     */
     f = flux_rpc_pack (h,
                        "job-list.list",
                        0,
                        0,
-                       "{s:i s:[ss] s:{s:[{s:[i]} {s:[i]}]}}",
+                       "{s:i s:[sss] s:{s:[{s:[ii]} {s:[s]} {s:[i]}]}}",
                        "max_entries", 0,
-                       "attrs", "ranks", "state",
+                       "attrs", "userid", "ranks", "annotations",
                        "constraint",
                         "and",
-                         "userid", uid,
-                         "states", FLUX_JOB_STATE_RUNNING);
+                         "userid", uid, allow_if_user,
+                         "ranks", rankstr,
+                         "states", FLUX_JOB_STATE_RUN);
     if (!f || flux_rpc_get_unpack (f, "{s:o}", "jobs", &jobs) < 0) {
-        flux_future_destroy (f);
         log_msg (LOG_ERR, "flux_job_list: %m");
         goto out;
     }
 
-    json_array_foreach (jobs, index, value) {
-        const char *ranks;
-        struct idset *ids;
+    authorized = check_jobs_array (jobs, rank, uid, allow_if_user);
 
-        if (json_unpack (value,
-                         "{s:s s:i}",
-                         "ranks", &ranks,
-                         "state", &state) < 0
-            || !(ids = idset_decode (ranks))) {
-            log_msg (LOG_ERR, "Failed to unpack job response");
-            goto out;
-        }
-        /*  Job must have an R which includes this rank _and_ the job
-         *   must be in FLUX_JOB_STATE_RUN (not CLEANUP)
-         */
-        if (idset_test (ids, rank) && state == FLUX_JOB_STATE_RUN)
-            authorized = 1;
-        idset_destroy (ids);
-        if (authorized)
-            goto out;
-    }
 out:
     flux_future_destroy (f);
     flux_close (h);
@@ -167,12 +308,34 @@ static void send_denial_msg (pam_handle_t *pamh,
         log_msg (LOG_ERR,
                  "unable to converse with app: %s",
                  pam_strerror (pamh, retval));
-    if (prsp != NULL)
-        _pam_drop_reply (prsp, 1);
+    if (prsp != NULL) {
+        /* N.B. _pam_drop_reply() deprecated in recent versions
+         * of Linux-PAM. Free reply without use of macros:
+         */
+        free (prsp[0].resp);
+        free (prsp);
+    }
 
     return;
 }
 
+static int parse_options (struct options *opts, int argc, const char **argv)
+{
+    for (int i = 0; i < argc; i++) {
+        if (strcmp ("allow-guest-user", argv[i]) == 0) {
+            opts->allow_guest_user = true;
+        }
+        else {
+            log_msg (LOG_ERR,
+                    "unrecognized option: %s",
+                    argv[i]);
+            return -1;
+        }
+    }
+    return 0;
+}
+
+
 PAM_EXTERN int
 pam_sm_acct_mgmt (pam_handle_t *pamh, int flags, int argc, const char **argv)
 {
@@ -181,6 +344,7 @@ pam_sm_acct_mgmt (pam_handle_t *pamh, int flags, int argc, const char **argv)
     struct passwd *pw;
     uid_t uid;
     int auth = PAM_PERM_DENIED;
+    struct options opts = { .allow_guest_user = false };
 
     retval = pam_get_item (pamh, PAM_USER, (const void **) &user);
     if ((retval != PAM_SUCCESS) || (user == NULL) || (*user == '\0')) {
@@ -195,7 +359,10 @@ pam_sm_acct_mgmt (pam_handle_t *pamh, int flags, int argc, const char **argv)
     }
     uid = pw->pw_uid;
 
-    if (flux_check_user (uid))
+    if (parse_options (&opts, argc, argv) < 0)
+        return PAM_SYSTEM_ERR;
+
+    if (flux_check_user (&opts, uid))
         auth = PAM_SUCCESS;
 
     if (auth != PAM_SUCCESS)

src/test/docker/fedora38/Dockerfile → src/test/docker/fedora40/Dockerfile

@@ -1,4 +1,4 @@
-FROM fluxrm/flux-core:fedora38
+FROM fluxrm/flux-core:fedora40
 
 ARG USER=fluxuser
 ARG UID=1000