Make ECC self tests lazy.
Change-Id: I1b7e4bd5403031232fc1e1ffb3c6e40decac23b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51565
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c
index 1f03e15..93fdcfc 100644
--- a/crypto/fipsmodule/ec/ec.c
+++ b/crypto/fipsmodule/ec/ec.c
@@ -943,8 +943,9 @@
return ok;
}
-int EC_POINT_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar,
- const EC_POINT *p, const BIGNUM *p_scalar, BN_CTX *ctx) {
+int ec_point_mul_no_self_test(const EC_GROUP *group, EC_POINT *r,
+ const BIGNUM *g_scalar, const EC_POINT *p,
+ const BIGNUM *p_scalar, BN_CTX *ctx) {
// Previously, this function set |r| to the point at infinity if there was
// nothing to multiply. But, nobody should be calling this function with
// nothing to multiply in the first place.
@@ -1010,6 +1011,13 @@
return ret;
}
+int EC_POINT_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar,
+ const EC_POINT *p, const BIGNUM *p_scalar, BN_CTX *ctx) {
+ boringssl_ensure_ecc_self_test();
+
+ return ec_point_mul_no_self_test(group, r, g_scalar, p, p_scalar, ctx);
+}
+
int ec_point_mul_scalar_public(const EC_GROUP *group, EC_RAW_POINT *r,
const EC_SCALAR *g_scalar, const EC_RAW_POINT *p,
const EC_SCALAR *p_scalar) {
diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c
index 43d7391..d7acf96 100644
--- a/crypto/fipsmodule/ec/ec_key.c
+++ b/crypto/fipsmodule/ec/ec_key.c
@@ -439,6 +439,8 @@
}
int EC_KEY_generate_key_fips(EC_KEY *eckey) {
+ boringssl_ensure_ecc_self_test();
+
if (EC_KEY_generate_key(eckey) && EC_KEY_check_fips(eckey)) {
return 1;
}
diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h
index 289c3aa..488adb8 100644
--- a/crypto/fipsmodule/ec/internal.h
+++ b/crypto/fipsmodule/ec/internal.h
@@ -301,6 +301,13 @@
int ec_point_set_affine_coordinates(const EC_GROUP *group, EC_AFFINE *out,
const EC_FELEM *x, const EC_FELEM *y);
+// ec_point_mul_no_self_test does the same as |EC_POINT_mul|, but doesn't try to
+// run the self-test first. This is for use in the self tests themselves, to
+// prevent an infinite loop.
+int ec_point_mul_no_self_test(const EC_GROUP *group, EC_POINT *r,
+ const BIGNUM *g_scalar, const EC_POINT *p,
+ const BIGNUM *p_scalar, BN_CTX *ctx);
+
// ec_point_mul_scalar sets |r| to |p| * |scalar|. Both inputs are considered
// secret.
int ec_point_mul_scalar(const EC_GROUP *group, EC_RAW_POINT *r,
diff --git a/crypto/fipsmodule/ecdh/ecdh.c b/crypto/fipsmodule/ecdh/ecdh.c
index 4e6d0bf..36fbadc 100644
--- a/crypto/fipsmodule/ecdh/ecdh.c
+++ b/crypto/fipsmodule/ecdh/ecdh.c
@@ -75,10 +75,13 @@
#include <openssl/sha.h>
#include "../ec/internal.h"
+#include "../../internal.h"
int ECDH_compute_key_fips(uint8_t *out, size_t out_len, const EC_POINT *pub_key,
const EC_KEY *priv_key) {
+ boringssl_ensure_ecc_self_test();
+
if (priv_key->priv_key == NULL) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_NO_PRIVATE_VALUE);
return 0;
diff --git a/crypto/fipsmodule/ecdsa/ecdsa.c b/crypto/fipsmodule/ecdsa/ecdsa.c
index 5d99903..db0c6e5 100644
--- a/crypto/fipsmodule/ecdsa/ecdsa.c
+++ b/crypto/fipsmodule/ecdsa/ecdsa.c
@@ -151,8 +151,8 @@
return 1;
}
-int ECDSA_do_verify(const uint8_t *digest, size_t digest_len,
- const ECDSA_SIG *sig, const EC_KEY *eckey) {
+int ecdsa_do_verify_no_self_test(const uint8_t *digest, size_t digest_len,
+ const ECDSA_SIG *sig, const EC_KEY *eckey) {
const EC_GROUP *group = EC_KEY_get0_group(eckey);
const EC_POINT *pub_key = EC_KEY_get0_public_key(eckey);
if (group == NULL || pub_key == NULL || sig == NULL) {
@@ -198,6 +198,13 @@
return 1;
}
+int ECDSA_do_verify(const uint8_t *digest, size_t digest_len,
+ const ECDSA_SIG *sig, const EC_KEY *eckey) {
+ boringssl_ensure_ecc_self_test();
+
+ return ecdsa_do_verify_no_self_test(digest, digest_len, sig, eckey);
+}
+
static ECDSA_SIG *ecdsa_sign_impl(const EC_GROUP *group, int *out_retry,
const EC_SCALAR *priv_key, const EC_SCALAR *k,
const uint8_t *digest, size_t digest_len) {
@@ -292,12 +299,16 @@
ECDSA_SIG *ECDSA_sign_with_nonce_and_leak_private_key_for_testing(
const uint8_t *digest, size_t digest_len, const EC_KEY *eckey,
const uint8_t *nonce, size_t nonce_len) {
+ boringssl_ensure_ecc_self_test();
+
return ecdsa_sign_with_nonce_for_known_answer_test(digest, digest_len, eckey,
nonce, nonce_len);
}
ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len,
const EC_KEY *eckey) {
+ boringssl_ensure_ecc_self_test();
+
if (eckey->ecdsa_meth && eckey->ecdsa_meth->sign) {
OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_NOT_IMPLEMENTED);
return NULL;
diff --git a/crypto/fipsmodule/ecdsa/internal.h b/crypto/fipsmodule/ecdsa/internal.h
index 5115dfa..645959f 100644
--- a/crypto/fipsmodule/ecdsa/internal.h
+++ b/crypto/fipsmodule/ecdsa/internal.h
@@ -31,6 +31,12 @@
const uint8_t *nonce,
size_t nonce_len);
+// ecdsa_do_verify_no_self_test does the same as |ECDSA_do_verify|, but doesn't
+// try to run the self-test first. This is for use in the self tests themselves,
+// to prevent an infinite loop.
+int ecdsa_do_verify_no_self_test(const uint8_t *digest, size_t digest_len,
+ const ECDSA_SIG *sig, const EC_KEY *eckey);
+
#if defined(__cplusplus)
}
diff --git a/crypto/fipsmodule/self_check/self_check.c b/crypto/fipsmodule/self_check/self_check.c
index 21e9e0a..7191ca4 100644
--- a/crypto/fipsmodule/self_check/self_check.c
+++ b/crypto/fipsmodule/self_check/self_check.c
@@ -400,28 +400,7 @@
return ret;
}
-#if defined(BORINGSSL_FIPS)
-
-static void run_self_test_rsa(void) {
- if (!boringssl_self_test_rsa()) {
- BORINGSSL_FIPS_abort();
- }
-}
-
-DEFINE_STATIC_ONCE(g_self_test_once_rsa);
-
-void boringssl_ensure_rsa_self_test(void) {
- CRYPTO_once(g_self_test_once_rsa_bss_get(), run_self_test_rsa);
-}
-
-#endif // BORINGSSL_FIPS
-
-
-// Startup self tests.
-//
-// These tests are run at process start when in FIPS mode.
-
-static int boringssl_self_test_slow(void) {
+static int boringssl_self_test_ecc(void) {
int ret = 0;
EC_KEY *ec_key = NULL;
EC_GROUP *ec_group = NULL;
@@ -429,8 +408,6 @@
EC_POINT *ec_point_out = NULL;
BIGNUM *ec_scalar = NULL;
ECDSA_SIG *sig = NULL;
- DH *dh = NULL;
- BIGNUM *ffdhe2048_value = NULL;
ec_key = self_test_ecdsa_key();
if (ec_key == NULL) {
@@ -487,8 +464,9 @@
ECDSA_SIG_free(sig);
sig = parse_ecdsa_sig(kECDSAVerifySig, sizeof(kECDSAVerifySig));
- if (!sig || !ECDSA_do_verify(kECDSAVerifyDigest, sizeof(kECDSAVerifyDigest),
- sig, ec_key)) {
+ if (!sig ||
+ !ecdsa_do_verify_no_self_test(kECDSAVerifyDigest,
+ sizeof(kECDSAVerifyDigest), sig, ec_key)) {
fprintf(stderr, "ECDSA-verify KAT failed.\n");
goto err;
}
@@ -533,8 +511,8 @@
!EC_POINT_oct2point(ec_group, ec_point_in, kP256Point, sizeof(kP256Point),
NULL) ||
!BN_bin2bn(kP256Scalar, sizeof(kP256Scalar), ec_scalar) ||
- !EC_POINT_mul(ec_group, ec_point_out, NULL, ec_point_in, ec_scalar,
- NULL) ||
+ !ec_point_mul_no_self_test(ec_group, ec_point_out, NULL, ec_point_in,
+ ec_scalar, NULL) ||
!EC_POINT_point2oct(ec_group, ec_point_out, POINT_CONVERSION_UNCOMPRESSED,
z_comp_result, sizeof(z_comp_result), NULL) ||
!check_test(kP256PointResult, z_comp_result, sizeof(z_comp_result),
@@ -543,6 +521,57 @@
goto err;
}
+ ret = 1;
+
+err:
+ EC_KEY_free(ec_key);
+ EC_POINT_free(ec_point_in);
+ EC_POINT_free(ec_point_out);
+ EC_GROUP_free(ec_group);
+ BN_free(ec_scalar);
+ ECDSA_SIG_free(sig);
+
+ return ret;
+}
+
+#if defined(BORINGSSL_FIPS)
+
+static void run_self_test_rsa(void) {
+ if (!boringssl_self_test_rsa()) {
+ BORINGSSL_FIPS_abort();
+ }
+}
+
+DEFINE_STATIC_ONCE(g_self_test_once_rsa);
+
+void boringssl_ensure_rsa_self_test(void) {
+ CRYPTO_once(g_self_test_once_rsa_bss_get(), run_self_test_rsa);
+}
+
+static void run_self_test_ecc(void) {
+ if (!boringssl_self_test_ecc()) {
+ BORINGSSL_FIPS_abort();
+ }
+}
+
+DEFINE_STATIC_ONCE(g_self_test_once_ecc);
+
+void boringssl_ensure_ecc_self_test(void) {
+ CRYPTO_once(g_self_test_once_ecc_bss_get(), run_self_test_ecc);
+}
+
+#endif // BORINGSSL_FIPS
+
+
+// Startup self tests.
+//
+// These tests are run at process start when in FIPS mode.
+
+static int boringssl_self_test_slow(void) {
+ int ret = 0;
+ DH *dh = NULL;
+ BIGNUM *ffdhe2048_value = NULL;
+
// FFC Diffie-Hellman KAT
// kFFDHE2048PublicValueData is an arbitrary public value, mod
@@ -608,12 +637,6 @@
ret = 1;
err:
- EC_KEY_free(ec_key);
- EC_POINT_free(ec_point_in);
- EC_POINT_free(ec_point_out);
- EC_GROUP_free(ec_group);
- BN_free(ec_scalar);
- ECDSA_SIG_free(sig);
DH_free(dh);
BN_free(ffdhe2048_value);
@@ -909,7 +932,8 @@
if (!boringssl_self_test_fast() ||
!boringssl_self_test_slow() ||
// When requested to run self tests, also run the lazy tests.
- !boringssl_self_test_rsa()) {
+ !boringssl_self_test_rsa() ||
+ !boringssl_self_test_ecc()) {
return 0;
}
diff --git a/crypto/internal.h b/crypto/internal.h
index 032660a..d8647eb 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -952,11 +952,17 @@
// unsuccessful.
void boringssl_ensure_rsa_self_test(void);
+// boringssl_ensure_ecc_self_test checks whether the ECDSA and ECDH self-test
+// has been run in this address space. If not, it runs it and crashes the
+// address space if unsuccessful.
+void boringssl_ensure_ecc_self_test(void);
+
#else
// Outside of FIPS mode, the lazy tests are no-ops.
OPENSSL_INLINE void boringssl_ensure_rsa_self_test(void) {}
+OPENSSL_INLINE void boringssl_ensure_ecc_self_test(void) {}
#endif // FIPS
