Bug 1393329 - zero-check x25519 result, r=bbeurdouche,ttaubert
authorFranziskus Kiefer <franziskuskiefer@gmail.com>
Tue, 05 Sep 2017 10:18:35 +0200
changeset 13562 4bf658832d8960a070dd0ce2baea49a4276461a1
parent 13561 d7c1635da93034480d015e4c85c71a06a752ab3c
child 13563 70fda6ba2043a72da5db34534d172c342b2562b9
push id2350
push userfranziskuskiefer@gmail.com
push dateTue, 05 Sep 2017 09:47:22 +0000
reviewersbbeurdouche, ttaubert
bugs1393329
Bug 1393329 - zero-check x25519 result, r=bbeurdouche,ttaubert Differential Revision: https://nss-review.dev.mozaws.net/D420
gtests/util_gtest/manifest.mn
gtests/util_gtest/util_gtest.gyp
gtests/util_gtest/util_memcmpzero_unittest.cc
lib/freebl/ecl/ecp_25519.c
lib/freebl/stubs.c
lib/freebl/stubs.h
lib/util/nssutil.def
lib/util/secport.c
lib/util/secport.h
--- a/gtests/util_gtest/manifest.mn
+++ b/gtests/util_gtest/manifest.mn
@@ -5,16 +5,18 @@
 CORE_DEPTH = ../..
 DEPTH      = ../..
 MODULE = nss
 
 CPPSRCS = \
 	util_utf8_unittest.cc \
 	util_b64_unittest.cc \
 	util_pkcs11uri_unittest.cc \
+	util_aligned_malloc_unittest.cc \
+	util_memcmpzero_unittest.cc \
 	$(NULL)
 
 INCLUDES += \
 	-I$(CORE_DEPTH)/gtests/google_test/gtest/include \
 	-I$(CORE_DEPTH)/gtests/common \
 	-I$(CORE_DEPTH)/cpputil \
 	$(NULL)
 
