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>

crypto/fipsmodule/ec/ec.c

@@ -943,8 +943,9 @@ static int arbitrary_bignum_to_scalar(const EC_GROUP *group, EC_SCALAR *out,
   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 @@ err:
   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) {

crypto/fipsmodule/ec/ec_key.c

@@ -439,6 +439,8 @@ int EC_KEY_generate_key(EC_KEY *key) {
 }
 
 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;
   }

crypto/fipsmodule/ec/internal.h

@@ -301,6 +301,13 @@ int ec_jacobian_to_affine_batch(const EC_GROUP *group, EC_AFFINE *out,
 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,

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;

crypto/fipsmodule/ecdsa/ecdsa.c

@@ -151,8 +151,8 @@ int ECDSA_SIG_set0(ECDSA_SIG *sig, BIGNUM *r, BIGNUM *s) {
   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 @@ int ECDSA_do_verify(const uint8_t *digest, size_t digest_len,
   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_for_known_answer_test(const uint8_t *digest,
 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;

crypto/fipsmodule/ecdsa/internal.h

@@ -31,6 +31,12 @@ ECDSA_SIG *ecdsa_sign_with_nonce_for_known_answer_test(const uint8_t *digest,
                                                        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)
 }

crypto/fipsmodule/self_check/self_check.c

@@ -400,28 +400,7 @@ err:
   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 @@ static int boringssl_self_test_slow(void) {
   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 @@ static int boringssl_self_test_slow(void) {
 
   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 @@ static int boringssl_self_test_slow(void) {
       !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 @@ static int boringssl_self_test_slow(void) {
     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 @@ static int boringssl_self_test_slow(void) {
   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 @@ int BORINGSSL_self_test(void) {
   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;
   }
 

crypto/internal.h

@@ -952,11 +952,17 @@ int boringssl_self_test_startup(void);
 // 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