Run ML-KEM and ML-DSA through constant-time validation We did this for Dilithium, but the tests themselves got lost for ML-DSA. This is just a matter of adding some declassifies so that the tests can survive running through with secret data marked secret. As for what to mark secret, I opted to mark: - Any internal secret output from the PRNG since that aligns better with crbug.com/42290551. Though it probably introduces a bunch of false positives on the TLS side because we haven't actually run all that through this yet. - Any *uniformly* secret inputs in test vectors. That is, seeds, but *not* the serialized long-form private keys (which ought to become seeds anyway). This is because a portion of those serializations include public keys and it's tricky to declassify that before the public key parser gets confused. To simplify comparisons, I added a Declassified() helper in test_util.h. I considered just making == on Bytes automatically declassify, but then we won't notice when we forget to, e.g. declassify ciphertext, so making the test do it explicitly seemed worthwhile? Change-Id: I2c53e25ca843ef876f2a89c15131a5b2b425603f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74387 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/mldsa/mldsa.cc.inc b/crypto/fipsmodule/mldsa/mldsa.cc.inc index 6092545..e9166b5 100644 --- a/crypto/fipsmodule/mldsa/mldsa.cc.inc +++ b/crypto/fipsmodule/mldsa/mldsa.cc.inc
@@ -1518,7 +1518,6 @@ return 0; } - OPENSSL_memcpy(pub->rho, priv->rho, sizeof(pub->rho)); OPENSSL_memcpy(pub->public_key_hash, priv->public_key_hash, sizeof(pub->public_key_hash)); @@ -1533,6 +1532,8 @@ vector_add(&values->t, &values->t, &priv->s2); vector_power2_round(&pub->t1, &values->t0, &values->t); + // t1 is part of the public key and thus is public. + CONSTTIME_DECLASSIFY(&pub->t1, sizeof(pub->t1)); return 1; } @@ -1830,6 +1831,7 @@ uint8_t out_seed[BCM_MLDSA_SEED_BYTES], struct BCM_mldsa65_private_key *out_private_key) { BCM_rand_bytes(out_seed, BCM_MLDSA_SEED_BYTES); + CONSTTIME_SECRET(out_seed, BCM_MLDSA_SEED_BYTES); return BCM_mldsa65_generate_key_external_entropy(out_encoded_public_key, out_private_key, out_seed); } @@ -1880,6 +1882,7 @@ BSSL_CHECK(context_len <= 255); uint8_t randomizer[BCM_MLDSA_SIGNATURE_RANDOMIZER_BYTES]; BCM_rand_bytes(randomizer, sizeof(randomizer)); + CONSTTIME_SECRET(randomizer, sizeof(randomizer)); const uint8_t context_prefix[2] = {0, static_cast<uint8_t>(context_len)}; return BCM_mldsa65_sign_internal(
diff --git a/crypto/fipsmodule/mlkem/mlkem.cc.inc b/crypto/fipsmodule/mlkem/mlkem.cc.inc index ec6d484..0ade4ce 100644 --- a/crypto/fipsmodule/mlkem/mlkem.cc.inc +++ b/crypto/fipsmodule/mlkem/mlkem.cc.inc
@@ -181,7 +181,7 @@ // reduce_once reduces 0 <= x < 2*kPrime, mod kPrime. uint16_t reduce_once(uint16_t x) { - assert(x < 2 * kPrime); + declassify_assert(x < 2 * kPrime); const uint16_t subtracted = x - kPrime; uint16_t mask = 0u - (subtracted >> 15); // On Aarch64, omitting a |value_barrier_u16| results in a 2x speedup of @@ -194,7 +194,7 @@ // constant time reduce x mod kPrime using Barrett reduction. x must be less // than kPrime + 2×kPrime². static uint16_t reduce(uint32_t x) { - assert(x < kPrime + 2u * kPrime * kPrime); + declassify_assert(x < kPrime + 2u * kPrime * kPrime); uint64_t product = (uint64_t)x * kBarrettMultiplier; uint32_t quotient = (uint32_t)(product >> kBarrettShift); uint32_t remainder = x - quotient * kPrime; @@ -528,7 +528,9 @@ element_bits_done += chunk_bits; } - if (element >= kPrime) { + // An element is only out of range in the case of invalid input, in which + // case it is okay to leak the comparison. + if (constant_time_declassify_int(element >= kPrime)) { return 0; } out->c[i] = element; @@ -577,7 +579,7 @@ // 0 <= remainder <= kHalfPrime round to 0 // kHalfPrime < remainder <= kPrime + kHalfPrime round to 1 // kPrime + kHalfPrime < remainder < 2 * kPrime round to 2 - assert(remainder < 2u * kPrime); + declassify_assert(remainder < 2u * kPrime); quotient += 1 & constant_time_lt_w(kHalfPrime, remainder); quotient += 1 & constant_time_lt_w(kPrime + kHalfPrime, remainder); return quotient & ((1 << bits) - 1); @@ -689,6 +691,8 @@ hash_g(hashed, augmented_seed, sizeof(augmented_seed)); const uint8_t *const rho = hashed; const uint8_t *const sigma = hashed + 32; + // rho is public. + CONSTTIME_DECLASSIFY(rho, 32); OPENSSL_memcpy(priv->pub.rho, hashed, sizeof(priv->pub.rho)); matrix_expand(&priv->pub.m, rho); uint8_t counter = 0; @@ -699,6 +703,8 @@ vector_ntt(&error); matrix_mult_transpose(&priv->pub.t, &priv->pub.m, &priv->s); vector_add(&priv->pub.t, &error); + // t is part of the public key and thus is public. + CONSTTIME_DECLASSIFY(&priv->pub.t, sizeof(priv->pub.t)); CBB cbb; CBB_init_fixed(&cbb, out_encoded_public_key, encoded_public_key_size(RANK)); @@ -889,7 +895,8 @@ uint8_t optional_out_seed[BCM_MLKEM_SEED_BYTES], struct BCM_mlkem768_private_key *out_private_key) { uint8_t seed[BCM_MLKEM_SEED_BYTES]; - RAND_bytes(seed, sizeof(seed)); + BCM_rand_bytes(seed, sizeof(seed)); + CONSTTIME_SECRET(seed, sizeof(seed)); if (optional_out_seed) { OPENSSL_memcpy(optional_out_seed, seed, sizeof(seed)); } @@ -915,7 +922,8 @@ uint8_t optional_out_seed[BCM_MLKEM_SEED_BYTES], struct BCM_mlkem1024_private_key *out_private_key) { uint8_t seed[BCM_MLKEM_SEED_BYTES]; - RAND_bytes(seed, sizeof(seed)); + BCM_rand_bytes(seed, sizeof(seed)); + CONSTTIME_SECRET(seed, sizeof(seed)); if (optional_out_seed) { OPENSSL_memcpy(optional_out_seed, seed, sizeof(seed)); } @@ -978,13 +986,15 @@ return bcm_infallible::approved; } -// Calls |MLKEM768_encap_external_entropy| with random bytes from |RAND_bytes| +// Calls |MLKEM768_encap_external_entropy| with random bytes from +// |BCM_rand_bytes| bcm_infallible BCM_mlkem768_encap( uint8_t out_ciphertext[BCM_MLKEM768_CIPHERTEXT_BYTES], uint8_t out_shared_secret[BCM_MLKEM_SHARED_SECRET_BYTES], const struct BCM_mlkem768_public_key *public_key) { uint8_t entropy[BCM_MLKEM_ENCAP_ENTROPY]; - RAND_bytes(entropy, BCM_MLKEM_ENCAP_ENTROPY); + BCM_rand_bytes(entropy, BCM_MLKEM_ENCAP_ENTROPY); + CONSTTIME_SECRET(entropy, BCM_MLKEM_ENCAP_ENTROPY); BCM_mlkem768_encap_external_entropy(out_ciphertext, out_shared_secret, public_key, entropy); return bcm_infallible::approved; @@ -995,7 +1005,8 @@ uint8_t out_shared_secret[BCM_MLKEM_SHARED_SECRET_BYTES], const struct BCM_mlkem1024_public_key *public_key) { uint8_t entropy[BCM_MLKEM_ENCAP_ENTROPY]; - RAND_bytes(entropy, BCM_MLKEM_ENCAP_ENTROPY); + BCM_rand_bytes(entropy, BCM_MLKEM_ENCAP_ENTROPY); + CONSTTIME_SECRET(entropy, BCM_MLKEM_ENCAP_ENTROPY); BCM_mlkem1024_encap_external_entropy(out_ciphertext, out_shared_secret, public_key, entropy); return bcm_infallible::approved; @@ -1015,6 +1026,8 @@ uint8_t key_and_randomness[64]; mlkem::hash_g(key_and_randomness, input, sizeof(input)); encrypt_cpa(out_ciphertext, pub, entropy, key_and_randomness + 32); + // The ciphertext is public. + CONSTTIME_DECLASSIFY(out_ciphertext, mlkem::ciphertext_size(RANK)); static_assert(BCM_MLKEM_SHARED_SECRET_BYTES == 32, ""); memcpy(out_shared_secret, key_and_randomness, 32); } @@ -1046,7 +1059,7 @@ const uint8_t *ciphertext, size_t ciphertext_len, const struct BCM_mlkem768_private_key *private_key) { if (ciphertext_len != BCM_MLKEM768_CIPHERTEXT_BYTES) { - RAND_bytes(out_shared_secret, BCM_MLKEM_SHARED_SECRET_BYTES); + BCM_rand_bytes(out_shared_secret, BCM_MLKEM_SHARED_SECRET_BYTES); return bcm_status::failure; } const struct mlkem::private_key<RANK768> *priv = @@ -1060,7 +1073,7 @@ const uint8_t *ciphertext, size_t ciphertext_len, const struct BCM_mlkem1024_private_key *private_key) { if (ciphertext_len != BCM_MLKEM1024_CIPHERTEXT_BYTES) { - RAND_bytes(out_shared_secret, BCM_MLKEM_SHARED_SECRET_BYTES); + BCM_rand_bytes(out_shared_secret, BCM_MLKEM_SHARED_SECRET_BYTES); return bcm_status::failure; } const struct mlkem::private_key<RANK1024> *priv =
diff --git a/crypto/mldsa/mldsa_test.cc b/crypto/mldsa/mldsa_test.cc index cb0e4f3..a0ab090 100644 --- a/crypto/mldsa/mldsa_test.cc +++ b/crypto/mldsa/mldsa_test.cc
@@ -24,6 +24,7 @@ #include <openssl/span.h> #include "../fipsmodule/bcm_interface.h" +#include "../internal.h" #include "../test/file_test.h" #include "../test/test_util.h" @@ -126,10 +127,11 @@ auto priv2 = std::make_unique<PrivateKey>(); EXPECT_TRUE(PrivateKeyFromSeed(priv2.get(), seed, sizeof(seed))); - EXPECT_EQ(Bytes(Marshal(MarshalPrivate, - reinterpret_cast<BCMPrivateKey *>(priv.get()))), - Bytes(Marshal(MarshalPrivate, - reinterpret_cast<BCMPrivateKey *>(priv2.get())))); + EXPECT_EQ( + Bytes(Declassified(Marshal( + MarshalPrivate, reinterpret_cast<BCMPrivateKey *>(priv.get())))), + Bytes(Declassified(Marshal( + MarshalPrivate, reinterpret_cast<BCMPrivateKey *>(priv2.get()))))); } TEST(MLDSATest, Basic65) { @@ -379,6 +381,7 @@ static void MLDSAKeyGenTest(FileTest *t) { std::vector<uint8_t> seed, expected_public_key, expected_private_key; ASSERT_TRUE(t->GetBytes(&seed, "seed")); + CONSTTIME_SECRET(seed.data(), seed.size()); ASSERT_TRUE(t->GetBytes(&expected_public_key, "pub")); ASSERT_TRUE(t->GetBytes(&expected_private_key, "priv")); @@ -391,7 +394,8 @@ Marshal(MarshalPrivate, priv.get()); EXPECT_EQ(Bytes(encoded_public_key), Bytes(expected_public_key)); - EXPECT_EQ(Bytes(encoded_private_key), Bytes(expected_private_key)); + EXPECT_EQ(Bytes(Declassified(encoded_private_key)), + Bytes(expected_private_key)); } TEST(MLDSATest, KeyGenTests65) {
diff --git a/crypto/mlkem/mlkem_test.cc b/crypto/mlkem/mlkem_test.cc index 68bc5fa..51e9e47 100644 --- a/crypto/mlkem/mlkem_test.cc +++ b/crypto/mlkem/mlkem_test.cc
@@ -26,6 +26,7 @@ #include "../fipsmodule/bcm_interface.h" #include "../fipsmodule/keccak/internal.h" +#include "../internal.h" #include "../test/file_test.h" #include "../test/test_util.h" @@ -127,8 +128,8 @@ { auto priv2 = std::make_unique<PRIVATE_KEY>(); ASSERT_TRUE(FROM_SEED(priv2.get(), seed, sizeof(seed))); - EXPECT_EQ(Bytes(Marshal(MARSHAL_PRIVATE, priv.get())), - Bytes(Marshal(MARSHAL_PRIVATE, priv2.get()))); + EXPECT_EQ(Bytes(Declassified(Marshal(MARSHAL_PRIVATE, priv.get()))), + Bytes(Declassified(Marshal(MARSHAL_PRIVATE, priv2.get())))); } uint8_t first_two_bytes[2]; @@ -172,8 +173,8 @@ 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(encoded_private_key), - Bytes(Marshal(MARSHAL_PRIVATE, priv2.get()))); + EXPECT_EQ(Bytes(Declassified(encoded_private_key)), + Bytes(Declassified(Marshal(MARSHAL_PRIVATE, priv2.get())))); uint8_t ciphertext[CIPHERTEXT_BYTES]; uint8_t shared_secret1[MLKEM_SHARED_SECRET_BYTES]; @@ -181,10 +182,12 @@ ENCAP(ciphertext, shared_secret1, pub.get()); ASSERT_TRUE( DECAP(shared_secret2, ciphertext, sizeof(ciphertext), priv.get())); - EXPECT_EQ(Bytes(shared_secret1), Bytes(shared_secret2)); + EXPECT_EQ(Bytes(Declassified(shared_secret1)), + Bytes(Declassified(shared_secret2))); ASSERT_TRUE( DECAP(shared_secret2, ciphertext, sizeof(ciphertext), priv2.get())); - EXPECT_EQ(Bytes(shared_secret1), Bytes(shared_secret2)); + EXPECT_EQ(Bytes(Declassified(shared_secret1)), + Bytes(Declassified(shared_secret2))); } TEST(MLKEMTest, Basic768) { @@ -213,6 +216,7 @@ void MLKEMKeyGenFileTest(FileTest *t) { std::vector<uint8_t> expected_pub_key_bytes, seed, expected_priv_key_bytes; ASSERT_TRUE(t->GetBytes(&seed, "seed")); + CONSTTIME_SECRET(seed.data(), seed.size()); ASSERT_TRUE(t->GetBytes(&expected_pub_key_bytes, "public_key")); ASSERT_TRUE(t->GetBytes(&expected_priv_key_bytes, "private_key")); @@ -225,7 +229,8 @@ Marshal(MARSHAL_PRIVATE, priv.get())); EXPECT_EQ(Bytes(pub_key_bytes), Bytes(expected_pub_key_bytes)); - EXPECT_EQ(Bytes(priv_key_bytes), Bytes(expected_priv_key_bytes)); + EXPECT_EQ(Bytes(Declassified(priv_key_bytes)), + Bytes(expected_priv_key_bytes)); } TEST(MLKEMTest, KeyGen768TestVectors) { @@ -297,6 +302,7 @@ std::vector<uint8_t> pub_key_bytes, entropy, expected_ciphertext, expected_shared_secret; ASSERT_TRUE(t->GetBytes(&entropy, "entropy")); + CONSTTIME_SECRET(entropy.data(), entropy.size()); ASSERT_TRUE(t->GetBytes(&pub_key_bytes, "public_key")); ASSERT_TRUE(t->GetBytes(&expected_ciphertext, "ciphertext")); ASSERT_TRUE(t->GetBytes(&expected_shared_secret, "shared_secret")); @@ -317,7 +323,7 @@ ENCAP(ciphertext, shared_secret, &pub_key, entropy.data()); ASSERT_EQ(Bytes(expected_ciphertext), Bytes(ciphertext)); - ASSERT_EQ(Bytes(expected_shared_secret), Bytes(shared_secret)); + ASSERT_EQ(Bytes(expected_shared_secret), Bytes(Declassified(shared_secret))); } TEST(MLKEMTest, Encap768TestVectors) {
diff --git a/crypto/test/test_util.h b/crypto/test/test_util.h index 16de9ab..7815093 100644 --- a/crypto/test/test_util.h +++ b/crypto/test/test_util.h
@@ -59,6 +59,13 @@ inline bool operator!=(const Bytes &a, const Bytes &b) { return !(a == b); } +// Declassified returns a declassified copy of some input. +inline std::vector<uint8_t> Declassified(bssl::Span<const uint8_t> in) { + std::vector<uint8_t> copy(in.begin(), in.end()); + CONSTTIME_DECLASSIFY(copy.data(), copy.size()); + return copy; +} + std::ostream &operator<<(std::ostream &os, const Bytes &in); // DecodeHex decodes |in| from hexadecimal and writes the output to |out|. It