--- a/gtests/util_gtest/util_gtest.gyp
+++ b/gtests/util_gtest/util_gtest.gyp
@@ -10,16 +10,17 @@
     {
       'target_name': 'util_gtest',
       'type': 'executable',
       'sources': [
         'util_utf8_unittest.cc',
         'util_b64_unittest.cc',
         'util_pkcs11uri_unittest.cc',
         'util_aligned_malloc_unittest.cc',
+        'util_memcmpzero_unittest.cc',
         '<(DEPTH)/gtests/common/gtests.cc',
       ],
       'dependencies': [
         '<(DEPTH)/exports.gyp:nss_exports',
         '<(DEPTH)/gtests/google_test/google_test.gyp:gtest',
         '<(DEPTH)/lib/util/util.gyp:nssutil',
         '<(DEPTH)/lib/nss/nss.gyp:nss_static',
         '<(DEPTH)/lib/pk11wrap/pk11wrap.gyp:pk11wrap_static',
new file mode 100644
--- /dev/null
+++ b/gtests/util_gtest/util_memcmpzero_unittest.cc
@@ -0,0 +1,45 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* 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 "scoped_ptrs_util.h"
+
+namespace nss_test {
+
+class MemcmpZeroTest : public ::testing::Test {
+ protected:
+  unsigned int test_memcmp_zero(const std::vector<uint8_t> &mem) {
+    return NSS_SecureMemcmpZero(mem.data(), mem.size());
+  };
+};
+
+TEST_F(MemcmpZeroTest, TestMemcmpZeroTrue) {
+  unsigned int rv = test_memcmp_zero(std::vector<uint8_t>(37, 0));
+  EXPECT_EQ(0U, rv);
+}
+
+TEST_F(MemcmpZeroTest, TestMemcmpZeroFalse5) {
+  std::vector<uint8_t> vec(37, 0);
+  vec[5] = 1;
+  unsigned int rv = test_memcmp_zero(vec);
+  EXPECT_NE(0U, rv);
+}
+
+TEST_F(MemcmpZeroTest, TestMemcmpZeroFalse37) {
+  std::vector<uint8_t> vec(37, 0);
+  vec[vec.size() - 1] = 0xFF;
+  unsigned int rv = test_memcmp_zero(vec);
+  EXPECT_NE(0U, rv);
+}
+
+TEST_F(MemcmpZeroTest, TestMemcmpZeroFalse0) {
+  std::vector<uint8_t> vec(37, 0);
+  vec[0] = 1;
+  unsigned int rv = test_memcmp_zero(vec);
+  EXPECT_NE(0U, rv);
+}
+
+}  // namespace nss_test
--- a/lib/freebl/ecl/ecp_25519.c
+++ b/lib/freebl/ecl/ecp_25519.c
@@ -110,10 +110,14 @@ ec_Curve25519_pt_mul(SECItem *X, SECItem
     } else {
         PORT_Assert(P->len == 32);
         if (P->len != 32) {
             return SECFailure;
         }
         px = P->data;
     }
 
-    return ec_Curve25519_mul(X->data, k->data, px);
+    SECStatus rv = ec_Curve25519_mul(X->data, k->data, px);
+    if (NSS_SecureMemcmpZero(X->data, X->len) == 0) {
+        return SECFailure;
+    }
+    return rv;
 }
--- a/lib/freebl/stubs.c
+++ b/lib/freebl/stubs.c
@@ -174,16 +174,17 @@ STUB_DECLARE(SECItem *, SECITEM_AllocIte
 STUB_DECLARE(SECComparison, SECITEM_CompareItem_Util, (const SECItem *a,
                                                        const SECItem *b));
 STUB_DECLARE(SECStatus, SECITEM_CopyItem_Util, (PLArenaPool * arena,
                                                 SECItem *to, const SECItem *from));
 STUB_DECLARE(void, SECITEM_FreeItem_Util, (SECItem * zap, PRBool freeit));
 STUB_DECLARE(void, SECITEM_ZfreeItem_Util, (SECItem * zap, PRBool freeit));
 STUB_DECLARE(SECOidTag, SECOID_FindOIDTag_Util, (const SECItem *oid));
 STUB_DECLARE(int, NSS_SecureMemcmp, (const void *a, const void *b, size_t n));
+STUB_DECLARE(unsigned int, NSS_SecureMemcmpZero, (const void *mem, size_t n));
 
 #define PORT_ZNew_stub(type) (type *)PORT_ZAlloc_stub(sizeof(type))
 #define PORT_New_stub(type) (type *)PORT_Alloc_stub(sizeof(type))
 #define PORT_ZNewArray_stub(type, num) \
     (type *)PORT_ZAlloc_stub(sizeof(type) * (num))
 #define PORT_ZNewAligned_stub(type, alignment, mem) \
     (type *)PORT_ZAllocAlignedOffset_stub(sizeof(type), alignment, offsetof(type, mem))
 
@@ -638,16 +639,23 @@ SECITEM_ZfreeItem_stub(SECItem *zap, PRB
 
 extern int
 NSS_SecureMemcmp_stub(const void *a, const void *b, size_t n)
 {
     STUB_SAFE_CALL3(NSS_SecureMemcmp, a, b, n);
     abort();
 }
 
+extern unsigned int
+NSS_SecureMemcmpZero_stub(const void *mem, size_t n)
+{
+    STUB_SAFE_CALL2(NSS_SecureMemcmpZero, mem, n);
+    abort();
+}
+
 #ifdef FREEBL_NO_WEAK
 
 static const char *nsprLibName = SHLIB_PREFIX "nspr4." SHLIB_SUFFIX;
 static const char *nssutilLibName = SHLIB_PREFIX "nssutil3." SHLIB_SUFFIX;
 
 static SECStatus
 freebl_InitNSPR(void *lib)
 {
@@ -690,16 +698,17 @@ freebl_InitNSSUtil(void *lib)
     STUB_FETCH_FUNCTION(PORT_SetError_Util);
     STUB_FETCH_FUNCTION(SECITEM_FreeItem_Util);
     STUB_FETCH_FUNCTION(SECITEM_AllocItem_Util);
     STUB_FETCH_FUNCTION(SECITEM_CompareItem_Util);
     STUB_FETCH_FUNCTION(SECITEM_CopyItem_Util);
     STUB_FETCH_FUNCTION(SECITEM_ZfreeItem_Util);
     STUB_FETCH_FUNCTION(SECOID_FindOIDTag_Util);
     STUB_FETCH_FUNCTION(NSS_SecureMemcmp);
+    STUB_FETCH_FUNCTION(NSS_SecureMemcmpZero);
     return SECSuccess;
 }
 
 /*
  * fetch the library if it's loaded. For NSS it should already be loaded
  */
 #define freebl_getLibrary(libName) \
     dlopen(libName, RTLD_LAZY | RTLD_NOLOAD)
--- a/lib/freebl/stubs.h
+++ b/lib/freebl/stubs.h
@@ -35,16 +35,17 @@
 
 #define SECITEM_AllocItem SECITEM_AllocItem_stub
 #define SECITEM_CompareItem SECITEM_CompareItem_stub
 #define SECITEM_CopyItem SECITEM_CopyItem_stub
 #define SECITEM_FreeItem SECITEM_FreeItem_stub
 #define SECITEM_ZfreeItem SECITEM_ZfreeItem_stub
 #define SECOID_FindOIDTag SECOID_FindOIDTag_stub
 #define NSS_SecureMemcmp NSS_SecureMemcmp_stub
+#define NSS_SecureMemcmpZero NSS_SecureMemcmpZero_stub
 
 #define PR_Assert PR_Assert_stub
 #define PR_Access PR_Access_stub
 #define PR_CallOnce PR_CallOnce_stub
 #define PR_Close PR_Close_stub
 #define PR_DestroyCondVar PR_DestroyCondVar_stub
 #define PR_DestroyLock PR_DestroyLock_stub
 #define PR_Free PR_Free_stub
--- a/lib/util/nssutil.def
+++ b/lib/util/nssutil.def
@@ -306,11 +306,12 @@ PK11URI_GetPathAttribute;
 PK11URI_GetQueryAttribute;
 ;+    local:
 ;+       *;
 ;+};
 ;+NSSUTIL_3.33 {         # NSS Utilities 3.33 release
 ;+    global:
 PORT_ZAllocAligned_Util;
 PORT_ZAllocAlignedOffset_Util;
+NSS_SecureMemcmpZero;
 ;+    local:
 ;+       *;
 ;+};
--- a/lib/util/secport.c
+++ b/lib/util/secport.c
@@ -775,8 +775,23 @@ NSS_SecureMemcmp(const void *ia, const v
     unsigned char r = 0;
 
     for (i = 0; i < n; ++i) {
         r |= *a++ ^ *b++;
     }
 
     return r;
 }
+
+/*
+ * Perform a constant-time check if a memory region is all 0. The return value
+ * is 0 if the memory region is all zero.
+ */
+unsigned int
+NSS_SecureMemcmpZero(const void *mem, size_t n)
+{
+    PRUint8 zero = 0;
+    int i;
+    for (i = 0; i < n; ++i) {
+        zero |= *(PRUint8 *)((uintptr_t)mem + i);
+    }
+    return zero;
+}
--- a/lib/util/secport.h
+++ b/lib/util/secport.h
@@ -247,16 +247,17 @@ sec_port_iso88591_utf8_conversion_functi
     unsigned int inBufLen,
     unsigned char *outBuf,
     unsigned int maxOutBufLen,
     unsigned int *outBufLen);
 
 extern int NSS_PutEnv(const char *envVarName, const char *envValue);
 
 extern int NSS_SecureMemcmp(const void *a, const void *b, size_t n);
+extern unsigned int NSS_SecureMemcmpZero(const void *mem, size_t n);
 
 /*
  * Load a shared library called "newShLibName" in the same directory as
  * a shared library that is already loaded, called existingShLibName.
  * A pointer to a static function in that shared library,
  * staticShLibFunc, is required.
  *
  * existingShLibName: