Merge 550f38cb14 into 9bf6ffe166

In the --help text we really should use the official spelling, just like
in the man page.

man/systemd-cryptenroll.xml

@@ -265,32 +265,11 @@
   </refsect1>
 
   <refsect1>
-    <title>Options</title>
+    <title>Unlocking</title>
 
-    <para>The following options are understood:</para>
+    <para>The following options are understood that may be used to unlock the device in preparation of the enrollment operations:</para>
 
     <variablelist>
-      <varlistentry>
-        <term><option>--password</option></term>
-
-        <listitem><para>Enroll a regular password/passphrase. This command is mostly equivalent to
-        <command>cryptsetup luksAddKey</command>, however may be combined with
-        <option>--wipe-slot=</option> in one call, see below.</para>
-
-        <xi:include href="version-info.xml" xpointer="v248"/></listitem>
-      </varlistentry>
-
-      <varlistentry>
-        <term><option>--recovery-key</option></term>
-
-        <listitem><para>Enroll a recovery key. Recovery keys are mostly identical to passphrases, but are
-        computer-generated instead of being chosen by a human, and thus have a guaranteed high entropy. The
-        key uses a character set that is easy to type in, and may be scanned off screen via a QR code.
-        </para>
-
-        <xi:include href="version-info.xml" xpointer="v248"/></listitem>
-      </varlistentry>
-
       <varlistentry>
         <term><option>--unlock-key-file=<replaceable>PATH</replaceable></option></term>
 
@@ -328,7 +307,45 @@
 
         <xi:include href="version-info.xml" xpointer="v256"/></listitem>
       </varlistentry>
+    </variablelist>
+  </refsect1>
 
+  <refsect1>
+    <title>Simple Enrollment</title>
+
+    <para>The following options are understood that may be used to enroll simple user input based
+    unlocking:</para>
+
+    <variablelist>
+      <varlistentry>
+        <term><option>--password</option></term>
+
+        <listitem><para>Enroll a regular password/passphrase. This command is mostly equivalent to
+        <command>cryptsetup luksAddKey</command>, however may be combined with
+        <option>--wipe-slot=</option> in one call, see below.</para>
+
+        <xi:include href="version-info.xml" xpointer="v248"/></listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term><option>--recovery-key</option></term>
+
+        <listitem><para>Enroll a recovery key. Recovery keys are mostly identical to passphrases, but are
+        computer-generated instead of being chosen by a human, and thus have a guaranteed high entropy. The
+        key uses a character set that is easy to type in, and may be scanned off screen via a QR code.
+        </para>
+
+        <xi:include href="version-info.xml" xpointer="v248"/></listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>PKCS#11 Enrollment</title>
+
+    <para>The following option is understood that may be used to enroll PKCS#11 tokens:</para>
+
+    <variablelist>
       <varlistentry>
         <term><option>--pkcs11-token-uri=<replaceable>URI</replaceable></option></term>
 
@@ -361,7 +378,15 @@
 
         <xi:include href="version-info.xml" xpointer="v248"/></listitem>
       </varlistentry>
+    </variablelist>
+  </refsect1>
 
+  <refsect1>
+    <title>FIDO2 Enrollment</title>
+
+    <para>The following options are understood that may be used to enroll PKCS#11 tokens:</para>
+
+    <variablelist>
       <varlistentry>
         <term><option>--fido2-credential-algorithm=<replaceable>STRING</replaceable></option></term>
         <listitem><para>Specify COSE algorithm used in credential generation. The default value is
@@ -461,7 +486,15 @@
 
         <xi:include href="version-info.xml" xpointer="v249"/></listitem>
       </varlistentry>
+    </variablelist>
+  </refsect1>
 
+  <refsect1>
+    <title>TPM2 Enrollment</title>
+
+    <para>The following options are understood that may be used to enroll TPM2 devices:</para>
+
+    <variablelist>
       <varlistentry>
         <term><option>--tpm2-device=<replaceable>PATH</replaceable></option></term>
 
@@ -636,7 +669,15 @@
 
         <xi:include href="version-info.xml" xpointer="v255"/></listitem>
       </varlistentry>
+    </variablelist>
+  </refsect1>
 
+  <refsect1>
+    <title>Other Options</title>
+
+    <para>The following additional options are understood:</para>
+
+    <variablelist>
       <varlistentry>
         <term><option>--wipe-slot=<replaceable>SLOT<optional>,SLOT...</optional></replaceable></option></term>
 

shell-completion/bash/systemd-cryptenroll

@@ -38,19 +38,12 @@ __get_tpm2_devices() {
     done
 }
 
