Handle nullptr arguments to FIPS key generation functions
FIPS needs these functions to handle nullptr arguments without crashing.
Change-Id: Ic3f2f4d988b80e621a990c28423e3323ea526241
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77308
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/crypto/fipsmodule/bcm_interface.h b/crypto/fipsmodule/bcm_interface.h
index c22ef77..2f53777 100644
--- a/crypto/fipsmodule/bcm_interface.h
+++ b/crypto/fipsmodule/bcm_interface.h
@@ -713,14 +713,22 @@
// SLH-DSA-SHA2-128s signature.
#define BCM_SLHDSA_SHA2_128S_SIGNATURE_BYTES 7856
-// SLHDSA_SHA2_128S_generate_key_from_seed generates an SLH-DSA-SHA2-128s key
-// pair from a 48-byte seed and writes the result to |out_public_key| and
+// BCM_slhdsa_sha2_128s_generate_key_from_seed generates an SLH-DSA-SHA2-128s
+// key pair from a 48-byte seed and writes the result to |out_public_key| and
// |out_secret_key|.
OPENSSL_EXPORT bcm_infallible BCM_slhdsa_sha2_128s_generate_key_from_seed(
uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
uint8_t out_secret_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES],
const uint8_t seed[3 * BCM_SLHDSA_SHA2_128S_N]);
+// BCM_slhdsa_sha2_128s_generate_key_from_seed_fips does the same thing as
+// `BCM_slhdsa_sha2_128s_generate_key_from_seed` but implements the required
+// second check before generating a key by testing for nullptr arguments.
+OPENSSL_EXPORT bcm_status BCM_slhdsa_sha2_128s_generate_key_from_seed_fips(
+ uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
+ uint8_t out_secret_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES],
+ const uint8_t seed[3 * BCM_SLHDSA_SHA2_128S_N]);
+
// BCM_slhdsa_sha2_128s_sign_internal acts like |SLHDSA_SHA2_128S_sign| but
// accepts an explicit entropy input, which can be PK.seed (bytes 32..48 of
// the private key) to generate deterministic signatures. It also takes the
@@ -748,6 +756,10 @@
uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
uint8_t out_private_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES]);
+OPENSSL_EXPORT bcm_status BCM_slhdsa_sha2_128s_generate_key_fips(
+ uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
+ uint8_t out_private_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES]);
+
OPENSSL_EXPORT bcm_infallible BCM_slhdsa_sha2_128s_public_from_private(
uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
const uint8_t private_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES]);
diff --git a/crypto/fipsmodule/mldsa/mldsa.cc.inc b/crypto/fipsmodule/mldsa/mldsa.cc.inc
index 52f5836..e89d87c 100644
--- a/crypto/fipsmodule/mldsa/mldsa.cc.inc
+++ b/crypto/fipsmodule/mldsa/mldsa.cc.inc
@@ -2032,6 +2032,9 @@
uint8_t out_encoded_public_key[BCM_MLDSA65_PUBLIC_KEY_BYTES],
uint8_t out_seed[BCM_MLDSA_SEED_BYTES],
struct BCM_mldsa65_private_key *out_private_key) {
+ if (out_encoded_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
if (BCM_mldsa65_generate_key(out_encoded_public_key, out_seed,
out_private_key) == bcm_status::failure) {
return bcm_status::failure;
@@ -2043,6 +2046,9 @@
uint8_t out_encoded_public_key[BCM_MLDSA65_PUBLIC_KEY_BYTES],
struct BCM_mldsa65_private_key *out_private_key,
const uint8_t entropy[BCM_MLDSA_SEED_BYTES]) {
+ if (out_encoded_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
if (BCM_mldsa65_generate_key_external_entropy(out_encoded_public_key,
out_private_key, entropy) ==
bcm_status::failure) {
@@ -2188,6 +2194,9 @@
uint8_t out_encoded_public_key[BCM_MLDSA87_PUBLIC_KEY_BYTES],
uint8_t out_seed[BCM_MLDSA_SEED_BYTES],
struct BCM_mldsa87_private_key *out_private_key) {
+ if (out_encoded_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
if (BCM_mldsa87_generate_key(out_encoded_public_key, out_seed,
out_private_key) == bcm_status::failure) {
return bcm_status::failure;
@@ -2199,6 +2208,9 @@
uint8_t out_encoded_public_key[BCM_MLDSA87_PUBLIC_KEY_BYTES],
struct BCM_mldsa87_private_key *out_private_key,
const uint8_t entropy[BCM_MLDSA_SEED_BYTES]) {
+ if (out_encoded_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
if (BCM_mldsa87_generate_key_external_entropy(out_encoded_public_key,
out_private_key, entropy) ==
bcm_status::failure) {
diff --git a/crypto/fipsmodule/mlkem/mlkem.cc.inc b/crypto/fipsmodule/mlkem/mlkem.cc.inc
index 265b0ca..c3773cc 100644
--- a/crypto/fipsmodule/mlkem/mlkem.cc.inc
+++ b/crypto/fipsmodule/mlkem/mlkem.cc.inc
@@ -1102,6 +1102,9 @@
uint8_t out_encoded_public_key[BCM_MLKEM768_PUBLIC_KEY_BYTES],
uint8_t optional_out_seed[BCM_MLKEM_SEED_BYTES],
struct BCM_mlkem768_private_key *out_private_key) {
+ if (out_encoded_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
BCM_mlkem768_generate_key(out_encoded_public_key, optional_out_seed,
out_private_key);
return BCM_mlkem768_check_fips(out_private_key);
@@ -1163,6 +1166,9 @@
uint8_t out_encoded_public_key[BCM_MLKEM1024_PUBLIC_KEY_BYTES],
uint8_t optional_out_seed[BCM_MLKEM_SEED_BYTES],
struct BCM_mlkem1024_private_key *out_private_key) {
+ if (out_encoded_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
BCM_mlkem1024_generate_key(out_encoded_public_key, optional_out_seed,
out_private_key);
return BCM_mlkem1024_check_fips(out_private_key);
diff --git a/crypto/fipsmodule/slhdsa/slhdsa.cc.inc b/crypto/fipsmodule/slhdsa/slhdsa.cc.inc
index e8f09b9..b3e5644 100644
--- a/crypto/fipsmodule/slhdsa/slhdsa.cc.inc
+++ b/crypto/fipsmodule/slhdsa/slhdsa.cc.inc
@@ -86,7 +86,7 @@
abort();
}
- return bcm_infallible::approved;
+ return bcm_infallible::not_approved;
}
// Note that this overreads by a byte. This is fine in the context that it's
@@ -294,6 +294,18 @@
seed);
}
+bcm_status BCM_slhdsa_sha2_128s_generate_key_from_seed_fips(
+ uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
+ uint8_t out_secret_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES],
+ const uint8_t seed[3 * BCM_SLHDSA_SHA2_128S_N]) {
+ if (out_public_key == nullptr || out_secret_key == nullptr) {
+ return bcm_status::failure;
+ }
+ BCM_slhdsa_sha2_128s_generate_key_from_seed(out_public_key, out_secret_key,
+ seed);
+ return bcm_status::approved;
+}
+
bcm_infallible BCM_slhdsa_sha2_128s_generate_key(
uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
uint8_t out_private_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES]) {
@@ -303,6 +315,16 @@
out_private_key, seed);
}
+bcm_status BCM_slhdsa_sha2_128s_generate_key_fips(
+ uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
+ uint8_t out_private_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES]) {
+ if (out_public_key == nullptr || out_private_key == nullptr) {
+ return bcm_status::failure;
+ }
+ BCM_slhdsa_sha2_128s_generate_key(out_public_key, out_private_key);
+ return bcm_status::approved;
+}
+
bcm_infallible BCM_slhdsa_sha2_128s_public_from_private(
uint8_t out_public_key[BCM_SLHDSA_SHA2_128S_PUBLIC_KEY_BYTES],
const uint8_t private_key[BCM_SLHDSA_SHA2_128S_PRIVATE_KEY_BYTES]) {
diff --git a/crypto/mldsa/mldsa_test.cc b/crypto/mldsa/mldsa_test.cc
index cc1d75c..112b444 100644
--- a/crypto/mldsa/mldsa_test.cc
+++ b/crypto/mldsa/mldsa_test.cc
@@ -547,4 +547,18 @@
bcm_status::approved);
}
+TEST(MLDSATest, NullptrArgumentsToCreate) {
+ // For FIPS reasons, this should fail rather than crash.
+ ASSERT_EQ(BCM_mldsa65_generate_key_fips(nullptr, nullptr, nullptr),
+ bcm_status::failure);
+ ASSERT_EQ(BCM_mldsa87_generate_key_fips(nullptr, nullptr, nullptr),
+ bcm_status::failure);
+ ASSERT_EQ(
+ BCM_mldsa65_generate_key_external_entropy_fips(nullptr, nullptr, nullptr),
+ bcm_status::failure);
+ ASSERT_EQ(
+ BCM_mldsa87_generate_key_external_entropy_fips(nullptr, nullptr, nullptr),
+ bcm_status::failure);
+}
+
} // namespace
diff --git a/crypto/mlkem/mlkem_test.cc b/crypto/mlkem/mlkem_test.cc
index 213991a..5b4d686 100644
--- a/crypto/mlkem/mlkem_test.cc
+++ b/crypto/mlkem/mlkem_test.cc
@@ -544,4 +544,12 @@
bcm_status::approved);
}
+TEST(MLKEMTest, NullptrArgumentsToCreate) {
+ // For FIPS reasons, this should fail rather than crash.
+ ASSERT_EQ(BCM_mlkem768_generate_key_fips(nullptr, nullptr, nullptr),
+ bcm_status::failure);
+ ASSERT_EQ(BCM_mlkem1024_generate_key_fips(nullptr, nullptr, nullptr),
+ bcm_status::failure);
+}
+
} // namespace
diff --git a/crypto/slhdsa/slhdsa_test.cc b/crypto/slhdsa/slhdsa_test.cc
index d6a7011..c00a2f9 100644
--- a/crypto/slhdsa/slhdsa_test.cc
+++ b/crypto/slhdsa/slhdsa_test.cc
@@ -222,4 +222,13 @@
TEST(SLHDSATest, Self) { boringssl_self_test_slhdsa(); }
+TEST(SLHDSATest, NullptrArgumentsToCreate) {
+ // For FIPS reasons, this should fail rather than crash.
+ ASSERT_EQ(BCM_slhdsa_sha2_128s_generate_key_fips(nullptr, nullptr),
+ bcm_status::failure);
+ ASSERT_EQ(BCM_slhdsa_sha2_128s_generate_key_from_seed_fips(nullptr, nullptr,
+ nullptr),
+ bcm_status::failure);
+}
+
} // namespace
