From 073391f7d69da2d94e06d087224f924fb6d33144 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 3 May 2017 15:03:35 -0400 Subject: [PATCH] Detach encrypt and keygen hooks from RSA_METHOD. Nothing is using them. For encrypt, there's generally no need to swap out public key operations. keygen seems especially pointless as one could just as easily call the other function directly. The one behavior change is RSA_encrypt now gracefully detects if called on an empty RSA, to match the other un-RSA_METHOD-ed functions which had similar treatments. (Conscrypt was filling in the encrypt function purely to provide a non-crashing no-op function. They leave the public bits blank and pass their custom keys through sufficiently many layers of Java crypto goo that it's not obvious whether this is reachable.) We still can't take the function pointers out, but once https://github.com/google/conscrypt/commit/96bbe03dfd2737f0c1461db59966ff41502a91e4 trickles back into everything, we can finally prune RSA_METHOD. Bump BORINGSSL_API_VERSION as a convenience so I can land the corresponding removal in Conscrypt immediately. Change-Id: Ia2ef4780a5dfcb869b224e1ff632daab8d378b2e Reviewed-on: https://boringssl-review.googlesource.com/15864 Reviewed-by: Adam Langley Commit-Queue: Adam Langley CQ-Verified: CQ bot account: commit-bot@chromium.org --- crypto/rsa/internal.h | 3 --- crypto/rsa/rsa.c | 17 ----------------- crypto/rsa/rsa_impl.c | 19 ++++++++++++------- include/openssl/base.h | 2 +- include/openssl/rsa.h | 7 ++++--- 5 files changed, 17 insertions(+), 31 deletions(-) diff --git a/crypto/rsa/internal.h b/crypto/rsa/internal.h index 29202b67..56b633b4 100644 --- a/crypto/rsa/internal.h +++ b/crypto/rsa/internal.h @@ -72,8 +72,6 @@ extern "C" { extern const RSA_METHOD RSA_default_method; size_t rsa_default_size(const RSA *rsa); -int rsa_default_encrypt(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_default_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); @@ -81,7 +79,6 @@ 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); int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in, size_t len); -int rsa_default_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb); BN_BLINDING *BN_BLINDING_new(void); diff --git a/crypto/rsa/rsa.c b/crypto/rsa/rsa.c index e3745fdb..afc1432e 100644 --- a/crypto/rsa/rsa.c +++ b/crypto/rsa/rsa.c @@ -192,23 +192,6 @@ void RSA_get0_crt_params(const RSA *rsa, const BIGNUM **out_dmp1, } } -int RSA_generate_key_ex(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) { - if (rsa->meth->keygen) { - return rsa->meth->keygen(rsa, bits, e_value, cb); - } - - return rsa_default_keygen(rsa, bits, e_value, cb); -} - -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) { - if (rsa->meth->encrypt) { - return rsa->meth->encrypt(rsa, out_len, out, max_out, in, in_len, padding); - } - - return rsa_default_encrypt(rsa, out_len, out, max_out, in, in_len, padding); -} - int RSA_public_encrypt(size_t flen, const uint8_t *from, uint8_t *to, RSA *rsa, int padding) { size_t out_len; diff --git a/crypto/rsa/rsa_impl.c b/crypto/rsa/rsa_impl.c index 4f9b4e6d..012a9849 100644 --- a/crypto/rsa/rsa_impl.c +++ b/crypto/rsa/rsa_impl.c @@ -112,8 +112,13 @@ size_t rsa_default_size(const RSA *rsa) { return BN_num_bytes(rsa->n); } -int rsa_default_encrypt(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_encrypt(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->n == NULL || rsa->e == NULL) { + OPENSSL_PUT_ERROR(RSA, RSA_R_VALUE_MISSING); + return 0; + } + const unsigned rsa_size = RSA_size(rsa); BIGNUM *f, *result; uint8_t *buf = NULL; @@ -902,7 +907,7 @@ err: return ret; } -int rsa_default_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) { +int RSA_generate_key_ex(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) { /* See FIPS 186-4 appendix B.3. This function implements a generalized version * of the FIPS algorithm. For FIPS compliance, the caller is responsible for * passing in 2048 or 3072 to |bits| and 65537 for |e_value|. */ @@ -1042,10 +1047,10 @@ const RSA_METHOD RSA_default_method = { NULL /* sign */, NULL /* verify */, - NULL /* encrypt (defaults to rsa_default_encrypt) */, + NULL /* encrypt (ignored) */, NULL /* sign_raw (defaults to rsa_default_sign_raw) */, NULL /* decrypt (defaults to rsa_default_decrypt) */, - NULL /* verify_raw (defaults to rsa_default_verify_raw) */, + NULL /* verify_raw (ignored) */, NULL /* private_transform (defaults to rsa_default_private_transform) */, @@ -1054,8 +1059,8 @@ const RSA_METHOD RSA_default_method = { RSA_FLAG_CACHE_PUBLIC | RSA_FLAG_CACHE_PRIVATE, - NULL /* keygen (defaults to rsa_default_keygen) */, + NULL /* keygen (ignored) */, NULL /* multi_prime_keygen (ignored) */, - NULL /* supports_digest */, + NULL /* supports_digest (ignored) */, }; diff --git a/include/openssl/base.h b/include/openssl/base.h index 70c20d76..c8ebee82 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -145,7 +145,7 @@ extern "C" { * A consumer may use this symbol in the preprocessor to temporarily build * against multiple revisions of BoringSSL at the same time. It is not * recommended to do so for longer than is necessary. */ -#define BORINGSSL_API_VERSION 3 +#define BORINGSSL_API_VERSION 4 #if defined(BORINGSSL_SHARED_LIBRARY) diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h index c5510c85..b697a7df 100644 --- a/include/openssl/rsa.h +++ b/include/openssl/rsa.h @@ -583,13 +583,13 @@ struct rsa_meth_st { int (*verify)(int dtype, const uint8_t *m, unsigned int m_length, const uint8_t *sigbuf, unsigned int siglen, const RSA *rsa); - - /* These functions mirror the |RSA_*| functions of the same name. */ + /* Ignored. Set this to NULL. */ int (*encrypt)(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out, const uint8_t *in, size_t in_len, int padding); + + /* These functions mirror the |RSA_*| functions of the same name. */ int (*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); - int (*decrypt)(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out, const uint8_t *in, size_t in_len, int padding); /* Ignored. Set this to NULL. */ @@ -621,6 +621,7 @@ struct rsa_meth_st { int flags; + /* Ignored. Set this to NULL. */ int (*keygen)(RSA *rsa, int bits, BIGNUM *e, BN_GENCB *cb); /* Ignored. Set this to NULL. */