-__get_block_devices() {
-    local i
-    for i in /dev/*; do
-        [ -b "$i" ] && printf '%s\n' "$i"
-    done
-}
-
 _systemd_cryptenroll() {
     local comps
     local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]} words cword
     local -A OPTS=(
         [STANDALONE]='-h --help --version
-                     --password --recovery-key'
+                     --password --recovery-key --list-devices'
         [ARG]='--unlock-key-file
                --unlock-fido2-device
                --unlock-tpm2-device
@@ -116,7 +109,7 @@ _systemd_cryptenroll() {
         return 0
     fi
 
-    comps=$(__get_block_devices)
+    comps=$(systemd-cryptenroll --list-devices)
     COMPREPLY=( $(compgen -W '$comps' -- "$cur") )
     return 0
 }

src/core/dbus-job.c

@@ -247,7 +247,7 @@ void bus_job_send_change_signal(Job *j) {
                 job_add_to_gc_queue(j);
         }
 
-        r = bus_foreach_bus(j->manager, j->bus_track, j->sent_dbus_new_signal ? send_changed_signal : send_new_signal, j);
+        r = bus_foreach_bus_signal(j->manager, j->bus_track, j->sent_dbus_new_signal ? send_changed_signal : send_new_signal, j);
         if (r < 0)
                 log_debug_errno(r, "Failed to send job change signal for %u: %m", j->id);
 
@@ -308,7 +308,7 @@ void bus_job_send_removed_signal(Job *j) {
         /* Make sure that any change signal on the unit is reflected before we send out the change signal on the job */
         bus_unit_send_pending_change_signal(j->unit, true);
 
-        r = bus_foreach_bus(j->manager, j->bus_track, send_removed_signal, j);
+        r = bus_foreach_bus_signal(j->manager, j->bus_track, send_removed_signal, j);
         if (r < 0)
                 log_debug_errno(r, "Failed to send job remove signal for %u: %m", j->id);
 }

src/core/dbus-manager.c

