From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 22 Dec 2024 17:32:09 +0000 (+0100)
Subject: tpm2-util: refuse hash algorithm/value specification when we only parse a mask
X-Git-Tag: v258-rc1~1720^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=308578202069c288e7f7dc395fa9f74582ab101e;p=thirdparty%2Fsystemd.git

tpm2-util: refuse hash algorithm/value specification when we only parse a mask

tpm2_parse_pcr_argument_to_mask() is supposed to parse a PCR mask
string, and uses the full blown tpm2_parse_pcr_argument() call at its
core, which parses more than just a mask, i.e. values and algorithms
too. Which is very confusing at times, because commands such as
"systemd-cryptenroll --tpm2-device=auto
--tpm2-public-key-pcrs=1:sha1=09dbdbc7f6cdd8029cc90b57a915c19a0ac21bce"
are very confusing, since they suggest enrollment with a specific
algorithm and has value, but this is not in fact what happens: both are
entirely ignored.

That this was accepted this way was more an accident than intended,
which is already visible in the fact that extensive test case entirely
ignores the fact that strings like this are accepted.
---

diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c
index f9a805964a7..29d9dbbe2e5 100644
--- a/src/shared/tpm2-util.c
+++ b/src/shared/tpm2-util.c
@@ -8090,14 +8090,15 @@ int tpm2_parse_pcr_argument_append(const char *arg, Tpm2PCRValue **pcr_values, s
 #endif
 }
 
-/* Same as tpm2_parse_pcr_argument() but converts the pcr values to a pcr mask. If more than one hash
- * algorithm is included in the pcr values array this results in error. This retains the previous behavior of
- * tpm2_parse_pcr_argument() of clearing the mask if 'arg' is empty, replacing the mask if it is set to
- * UINT32_MAX, and or-ing the mask otherwise. */
 int tpm2_parse_pcr_argument_to_mask(const char *arg, uint32_t *mask) {
 #if HAVE_TPM2
         int r;
 
+       /* Same as tpm2_parse_pcr_argument() but converts the pcr values to a pcr mask. If a hash algorithm or
+        * hash value is specified an error is generated (after all we only return the mask here, nothing
+        * else). This retains the previous behavior of tpm2_parse_pcr_argument() of clearing the mask if
+        * 'arg' is empty, replacing the mask if it is set to UINT32_MAX, and or-ing the mask otherwise. */
+
         assert(arg);
         assert(mask);
 
@@ -8113,6 +8114,15 @@ int tpm2_parse_pcr_argument_to_mask(const char *arg, uint32_t *mask) {
                 return 0;
         }
 
+        FOREACH_ARRAY(v, pcr_values, n_pcr_values) {
+                if (v->hash != 0)
+                        return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                               "Not expecting hash algorithm specification in PCR mask value, refusing: %s", arg);
+                if (v->value.size != 0)
+                        return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                               "Not expecting hash value specification in PCR mask value, refusing: %s", arg);
+        }
+
         uint32_t new_mask;
         r = tpm2_pcr_values_to_mask(pcr_values, n_pcr_values, /* algorithm= */ 0, &new_mask);
         if (r < 0)
diff --git a/src/test/test-tpm2.c b/src/test/test-tpm2.c
index e4f1f861b47..31ef6192b29 100644
--- a/src/test/test-tpm2.c
+++ b/src/test/test-tpm2.c
@@ -556,8 +556,13 @@ static void check_parse_pcr_argument(
 
                 assert_se(tpm2_pcr_values_to_mask(expected_values, n_expected_values, expected_values[0].hash, &expected_mask) == 0);
 
-                assert_se(tpm2_parse_pcr_argument_to_mask(arg, &mask) == 0);
-                assert_se(mask == expected_mask);
+                _cleanup_free_ Tpm2PCRValue *arg_pcr_values = NULL;
+                size_t n_arg_pcr_values = 0;
+                assert_se(tpm2_parse_pcr_argument(arg, &arg_pcr_values, &n_arg_pcr_values) >= 0);
+                uint32_t mask2 = UINT32_MAX;
+                assert_se(tpm2_pcr_values_to_mask(arg_pcr_values, n_arg_pcr_values, /* algorithm= */ 0, &mask2) >= 0);
+
+                assert_se((mask == UINT32_MAX ? mask2 : (mask|mask2)) == expected_mask);
         }
 
         size_t old_n_values = n_values;
@@ -701,6 +706,10 @@ TEST(parse_pcr_argument) {
         check_parse_pcr_argument_to_mask("sysexts+17+23", 0x822000);
         check_parse_pcr_argument_to_mask("6+boot-loader-code,44", -EINVAL);
         check_parse_pcr_argument_to_mask("debug+24", -EINVAL);
+        check_parse_pcr_argument_to_mask("5:sha1=f013d66c7f6817d08b7eb2a93e6d0440c1f3e7f8", -EINVAL);
+        check_parse_pcr_argument_to_mask("0:sha256=f013d66c7f6817d08b7eb2a93e6d0440c1f3e7f8", -EINVAL);
+        check_parse_pcr_argument_to_mask("5:sha1=f013d66c7f6817d08b7eb2a93e6d0440c1f3e7f8,3", -EINVAL);
+        check_parse_pcr_argument_to_mask("3,0:sha256=f013d66c7f6817d08b7eb2a93e6d0440c1f3e7f8", -EINVAL);
 }
 
 static const TPMT_PUBLIC test_rsa_template = {