Merge to fips-20250107: Check hashes when parsing test-only, semi-expand ML-KEM private keys ML-KEM has two private key formats: a short seed, and an internal "semi-expanded" format that is only used in unit and ACVP testing. Only the seed form is reachable from outside the library. Seeds simplify a lot of questions, including making it impossible for components of the private key to be inconsistent with each other. The semi-expanded form has a redundancy in that it carries both the public key and the hash. Matching what https://boringssl-review.googlesource.com/c/boringssl/+/82991 did for ML-DSA, check the hash is correct as part of parsing. This somewhat defeats the point of having the hash listed explicitly. (It's an optimization for systems that parse a trusted key over and over.) But since we've now concluded this wasn't a great private key format in the first place, this codepath is test-only anyway. That means there is no optimization potential and it is better to include the check to avoid having to think about it. See https://boringssl-review.googlesource.com/c/boringssl/+/93247 (cherry picked from commit c8eb36bae607541c5c6bc518e60ba6ec44e3d10a) Change-Id: I8b5c42bf38b7f7564dc7a125c23d9aa2fa7e568d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/100807 Reviewed-by: David Benjamin <davidben@google.com>

diff --git a/crypto/fipsmodule/mlkem/mlkem.cc.inc b/crypto/fipsmodule/mlkem/mlkem.cc.inc
index 07ed846..c4e5ae4 100644
--- a/crypto/fipsmodule/mlkem/mlkem.cc.inc
+++ b/crypto/fipsmodule/mlkem/mlkem.cc.inc

@@ -818,10 +818,12 @@
   mlkem_decap_no_self_test(out_shared_secret, ciphertext, priv);
 }
 
-// mlkem_parse_public_key_no_hash parses |in| into |pub| but doesn't calculate
-// the value of |pub->public_key_hash|.
+// mlkem_parse_public_key_with_trailing_data parses |in| into |pub| but leaves
+// trailing data in |in| for the caller.
 template <int RANK>
-int mlkem_parse_public_key_no_hash(struct public_key<RANK> *pub, CBS *in) {
+int mlkem_parse_public_key_with_trailing_data(struct public_key<RANK> *pub,
+                                              CBS *in) {
+  CBS orig_in = *in;
   CBS t_bytes;
   if (!CBS_get_bytes(in, &t_bytes, encoded_vector_size(RANK)) ||
       !vector_decode(&pub->t, CBS_data(&t_bytes), kLog2Prime) ||
@@ -829,28 +831,31 @@
     return 0;
   }
   matrix_expand(&pub->m, pub->rho);
+  size_t pub_key_len = CBS_len(&orig_in) - CBS_len(in);
+  assert(pub_key_len == encoded_public_key_size(RANK));
+  hash_h(pub->public_key_hash, CBS_data(&orig_in), pub_key_len);
   return 1;
 }
 
 template <int RANK>
 int mlkem_parse_public_key(struct public_key<RANK> *pub, CBS *in) {
-  CBS orig_in = *in;
-  if (!mlkem_parse_public_key_no_hash(pub, in) ||  //
+  if (!mlkem_parse_public_key_with_trailing_data(pub, in) ||  //
       CBS_len(in) != 0) {
     return 0;
   }
-  hash_h(pub->public_key_hash, CBS_data(&orig_in), CBS_len(&orig_in));
   return 1;
 }
 
 template <int RANK>
 int mlkem_parse_private_key(struct private_key<RANK> *priv, CBS *in) {
-  CBS s_bytes;
+  CBS s_bytes, public_key_hash;
   if (!CBS_get_bytes(in, &s_bytes, encoded_vector_size(RANK)) ||
       !vector_decode(&priv->s, CBS_data(&s_bytes), kLog2Prime) ||
-      !mlkem_parse_public_key_no_hash(&priv->pub, in) ||
-      !CBS_copy_bytes(in, priv->pub.public_key_hash,
-                      sizeof(priv->pub.public_key_hash)) ||
+      !mlkem_parse_public_key_with_trailing_data(&priv->pub, in) ||
+      // We compute the public key hash ourselves, but check it matched.
+      !CBS_get_bytes(in, &public_key_hash, sizeof(priv->pub.public_key_hash)) ||
+      !CBS_mem_equal(&public_key_hash, priv->pub.public_key_hash,
+                     sizeof(priv->pub.public_key_hash)) ||
       !CBS_copy_bytes(in, priv->fo_failure_secret,
                       sizeof(priv->fo_failure_secret)) ||
       CBS_len(in) != 0) {

diff --git a/crypto/mlkem/mlkem_test.cc b/crypto/mlkem/mlkem_test.cc
index 96d652d..401ff09 100644
--- a/crypto/mlkem/mlkem_test.cc
+++ b/crypto/mlkem/mlkem_test.cc

@@ -172,18 +172,32 @@
       Marshal(MARSHAL_PRIVATE, priv.get()));
   EXPECT_EQ(encoded_private_key.size(), size_t{PRIVATE_KEY_BYTES});
 
-  OPENSSL_memcpy(first_two_bytes, encoded_private_key.data(),
-                 sizeof(first_two_bytes));
-  OPENSSL_memset(encoded_private_key.data(), 0xff, sizeof(first_two_bytes));
+  // Parsing should fail if a coefficient is out of range.
+  {
+    std::vector<uint8_t> invalid = encoded_private_key;
+    invalid[0] = 0xff;
+    invalid[1] = 0xff;
+    CBS cbs;
+    CBS_init(&cbs, invalid.data(), invalid.size());
+    auto priv2 = std::make_unique<PRIVATE_KEY>();
+    ASSERT_FALSE(PARSE_PRIVATE(priv2.get(), &cbs));
+  }
+
+  // Parsing should fail if the public key hash is wrong.
+  {
+    std::vector<uint8_t> invalid = encoded_private_key;
+    // The final 64 bytes of the semi-expanded format are the 32-byte hash and
+    // the 32-byte FO (Fujisaki-Okamoto) failure secret. Flip a bit in the hash.
+    invalid[invalid.size() - 33] ^= 1;
+    CBS cbs;
+    CBS_init(&cbs, invalid.data(), invalid.size());
+    auto priv2 = std::make_unique<PRIVATE_KEY>();
+    ASSERT_FALSE(PARSE_PRIVATE(priv2.get(), &cbs));
+  }
+
   CBS cbs;
   CBS_init(&cbs, encoded_private_key.data(), encoded_private_key.size());
   auto priv2 = std::make_unique<PRIVATE_KEY>();
-  // Parsing should fail because the first coefficient is >= kPrime.
-  ASSERT_FALSE(PARSE_PRIVATE(priv2.get(), &cbs));
-
-  OPENSSL_memcpy(encoded_private_key.data(), first_two_bytes,
-                 sizeof(first_two_bytes));
-  CBS_init(&cbs, encoded_private_key.data(), encoded_private_key.size());
   ASSERT_TRUE(PARSE_PRIVATE(priv2.get(), &cbs));
   EXPECT_EQ(Bytes(Declassified(encoded_private_key)),
             Bytes(Declassified(Marshal(MARSHAL_PRIVATE, priv2.get()))));