@@ -1337,10 +1337,6 @@ static int method_subscribe(sd_bus_message *message, void *userdata, sd_bus_erro
                 return r;
 
         if (sd_bus_message_get_bus(message) == m->api_bus) {
-
-                /* Note that direct bus connection subscribe by
-                 * default, we only track peers on the API bus here */
-
                 if (!m->subscribed) {
                         r = sd_bus_track_new(sd_bus_message_get_bus(message), &m->subscribed, NULL, NULL);
                         if (r < 0)
@@ -1350,10 +1346,15 @@ static int method_subscribe(sd_bus_message *message, void *userdata, sd_bus_erro
                 r = sd_bus_track_add_sender(m->subscribed, message);
                 if (r < 0)
                         return r;
-                if (r == 0)
+        } else {
-                        return sd_bus_error_set(error, BUS_ERROR_ALREADY_SUBSCRIBED, "Client is already subscribed.");
+                r = set_ensure_put(&m->private_buses_subscribed, NULL, sd_bus_message_get_bus(message));
+                if (r < 0)
+                        return r;
         }
 
+        if (r == 0)
+                return sd_bus_error_set(error, BUS_ERROR_ALREADY_SUBSCRIBED, "Client is already subscribed.");
+
         return sd_bus_reply_method_return(message, NULL);
 }
 
@@ -1373,9 +1374,11 @@ static int method_unsubscribe(sd_bus_message *message, void *userdata, sd_bus_er
                 r = sd_bus_track_remove_sender(m->subscribed, message);
                 if (r < 0)
                         return r;
-                if (r == 0)
+        } else
-                        return sd_bus_error_set(error, BUS_ERROR_NOT_SUBSCRIBED, "Client is not subscribed.");
+                r = !!set_remove(m->private_buses_subscribed, sd_bus_message_get_bus(message));
-        }
+
+        if (r == 0)
+                return sd_bus_error_set(error, BUS_ERROR_NOT_SUBSCRIBED, "Client is not subscribed.");
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -2339,7 +2342,7 @@ static void manager_unit_files_changed(Manager *m, const InstallChange *changes,
         /* See comments for this variable in manager.h */
         m->unit_file_state_outdated = true;
 
-        r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
+        r = bus_foreach_bus_signal(m, NULL, send_unit_files_changed, NULL);
         if (r < 0)
                 log_debug_errno(r, "Failed to send UnitFilesChanged signal, ignoring: %m");
 }
@@ -3706,7 +3709,7 @@ void bus_manager_send_finished(
 
         assert(m);
 
-        r = bus_foreach_bus(
+        r = bus_foreach_bus_signal(
                         m,
                         NULL,
                         send_finished,
@@ -3744,7 +3747,7 @@ void bus_manager_send_reloading(Manager *m, bool active) {
 
         assert(m);
 
-        r = bus_foreach_bus(m, NULL, send_reloading, INT_TO_PTR(active));
+        r = bus_foreach_bus_signal(m, NULL, send_reloading, INT_TO_PTR(active));
         if (r < 0)
                 log_debug_errno(r, "Failed to send reloading signal: %m");
 }
@@ -3763,7 +3766,7 @@ void bus_manager_send_change_signal(Manager *m) {
 
         assert(m);
 
-        r = bus_foreach_bus(m, NULL, send_changed_signal, NULL);
+        r = bus_foreach_bus_signal(m, NULL, send_changed_signal, NULL);
         if (r < 0)
                 log_debug_errno(r, "Failed to send manager change signal: %m");
 }

src/core/dbus-unit.c

@@ -1705,7 +1705,7 @@ void bus_unit_send_change_signal(Unit *u) {
         if (!u->id)
                 return;
 
-        r = bus_foreach_bus(u->manager, u->bus_track, u->sent_dbus_new_signal ? send_changed_signal : send_new_signal, u);
+        r = bus_foreach_bus_signal(u->manager, u->bus_track, u->sent_dbus_new_signal ? send_changed_signal : send_new_signal, u);
         if (r < 0)
                 log_unit_debug_errno(u, r, "Failed to send unit change signal for %s: %m", u->id);
 
@@ -1800,7 +1800,7 @@ void bus_unit_send_removed_signal(Unit *u) {
         if (!u->id)
                 return;
 
-        r = bus_foreach_bus(u->manager, u->bus_track, send_removed_signal, u);
+        r = bus_foreach_bus_signal(u->manager, u->bus_track, send_removed_signal, u);
         if (r < 0)
                 log_unit_debug_errno(u, r, "Failed to send unit remove signal for %s: %m", u->id);
 }

src/core/dbus.c

@@ -138,6 +138,9 @@ static int signal_disconnected(sd_bus_message *message, void *userdata, sd_bus_e
 
         if (set_remove(m->private_buses, bus)) {
                 log_debug("Got disconnect on private connection.");
+
+                /* Don't bother checking if the bus was subscribed; try to remove it opportunistically. */
+                set_remove(m->private_buses_subscribed, bus);
                 destroy_bus(m, &bus);
         }
 
@@ -761,6 +764,9 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void
                 return 0;
         }
 
+        /* If this bus (i.e. object address) was subscribed previously let's drop it from that set. */
+        set_remove(m->private_buses_subscribed, bus);
+
         TAKE_PTR(bus);
 
         log_debug("Accepted new private connection.");
@@ -1044,6 +1050,8 @@ void bus_done_private(Manager *m) {
                 destroy_bus(m, &b);
 
         m->private_buses = set_free(m->private_buses);
+        m->private_buses_subscribed = set_free(m->private_buses_subscribed);
+
 
         m->private_listen_event_source = sd_event_source_disable_unref(m->private_listen_event_source);
         m->private_listen_fd = safe_close(m->private_listen_fd);
@@ -1101,33 +1109,33 @@ int bus_fdset_add_all(Manager *m, FDSet *fds) {
         return 0;
 }
 
-int bus_foreach_bus(
+int bus_foreach_bus_signal(
                 Manager *m,
                 sd_bus_track *subscribed2,
-                int (*send_message)(sd_bus *bus, void *userdata),
+                int (*send_signal)(sd_bus *bus, void *userdata),
                 void *userdata) {
 
         int r = 0;
 
         assert(m);
-        assert(send_message);
+        assert(send_signal);
 
         /* Send to all direct buses, unconditionally */
         sd_bus *b;
-        SET_FOREACH(b, m->private_buses) {
+        SET_FOREACH(b, m->private_buses_subscribed) {
 
                 /* Don't bother with enqueuing these messages to clients that haven't started yet */
                 if (sd_bus_is_ready(b) <= 0)
                         continue;
 
-                RET_GATHER(r, send_message(b, userdata));
+                RET_GATHER(r, send_signal(b, userdata));
         }
 
         /* Send to API bus, but only if somebody is subscribed */
         if (m->api_bus &&
             (sd_bus_track_count(m->subscribed) > 0 ||
              sd_bus_track_count(subscribed2) > 0))
-                RET_GATHER(r, send_message(m->api_bus, userdata));
+                RET_GATHER(r, send_signal(m->api_bus, userdata));
 
         return r;
 }

src/core/dbus.h

@@ -21,7 +21,7 @@ int bus_fdset_add_all(Manager *m, FDSet *fds);
 void bus_track_serialize(sd_bus_track *t, FILE *f, const char *prefix);
 int bus_track_coldplug(Manager *m, sd_bus_track **t, bool recursive, char **l);
 
-int bus_foreach_bus(Manager *m, sd_bus_track *subscribed2, int (*send_message)(sd_bus *bus, void *userdata), void *userdata);
+int bus_foreach_bus_signal(Manager *m, sd_bus_track *subscribed2, int (*send_signal)(sd_bus *bus, void *userdata), void *userdata);
 
 int bus_forward_agent_released(Manager *m, const char *path);
 

src/core/manager.h

@@ -332,6 +332,8 @@ struct Manager {
         /* Data specific to the D-Bus subsystem */
         sd_bus *api_bus, *system_bus;
         Set *private_buses;
+        /* Private buses on which client called Subscribe() method */
+        Set *private_buses_subscribed;
         int private_listen_fd;
         sd_event_source *private_listen_event_source;
 

src/core/service.c

@@ -3426,14 +3426,12 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                         return 0;
                 }
 
-                r = service_add_fd_store(s, fd, fdn, do_poll);
+                r = service_add_fd_store(s, TAKE_FD(fd), fdn, do_poll);
                 if (r < 0) {
                         log_unit_debug_errno(u, r,
                                              "Failed to store deserialized fd '%s', ignoring: %m", fdn);
                         return 0;
                 }
-
-                TAKE_FD(fd);
         } else if (streq(key, "extra-fd")) {
                 _cleanup_free_ char *fdv = NULL, *fdn = NULL;
                 _cleanup_close_ int fd = -EBADF;

src/cryptenroll/cryptenroll.c

@@ -193,7 +193,7 @@ static int help(void) {
                "\n%3$sSimple Enrollment:%4$s\n"
                "     --password        Enroll a user-supplied password\n"
                "     --recovery-key    Enroll a recovery key\n"
-               "\n%3$sPKCS11 Enrollment:%4$s\n"
+               "\n%3$sPKCS#11 Enrollment:%4$s\n"
                "     --pkcs11-token-uri=URI\n"
                "                       Specify PKCS#11 security token URI\n"
                "\n%3$sFIDO2 Enrollment:%4$s\n"

src/libsystemd/sd-bus/bus-internal.h

@@ -327,6 +327,8 @@ struct sd_bus {
 
         /* zero means use value specified by $SYSTEMD_BUS_TIMEOUT= environment variable or built-in default */
         usec_t method_call_timeout;
+
+        bool queue_toggle;
 };
 
 /* For method calls we timeout at 25s, like in the D-Bus reference implementation */

src/libsystemd/sd-bus/sd-bus.c

@@ -3064,9 +3064,14 @@ static int process_running(sd_bus *bus, sd_bus_message **ret) {
         if (r != 0)
                 goto null_message;
 
-        r = dispatch_wqueue(bus);
+        /* Toggle which queue we process preferably in this iteration, the rq or the wq */
-        if (r != 0)
+        bus->queue_toggle = !bus->queue_toggle;
-                goto null_message;
+
+        if (bus->queue_toggle) { /* wqueue preferably */
+                r = dispatch_wqueue(bus);
+                if (r != 0)
+                        goto null_message;
+        }
 
         r = dispatch_track(bus);
         if (r != 0)
@@ -3075,6 +3080,12 @@ static int process_running(sd_bus *bus, sd_bus_message **ret) {
         r = dispatch_rqueue(bus, &m);
         if (r < 0)
                 return r;
+        if (r == 0 && !bus->queue_toggle) {
+                /* Nothing happened on the rq, and we didn't check the wq in this iteration, hence do that now. */
+                r = dispatch_wqueue(bus);
+                goto null_message;
+        }
+
         if (!m)
                 goto null_message;
 

src/shared/bus-wait-for-jobs.c

@@ -112,6 +112,19 @@ int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret) {
         if (r < 0)
                 return r;
 
+        r = sd_bus_call_method_async(
+                        bus,
+                        /* slot= */ NULL,
+                        "org.freedesktop.systemd1",
+                        "/org/freedesktop/systemd1",
+                        "org.freedesktop.systemd1.Manager",
+                        "Subscribe",
+                        /* callback= */ NULL,
+                        /* userdata= */ NULL,
+                        /* types= */ NULL);
+        if (r < 0)
+                return r;
+
         *ret = TAKE_PTR(d);
 
         return 0;