Make RSA self-test lazy.

We need to ensure that all public functions that end up doing a cryptographic RSA operation run the self-tests first. We could do that by putting calls in the lower-most functions but the self-tests must run operations without creating a cycle. Therefore calls are placed as low down as possible except where it would conflict with the self-tests. Some functions need to be split so that there's a private version that doesn't require that the self tests have passed. Here's the call-graph that I used for this: ┌───────────────────────────┐ │ private_decrypt │ └───────────────────────────┘ │ │ ▼ ┌───────────────────────────┐ │ decrypt │ └───────────────────────────┘ │ │ ▼ ┌───────────────────────────┐ │ default_decrypt │ └───────────────────────────┘ │ │ ▼ ┌───────────────────────────┐ │ private_transform │ ◀┐ └───────────────────────────┘ │ │ │ │ │ ▼ │ ┌───────────────────────────┐ │ │ default_private_transform │ │ └───────────────────────────┘ │ ┌───────────────────────────┐ │ │ private_encrypt │ │ └───────────────────────────┘ │ ┌───────────────┐ │ │ │ sign_pss_mgf1 │ │ │ └───────────────┘\ ▼ │ ┌────────┐ ┌───────────────────────────┐ │ │ sign │ ──▶ │ sign_raw │ │ └────────┘ └───────────────────────────┘ │ │ │ │ │ ▼ │ ┌───────────────────────────┐ │ │ default_sign_raw │ ─┘ └───────────────────────────┘ ┌−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−┐ ╎ Verification ╎ ╎ ╎ ╎ ┌───────────────────────────┐ ╎ ╎ │ public_decrypt │ ╎ ╎ └───────────────────────────┘ ╎ ╎ │ ╎ ╎ │ ╎ ╎ │ ╎ ┌−−−−−−−−−−−−−−−− │ ╎ ╎ ▼ ╎ ╎ ┌────────┐ ┌───────────────────────────┐ ╎ ╎ │ verify │ ────▶ │ verify_raw │ ╎ ╎ └────────┘ └───────────────────────────┘ ╎ ╎ ╎ └−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−┘ ┌−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−┐ ╎ Encryption ╎ ╎ ╎ ╎ ┌───────────────────────────┐ ╎ ╎ │ public_encrypt │ ╎ ╎ └───────────────────────────┘ ╎ ╎ │ ╎ ╎ │ ╎ ╎ ▼ ╎ ╎ ┌───────────────────────────┐ ╎ ╎ │ encrypt │ ╎ ╎ └───────────────────────────┘ ╎ ╎ ╎ └−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−−┘ Speed difference looks to be in the noise. Before: Did 19716 RSA 2048 signing operations in 10050000us (1961.8 ops/sec) Did 712000 RSA 2048 verify (same key) operations in 10007156us (71149.1 ops/sec) Did 590000 RSA 2048 verify (fresh key) operations in 10004296us (58974.7 ops/sec) Did 101866 RSA 2048 private key parse operations in 10090285us (10095.5 ops/sec) Did 2919 RSA 4096 signing operations in 10019359us (291.3 ops/sec) Did 203000 RSA 4096 verify (same key) operations in 10008421us (20282.9 ops/sec) Did 175000 RSA 4096 verify (fresh key) operations in 10026353us (17454.0 ops/sec) Did 30900 RSA 4096 private key parse operations in 10090073us (3062.4 ops/sec) After: Did 19525 RSA 2048 signing operations in 10000499us (1952.4 ops/sec) Did 706000 RSA 2048 verify (same key) operations in 10002172us (70584.7 ops/sec) Did 588000 RSA 2048 verify (fresh key) operations in 10010856us (58736.2 ops/sec) Did 101864 RSA 2048 private key parse operations in 10063474us (10122.2 ops/sec) Did 2919 RSA 4096 signing operations in 10037480us (290.8 ops/sec) Did 203000 RSA 4096 verify (same key) operations in 10026966us (20245.4 ops/sec) Did 175000 RSA 4096 verify (fresh key) operations in 10032281us (17443.7 ops/sec) Did 31416 RSA 4096 private key parse operations in 10031047us (3131.9 ops/sec) Change-Id: I8dec8a33066717b7078f160e3f93c33cd354bb0c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51426 Reviewed-by: David Benjamin <davidben@google.com>
2022-02-19 13:08:10 +00:00
parent 263f489973
commit 1c2e61efef
6 changed files with 171 additions and 39 deletions
@@ -252,7 +252,7 @@ BORINGSSL_bcm_power_on_self_test(void) {
  OPENSSL_cleanse(result, sizeof(result)); // FIPS 140-3, AS05.10.
 #endif  // OPENSSL_ASAN

-  if (!BORINGSSL_self_test()) {
+  if (!boringssl_self_test_startup()) {
    goto err;
  }

@@ -124,6 +124,28 @@ extern const BN_ULONG kBoringSSLRSASqrtTwo[];
 extern const size_t kBoringSSLRSASqrtTwoLen;


+// Functions that avoid self-tests.
+//
+// Self-tests need to call functions that don't try and ensure that the
+// self-tests have passed. These functions, in turn, need to limit themselves
+// to such functions too.
+//
+// These functions are the same as their public versions, but skip the self-test
+// check.
+
+int rsa_verify_no_self_test(int hash_nid, const uint8_t *digest,
+                            size_t digest_len, const uint8_t *sig,
+                            size_t sig_len, RSA *rsa);
+
+int rsa_verify_raw_no_self_test(RSA *rsa, size_t *out_len, uint8_t *out,
+                                size_t max_out, const uint8_t *in,
+                                size_t in_len, int padding);
+
+int rsa_sign_no_self_test(int hash_nid, const uint8_t *digest,
+                          unsigned digest_len, uint8_t *out, unsigned *out_len,
+                          RSA *rsa);
+
+
 #if defined(__cplusplus)
 }  // extern C
 #endif
@@ -304,8 +304,9 @@ int RSA_public_encrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
  return out_len;
 }

