Bug 1651411 New tlsfuzzer code can still detect timing issues in RSA …

…operations.

This patch defeats Bleichenbacher by not trying to hide the size of the
decrypted text, but to hide if the text succeeded for failed. This is done
by generating a fake returned text that's based on the key and the cipher text,
so the fake data is always the same for the same key and cipher text. Both the
length and the plain text are generated with a prf.

Here's the proposed spec the patch codes to:

    1. Use SHA-256 to hash the private exponent encoded as a big-endian integer to a string the same length as the public modulus. Keep this value secret. (this is just an optimisation so that the implementation doesn't have to serialise the key over and over again)
    2. Check the length of input according to step one of https://tools.ietf.org/html/rfc8017#section-7.2.2
    3. When provided with a ciphertext, use SHA-256 HMAC(key=hash_from_step1, text=ciphertext) to generate the key derivation key
    4. Use SHA-256 HMAC with key derivation key as the key and a two-byte big-endian iterator concatenated with byte string "length" with the big-endian representation of 2048 (0x0800) as the bit length of the generated string.
      - Iterate this PRF 8 times to generate a 256 byte string
    5. initialise the length of synthetic message to 0
    6. split the PRF output into 2 byte strings, convert into big-endian integers, zero-out high-order bits so that they have the same bit length as the octet length of the maximum acceptable message size (k-11), select the last integer that is no larger than (k-11) or remain at 0 if no integer is smaller than (k-11); this selection needs to be performed using a side-channel free operators
    7. Use SHA-256 HMAC with key derivation key as the key and a two-byte big-endian iterator concatenated with byte string "message" with the big-endian representation of k*8
      - use this PRF to generate k bytes of output (right-truncate last HMAC call if the number of generated bytes is not a multiple of SHA-256 output size)
    8. perform the RSA decryption as described in step 2 of section 7.2.2 of rfc8017
    9. Verify the EM message padding as described in step 3 of section 7.2.2 of rfc8017, but instead of outputting "decryption error", return the last l bytes of the "message" PRF, when l is the selected synthetic message length using the "length" PRF, make this decision and copy using side-channel free operation

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

gtests/freebl_gtest/Makefile

@@ -0,0 +1,43 @@
+#! gmake
+#
+# 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/.
+
+#######################################################################
+# (1) Include initial platform-independent assignments (MANDATORY).   #
+#######################################################################
+
+include manifest.mn
+
+#######################################################################
+# (2) Include "global" configuration information. (OPTIONAL)          #
+#######################################################################
+
+include $(CORE_DEPTH)/coreconf/config.mk
+
+#######################################################################
+# (3) Include "component" configuration information. (OPTIONAL)       #
+#######################################################################
+
+
+#######################################################################
+# (4) Include "local" platform-dependent assignments (OPTIONAL).      #
+#######################################################################
+
+include ../common/gtest.mk
+
+#######################################################################
+# (5) Execute "global" rules. (OPTIONAL)                              #
+#######################################################################
+
+include $(CORE_DEPTH)/coreconf/rules.mk
+
+#######################################################################
+# (6) Execute "component" rules. (OPTIONAL)                           #
+#######################################################################
+
+
+#######################################################################
+# (7) Execute "local" rules. (OPTIONAL).                              #
+#######################################################################

gtests/freebl_gtest/manifest.mn

@@ -0,0 +1,38 @@
+#
+# 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/.
+CORE_DEPTH = ../..
+DEPTH      = ../..
+MODULE = nss
+
+# we'll need to figure out how to get these symbols linked
+# in before we include these tests:
+#     mpi_unittest.cc 
+#     ghash_unittest.cc
+CPPSRCS = \
+        dh_unittest.cc \
+        ecl_unittest.cc \
+        rsa_unittest.cc \
+        cmac_unittests.cc \
+      $(NULL)
+
+DEFINES += -DDLL_PREFIX=\"$(DLL_PREFIX)\" -DDLL_SUFFIX=\"$(DLL_SUFFIX)\"
+
+INCLUDES += -I$(CORE_DEPTH)/gtests/google_test/gtest/include \
+            -I$(CORE_DEPTH)/lib/freebl/ecl \
+            -I$(CORE_DEPTH)/lib/freebl/mpi \
+            -I$(CORE_DEPTH)/lib/freebl \
+            -I$(CORE_DEPTH)/gtests/common \
+            -I$(CORE_DEPTH)/cpputil
+
+REQUIRES = nspr nss libdbm gtest cpputil
+
+PROGRAM = freebl_gtest
+
+EXTRA_LIBS = $(DIST)/lib/$(LIB_PREFIX)gtest.$(LIB_SUFFIX) \
+             $(DIST)/lib/$(LIB_PREFIX)cpputil.$(LIB_SUFFIX) \
+             $(DIST)/lib/$(LIB_PREFIX)gtestutil.$(LIB_SUFFIX) \
+             $(NULL)
+
+USE_STATIC_LIBS=1

gtests/freebl_gtest/rsa_unittest.cc

@@ -78,18 +78,22 @@ TEST_F(RSATest, DecryptBlockTestErrors) {
 
   uint8_t in[256] = {0};
   // This should fail because the padding checks will fail.
+  // however, Bleichenbacher preventions means that failure would be
+  // a different output.
   rv = RSA_DecryptBlock(key.get(), out, &outputLen, maxOutputLen, in,
                         sizeof(in));
-  EXPECT_EQ(SECFailure, rv);
-  // outputLen should be maxOutputLen.
-  EXPECT_EQ(maxOutputLen, outputLen);
+  EXPECT_EQ(SECSuccess, rv);
+  // outputLen should <= 256-11=245.
+  EXPECT_LE(outputLen, 245u);
 
   // This should fail because the padding checks will fail.
+  // however, Bleichenbacher preventions means that failure would be
+  // a different output.
   uint8_t out_long[260] = {0};
   maxOutputLen = sizeof(out_long);
   rv = RSA_DecryptBlock(key.get(), out_long, &outputLen, maxOutputLen, in,
                         sizeof(in));
-  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SECSuccess, rv);
   // outputLen should <= 256-11=245.
   EXPECT_LE(outputLen, 245u);
   // Everything over 256 must be 0 in the output.

gtests/manifest.mn

@@ -27,6 +27,7 @@ NSS_SRCDIRS = \
 	certhigh_gtest \
 	cryptohi_gtest \
 	der_gtest \
+	freebl_gtest \
 	pk11_gtest \
 	smime_gtest \
 	softoken_gtest \

gtests/pk11_gtest/pk11_rsaencrypt_unittest.cc

@@ -13,6 +13,7 @@
 #include "nss.h"
 #include "nss_scoped_ptrs.h"
 #include "pk11pub.h"
+#include "databuffer.h"
 
 #include "testvectors/rsa_pkcs1_2048_test-vectors.h"
 #include "testvectors/rsa_pkcs1_3072_test-vectors.h"
@@ -44,13 +45,15 @@ class RsaDecryptWycheproofTest
     rv = PK11_PrivDecryptPKCS1(priv_key.get(), decrypted.data(), &decrypted_len,
                                decrypted.size(), vec.ct.data(), vec.ct.size());
 
-    // RSA_DecryptBlock returns SECFailure with an empty message.
-    if (vec.valid && vec.msg.size()) {
+    if (vec.valid) {
       EXPECT_EQ(SECSuccess, rv);
       decrypted.resize(decrypted_len);
       EXPECT_EQ(vec.msg, decrypted);
     } else {
-      EXPECT_EQ(SECFailure, rv);
+      DataBuffer::SetLogLimit(512);
+      decrypted.resize(decrypted_len);
+      EXPECT_EQ(SECFailure, rv)
+          << "Returned:" << DataBuffer(decrypted.data(), decrypted.size());
     }
   };
 };

