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>
diff --git a/crypto/fipsmodule/bcm.c b/crypto/fipsmodule/bcm.c
index 4b0f7e1..505510c 100644
--- a/crypto/fipsmodule/bcm.c
+++ b/crypto/fipsmodule/bcm.c
@@ -252,7 +252,7 @@
OPENSSL_cleanse(result, sizeof(result)); // FIPS 140-3, AS05.10.
#endif // OPENSSL_ASAN
- if (!BORINGSSL_self_test()) {
+ if (!boringssl_self_test_startup()) {
goto err;
}
diff --git a/crypto/fipsmodule/rsa/internal.h b/crypto/fipsmodule/rsa/internal.h
index d9d6fac..1cb3b5f 100644
--- a/crypto/fipsmodule/rsa/internal.h
+++ b/crypto/fipsmodule/rsa/internal.h
@@ -124,6 +124,28 @@
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
diff --git a/crypto/fipsmodule/rsa/rsa.c b/crypto/fipsmodule/rsa/rsa.c
index 83649d3..03acd27 100644
--- a/crypto/fipsmodule/rsa/rsa.c
+++ b/crypto/fipsmodule/rsa/rsa.c
@@ -304,8 +304,9 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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) {
diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c
index a6865c0..1046f35 100644
--- a/crypto/fipsmodule/rsa/rsa_impl.c
+++ b/crypto/fipsmodule/rsa/rsa_impl.c
@@ -261,6 +261,8 @@
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 @@
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 @@
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 @@
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 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;
diff --git a/crypto/fipsmodule/self_check/self_check.c b/crypto/fipsmodule/self_check/self_check.c
index fe40d90..21e9e0a 100644
--- a/crypto/fipsmodule/self_check/self_check.c
+++ b/crypto/fipsmodule/self_check/self_check.c
@@ -296,27 +296,22 @@
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 @@
};
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 @@
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 @@
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 @@
"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 @@
goto err;
}
- ret = boringssl_self_test_slow();
+ ret = 1;
err:
EVP_AEAD_CTX_cleanup(&aead_ctx);
@@ -870,4 +905,26 @@
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
diff --git a/crypto/internal.h b/crypto/internal.h
index fe2ba39..dac515c 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -938,6 +938,22 @@
// 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.