-int RSA_sign_raw(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
-                 const uint8_t *in, size_t in_len, int padding) {
+static int rsa_sign_raw_no_self_test(RSA *rsa, size_t *out_len, uint8_t *out,
+                                     size_t max_out, const uint8_t *in,
+                                     size_t in_len, int padding) {
  if (rsa->meth->sign_raw) {
    return rsa->meth->sign_raw(rsa, out_len, out, max_out, in, in_len, padding);
  }
@@ -313,6 +314,13 @@ int RSA_sign_raw(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
  return rsa_default_sign_raw(rsa, out_len, out, max_out, in, in_len, padding);
 }

+int RSA_sign_raw(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
+                 const uint8_t *in, size_t in_len, int padding) {
+  boringssl_ensure_rsa_self_test();
+  return rsa_sign_raw_no_self_test(rsa, out_len, out, max_out, in, in_len,
+                                   padding);
+}
+
 int RSA_private_encrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa,
                        int padding) {
  size_t out_len;
@@ -524,8 +532,9 @@ int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
  return 0;
 }

-int RSA_sign(int hash_nid, const uint8_t *digest, unsigned digest_len,
-             uint8_t *out, unsigned *out_len, RSA *rsa) {
+int rsa_sign_no_self_test(int hash_nid, const uint8_t *digest,
+                          unsigned digest_len, uint8_t *out, unsigned *out_len,
+                          RSA *rsa) {
  const unsigned rsa_size = RSA_size(rsa);
  int ret = 0;
  uint8_t *signed_msg = NULL;
@@ -540,8 +549,9 @@ int RSA_sign(int hash_nid, const uint8_t *digest, unsigned digest_len,
  if (!RSA_add_pkcs1_prefix(&signed_msg, &signed_msg_len,
                            &signed_msg_is_alloced, hash_nid, digest,
                            digest_len) ||
-      !RSA_sign_raw(rsa, &size_t_out_len, out, rsa_size, signed_msg,
-                    signed_msg_len, RSA_PKCS1_PADDING)) {
+      !rsa_sign_raw_no_self_test(rsa, &size_t_out_len, out, rsa_size,
+                                 signed_msg, signed_msg_len,
+                                 RSA_PKCS1_PADDING)) {
    goto err;
  }

@@ -555,6 +565,13 @@ err:
  return ret;
 }

+int RSA_sign(int hash_nid, const uint8_t *digest, unsigned digest_len,
+             uint8_t *out, unsigned *out_len, RSA *rsa) {
+  boringssl_ensure_rsa_self_test();
+
+  return rsa_sign_no_self_test(hash_nid, digest, digest_len, out, out_len, rsa);
+}
+
 int RSA_sign_pss_mgf1(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
                      const uint8_t *digest, size_t digest_len,
                      const EVP_MD *md, const EVP_MD *mgf1_md, int salt_len) {
@@ -578,8 +595,9 @@ int RSA_sign_pss_mgf1(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
  return ret;
 }

-int RSA_verify(int hash_nid, const uint8_t *digest, size_t digest_len,
-               const uint8_t *sig, size_t sig_len, RSA *rsa) {
+int rsa_verify_no_self_test(int hash_nid, const uint8_t *digest,
+                            size_t digest_len, const uint8_t *sig,
+                            size_t sig_len, RSA *rsa) {
  if (rsa->n == NULL || rsa->e == NULL) {
    OPENSSL_PUT_ERROR(RSA, RSA_R_VALUE_MISSING);
    return 0;
@@ -603,12 +621,9 @@ int RSA_verify(int hash_nid, const uint8_t *digest, size_t digest_len,
    return 0;
  }

-  if (!RSA_verify_raw(rsa, &len, buf, rsa_size, sig, sig_len,
-                      RSA_PKCS1_PADDING)) {
-    goto out;
-  }
-
-  if (!RSA_add_pkcs1_prefix(&signed_msg, &signed_msg_len,
+  if (!rsa_verify_raw_no_self_test(rsa, &len, buf, rsa_size, sig, sig_len,
+                                   RSA_PKCS1_PADDING) ||
+      !RSA_add_pkcs1_prefix(&signed_msg, &signed_msg_len,
                            &signed_msg_is_alloced, hash_nid, digest,
                            digest_len)) {
    goto out;
@@ -631,6 +646,13 @@ out:
  return ret;
 }

+int RSA_verify(int hash_nid, const uint8_t *digest, size_t digest_len,
+               const uint8_t *sig, size_t sig_len, RSA *rsa) {
+  boringssl_ensure_rsa_self_test();
+  return rsa_verify_no_self_test(hash_nid, digest, digest_len, sig, sig_len,
+                                 rsa);
+}
+
 int RSA_verify_pss_mgf1(RSA *rsa, const uint8_t *digest, size_t digest_len,
                        const EVP_MD *md, const EVP_MD *mgf1_md, int salt_len,
                        const uint8_t *sig, size_t sig_len) {
@@ -261,6 +261,8 @@ size_t rsa_default_size(const RSA *rsa) {

 int RSA_encrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
                const uint8_t *in, size_t in_len, int padding) {
+  boringssl_ensure_rsa_self_test();
+
  if (!rsa_check_public_key(rsa)) {
    return 0;
  }
@@ -528,6 +530,8 @@ err:

 int rsa_default_decrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
                        const uint8_t *in, size_t in_len, int padding) {
+  boringssl_ensure_rsa_self_test();
+
  const unsigned rsa_size = RSA_size(rsa);
  uint8_t *buf = NULL;
  int ret = 0;
@@ -593,8 +597,9 @@ err:

 static int mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx);

-int RSA_verify_raw(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
-                   const uint8_t *in, size_t in_len, int padding) {
+int rsa_verify_raw_no_self_test(RSA *rsa, size_t *out_len, uint8_t *out,
+                                size_t max_out, const uint8_t *in,
+                                size_t in_len, int padding) {
  if (!rsa_check_public_key(rsa)) {
    return 0;
  }
@@ -686,6 +691,14 @@ err:
  return ret;
 }

+int RSA_verify_raw(RSA *rsa, size_t *out_len, uint8_t *out,
+                                size_t max_out, const uint8_t *in,
+                                size_t in_len, int padding) {
+  boringssl_ensure_rsa_self_test();
+  return rsa_verify_raw_no_self_test(rsa, out_len, out, max_out, in, in_len,
+                                     padding);
+}
+
 int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in,
                                  size_t len) {
  if (rsa->n == NULL || rsa->d == NULL) {
@@ -1324,6 +1337,8 @@ static void replace_bn_mont_ctx(BN_MONT_CTX **out, BN_MONT_CTX **in) {
 static int RSA_generate_key_ex_maybe_fips(RSA *rsa, int bits,
                                          const BIGNUM *e_value, BN_GENCB *cb,
                                          int check_fips) {
+  boringssl_ensure_rsa_self_test();
+
  RSA *tmp = NULL;
  uint32_t err;
  int ret = 0;
@@ -296,27 +296,22 @@ err:
  return NULL;
 }

-static int boringssl_self_test_slow(void) {
+
+// Lazy self-tests
+//
+// Self tests that are slow are deferred until the corresponding algorithm is
+// actually exercised, in FIPS mode. (In non-FIPS mode these tests are only run
+// when requested by |BORINGSSL_self_test|.)
+
+static int boringssl_self_test_rsa(void) {
  int ret = 0;
-  RSA *rsa_key = NULL;
-  EC_KEY *ec_key = NULL;
-  EC_GROUP *ec_group = NULL;
-  EC_POINT *ec_point_in = NULL;
-  EC_POINT *ec_point_out = NULL;
-  BIGNUM *ec_scalar = NULL;
-  ECDSA_SIG *sig = NULL;
-  DH *dh = NULL;
-  BIGNUM *ffdhe2048_value = NULL;
  uint8_t output[256];

-  rsa_key = self_test_rsa_key();
+  RSA *const rsa_key = self_test_rsa_key();
  if (rsa_key == NULL) {
-    fprintf(stderr, "RSA KeyGen failed\n");
+    fprintf(stderr, "RSA key construction failed\n");
    goto err;
  }
-  // Disable blinding for the power-on tests because it's not needed and
-  // triggers an entropy draw.
-  rsa_key->flags |= RSA_FLAG_NO_BLINDING;

  // RSA Sign KAT

@@ -351,8 +346,8 @@ static int boringssl_self_test_slow(void) {
  };

  unsigned sig_len;
-  if (!RSA_sign(NID_sha256, kRSASignDigest, sizeof(kRSASignDigest), output,
-                &sig_len, rsa_key) ||
+  if (!rsa_sign_no_self_test(NID_sha256, kRSASignDigest, sizeof(kRSASignDigest),
+                             output, &sig_len, rsa_key) ||
      !check_test(kRSASignSignature, output, sizeof(kRSASignSignature),
                  "RSA-sign KAT")) {
    fprintf(stderr, "RSA signing test failed.\n");
@@ -390,12 +385,53 @@ static int boringssl_self_test_slow(void) {
      0x4d, 0xbe, 0xa4, 0x16, 0x15, 0x34, 0x5c, 0x88, 0x53, 0x25, 0x92, 0x67,
      0x44, 0xa5, 0x39, 0x15,
  };
-  if (!RSA_verify(NID_sha256, kRSAVerifyDigest, sizeof(kRSAVerifyDigest),
-                  kRSAVerifySignature, sizeof(kRSAVerifySignature), rsa_key)) {
+  if (!rsa_verify_no_self_test(NID_sha256, kRSAVerifyDigest,
+                               sizeof(kRSAVerifyDigest), kRSAVerifySignature,
+                               sizeof(kRSAVerifySignature), rsa_key)) {
    fprintf(stderr, "RSA-verify KAT failed.\n");
    goto err;
  }

+  ret = 1;
+
+err:
+  RSA_free(rsa_key);
+
+  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) {
+  int ret = 0;
+  EC_KEY *ec_key = NULL;
+  EC_GROUP *ec_group = NULL;
+  EC_POINT *ec_point_in = NULL;
+  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) {
    fprintf(stderr, "ECDSA KeyGen failed\n");
@@ -572,7 +608,6 @@ static int boringssl_self_test_slow(void) {
  ret = 1;

 err:
-  RSA_free(rsa_key);
  EC_KEY_free(ec_key);
  EC_POINT_free(ec_point_in);
  EC_POINT_free(ec_point_out);
@@ -644,7 +679,7 @@ int boringssl_self_test_hmac_sha256(void) {
                    "HMAC-SHA-256 KAT");
 }

-int BORINGSSL_self_test(void) {
+static int boringssl_self_test_fast(void) {
  static const uint8_t kAESKey[16] = "BoringCrypto Key";
  static const uint8_t kAESIV[16] = {0};

@@ -862,7 +897,7 @@ int BORINGSSL_self_test(void) {
    goto err;
  }

-  ret = boringssl_self_test_slow();
+  ret = 1;

 err:
  EVP_AEAD_CTX_cleanup(&aead_ctx);
@@ -870,4 +905,26 @@ err:
  return ret;
 }

+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()) {
+    return 0;
+  }
+
+  return 1;
+}
+
+#if defined(BORINGSSL_FIPS)
+int boringssl_self_test_startup(void) {
+  if (!boringssl_self_test_fast() ||
+      !boringssl_self_test_slow()) {
+    return 0;
+  }
+
+  return 1;
+}
+#endif
+
 #endif  // !_MSC_VER
@@ -938,6 +938,22 @@ static inline uint64_t CRYPTO_rotr_u64(uint64_t value, int shift) {
 // process.
 void BORINGSSL_FIPS_abort(void) __attribute__((noreturn));

+// boringssl_self_test_startup runs all startup self tests and returns one on
+// success or zero on error. Startup self tests do not include lazy tests.
+// Call |BORINGSSL_self_test| to run every self test.
+int boringssl_self_test_startup(void);
+
+// boringssl_ensure_rsa_self_test checks whether the RSA self-test has been run
+// in this address space. If not, it runs it and crashes the address space if
+// unsuccessful.
+void boringssl_ensure_rsa_self_test(void);
+
+#else
+
+// Outside of FIPS mode, the lazy tests are no-ops.
+
+OPENSSL_INLINE void boringssl_ensure_rsa_self_test(void) {}
+
 #endif  // FIPS

 // boringssl_self_test_sha256 performs a SHA-256 KAT.