BUG: `clevis luks bind -d /dev/sda3 tpm2 '{"pcr_ids":"1,4,5,7,9"}'` fails on an empty password

[jarkkojs]

[originally posted here: https://social.kernel.org/notice/AnAEEyg3ULvyJzNq88]

Clevis has a bug that the following ends up failing unless the passphrase is non-empty:

sudo clevis luks bind -d /dev/nvme... tpm2 '{"pcr_ids":"1,4,5,7,9"}'

An empty passphrase can be created by the means of:

sudo cryptsetup luksChangeKey --force-password /dev/sda3

It is a totally legit configuration and scenario for my NUC7CJYH, which I use for only kernel testing. Threats are protected by the requirement of having physical presence checked.

Transcript:

$ sudo clevis luks bind -d /dev/sda3 tpm2 '{"pcr_ids":"1,4,5,7,9"}'
Enter existing LUKS password:
Enter existing LUKS password:
Error adding new binding to /dev/sda3

[jarkkojs]

This is not right:

clevis_luks_check_valid_key_or_keyfile() {
    local DEV="${1}"
    local KEY="${2:-}"
    local KEYFILE="${3:-}"
    local SLT="${4:-}"
    local EXISTING_TOKEN_ID="${5:-}"

    [ -z "${DEV}" ] && return 1
    [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEYFILE}" ] && [ -z "${KEY}" ] && return 1

    local extra_args
    extra_args="$([ -n "${SLT}" ] && printf -- '--key-slot %s' "${SLT}")"
    if [ -n "${KEYFILE}" ]; then
        cryptsetup open --test-passphrase "${DEV}" --key-file "${KEYFILE}" \
                   ${extra_args}
        return
    fi
    if [ -n "${EXISTING_TOKEN_ID}" ]; then
        cryptsetup open --test-passphrase "${DEV}" --token-id "${EXISTING_TOKEN_ID}" \
                   ${extra_args}
        return
    fi

    printf '%s' "${KEY}" | cryptsetup open --test-passphrase "${DEV}" \
                                      ${extra_args}
}

There should be means to pass --force-password to cryptsetup open, and allow EXISTING_TOKEN_ID to be empty. I.e. a new option is needed for clevis luks bind equivalent to --force-password.

[sergio-correia]

What about something like this?

diff --git a/src/luks/clevis-luks-common-functions.in b/src/luks/clevis-luks-common-functions.in
index 29e4631..2b39316 100644
--- a/src/luks/clevis-luks-common-functions.in
+++ b/src/luks/clevis-luks-common-functions.in
@@ -334,10 +334,18 @@ clevis_luks_check_valid_key_or_keyfile() {
     local EXISTING_TOKEN_ID="${5:-}"

     [ -z "${DEV}" ] && return 1
-    [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEYFILE}" ] && [ -z "${KEY}" ] && return 1

     local extra_args
     extra_args="$([ -n "${SLT}" ] && printf -- '--key-slot %s' "${SLT}")"
+
+    # We have an empty key here.
+    if [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEYFILE}" ] \
+         && [ -z "${KEY}" ]; then
+        echo | cryptsetup open --force-password --test-passphrase "${DEV}" \
+               ${extra_args}
+        return
+    fi
+
     if [ -n "${KEYFILE}" ]; then
         cryptsetup open --test-passphrase "${DEV}" --key-file "${KEYFILE}" \
                    ${extra_args}
@@ -798,7 +806,6 @@ clevis_luks_add_key() {

     [ -z "${DEV}" ] && return 1
     [ -z "${NEWKEY}" ] && return 1
-    [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEY}" ] && [ -z "${KEYFILE}" ] && return 1

     local extra_args='' input
     input="$(printf '%s\n%s' "${KEY}" "${NEWKEY}")"

[jarkkojs]

What about something like this?

diff --git a/src/luks/clevis-luks-common-functions.in b/src/luks/clevis-luks-common-functions.in
index 29e4631..2b39316 100644
--- a/src/luks/clevis-luks-common-functions.in
+++ b/src/luks/clevis-luks-common-functions.in
@@ -334,10 +334,18 @@ clevis_luks_check_valid_key_or_keyfile() {
     local EXISTING_TOKEN_ID="${5:-}"

     [ -z "${DEV}" ] && return 1
-    [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEYFILE}" ] && [ -z "${KEY}" ] && return 1

     local extra_args
     extra_args="$([ -n "${SLT}" ] && printf -- '--key-slot %s' "${SLT}")"
+
+    # We have an empty key here.
+    if [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEYFILE}" ] \
+         && [ -z "${KEY}" ]; then
+        echo | cryptsetup open --force-password --test-passphrase "${DEV}" \
+               ${extra_args}
+        return
+    fi
+
     if [ -n "${KEYFILE}" ]; then
         cryptsetup open --test-passphrase "${DEV}" --key-file "${KEYFILE}" \
                    ${extra_args}
@@ -798,7 +806,6 @@ clevis_luks_add_key() {

     [ -z "${DEV}" ] && return 1
     [ -z "${NEWKEY}" ] && return 1
-    [ -z "${EXISTING_TOKEN_ID}" ] && [ -z "${KEY}" ] && [ -z "${KEYFILE}" ] && return 1

     local extra_args='' input
     input="$(printf '%s\n%s' "${KEY}" "${NEWKEY}")"

I still get with this patch:

❯ sudo clevis luks bind -d /dev/sda3 tpm2 '{"pcr_ids":"1,4,5,7,9"}'
Enter existing LUKS password:
Enter existing LUKS password:
Error adding new binding to /dev/sda3

I built a local version installed under ~/local prefix in order to test this in Fedora 40 Server Edition.

[sergio-correia]

I still get with this patch:

❯ sudo clevis luks bind -d /dev/sda3 tpm2 '{"pcr_ids":"1,4,5,7,9"}'
Enter existing LUKS password:
Enter existing LUKS password:
Error adding new binding to /dev/sda3

Are you sure it is using the updated version? I get this exact output without the patch. With it, it should not show that Enter existing LUKS password twice.

[jarkkojs]

I still get with this patch:

❯ sudo clevis luks bind -d /dev/sda3 tpm2 '{"pcr_ids":"1,4,5,7,9"}'
Enter existing LUKS password:
Enter existing LUKS password:
Error adding new binding to /dev/sda3

Are you sure it is using the updated version? I get this exact output without the patch. With it, it should not show that Enter existing LUKS password twice.

I'll sanity check this.

[jarkkojs]

Sorry have been busy work with other not forgotten.

[jarkkojs]

It fixes the issue:

jarkko@mustatorvisieni:~$ sudo clevis luks list -d /dev/sda3
1: tpm2 '{"hash":"sha256","key":"ecc","pcr_bank":"sha1","pcr_ids":"1,4,5,7,9"}'
j

jarkko@mustatorvisieni:~$ sudo systemd-cryptenroll /dev/sda3 --tpm2-device=auto --tpm2-pcrs=1,4,5,7,
9
🔐 Please enter current passphrase for disk /dev/sda3:                         
New TPM2 token enrolled as key slot 2.

[sergio-correia]

It fixes the issue:

Thanks for testing; I added a PR to address it.