From 96bf8aa051c5687fba14fef9f841c5ffe16d57f9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 May 2024 13:53:22 +0200 Subject: [PATCH 1/4] cryptenroll: explicitly pick PCR bank if literal PCR binding is off, but signed PCR binding is on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We so far derived the PCR bank to use from the PCR values specified fr literal PCR binding. However, when that's not used then we left the bank uninitialized – which will break if signed PCR binds are used (where we need to pick a bank too after all). Hence, let's explicitly pick a bank to use if literal PCR values are not used, to make things just work. Fixes: #32946 --- src/cryptenroll/cryptenroll-tpm2.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/cryptenroll/cryptenroll-tpm2.c b/src/cryptenroll/cryptenroll-tpm2.c index 1423f3b2ac3..4e5d02a97e7 100644 --- a/src/cryptenroll/cryptenroll-tpm2.c +++ b/src/cryptenroll/cryptenroll-tpm2.c @@ -371,8 +371,10 @@ int enroll_tpm2(struct crypt_device *cd, uint16_t hash_pcr_bank = 0; uint32_t hash_pcr_mask = 0; + if (n_hash_pcr_values > 0) { size_t hash_count; + r = tpm2_pcr_values_hash_count(hash_pcr_values, n_hash_pcr_values, &hash_count); if (r < 0) return log_error_errno(r, "Could not get hash count: %m"); @@ -380,10 +382,21 @@ int enroll_tpm2(struct crypt_device *cd, if (hash_count > 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Multiple PCR banks selected."); + /* If we use a literal PCR value policy, derive the bank to use from the algorithm specified on the hash values */ hash_pcr_bank = hash_pcr_values[0].hash; r = tpm2_pcr_values_to_mask(hash_pcr_values, n_hash_pcr_values, hash_pcr_bank, &hash_pcr_mask); if (r < 0) return log_error_errno(r, "Could not get hash mask: %m"); + } else if (pubkey_pcr_mask != 0) { + + /* If no literal PCR value policy is used, then let's determine the mask to use automatically + * from the measurements of the TPM. */ + r = tpm2_get_best_pcr_bank( + tpm2_context, + pubkey_pcr_mask, + &hash_pcr_bank); + if (r < 0) + return log_error_errno(r, "Failed to determine best PCR bank: %m"); } TPM2B_DIGEST policy = TPM2B_DIGEST_MAKE(NULL, TPM2_SHA256_DIGEST_SIZE); From 360198efc33deac69b4cb833572372dd4b8d01f1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 May 2024 13:57:07 +0200 Subject: [PATCH 2/4] tpm2-util: do not serialize tpm2 bank if none is specified If both literal and signed PCR bindings are not used then we won't determine a PCR bank to use, and hence we shouldnt attempt to serialize it either. Hence, if the bank is zero, skip serialization. (And while we are at it, also skip serialization of the primary algorithm if not set, purely to make things systematic). [This effectively results in little change, as previously we'd then seralize a json "null", while now we simply won't genreate the field] --- src/shared/tpm2-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index a64c2738bf2..cc26441a1e0 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -7391,8 +7391,8 @@ int tpm2_make_luks2_json( JSON_BUILD_PAIR("keyslots", JSON_BUILD_ARRAY(JSON_BUILD_STRING(keyslot_as_string))), JSON_BUILD_PAIR("tpm2-blob", JSON_BUILD_IOVEC_BASE64(blob)), JSON_BUILD_PAIR("tpm2-pcrs", JSON_BUILD_VARIANT(hmj)), - JSON_BUILD_PAIR_CONDITION(!!tpm2_hash_alg_to_string(pcr_bank), "tpm2-pcr-bank", JSON_BUILD_STRING(tpm2_hash_alg_to_string(pcr_bank))), - JSON_BUILD_PAIR_CONDITION(!!tpm2_asym_alg_to_string(primary_alg), "tpm2-primary-alg", JSON_BUILD_STRING(tpm2_asym_alg_to_string(primary_alg))), + JSON_BUILD_PAIR_CONDITION(pcr_bank != 0 && tpm2_hash_alg_to_string(pcr_bank), "tpm2-pcr-bank", JSON_BUILD_STRING(tpm2_hash_alg_to_string(pcr_bank))), + JSON_BUILD_PAIR_CONDITION(primary_alg != 0 && tpm2_asym_alg_to_string(primary_alg), "tpm2-primary-alg", JSON_BUILD_STRING(tpm2_asym_alg_to_string(primary_alg))), JSON_BUILD_PAIR("tpm2-policy-hash", JSON_BUILD_IOVEC_HEX(policy_hash)), JSON_BUILD_PAIR("tpm2-pin", JSON_BUILD_BOOLEAN(flags & TPM2_FLAGS_USE_PIN)), JSON_BUILD_PAIR("tpm2_pcrlock", JSON_BUILD_BOOLEAN(flags & TPM2_FLAGS_USE_PCRLOCK)), From b3efb67ddcbad0009a932e1a9bff3dcc22e10993 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 May 2024 13:59:23 +0200 Subject: [PATCH 3/4] tpm2-util: improve compat with older unlocking tools Let's only generate the pin and pcrlock booleans if they are enabled, in order to not unnecessarily confuse older unlocking tools. --- src/shared/tpm2-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index cc26441a1e0..11750333e4f 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -7394,8 +7394,8 @@ int tpm2_make_luks2_json( JSON_BUILD_PAIR_CONDITION(pcr_bank != 0 && tpm2_hash_alg_to_string(pcr_bank), "tpm2-pcr-bank", JSON_BUILD_STRING(tpm2_hash_alg_to_string(pcr_bank))), JSON_BUILD_PAIR_CONDITION(primary_alg != 0 && tpm2_asym_alg_to_string(primary_alg), "tpm2-primary-alg", JSON_BUILD_STRING(tpm2_asym_alg_to_string(primary_alg))), JSON_BUILD_PAIR("tpm2-policy-hash", JSON_BUILD_IOVEC_HEX(policy_hash)), - JSON_BUILD_PAIR("tpm2-pin", JSON_BUILD_BOOLEAN(flags & TPM2_FLAGS_USE_PIN)), - JSON_BUILD_PAIR("tpm2_pcrlock", JSON_BUILD_BOOLEAN(flags & TPM2_FLAGS_USE_PCRLOCK)), + JSON_BUILD_PAIR_CONDITION(FLAGS_SET(flags, TPM2_FLAGS_USE_PIN), "tpm2-pin", JSON_BUILD_BOOLEAN(true)), + JSON_BUILD_PAIR_CONDITION(FLAGS_SET(flags, TPM2_FLAGS_USE_PCRLOCK), "tpm2_pcrlock", JSON_BUILD_BOOLEAN(true)), JSON_BUILD_PAIR_CONDITION(pubkey_pcr_mask != 0, "tpm2_pubkey_pcrs", JSON_BUILD_VARIANT(pkmj)), JSON_BUILD_PAIR_CONDITION(iovec_is_set(pubkey), "tpm2_pubkey", JSON_BUILD_IOVEC_BASE64(pubkey)), JSON_BUILD_PAIR_CONDITION(iovec_is_set(salt), "tpm2_salt", JSON_BUILD_IOVEC_BASE64(salt)), From 51a9a006a55f6c7432afcb95c20eb0851bf9ba4b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 May 2024 14:21:59 +0200 Subject: [PATCH 4/4] update TODO --- TODO | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/TODO b/TODO index 102bdd0a0a9..9fbf041546d 100644 --- a/TODO +++ b/TODO @@ -130,6 +130,10 @@ Deprecations and removals: Features: +* rework tpm2_parse_pcr_argument_to_mask() to refuse literal hash value + specifications. They are currently parsed but ignored. We should refuse them + however, to not confuse people. + * use name_to_handle_at() with AT_HANDLE_FID instead of .st_ino (inode number) for identifying inodes, for example in copy.c when finding hard links, or loop-util.c for tracking backing files, and other places.