Merge pull request #5162 from garlick/issue#5136

restrict remote access to sdbus on rank 0

Author: mergify[bot]

src/broker/broker.c

@@ -217,7 +217,7 @@ int main (int argc, char *argv[])
     ctx.cred.userid = getuid ();
     /* Set default rolemask for messages sent with flux_send()
      * on the broker's internal handle. */
-    ctx.cred.rolemask = FLUX_ROLE_OWNER;
+    ctx.cred.rolemask = FLUX_ROLE_OWNER | FLUX_ROLE_LOCAL;
 
     init_attrs (ctx.attrs, getpid (), &ctx.cred);
 

src/broker/exec.c

@@ -24,7 +24,7 @@ static int reject_nonlocal (const flux_msg_t *msg,
                             void *arg,
                             flux_error_t *error)
 {
-    if (!overlay_msg_is_local (msg)) {
+    if (!flux_msg_is_local (msg)) {
         errprintf (error,
                "Remote rexec requests are not allowed on rank 0");
         return -1;

src/broker/module.c

@@ -658,7 +658,7 @@ module_t *module_add (modhash_t *mh,
      * credentials are always those of the instance owner.
      */
     p->cred.userid = getuid ();
-    p->cred.rolemask = FLUX_ROLE_OWNER;
+    p->cred.rolemask = FLUX_ROLE_OWNER | FLUX_ROLE_LOCAL;
 
     /* Update the modhash.
      */

src/broker/overlay.c

@@ -454,14 +454,6 @@ static struct child *child_lookup (struct overlay *ov, const char *id)
     return NULL;
 }
 
-bool overlay_msg_is_local (const flux_msg_t *msg)
-{
-    /*  Return true only if msg is non-NULL and the message does not
-     *  have the overlay supplied "remote" tag.
-     */
-    return (msg && flux_msg_aux_get (msg, "overlay::remote") == NULL);
-}
-
 /* Lookup (direct) child peer by rank.
  * Returns NULL on lookup failure.
  */
@@ -869,6 +861,18 @@ static void logdrop (struct overlay *ov,
               reason);
 }
 
+static int clear_msg_role (flux_msg_t *msg, uint32_t role)
+{
+    uint32_t rolemask;
+
+    if (flux_msg_get_rolemask (msg, &rolemask) < 0)
+        return -1;
+    rolemask &= ~role;
+    if (flux_msg_set_rolemask (msg, rolemask) < 0)
+        return -1;
+    return 0;
+}
+
 /* Handle a message received from TBON child (downstream).
  */
 static void child_cb (flux_reactor_t *r, flux_watcher_t *w,
@@ -883,11 +887,8 @@ static void child_cb (flux_reactor_t *r, flux_watcher_t *w,
 
     if (!(msg = zmqutil_msg_recv (ov->bind_zsock)))
         return;
-    /* Flag this message as remotely received. This allows efficient
-     * operation of the overlay_msg_is_local() function.
-     */
-    if (flux_msg_aux_set (msg, "overlay::remote", int2ptr (1), NULL) < 0) {
-        logdrop (ov, OVERLAY_DOWNSTREAM, msg, "failed to tag msg as remote");
+    if (clear_msg_role (msg, FLUX_ROLE_LOCAL) < 0) {
+        logdrop (ov, OVERLAY_DOWNSTREAM, msg, "failed to clear local role");
         goto done;
     }
     if (flux_msg_get_type (msg, &type) < 0
@@ -992,11 +993,8 @@ static void parent_cb (flux_reactor_t *r, flux_watcher_t *w,
 
     if (!(msg = zmqutil_msg_recv (ov->parent.zsock)))
         return;
-    /* Flag this message as remotely received. This allows efficient
-     * operation of the overlay_msg_is_local() function.
-     */
-    if (flux_msg_aux_set (msg, "overlay::remote", int2ptr (1), NULL) < 0) {
-        logdrop (ov, OVERLAY_UPSTREAM, msg, "failed to tag msg as remote");
+    if (clear_msg_role (msg, FLUX_ROLE_LOCAL) < 0) {
+        logdrop (ov, OVERLAY_UPSTREAM, msg, "failed to clear local role");
         goto done;
     }
     if (flux_msg_get_type (msg, &type) < 0) {

src/broker/overlay.h

@@ -142,11 +142,6 @@ int overlay_set_monitor_cb (struct overlay *ov,
  */
 int overlay_register_attrs (struct overlay *overlay);
 
-/* Return true if this message was routed locally, i.e. wasn't delivered
- *  via an overlay parent or child.
- */
-bool overlay_msg_is_local (const flux_msg_t *msg);
-
 /* Stop allowing new connections from downstream peers.
  */
 void overlay_shutdown (struct overlay *overlay);

src/broker/test/overlay.c

@@ -330,8 +330,8 @@ void trio (flux_t *h)
     rmsg = recvmsg_timeout (ctx[0], 5);
     ok (rmsg != NULL,
         "%s: request was received by overlay", ctx[0]->name);
-    ok (!overlay_msg_is_local (rmsg),
-        "%s: overlay_msg_is_local fails on parent from child",
+    ok (!flux_msg_is_local (rmsg),
+        "%s: flux_msg_is_local fails on parent from child",
         ctx[1]->name);
     ok (flux_msg_get_topic (rmsg, &topic) == 0 && !strcmp (topic, "meep"),
         "%s: received message has expected topic", ctx[0]->name);
@@ -354,8 +354,8 @@ void trio (flux_t *h)
     rmsg = recvmsg_timeout (ctx[1], 5);
     ok (rmsg != NULL,
         "%s: request was received by overlay", ctx[1]->name);
-    ok (!overlay_msg_is_local (rmsg),
-        "%s: overlay_msg_is_local fails on child from parent",
+    ok (!flux_msg_is_local (rmsg),
+        "%s: flux_msg_is_local fails on child from parent",
         ctx[1]->name);
     ok (flux_msg_get_topic (rmsg, &topic) == 0 && !strcmp (topic, "errr"),
         "%s: request has expected topic", ctx[1]->name);
@@ -376,8 +376,8 @@ void trio (flux_t *h)
     rmsg = recvmsg_timeout (ctx[0], 5);
     ok (rmsg != NULL,
         "%s: response was received by overlay", ctx[0]->name);
-    ok (!overlay_msg_is_local (rmsg),
-        "%s: overlay_msg_is_local returns false for response from child");
+    ok (!flux_msg_is_local (rmsg),
+        "%s: flux_msg_is_local returns false for response from child");
     ok (flux_msg_get_topic (rmsg, &topic) == 0 && !strcmp (topic, "m000"),
         "%s: received message has expected topic", ctx[0]->name);
     ok (flux_msg_route_count (rmsg) == 0,
@@ -396,8 +396,8 @@ void trio (flux_t *h)
         "%s: event was received by overlay", ctx[0]->name);
     ok (flux_msg_get_topic (rmsg, &topic) == 0 && !strcmp (topic, "eeek"),
         "%s: received message has expected topic", ctx[0]->name);
-    ok (!overlay_msg_is_local (rmsg),
-        "%s: overlay_msg_is_local returns false for event from child");
+    ok (!flux_msg_is_local (rmsg),
+        "%s: flux_msg_is_local returns false for event from child");
 
     /* Response 0->1
      */
@@ -664,8 +664,8 @@ void wrongness (flux_t *h)
     ok (overlay_bind (ov, "ipc://@foobar") < 0 && errno == EINVAL,
         "overlay_bind fails if called before rank is known");
 
-    ok (!overlay_msg_is_local (NULL),
-        "overlay_msg_is_local (NULL) returns false");
+    ok (!flux_msg_is_local (NULL),
+        "flux_msg_is_local (NULL) returns false");
 
     overlay_destroy (ov);
     attr_destroy (attrs);

src/common/libflux/message.c

@@ -507,6 +507,15 @@ int flux_msg_authorize (const flux_msg_t *msg, uint32_t userid)
     return 0;
 }
 
+bool flux_msg_is_local (const flux_msg_t *msg)
+{
+    uint32_t rolemask;
+    if (flux_msg_get_rolemask (msg, &rolemask) == 0
+        && (rolemask & FLUX_ROLE_LOCAL))
+        return true;
+    return false;
+}
+
 int flux_msg_set_nodeid (flux_msg_t *msg, uint32_t nodeid)
 {
     if (msg_validate (msg) < 0)
@@ -1238,26 +1247,39 @@ static void userid2str (uint32_t userid, char *buf, int buflen)
     assert (n < buflen);
 }
 
-static void rolemask2str (uint32_t rolemask, char *buf, int buflen)
+static int roletostr (uint32_t role, const char *sep, char *buf, int buflen)
 {
     int n;
-    switch (rolemask) {
-        case FLUX_ROLE_NONE:
-            n = snprintf (buf, buflen, "none");
-            break;
-        case FLUX_ROLE_OWNER:
-            n = snprintf (buf, buflen, "owner");
-            break;
-        case FLUX_ROLE_USER:
-            n = snprintf (buf, buflen, "user");
-            break;
-        case FLUX_ROLE_ALL:
-            n = snprintf (buf, buflen, "all");
-            break;
-        default:
-            n = snprintf (buf, buflen, "unknown");
+    if (role == FLUX_ROLE_OWNER)
+        n = snprintf (buf, buflen, "%sowner", sep);
+    else if (role == FLUX_ROLE_USER)
+        n = snprintf (buf, buflen, "%suser", sep);
+    else if (role == FLUX_ROLE_LOCAL)
+        n = snprintf (buf, buflen, "%slocal", sep);
+    else
+        n = snprintf (buf, buflen, "%s0x%x", sep, role);
+    if (n >= buflen)
+        n = buflen;
+    return n;
+}
+
+static void rolemask2str (uint32_t rolemask, char *buf, int buflen)
+{
+    if (rolemask == FLUX_ROLE_NONE)
+        snprintf (buf, buflen, "none");
+    else if (rolemask == FLUX_ROLE_ALL)
+        snprintf (buf, buflen, "all");
+    else {
+        int offset = 0;
+        for (int i = 0; i < sizeof (rolemask)*8; i++) {
+            if (rolemask & 1<<i) {
+                offset += roletostr (rolemask & 1<<i,
+                                     offset > 0 ? "," : "",
+                                     buf + offset,
+                                     buflen - offset);
+            }
+        }
     }
-    assert (n < buflen);
 }
 
 static void nodeid2str (uint32_t nodeid, char *buf, int buflen)

src/common/libflux/message.h

@@ -233,6 +233,7 @@ enum {
     FLUX_ROLE_NONE = 0,
     FLUX_ROLE_OWNER = 1,
     FLUX_ROLE_USER = 2,
+    FLUX_ROLE_LOCAL = 4,
     FLUX_ROLE_ALL = 0xFFFFFFFF,
 };
 int flux_msg_set_rolemask (flux_msg_t *msg, uint32_t rolemask);
@@ -260,6 +261,11 @@ int flux_msg_cred_authorize (struct flux_msg_cred cred, uint32_t userid);
  */
 int flux_msg_authorize (const flux_msg_t *msg, uint32_t userid);
 
+/* Return true if 'msg' credential carries FLUX_ROLE_LOCAL, indicating
+ * that the message has not traversed brokers.
+ */
+bool flux_msg_is_local (const flux_msg_t *msg);
+
 /* Get/set errnum (response only)
  */
 int flux_msg_set_errnum (flux_msg_t *msg, int errnum);

src/common/libflux/test/message.c

@@ -1161,6 +1161,29 @@ void check_print (void)
     fclose (f);
 }
 
+void check_print_rolemask (void)
+{
+    flux_msg_t *msg;
+    FILE *fp;
+    uint32_t rolemask;
+    char *buf = NULL;
+    size_t size = 0;
+
+    rolemask = FLUX_ROLE_LOCAL | FLUX_ROLE_USER | 0x10;
+    if (!(msg = flux_msg_create (FLUX_MSGTYPE_REQUEST))
+        || flux_msg_set_rolemask (msg, rolemask) < 0)
+        BAIL_OUT ("failed to create test request");
+    if (!(fp = open_memstream (&buf, &size)))
+        BAIL_OUT ("open_memstream failed");
+    flux_msg_fprint (fp, msg);
+    fclose (fp); // close flushes content
+    diag ("%s", buf);
+    ok (buf && strstr (buf, "rolemask=user,local,0x10") != NULL,
+        "flux_msg_fprint() rolemask string is correct");
+    free (buf);
+    flux_msg_destroy (msg);
+}
+
 void check_flags (void)
 {
     flux_msg_t *msg;
@@ -1404,6 +1427,7 @@ int main (int argc, char *argv[])
     check_refcount();
 
     check_print ();
+    check_print_rolemask ();
 
     check_proto_internal ();
 

src/connectors/local/local.c

@@ -77,7 +77,7 @@ static int op_send (void *impl, const flux_msg_t *msg, int flags)
     struct local_connector *ctx = impl;
 
     if (ctx->testing_userid != FLUX_USERID_UNKNOWN
-                                || ctx->testing_rolemask != FLUX_ROLE_NONE)
+        || ctx->testing_rolemask != FLUX_ROLE_NONE)
         return send_testing (ctx, msg, flags);
 
     return usock_client_send (ctx->uclient, msg, flags);
@@ -104,14 +104,16 @@ static int op_setopt (void *impl, const char *option,
             goto done;
         }
         memcpy (&ctx->testing_userid, val, val_size);
-    } else if (option && !strcmp (option, FLUX_OPT_TESTING_ROLEMASK)) {
+    }
+    else if (option && !strcmp (option, FLUX_OPT_TESTING_ROLEMASK)) {
         val_size = sizeof (ctx->testing_rolemask);
         if (size != val_size) {
             errno = EINVAL;
             goto done;
         }
         memcpy (&ctx->testing_rolemask, val, val_size);
-    } else {
+    }
+    else {
         errno = EINVAL;
         goto done;
     }
@@ -134,7 +136,8 @@ static int op_getopt (void *impl, const char *option,
             goto done;
         }
         memcpy (val, &ctx->owner, val_size);
-    } else {
+    }
+    else {
         errno = EINVAL;
         goto done;
     }