commit 625475f3e3a824638903b7366366a28ea69316a3
author Brian Smith <brian@briansmith.org> Tue Jan 12 10:47:25 2016 -1000
committer Adam Langley <agl@google.com> Wed Jan 13 22:28:54 2016 +0000
tree 4a84b71c0652dceb43e3ef5dc20f1c444af4cf41
parent 7b8b9c17db93ea5287575b437c77fb36eeb81b31

Fix bits vs. bytes confusion in RSA encryption.

rsa_default_encrypt allowed an RSA modulus 8 times larger than the
intended maximum size due to bits vs. bytes confusion.

Further, as |rsa_default_encrypt| got this wrong while
|rsa_default_verify_raw| got it right, factor out the duplicated logic
so that such inconsistencies are less likely to occur.

BUG=576856

Change-Id: Ic842fadcbb3b140d2ba4295793457af2b62d9444
Reviewed-on: https://boringssl-review.googlesource.com/6900
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>

crypto/rsa/rsa_impl.c

diff --git a/crypto/rsa/rsa_impl.c b/crypto/rsa/rsa_impl.c
index b1cfaa6..41acf0d 100644
--- a/crypto/rsa/rsa_impl.c
+++ b/crypto/rsa/rsa_impl.c
@@ -67,11 +67,26 @@
 #include "../internal.h"
 
 
-#define OPENSSL_RSA_MAX_MODULUS_BITS 16384
-#define OPENSSL_RSA_SMALL_MODULUS_BITS 3072
-#define OPENSSL_RSA_MAX_PUBEXP_BITS \
-  64 /* exponent limit enforced for "large" modulus only */
+static int check_modulus_and_exponent_sizes(const RSA *rsa) {
+  unsigned rsa_bits = BN_num_bits(rsa->n);
+  if (rsa_bits > 16 * 1024) {
+    OPENSSL_PUT_ERROR(RSA, RSA_R_MODULUS_TOO_LARGE);
+    return 0;
+  }
 
+  if (BN_ucmp(rsa->n, rsa->e) <= 0) {
+    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE);
+    return 0;
+  }
+
+  /* For large moduli only, enforce exponent limit. */
+  if (rsa_bits > 3072 && BN_num_bits(rsa->e) > 64) {
+    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE);
+    return 0;
+  }
+
+  return 1;
+}
 
 size_t rsa_default_size(const RSA *rsa) {
   return BN_num_bytes(rsa->n);
@@ -85,25 +100,12 @@
   BN_CTX *ctx = NULL;
   int i, ret = 0;
 
-  if (rsa_size > OPENSSL_RSA_MAX_MODULUS_BITS) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_MODULUS_TOO_LARGE);
-    return 0;
-  }
-
   if (max_out < rsa_size) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_OUTPUT_BUFFER_TOO_SMALL);
     return 0;
   }
 
-  if (BN_ucmp(rsa->n, rsa->e) <= 0) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE);
-    return 0;
-  }
-
-  /* for large moduli, enforce exponent limit */
-  if (BN_num_bits(rsa->n) > OPENSSL_RSA_SMALL_MODULUS_BITS &&
-      BN_num_bits(rsa->e) > OPENSSL_RSA_MAX_PUBEXP_BITS) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE);
+  if (!check_modulus_and_exponent_sizes(rsa)) {
     return 0;
   }
 
@@ -417,25 +419,12 @@
   uint8_t *buf = NULL;
   BN_CTX *ctx = NULL;
 
-  if (BN_num_bits(rsa->n) > OPENSSL_RSA_MAX_MODULUS_BITS) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_MODULUS_TOO_LARGE);
-    return 0;
-  }
-
-  if (BN_ucmp(rsa->n, rsa->e) <= 0) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE);
-    return 0;
-  }
-
   if (max_out < rsa_size) {
     OPENSSL_PUT_ERROR(RSA, RSA_R_OUTPUT_BUFFER_TOO_SMALL);
     return 0;
   }
 
-  /* for large moduli, enforce exponent limit */
-  if (BN_num_bits(rsa->n) > OPENSSL_RSA_SMALL_MODULUS_BITS &&
-      BN_num_bits(rsa->e) > OPENSSL_RSA_MAX_PUBEXP_BITS) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_E_VALUE);
+  if (!check_modulus_and_exponent_sizes(rsa)) {
     return 0;
   }