Switch RSA_sign to size_t.
While I'm here, use a fixed-size uint64_t in RSA_generate_key, rather
than unsigned long. This code also assumes unsigned long fits in
BN_ULONG, which is probably true on all platforms we care about, but
unnecessarily fussy.
The RSA_sign -> RSA_METHOD transition does require a cast. Go ahead and
check length/hash_nid consistency so we know it fits in the cast. This
does mean RSA_METHOD-backed keys are restricted to implementing digests
that we support, but that's probably fine. If anything, I think we
should try to shift away from RSA_METHOD as a story for custom keys.
Bug: 516
Change-Id: I3969da67d1daeff882279a534eb48ca831eb16cd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54465
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/fipsmodule/rsa/internal.h b/crypto/fipsmodule/rsa/internal.h
index 1cb3b5f..d0b5a4a 100644
--- a/crypto/fipsmodule/rsa/internal.h
+++ b/crypto/fipsmodule/rsa/internal.h
@@ -142,7 +142,7 @@
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,
+ size_t digest_len, uint8_t *out, unsigned *out_len,
RSA *rsa);
diff --git a/crypto/fipsmodule/rsa/rsa.c b/crypto/fipsmodule/rsa/rsa.c
index 14cdae5..6b3e228 100644
--- a/crypto/fipsmodule/rsa/rsa.c
+++ b/crypto/fipsmodule/rsa/rsa.c
@@ -56,6 +56,7 @@
#include <openssl/rsa.h>
+#include <assert.h>
#include <limits.h>
#include <string.h>
@@ -470,18 +471,43 @@
},
};
-int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
- int *is_alloced, int hash_nid, const uint8_t *digest,
- size_t digest_len) {
+static int rsa_check_digest_size(int hash_nid, size_t digest_len) {
if (hash_nid == NID_md5_sha1) {
- // Special case: SSL signature, just check the length.
if (digest_len != SSL_SIG_LENGTH) {
OPENSSL_PUT_ERROR(RSA, RSA_R_INVALID_MESSAGE_LENGTH);
return 0;
}
+ return 1;
+ }
+ for (size_t i = 0; kPKCS1SigPrefixes[i].nid != NID_undef; i++) {
+ const struct pkcs1_sig_prefix *sig_prefix = &kPKCS1SigPrefixes[i];
+ if (sig_prefix->nid == hash_nid) {
+ if (digest_len != sig_prefix->hash_len) {
+ OPENSSL_PUT_ERROR(RSA, RSA_R_INVALID_MESSAGE_LENGTH);
+ return 0;
+ }
+ return 1;
+ }
+ }
+
+ OPENSSL_PUT_ERROR(RSA, RSA_R_UNKNOWN_ALGORITHM_TYPE);
+ return 0;
+
+}
+
+int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
+ int *is_alloced, int hash_nid, const uint8_t *digest,
+ size_t digest_len) {
+ if (!rsa_check_digest_size(hash_nid, digest_len)) {
+ return 0;
+ }
+
+ if (hash_nid == NID_md5_sha1) {
+ // The length should already have been checked.
+ assert(digest_len == SSL_SIG_LENGTH);
*out_msg = (uint8_t *)digest;
- *out_msg_len = SSL_SIG_LENGTH;
+ *out_msg_len = digest_len;
*is_alloced = 0;
return 1;
}
@@ -492,11 +518,8 @@
continue;
}
- if (digest_len != sig_prefix->hash_len) {
- OPENSSL_PUT_ERROR(RSA, RSA_R_INVALID_MESSAGE_LENGTH);
- return 0;
- }
-
+ // The length should already have been checked.
+ assert(digest_len == sig_prefix->hash_len);
const uint8_t* prefix = sig_prefix->bytes;
size_t prefix_len = sig_prefix->len;
size_t signed_msg_len = prefix_len + digest_len;
@@ -526,19 +549,25 @@
}
int rsa_sign_no_self_test(int hash_nid, const uint8_t *digest,
- unsigned digest_len, uint8_t *out, unsigned *out_len,
+ size_t digest_len, uint8_t *out, unsigned *out_len,
RSA *rsa) {
+ if (rsa->meth->sign) {
+ if (!rsa_check_digest_size(hash_nid, digest_len)) {
+ return 0;
+ }
+ // All supported digest lengths fit in |unsigned|.
+ assert(digest_len <= EVP_MAX_MD_SIZE);
+ static_assert(EVP_MAX_MD_SIZE <= UINT_MAX, "digest too long");
+ return rsa->meth->sign(hash_nid, digest, (unsigned)digest_len, out, out_len,
+ rsa);
+ }
+
const unsigned rsa_size = RSA_size(rsa);
int ret = 0;
uint8_t *signed_msg = NULL;
size_t signed_msg_len = 0;
int signed_msg_is_alloced = 0;
size_t size_t_out_len;
-
- if (rsa->meth->sign) {
- return rsa->meth->sign(hash_nid, digest, digest_len, out, out_len, rsa);
- }
-
if (!RSA_add_pkcs1_prefix(&signed_msg, &signed_msg_len,
&signed_msg_is_alloced, hash_nid, digest,
digest_len) ||
@@ -563,7 +592,7 @@
return ret;
}
-int RSA_sign(int hash_nid, const uint8_t *digest, unsigned digest_len,
+int RSA_sign(int hash_nid, const uint8_t *digest, size_t digest_len,
uint8_t *out, unsigned *out_len, RSA *rsa) {
boringssl_ensure_rsa_self_test();
diff --git a/decrepit/rsa/rsa_decrepit.c b/decrepit/rsa/rsa_decrepit.c
index 54be9b2..2c06fe3 100644
--- a/decrepit/rsa/rsa_decrepit.c
+++ b/decrepit/rsa/rsa_decrepit.c
@@ -61,7 +61,7 @@
#include <openssl/bn.h>
-RSA *RSA_generate_key(int bits, unsigned long e_value, void *callback,
+RSA *RSA_generate_key(int bits, uint64_t e_value, void *callback,
void *cb_arg) {
assert(callback == NULL);
assert(cb_arg == NULL);
@@ -71,7 +71,7 @@
if (rsa == NULL ||
e == NULL ||
- !BN_set_word(e, e_value) ||
+ !BN_set_u64(e, e_value) ||
!RSA_generate_key_ex(rsa, bits, e, NULL)) {
goto err;
}
diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h
index 57a2cb2..15ce1c7 100644
--- a/include/openssl/rsa.h
+++ b/include/openssl/rsa.h
@@ -298,8 +298,8 @@
// |hash_nid|. Passing unhashed inputs will not result in a secure signature
// scheme.
OPENSSL_EXPORT int RSA_sign(int hash_nid, const uint8_t *digest,
- unsigned digest_len, uint8_t *out,
- unsigned *out_len, RSA *rsa);
+ size_t digest_len, uint8_t *out, unsigned *out_len,
+ RSA *rsa);
// RSA_sign_pss_mgf1 signs |digest_len| bytes from |digest| with the public key
// from |rsa| using RSASSA-PSS with MGF1 as the mask generation function. It
@@ -625,7 +625,7 @@
// should use instead. It returns NULL on error, or a newly-allocated |RSA| on
// success. This function is provided for compatibility only. The |callback|
// and |cb_arg| parameters must be NULL.
-OPENSSL_EXPORT RSA *RSA_generate_key(int bits, unsigned long e, void *callback,
+OPENSSL_EXPORT RSA *RSA_generate_key(int bits, uint64_t e, void *callback,
void *cb_arg);
// d2i_RSAPublicKey parses a DER-encoded RSAPublicKey structure (RFC 8017) from
