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>

crypto/fipsmodule/rsa/internal.h

@@ -142,7 +142,7 @@ int rsa_verify_raw_no_self_test(RSA *rsa, size_t *out_len, uint8_t *out,
                                 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);
 
 

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 @@ static const struct pkcs1_sig_prefix kPKCS1SigPrefixes[] = {
     },
 };
 
-int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
+static int rsa_check_digest_size(int hash_nid, size_t digest_len) {
-                         int *is_alloced, int hash_nid, const uint8_t *digest,
-                         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 @@ int RSA_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
       continue;
     }
 
-    if (digest_len != sig_prefix->hash_len) {
+    // The length should already have been checked.
-      OPENSSL_PUT_ERROR(RSA, RSA_R_INVALID_MESSAGE_LENGTH);
+    assert(digest_len == sig_prefix->hash_len);
-      return 0;
-    }
-
     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_add_pkcs1_prefix(uint8_t **out_msg, size_t *out_msg_len,
 }
 
 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 @@ err:
   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();
 

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 @@ RSA *RSA_generate_key(int bits, unsigned long e_value, void *callback,
 
   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;
   }

include/openssl/rsa.h

@@ -298,8 +298,8 @@ OPENSSL_EXPORT int RSA_private_decrypt(size_t flen, const uint8_t *from,
 // |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,
+                            size_t digest_len, uint8_t *out, unsigned *out_len,
-                            unsigned *out_len, RSA *rsa);
+                            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 @@ OPENSSL_EXPORT int RSA_blinding_on(RSA *rsa, BN_CTX *ctx);
 // 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