Merge c7e8c56c1b into d99198819c

Since currently there is only one device type (devicetree), chid_match()
compares the descriptor value of this device type directly and errors out for
other descriptors. When other device types will be added, this logic will fail.

Introduce DEVICE_TYPE_MAX to denote maximum value of a device type and use
the corresponding descriptor to do validations in chid_match.

man/systemd-cryptenroll.xml

@@ -265,11 +265,32 @@
   </refsect1>
 
   <refsect1>
-    <title>Unlocking</title>
+    <title>Options</title>
 
-    <para>The following options are understood that may be used to unlock the device in preparation of the enrollment operations:</para>
+    <para>The following options are understood:</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>
 
@@ -307,45 +328,7 @@
 
         <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>
 
@@ -378,15 +361,7 @@
 
         <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
@@ -486,15 +461,7 @@
 
         <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>
 
@@ -669,15 +636,7 @@
 
         <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,12 +38,19 @@ __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 --list-devices'
+                     --password --recovery-key'
         [ARG]='--unlock-key-file
                --unlock-fido2-device
                --unlock-tpm2-device
@@ -109,7 +116,7 @@ _systemd_cryptenroll() {
         return 0
     fi
 
-    comps=$(systemd-cryptenroll --list-devices)
+    comps=$(__get_block_devices)
     COMPREPLY=( $(compgen -W '$comps' -- "$cur") )
     return 0
 }

src/boot/chid.c

@@ -22,12 +22,9 @@
 #include "util.h"
 
 /* Validate the descriptor macros a bit that they match our expectations */
-assert_cc(DEVICE_DESCRIPTOR_DEVICETREE == UINT32_C(0x10000020));
-assert_cc(DEVICE_DESCRIPTOR_EFIFW == UINT32_C(0x20000020));
+assert_cc(DEVICE_DESCRIPTOR_DEVICETREE == UINT32_C(0x1000001C));
 assert_cc(DEVICE_SIZE_FROM_DESCRIPTOR(DEVICE_DESCRIPTOR_DEVICETREE) == sizeof(Device));
 assert_cc(DEVICE_TYPE_FROM_DESCRIPTOR(DEVICE_DESCRIPTOR_DEVICETREE) == DEVICE_TYPE_DEVICETREE);
-assert_cc(DEVICE_SIZE_FROM_DESCRIPTOR(DEVICE_DESCRIPTOR_EFIFW) == sizeof(Device));
-assert_cc(DEVICE_TYPE_FROM_DESCRIPTOR(DEVICE_DESCRIPTOR_EFIFW) == DEVICE_TYPE_EFIFW);
 
 /**
  * smbios_to_hashable_string() - Convert ascii smbios string to stripped char16_t.
@@ -110,15 +107,13 @@ EFI_STATUS chid_match(const void *hwid_buffer, size_t hwid_length, const Device
                 return log_error_status(status, "Failed to populate board CHIDs: %m");
 
         size_t n_devices = 0;
-        uint32_t dev_type;
 
         /* Count devices and check validity */
         for (; (n_devices + 1) * sizeof(*devices) < hwid_length;) {
 
-                dev_type = DEVICE_TYPE_FROM_DESCRIPTOR(devices[n_devices].descriptor);
                 if (devices[n_devices].descriptor == DEVICE_DESCRIPTOR_EOL)
                         break;
-                if ((dev_type != DEVICE_TYPE_EFIFW) && (dev_type != DEVICE_TYPE_DEVICETREE))
+                if (devices[n_devices].descriptor >= DEVICE_DESCRIPTOR_MAX)
                         return EFI_UNSUPPORTED;
                 n_devices++;
         }

src/boot/chid.h

@@ -11,7 +11,6 @@
 
 enum {
         DEVICE_TYPE_DEVICETREE = 0x1, /* A devicetree blob */
-        DEVICE_TYPE_EFIFW, /* an efi firmware blob */
 
         /* Maybe later additional types for:
          *   - CoCo Bring-Your-Own-Firmware
@@ -25,7 +24,7 @@ enum {
 #define DEVICE_MAKE_DESCRIPTOR(type, size) (((uint32_t) (size) | ((uint32_t) type << 28)))
 
 #define DEVICE_DESCRIPTOR_DEVICETREE DEVICE_MAKE_DESCRIPTOR(DEVICE_TYPE_DEVICETREE, sizeof(Device))
-#define DEVICE_DESCRIPTOR_EFIFW DEVICE_MAKE_DESCRIPTOR(DEVICE_TYPE_EFIFW, sizeof(Device))
+#define DEVICE_DESCRIPTOR_MAX DEVICE_MAKE_DESCRIPTOR(_DEVICE_TYPE_MAX, sizeof(Device))
 #define DEVICE_DESCRIPTOR_EOL UINT32_C(0)
 
 typedef struct Device {
@@ -39,14 +38,6 @@ typedef struct Device {
                         uint32_t name_offset;          /* nul-terminated string or 0 if not present */
                         uint32_t compatible_offset;    /* nul-terminated string or 0 if not present */
                 } devicetree;
-                struct {
-                        /* Offsets are relative to the beginning of the .hwids PE section.
-                           They are nul-terminated strings when present or 0 if not present */
-                        uint32_t id_offset;         /* identifier for the firmware blob */
-                        uint32_t metadata_offset;   /* firmware metadata string */
-                        uint32_t compatible_offset; /* compatibility identifier to match a specific fw blob */
-                } efifw;
-
                 /* fields for other descriptor types… */
         };
 } _packed_ Device;
@@ -56,19 +47,16 @@ assert_cc(offsetof(Device, descriptor) == 0);
 assert_cc(offsetof(Device, chid) == 4);
 assert_cc(offsetof(Device, devicetree.name_offset) == 20);
 assert_cc(offsetof(Device, devicetree.compatible_offset) == 24);
-assert_cc(offsetof(Device, efifw.id_offset) == 20);
-assert_cc(offsetof(Device, efifw.metadata_offset) == 24);
-assert_cc(offsetof(Device, efifw.compatible_offset) == 28);
-assert_cc(sizeof(Device) == 32);
+assert_cc(sizeof(Device) == 28);
 
-static inline const char* device_get_devicetree_name(const void *base, const Device *device) {
+static inline const char* device_get_name(const void *base, const Device *device) {
         if (device->descriptor != DEVICE_DESCRIPTOR_DEVICETREE)
                 return NULL;
 
         return device->devicetree.name_offset == 0 ? NULL : (const char *) ((const uint8_t *) base + device->devicetree.name_offset);
 }
 
-static inline const char* device_get_devicetree_compatible(const void *base, const Device *device) {
+static inline const char* device_get_compatible(const void *base, const Device *device) {
         if (device->descriptor != DEVICE_DESCRIPTOR_DEVICETREE)
                 return NULL;
 

src/boot/pe.c

@@ -185,7 +185,7 @@ static bool pe_use_this_dtb(
         if (!device || !base)
                 return false;
 
-        const char *compatible = device_get_devicetree_compatible(base, device);
+        const char *compatible = device_get_compatible(base, device);
         if (!compatible)
                 return false;
 

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$sPKCS#11 Enrollment:%4$s\n"
+               "\n%3$sPKCS11 Enrollment:%4$s\n"
                "     --pkcs11-token-uri=URI\n"
                "                       Specify PKCS#11 security token URI\n"
                "\n%3$sFIDO2 Enrollment:%4$s\n"