Merge pull request #4342 from garlick/cert_name

improve logging of overlay peer authentication

Author: mergify[bot]

doc/man1/flux-keygen.rst

@@ -6,7 +6,7 @@ flux-keygen(1)
 SYNOPSIS
 ========
 
-**flux** **keygen** *PATH*
+**flux** **keygen** [*--name=NAME*] *PATH*
 
 
 DESCRIPTION
@@ -28,6 +28,17 @@ In that case, each broker self-generates a unique certificate and the
 public keys are exchanged with PMI.
 
 
+OPTIONS
+=======
+
+``flux-keygen`` accepts the following options:
+
+**-n, --name=NAME**
+   Set the certificate metadata ``name`` field.  The value is logged when
+   :man1:`flux-broker` authenticates a peer that presents this certificate.
+   A cluster name might be appropriate here.  Default: the local hostname.
+
+
 RESOURCES
 =========
 

src/cmd/flux-keygen.c

@@ -13,18 +13,16 @@
 #endif
 #include <unistd.h>
 #include <flux/core.h>
+#include <flux/optparse.h>
 #include <czmq.h>
 
 #include "src/common/libutil/log.h"
 
-
-void usage (void)
-{
-    fprintf (stderr,
-"Usage: flux-keygen PATH\n"
-);
-    exit (1);
-}
+static struct optparse_option opts[] = {
+    { .name = "name", .key = 'n', .has_arg = 1, .arginfo = "NAME",
+      .usage = "Set certificate name (default: hostname)", },
+    OPTPARSE_TABLE_END,
+};
 
 static char * ctime_iso8601_now (char *buf, size_t sz)
 {
@@ -42,26 +40,37 @@ static char * ctime_iso8601_now (char *buf, size_t sz)
 
 int main (int argc, char *argv[])
 {
+    const char *usage_msg = "[OPTIONS] [PATH]";
+    optparse_t *p;
+    int optindex;
     zcert_t *cert;
     char buf[64];
     char *path = NULL;
 
     log_init ("flux-keygen");
-
-    if (argc == 1)
+    if (!(p = optparse_create ("flux-keygen"))
+        || optparse_add_option_table (p, opts) != OPTPARSE_SUCCESS
+        || optparse_set (p, OPTPARSE_USAGE, usage_msg) != OPTPARSE_SUCCESS)
+        log_err_exit ("error setting up otpion parsing");
+    if ((optindex = optparse_parse_args (p, argc, argv)) < 0)
+        exit (1);
+    if (optindex < argc)
+        path = argv[optindex++];
+    if (optindex < argc) {
+        optparse_print_usage (p);
+        exit (1);
+    }
+    if (!path)
         log_msg ("WARNING: add PATH argument to save generated certificate");
-    else if (argc == 2 && *argv[1] != '-')
-        path = argv[1];
-    else
-        usage ();
 
     if (!(cert = zcert_new ()))
         log_msg_exit ("zcert_new: %s", zmq_strerror (errno));
 
     if (gethostname (buf, sizeof (buf)) < 0)
         log_err_exit ("gethostname");
     zcert_set_meta (cert, "hostname", "%s", buf);
-    zcert_set_meta (cert, "name", "%s", buf); // used in overlay logging
+    // name is used in overlay logging
+    zcert_set_meta (cert, "name", "%s", optparse_get_str (p, "name", buf));
     zcert_set_meta (cert, "time", "%s", ctime_iso8601_now (buf, sizeof (buf)));
     zcert_set_meta (cert, "userid", "%d", getuid ());
 
@@ -70,6 +79,7 @@ int main (int argc, char *argv[])
 
     zcert_destroy (&cert);
 
+    optparse_destroy (p);
     log_fini ();
 
     return 0;

src/common/libzmqutil/zap.c

@@ -123,11 +123,15 @@ static void zap_cb (flux_reactor_t *r,
             status_text = "OK";
             user_id = pubkey;
             name = zcert_meta (cert, "name");
-            log_level = LOG_INFO;
+            log_level = LOG_DEBUG;
         }
         if (!name)
             name = "unknown";
-        logger (zap, log_level, "overlay auth %s %s", name, status_text);
+        logger (zap,
+                log_level,
+                "overlay auth cert-name=%s %s",
+                name,
+                status_text);
 
         if (!(rep = zmsg_new ()))
             goto done;

t/sharness.d/flux-sharness.sh

@@ -99,7 +99,7 @@ make_bootstrap_config() {
 
     mkdir $workdir/conf.d
     mkdir $workdir/state
-    flux keygen $workdir/cert
+    flux keygen --name testcert $workdir/cert
     cat >$workdir/conf.d/bootstrap.toml <<-EOT
 	[bootstrap]
 	    curve_cert = "$workdir/cert"

t/t0001-basic.t

@@ -37,6 +37,14 @@ test_expect_success 'flux-keygen works' '
 	flux keygen cert &&
 	test -f cert
 '
+test_expect_success 'flux-keygen --name=test works' '
+	flux keygen --name=testcert cert2 &&
+	test -f cert2 &&
+	grep testcert cert2
+'
+test_expect_success 'flux-keygen fails with extra positional argument' '
+	test_must_fail flux keygen cert xyz
+'
 test_expect_success 'flux-keygen generated cert with u=rw access' '
 	echo '-rw-------' >cert-access.exp &&
 	stat --format=%A cert >cert-access.out &&

t/t3300-system-basic.t

@@ -31,6 +31,10 @@ test_expect_success 'startctl status works' '
 test_expect_success HAVE_JQ 'broker overlay shows 2 connected children' '
 	test $(overlay_connected_children) -eq 2
 '
+test_expect_success 'testcert was used to authenticate' '
+	flux dmesg |grep "overlay auth" >auth.log &&
+	grep testcert auth.log
+'
 
 test_expect_success 'overlay status is full' '
 	test "$(flux overlay status --timeout=0 --summary)" = "full"