commit 3ef608594db650af005df6cae18e9f21154c11f6
author Luke Granger-Brown <git@lukegb.com> Sat Dec 12 13:07:42 2015 +0000
committer Adam Langley <agl@google.com> Mon Dec 21 23:49:02 2015 +0000
tree d09266d01404efd17111c9f418bf8f0dcdfdff39
parent afe57cb14d36f70ad4a109fc5e7765d1adc67035

Refuse to parse RSA pubkeys with invalid exponents.

We should reject RSA public keys with exponents of less than 3.

This change also rejects even exponents, although the usefulness
of such a public key is somewhat questionable.

BUG=chromium:541257

Change-Id: I1499e9762ba40a7cf69155d21d55bc210cd6d273
Reviewed-on: https://boringssl-review.googlesource.com/6710
Reviewed-by: Adam Langley <agl@google.com>

crypto/rsa/rsa_asn1.c

diff --git a/crypto/rsa/rsa_asn1.c b/crypto/rsa/rsa_asn1.c
index 6144e74..b73a0e1 100644
--- a/crypto/rsa/rsa_asn1.c
+++ b/crypto/rsa/rsa_asn1.c
@@ -108,6 +108,14 @@
     RSA_free(ret);
     return NULL;
   }
+
+  if (!BN_is_odd(ret->e) ||
+      BN_num_bits(ret->e) < 2) {
+    OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_RSA_PARAMETERS);
+    RSA_free(ret);
+    return NULL;
+  }
+
   return ret;
 }
 

crypto/rsa/rsa_test.cc

diff --git a/crypto/rsa/rsa_test.cc b/crypto/rsa/rsa_test.cc
index 57b360c..5545161 100644
--- a/crypto/rsa/rsa_test.cc
+++ b/crypto/rsa/rsa_test.cc
@@ -495,6 +495,34 @@
     0x02, 0x03, 0x01, 0x00, 0x01,
 };
 
+// kExponent1RSAKey is an RSAPublicKey encoded with an exponent of 1. See
+// https://crbug.com/541257
+static const uint8_t kExponent1RSAKey[] = {
+    0x30, 0x82, 0x01, 0x08, 0x02, 0x82, 0x01, 0x01, 0x00, 0xcf, 0x86, 0x9a,
+    0x7d, 0x5c, 0x9f, 0xbd, 0x33, 0xbb, 0xc2, 0xb1, 0x06, 0xa8, 0x3e, 0xc5,
+    0x18, 0xf3, 0x01, 0x04, 0xdd, 0x7a, 0x38, 0x0e, 0x8e, 0x8d, 0x10, 0xaa,
+    0xf8, 0x64, 0x49, 0x82, 0xa6, 0x16, 0x9d, 0xd9, 0xae, 0x5e, 0x7f, 0x9b,
+    0x53, 0xcb, 0xbb, 0x29, 0xda, 0x98, 0x47, 0x26, 0x88, 0x2e, 0x1d, 0x64,
+    0xb3, 0xbc, 0x7e, 0x96, 0x3a, 0xa7, 0xd6, 0x87, 0xf6, 0xf5, 0x3f, 0xa7,
+    0x3b, 0xd3, 0xc5, 0xd5, 0x61, 0x3c, 0x63, 0x05, 0xf9, 0xbc, 0x64, 0x1d,
+    0x71, 0x65, 0xf5, 0xc8, 0xe8, 0x64, 0x41, 0x35, 0x88, 0x81, 0x6b, 0x2a,
+    0x24, 0xbb, 0xdd, 0x9f, 0x75, 0x4f, 0xea, 0x35, 0xe5, 0x32, 0x76, 0x5a,
+    0x8b, 0x7a, 0xb5, 0x92, 0x65, 0x34, 0xb7, 0x88, 0x42, 0x5d, 0x41, 0x0b,
+    0xd1, 0x00, 0x2d, 0x43, 0x47, 0x55, 0x60, 0x3c, 0x0e, 0x60, 0x04, 0x5c,
+    0x88, 0x13, 0xc7, 0x42, 0x55, 0x16, 0x31, 0x32, 0x81, 0xba, 0xde, 0xa9,
+    0x56, 0xeb, 0xdb, 0x66, 0x7f, 0x31, 0xba, 0xe8, 0x87, 0x1a, 0xcc, 0xad,
+    0x90, 0x86, 0x4b, 0xa7, 0x6d, 0xd5, 0xc1, 0xb7, 0xe7, 0x67, 0x56, 0x41,
+    0xf7, 0x03, 0xb3, 0x09, 0x61, 0x63, 0xb5, 0xb0, 0x19, 0x7b, 0xc5, 0x91,
+    0xc8, 0x96, 0x5b, 0x6a, 0x80, 0xa1, 0x53, 0x0f, 0x9a, 0x47, 0xb5, 0x9a,
+    0x44, 0x53, 0xbd, 0x93, 0xe3, 0xe4, 0xce, 0x0c, 0x17, 0x11, 0x51, 0x1d,
+    0xfd, 0x6c, 0x74, 0xe4, 0xec, 0x2a, 0xce, 0x57, 0x27, 0xcc, 0x83, 0x98,
+    0x08, 0x32, 0x2c, 0xd5, 0x75, 0xa9, 0x27, 0xfe, 0xaa, 0x5e, 0x48, 0xc9,
+    0x46, 0x9a, 0x29, 0x3f, 0xe6, 0x01, 0x4d, 0x97, 0x4a, 0x70, 0xd1, 0x5d,
+    0xf8, 0xc0, 0x0b, 0x23, 0xcb, 0xbe, 0xf5, 0x70, 0x0b, 0xc2, 0xf2, 0xc0,
+    0x33, 0x9c, 0xc4, 0x8b, 0x39, 0x7e, 0x3d, 0xc6, 0x23, 0x39, 0x9a, 0x98,
+    0xdd, 0x02, 0x01, 0x01,
+};
+
 static bool TestRSA(const uint8_t *der, size_t der_len,
                     const uint8_t *oaep_ciphertext,
                     size_t oaep_ciphertext_len) {
@@ -845,6 +873,19 @@
   return true;
 }
 
+static bool TestBadExponent() {
+  ScopedRSA rsa(RSA_public_key_from_bytes(kExponent1RSAKey,
+                                          sizeof(kExponent1RSAKey)));
+
+  if (rsa) {
+    fprintf(stderr, "kExponent1RSAKey parsed but should have failed.\n");
+    return false;
+  }
+
+  ERR_clear_error();
+  return true;
+}
+
 int main(int argc, char *argv[]) {
   CRYPTO_library_init();
 
@@ -867,7 +908,8 @@
                             kSixPrimeEncryptedMessage,
                             sizeof(kSixPrimeEncryptedMessage)) ||
       !TestMultiPrimeKeygen() ||
-      !TestASN1()) {
+      !TestASN1() ||
+      !TestBadExponent()) {
     return 1;
   }