gtests/pk11_gtest/pk11_rsaoaep_unittest.cc

@@ -157,11 +157,32 @@ TEST(Pkcs11RsaOaepTest, TestOaepWrapUnwrap) {
 
   PK11SymKey* p_unwrapped_tmp = nullptr;
 
-  // This fails because this method is broken and assumes CKM_RSA_PKCS and
-  // doesn't understand OAEP.
+  // Extract key's value in order to validate decryption worked.
+  rv = PK11_ExtractKeyValue(to_wrap.get());
+  ASSERT_EQ(rv, SECSuccess);
+
+  // References owned by PKCS#11 layer; no need to scope and free.
+  SECItem* expectedItem = PK11_GetKeyData(to_wrap.get());
+
+  // This assumes CKM_RSA_PKCS and doesn't understand OAEP.
+  // CKM_RSA_PKCS cannot safely return errors, however, as it can lead
+  // to Blecheinbaucher-like attacks. To solve this there's a new definition
+  // that generates fake key material based on the message and private key.
+  // This returned key material will not be the key we were expecting, so
+  // make sure that's the case:
   p_unwrapped_tmp = PK11_PubUnwrapSymKey(priv.get(), wrapped.get(), CKM_AES_CBC,
                                          CKA_DECRYPT, 16);
-  ASSERT_EQ(p_unwrapped_tmp, nullptr);
+  // as long as the wrapped data is legal RSA length of the key
+  // (which is should be), then CKM_RSA_PKCS should not fail.
+  ASSERT_NE(p_unwrapped_tmp, nullptr);
+  ScopedPK11SymKey fakeUnwrapped;
+  fakeUnwrapped.reset(p_unwrapped_tmp);
+  rv = PK11_ExtractKeyValue(fakeUnwrapped.get());
+  ASSERT_EQ(rv, SECSuccess);
+
+  // References owned by PKCS#11 layer; no need to scope and free.
+  SECItem* fakeItem = PK11_GetKeyData(fakeUnwrapped.get());
+  ASSERT_NE(SECITEM_CompareItem(fakeItem, expectedItem), 0);
 
   ScopedPK11SymKey unwrapped;
   p_unwrapped_tmp = PK11_PubUnwrapSymKeyWithMechanism(
@@ -171,15 +192,10 @@ TEST(Pkcs11RsaOaepTest, TestOaepWrapUnwrap) {
 
   unwrapped.reset(p_unwrapped_tmp);
 
-  // Extract key's value in order to validate decryption worked.
-  rv = PK11_ExtractKeyValue(to_wrap.get());
-  ASSERT_EQ(rv, SECSuccess);
-
   rv = PK11_ExtractKeyValue(unwrapped.get());
   ASSERT_EQ(rv, SECSuccess);
 
   // References owned by PKCS#11 layer; no need to scope and free.
-  SECItem* expectedItem = PK11_GetKeyData(to_wrap.get());
   SECItem* actualItem = PK11_GetKeyData(unwrapped.get());
 
   ASSERT_EQ(SECITEM_CompareItem(actualItem, expectedItem), 0);

lib/freebl/alghmac.c

@@ -37,27 +37,20 @@ HMAC_Destroy(HMACContext *cx, PRBool freeit)
         PORT_Free(cx);
 }
 
-SECStatus
-HMAC_Init(HMACContext *cx, const SECHashObject *hash_obj,
-          const unsigned char *secret, unsigned int secret_len, PRBool isFIPS)
+/* just setup the hmac key */
+static SECStatus
+hmac_initKey(HMACContext *cx, const unsigned char *secret,
+             unsigned int secret_len, PRBool isFIPS)
 {
+
     unsigned int i;
     unsigned char hashed_secret[HASH_LENGTH_MAX];
 
     /* required by FIPS 198 Section 3 */
-    if (isFIPS && secret_len < hash_obj->length / 2) {
-        PORT_SetError(SEC_ERROR_INVALID_ARGS);
-        return SECFailure;
-    }
-    if (cx == NULL) {
+    if (isFIPS && secret_len < cx->hashobj->length / 2) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
     }
-    cx->wasAllocated = PR_FALSE;
-    cx->hashobj = hash_obj;
-    cx->hash = cx->hashobj->create();
-    if (cx->hash == NULL)
-        goto loser;
 
     if (secret_len > cx->hashobj->blocklength) {
         cx->hashobj->begin(cx->hash);
@@ -85,6 +78,31 @@ HMAC_Init(HMACContext *cx, const SECHashObject *hash_obj,
 
 loser:
     PORT_Memset(hashed_secret, 0, sizeof hashed_secret);
+    return SECFailure;
+}
+
+SECStatus
+HMAC_Init(HMACContext *cx, const SECHashObject *hash_obj,
+          const unsigned char *secret, unsigned int secret_len, PRBool isFIPS)
+{
+    SECStatus rv;
+
+    if (cx == NULL) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+    cx->wasAllocated = PR_FALSE;
+    cx->hashobj = hash_obj;
+    cx->hash = cx->hashobj->create();
+    if (cx->hash == NULL)
+        goto loser;
+
+    rv = hmac_initKey(cx, secret, secret_len, isFIPS);
+    if (rv != SECSuccess)
+        goto loser;
+
+    return rv;
+loser:
     if (cx->hash != NULL)
         cx->hashobj->destroy(cx->hash, PR_TRUE);
     return SECFailure;
@@ -107,6 +125,34 @@ HMAC_Create(const SECHashObject *hash_obj, const unsigned char *secret,
     return cx;
 }
 
+/* this allows us to reuse an existing HMACContext with a new key and
+ * Hash function */
+SECStatus
+HMAC_ReInit(HMACContext *cx, const SECHashObject *hash_obj,
+            const unsigned char *secret, unsigned int secret_len, PRBool isFIPS)
+{
+    PRBool wasAllocated;
+    SECStatus rv;
+
+    /* if we are using the same hash, keep the hash contexts and only
+     * init the key */
+    if ((cx->hashobj == hash_obj) && (cx->hash != NULL)) {
+        return hmac_initKey(cx, secret, secret_len, isFIPS);
+    }
+    /* otherwise we destroy the contents of the context and
+     * initalize it from scratch. We need to preseve the current state
+     * of wasAllocated to the final destroy works correctly */
+    wasAllocated = cx->wasAllocated;
+    cx->wasAllocated = PR_FALSE;
+    HMAC_Destroy(cx, PR_FALSE);
+    rv = HMAC_Init(cx, hash_obj, secret, secret_len, isFIPS);
+    if (rv != SECSuccess) {
+        return rv;
+    }
+    cx->wasAllocated = wasAllocated;
+    return SECSuccess;
+}
+
 void
 HMAC_Begin(HMACContext *cx)
 {

lib/freebl/alghmac.h

@@ -30,6 +30,12 @@ SECStatus
 HMAC_Init(HMACContext *cx, const SECHashObject *hash_obj,
           const unsigned char *secret, unsigned int secret_len, PRBool isFIPS);
 
+/* like HMAC_Init, except caller passes in an existing context
+ * previously used by either HMAC_Create or HMAC_Init. */
+SECStatus
+HMAC_ReInit(HMACContext *cx, const SECHashObject *hash_obj,
+            const unsigned char *secret, unsigned int secret_len, PRBool isFIPS);
+
 /* reset HMAC for a fresh round */
 extern void
 HMAC_Begin(HMACContext *cx);