Bug 1369091 - check that e > 2 and odd in RSA_NewKey, r=franziskus

Summary: Patch for bug 1369091

Reviewers: franziskus

Reviewed By: franziskus

Bug #: 1369091

Differential Revision: https://phabricator.services.mozilla.com/D165

--HG--
extra : amend_source : 849b90486eedd57ecda4f2c0e95791e53aa1f7da

gtests/freebl_gtest/freebl_gtest.gyp

@@ -33,6 +33,7 @@
         'dh_unittest.cc',
         'ecl_unittest.cc',
         'ghash_unittest.cc',
+        'rsa_unittest.cc',
         '<(DEPTH)/gtests/common/gtests.cc'
       ],
       'dependencies': [

gtests/freebl_gtest/rsa_unittest.cc

@@ -0,0 +1,57 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include "gtest/gtest.h"
+
+#include <stdint.h>
+
+#include "blapi.h"
+#include "secitem.h"
+
+template <class T>
+struct ScopedDelete {
+  void operator()(T* ptr) {
+    if (ptr) {
+      PORT_FreeArena(ptr->arena, PR_TRUE);
+    }
+  }
+};
+
+typedef std::unique_ptr<RSAPrivateKey, ScopedDelete<RSAPrivateKey>>
+    ScopedRSAPrivateKey;
+
+class RSANewKeyTest : public ::testing::Test {
+ protected:
+  RSAPrivateKey* CreateKeyWithExponent(int keySizeInBits,
+                                       unsigned char publicExponent) {
+    SECItem exp = {siBuffer, 0, 0};
+    unsigned char pubExp[1] = {publicExponent};
+    exp.data = pubExp;
+    exp.len = 1;
+
+    return RSA_NewKey(keySizeInBits, &exp);
+  }
+};
+
+TEST_F(RSANewKeyTest, expOneTest) {
+  ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x01));
+  ASSERT_TRUE(key == nullptr);
+}
+TEST_F(RSANewKeyTest, expTwoTest) {
+  ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x02));
+  ASSERT_TRUE(key == nullptr);
+}
+TEST_F(RSANewKeyTest, expFourTest) {
+  ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x04));
+  ASSERT_TRUE(key == nullptr);
+}
+TEST_F(RSANewKeyTest, WrongKeysizeTest) {
+  ScopedRSAPrivateKey key(CreateKeyWithExponent(2047, 0x03));
+  ASSERT_TRUE(key == nullptr);
+}
+
+TEST_F(RSANewKeyTest, expThreeTest) {
+  ScopedRSAPrivateKey key(CreateKeyWithExponent(2048, 0x03));
+  ASSERT_TRUE(key != nullptr);
+}

lib/freebl/rsa.c

@@ -276,7 +276,10 @@ RSAPrivateKey *
 RSA_NewKey(int keySizeInBits, SECItem *publicExponent)
 {
     unsigned int primeLen;
-    mp_int p, q, e, d;
+    mp_int p = { 0, 0, 0, NULL };
+    mp_int q = { 0, 0, 0, NULL };
+    mp_int e = { 0, 0, 0, NULL };
+    mp_int d = { 0, 0, 0, NULL };
     int kiter;
     int max_attempts;
     mp_err err = MP_OKAY;
@@ -290,41 +293,46 @@ RSA_NewKey(int keySizeInBits, SECItem *publicExponent)
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return NULL;
     }
-    /* 1. Allocate arena & key */
+    /* 1.  Set the public exponent and check if it's uneven and greater than 2.*/
+    MP_DIGITS(&e) = 0;
+    CHECK_MPI_OK(mp_init(&e));
+    SECITEM_TO_MPINT(*publicExponent, &e);
+    if (mp_iseven(&e) || !(mp_cmp_d(&e, 2) > 0)) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        goto cleanup;
+    }
+#ifndef NSS_FIPS_DISABLED
+    /* Check that the exponent is not smaller than 65537  */
+    if (mp_cmp_d(&e, 0x10001) < 0) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        goto cleanup;
+    }
+#endif
+
+    /* 2. Allocate arena & key */
     arena = PORT_NewArena(NSS_FREEBL_DEFAULT_CHUNKSIZE);
     if (!arena) {
         PORT_SetError(SEC_ERROR_NO_MEMORY);
-        return NULL;
+        goto cleanup;
     }
     key = PORT_ArenaZNew(arena, RSAPrivateKey);
     if (!key) {
         PORT_SetError(SEC_ERROR_NO_MEMORY);
-        PORT_FreeArena(arena, PR_TRUE);
-        return NULL;
+        goto cleanup;
     }
     key->arena = arena;
     /* length of primes p and q (in bytes) */
     primeLen = keySizeInBits / (2 * PR_BITS_PER_BYTE);
     MP_DIGITS(&p) = 0;
     MP_DIGITS(&q) = 0;
-    MP_DIGITS(&e) = 0;
     MP_DIGITS(&d) = 0;
     CHECK_MPI_OK(mp_init(&p));
     CHECK_MPI_OK(mp_init(&q));
-    CHECK_MPI_OK(mp_init(&e));
     CHECK_MPI_OK(mp_init(&d));
-    /* 2.  Set the version number (PKCS1 v1.5 says it should be zero) */
+    /* 3.  Set the version number (PKCS1 v1.5 says it should be zero) */
     SECITEM_AllocItem(arena, &key->version, 1);
     key->version.data[0] = 0;
-    /* 3.  Set the public exponent */
-    SECITEM_TO_MPINT(*publicExponent, &e);
-#ifndef NSS_FIPS_DISABLED
-    /* check the exponent size  we  */
-    if (mp_cmp_d(&e, 0x10001) < 0) {
-        PORT_SetError(SEC_ERROR_INVALID_ARGS);
-        goto cleanup;
-    }
-#endif
+
     kiter = 0;
     max_attempts = 5 * (keySizeInBits / 2); /* FIPS 186-4 B.3.3 steps 4.7 and 5.8 */
     do {