Bug 1463539 - Land NSS NSS_3_37_1_RTM UPGRADE_NSS_RELEASE. r=me, a=RyanVM
authorJ.C. Jones <jjones@mozilla.com>
Wed, 23 May 2018 14:09:30 -0700
changeset 470888 4fa86932be2d7a90b2494de9ae6c379c86a031f9
parent 470887 88c81330970bd1f7336157648668366af72e57bc
child 470889 6b22684df52daec5a18b090a48fba026bd5da396
push id9256
push userryanvm@gmail.com
push dateThu, 24 May 2018 15:32:42 +0000
treeherdermozilla-beta@54ba8ee1c9ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme, RyanVM
bugs1463539
milestone61.0
Bug 1463539 - Land NSS NSS_3_37_1_RTM UPGRADE_NSS_RELEASE. r=me, a=RyanVM
old-configure.in
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/cpputil/scoped_ptrs.h
security/nss/gtests/der_gtest/der_gtest.gyp
security/nss/gtests/der_gtest/manifest.mn
security/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
security/nss/lib/nss/nss.h
security/nss/lib/pkcs12/p12d.c
security/nss/lib/pkcs7/p7decode.c
security/nss/lib/softoken/softkver.h
security/nss/lib/ssl/ssl3con.c
security/nss/lib/util/nssutil.h
security/nss/lib/util/secasn1d.c
--- a/old-configure.in
+++ b/old-configure.in
@@ -1768,17 +1768,17 @@ dnl = If NSS was not detected in the sys
 dnl = use the one in the source tree (mozilla/security/nss)
 dnl ========================================================
 
 MOZ_ARG_WITH_BOOL(system-nss,
 [  --with-system-nss       Use system installed NSS],
     _USE_SYSTEM_NSS=1 )
 
 if test -n "$_USE_SYSTEM_NSS"; then
-    AM_PATH_NSS(3.37, [MOZ_SYSTEM_NSS=1], [AC_MSG_ERROR([you don't have NSS installed or your version is too old])])
+    AM_PATH_NSS(3.37.1, [MOZ_SYSTEM_NSS=1], [AC_MSG_ERROR([you don't have NSS installed or your version is too old])])
 fi
 
 if test -z "$MOZ_SYSTEM_NSS"; then
    NSS_CFLAGS="-I${DIST}/include/nss"
    case "${OS_ARCH}" in
         # Only few platforms have been tested with GYP
         WINNT|Darwin|Linux|DragonFly|FreeBSD|NetBSD|OpenBSD|SunOS)
             ;;
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-NSS_3_37_RTM
+NSS_3_37_1_RTM
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,8 +5,9 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
+
--- a/security/nss/cpputil/scoped_ptrs.h
+++ b/security/nss/cpputil/scoped_ptrs.h
@@ -8,16 +8,17 @@
 #define scoped_ptrs_h__
 
 #include <memory>
 #include "cert.h"
 #include "keyhi.h"
 #include "pk11pub.h"
 #include "pkcs11uri.h"
 #include "sslexp.h"
+#include "p12.h"
 
 struct ScopedDelete {
   void operator()(CERTCertificate* cert) { CERT_DestroyCertificate(cert); }
   void operator()(CERTCertificateList* list) {
     CERT_DestroyCertificateList(list);
   }
   void operator()(CERTName* name) { CERT_DestroyName(name); }
   void operator()(CERTCertList* list) { CERT_DestroyCertList(list); }
@@ -36,16 +37,19 @@ struct ScopedDelete {
   }
   void operator()(PK11URI* uri) { PK11URI_DestroyURI(uri); }
   void operator()(PLArenaPool* arena) { PORT_FreeArena(arena, PR_FALSE); }
   void operator()(PK11Context* context) { PK11_DestroyContext(context, true); }
   void operator()(PK11GenericObject* obj) { PK11_DestroyGenericObject(obj); }
   void operator()(SSLResumptionTokenInfo* token) {
     SSL_DestroyResumptionTokenInfo(token);
   }
+  void operator()(SEC_PKCS12DecoderContext* dcx) {
+    SEC_PKCS12DecoderFinish(dcx);
+  }
 };
 
 template <class T>
 struct ScopedMaybeDelete {
   void operator()(T* ptr) {
     if (ptr) {
       ScopedDelete del;
       del(ptr);
@@ -68,12 +72,13 @@ SCOPED(SECItem);
 SCOPED(SECKEYPublicKey);
 SCOPED(SECKEYPrivateKey);
 SCOPED(SECKEYPrivateKeyList);
 SCOPED(PK11URI);
 SCOPED(PLArenaPool);
 SCOPED(PK11Context);
 SCOPED(PK11GenericObject);
 SCOPED(SSLResumptionTokenInfo);
+SCOPED(SEC_PKCS12DecoderContext);
 
 #undef SCOPED
 
 #endif  // scoped_ptrs_h__
--- a/security/nss/gtests/der_gtest/der_gtest.gyp
+++ b/security/nss/gtests/der_gtest/der_gtest.gyp
@@ -8,23 +8,26 @@
   ],
   'targets': [
     {
       'target_name': 'der_gtest',
       'type': 'executable',
       'sources': [
         'der_getint_unittest.cc',
         'der_quickder_unittest.cc',
+        'p12_import_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:nssutil3',
         '<(DEPTH)/lib/ssl/ssl.gyp:ssl3',
         '<(DEPTH)/lib/nss/nss.gyp:nss3',
+        '<(DEPTH)/lib/pkcs12/pkcs12.gyp:pkcs12',
+        '<(DEPTH)/lib/pkcs7/pkcs7.gyp:pkcs7',
       ]
     }
   ],
   'variables': {
     'module': 'nss'
   }
 }
--- a/security/nss/gtests/der_gtest/manifest.mn
+++ b/security/nss/gtests/der_gtest/manifest.mn
@@ -4,16 +4,17 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 CORE_DEPTH = ../..
 DEPTH      = ../..
 MODULE = nss
 
 CPPSRCS = \
       der_getint_unittest.cc \
       der_quickder_unittest.cc \
+      p12_import_unittest.cc \
       $(NULL)
 
 INCLUDES += -I$(CORE_DEPTH)/gtests/google_test/gtest/include \
             -I$(CORE_DEPTH)/gtests/common \
             -I$(CORE_DEPTH)/cpputil
 
 REQUIRES = nspr gtest
 
--- a/security/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
@@ -431,9 +431,25 @@ TEST_F(TlsConnectDatagram13, CompatModeD
               server_records->record(i).header.content_type());
   }
 
   uint32_t session_id_len = 0;
   EXPECT_TRUE(server_hello->buffer().Read(2 + 32, 1, &session_id_len));
   EXPECT_EQ(0U, session_id_len);
 }
 
+TEST_F(Tls13CompatTest, ConnectWith12ThenAttemptToResume13CompatMode) {
+  ConfigureSessionCache(RESUME_SESSIONID, RESUME_SESSIONID);
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_2);
+  Connect();
+
+  Reset();
+  ExpectResumption(RESUME_NONE);
+  version_ = SSL_LIBRARY_VERSION_TLS_1_3;
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  EnableCompatMode();
+  Connect();
+}
+
 }  // namespace nss_test
--- a/security/nss/lib/nss/nss.h
+++ b/security/nss/lib/nss/nss.h
@@ -17,20 +17,20 @@
 
 /*
  * NSS's major version, minor version, patch level, build number, and whether
  * this is a beta release.
  *
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <ECC>][ <Beta>]"
  */
-#define NSS_VERSION "3.37" _NSS_CUSTOMIZED
+#define NSS_VERSION "3.37.1" _NSS_CUSTOMIZED
 #define NSS_VMAJOR 3
 #define NSS_VMINOR 37
-#define NSS_VPATCH 0
+#define NSS_VPATCH 1
 #define NSS_VBUILD 0
 #define NSS_BETA PR_FALSE
 
 #ifndef RC_INVOKED
 
 #include "seccomon.h"
 
 typedef struct NSSInitParametersStr NSSInitParameters;
--- a/security/nss/lib/pkcs12/p12d.c
+++ b/security/nss/lib/pkcs12/p12d.c
@@ -808,16 +808,17 @@ sec_pkcs12_decoder_asafes_notify(void *a
 
     if (!before) {
         /* if one is being decoded, finish the decode */
         if (p12dcx->currentASafeP7Dcx != NULL) {
             SEC_PKCS7ContentInfo *cinfo;
             unsigned int cnt = p12dcx->safeContentsCnt - 1;
             safeContentsCtx = p12dcx->safeContentsList[cnt];
             if (safeContentsCtx->safeContentsA1Dcx) {
+                SEC_ASN1DecoderClearFilterProc(p12dcx->aSafeA1Dcx);
                 SEC_ASN1DecoderFinish(safeContentsCtx->safeContentsA1Dcx);
                 safeContentsCtx->safeContentsA1Dcx = NULL;
             }
             cinfo = SEC_PKCS7DecoderFinish(p12dcx->currentASafeP7Dcx);
             p12dcx->currentASafeP7Dcx = NULL;
             if (!cinfo) {
                 p12dcx->errorValue = PORT_GetError();
                 goto loser;
--- a/security/nss/lib/pkcs7/p7decode.c
+++ b/security/nss/lib/pkcs7/p7decode.c
@@ -555,16 +555,17 @@ sec_pkcs7_decoder_start_decrypt(SEC_PKCS
                                  (PRBool)(p7dcx->cb != NULL));
 
     p7dcx->worker.depth = depth;
     p7dcx->worker.decryptobj = decryptobj;
 
     return SECSuccess;
 
 no_decryption:
+    PK11_FreeSymKey(bulkkey);
     /*
      * For some reason (error set already, if appropriate), we cannot
      * decrypt the content.  I am not sure what exactly is the right
      * thing to do here; in some cases we want to just stop, and in
      * others we want to let the decoding finish even though we cannot
      * decrypt the content.  My current thinking is that if the caller
      * set up a content callback, then they are really interested in
      * getting (decrypted) content, and if they cannot they will want
@@ -1026,16 +1027,21 @@ SEC_PKCS7DecoderStart(SEC_PKCS7DecoderCo
  * again in case that is the easiest route for our caller to take.
  * We simply detect it and do not do anything except keep setting
  * that error in case our caller has not noticed it yet...
  */
 SECStatus
 SEC_PKCS7DecoderUpdate(SEC_PKCS7DecoderContext *p7dcx,
                        const char *buf, unsigned long len)
 {
+    if (!p7dcx) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+
     if (p7dcx->cinfo != NULL && p7dcx->dcx != NULL) {
         PORT_Assert(p7dcx->error == 0);
         if (p7dcx->error == 0) {
             if (SEC_ASN1DecoderUpdate(p7dcx->dcx, buf, len) != SECSuccess) {
                 p7dcx->error = PORT_GetError();
                 PORT_Assert(p7dcx->error);
                 if (p7dcx->error == 0)
                     p7dcx->error = -1;
--- a/security/nss/lib/softoken/softkver.h
+++ b/security/nss/lib/softoken/softkver.h
@@ -12,16 +12,16 @@
 
 /*
  * Softoken's major version, minor version, patch level, build number,
  * and whether this is a beta release.
  *
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <ECC>][ <Beta>]"
  */
-#define SOFTOKEN_VERSION "3.37" SOFTOKEN_ECC_STRING
+#define SOFTOKEN_VERSION "3.37.1" SOFTOKEN_ECC_STRING
 #define SOFTOKEN_VMAJOR 3
 #define SOFTOKEN_VMINOR 37
-#define SOFTOKEN_VPATCH 0
+#define SOFTOKEN_VPATCH 1
 #define SOFTOKEN_VBUILD 0
 #define SOFTOKEN_BETA PR_FALSE
 
 #endif /* _SOFTKVER_H_ */
--- a/security/nss/lib/ssl/ssl3con.c
+++ b/security/nss/lib/ssl/ssl3con.c
@@ -6159,38 +6159,48 @@ ssl_ClientSetCipherSuite(sslSocket *ss, 
 
 /* Check that session ID we received from the server, if any, matches our
  * expectations, depending on whether we're in compat mode and whether we
  * negotiated TLS 1.3+ or TLS 1.2-.
  */
 static PRBool
 ssl_CheckServerSessionIdCorrectness(sslSocket *ss, SECItem *sidBytes)
 {
-    PRBool sid_match = PR_FALSE;
-    PRBool sent_fake_sid = ss->opt.enableTls13CompatMode && !IS_DTLS(ss);
-
-    /* If in compat mode and we received a session ID with the right length
-     * then compare it to the fake one we sent in the ClientHello. */
-    if (sent_fake_sid && sidBytes->len == SSL3_SESSIONID_BYTES) {
-        PRUint8 buf[SSL3_SESSIONID_BYTES];
-        ssl_MakeFakeSid(ss, buf);
-        sid_match = PORT_Memcmp(buf, sidBytes->data, sidBytes->len) == 0;
-    }
-
-    /* TLS 1.2: SessionID shouldn't match the fake one. */
+    sslSessionID *sid = ss->sec.ci.sid;
+    PRBool sidMatch = PR_FALSE;
+    PRBool sentFakeSid = PR_FALSE;
+    PRBool sentRealSid = sid && sid->version < SSL_LIBRARY_VERSION_TLS_1_3;
+
+    /* If attempting to resume a TLS 1.2 connection, the session ID won't be a
+     * fake. Check for the real value. */
+    if (sentRealSid) {
+        sidMatch = (sidBytes->len == sid->u.ssl3.sessionIDLength) &&
+                   PORT_Memcmp(sid->u.ssl3.sessionID, sidBytes->data, sidBytes->len) == 0;
+    } else {
+        /* Otherwise, the session ID was a fake if TLS 1.3 compat mode is
+         * enabled.  If so, check for the fake value. */
+        sentFakeSid = ss->opt.enableTls13CompatMode && !IS_DTLS(ss);
+        if (sentFakeSid && sidBytes->len == SSL3_SESSIONID_BYTES) {
+            PRUint8 buf[SSL3_SESSIONID_BYTES];
+            ssl_MakeFakeSid(ss, buf);
+            sidMatch = PORT_Memcmp(buf, sidBytes->data, sidBytes->len) == 0;
+        }
+    }
+
+    /* TLS 1.2: Session ID shouldn't match if we sent a fake. */
     if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
-        return !sid_match;
-    }
-
-    /* TLS 1.3: [Compat Mode] Session ID should match the fake one. */
-    if (sent_fake_sid) {
-        return sid_match;
-    }
-
-    /* TLS 1.3: [Non-Compat Mode] Server shouldn't send a session ID. */
+        return !sentFakeSid || !sidMatch;
+    }
+
+    /* TLS 1.3: We sent a session ID.  The server's should match. */
+    if (sentRealSid || sentFakeSid) {
+        return sidMatch;
+    }
+
+    /* TLS 1.3: The server shouldn't send a session ID. */
     return sidBytes->len == 0;
 }
 
 /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete
  * ssl3 ServerHello message.
  * Caller must hold Handshake and RecvBuf locks.
  */
 static SECStatus
--- a/security/nss/lib/util/nssutil.h
+++ b/security/nss/lib/util/nssutil.h
@@ -14,20 +14,20 @@
 
 /*
  * NSS utilities's major version, minor version, patch level, build number,
  * and whether this is a beta release.
  *
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>[.<build number>]][ <Beta>]"
  */
-#define NSSUTIL_VERSION "3.37"
+#define NSSUTIL_VERSION "3.37.1"
 #define NSSUTIL_VMAJOR 3
 #define NSSUTIL_VMINOR 37
-#define NSSUTIL_VPATCH 0
+#define NSSUTIL_VPATCH 1
 #define NSSUTIL_VBUILD 0
 #define NSSUTIL_BETA PR_FALSE
 
 SEC_BEGIN_PROTOS
 
 /*
  * Returns a const string of the UTIL library version.
  */
--- a/security/nss/lib/util/secasn1d.c
+++ b/security/nss/lib/util/secasn1d.c
@@ -170,17 +170,17 @@ static int /* bool */
             sprintf(buf, " %s", type_names[k]);
             if ((k == SEC_ASN1_SET || k == SEC_ASN1_SEQUENCE) &&
                 (kind & SEC_ASN1_GROUP)) {
                 buf += strlen(buf);
                 sprintf(buf, "_OF");
             }
         }
     } else {
-        sprintf(buf, " [%d]", k);
+        sprintf(buf, " [%lu]", k);
     }
     buf += strlen(buf);
 
     for (k = kind >> 8, i = 0; k; k >>= 1, ++i) {
         if (k & 1) {
             sprintf(buf, " %s", flag_names[i]);
             buf += strlen(buf);
         }
@@ -977,17 +977,17 @@ sec_asn1d_prepare_for_contents(sec_asn1d
 {
     SECItem *item;
     PLArenaPool *poolp;
     unsigned long alloc_len;
     sec_asn1d_state *parent;
 
 #ifdef DEBUG_ASN1D_STATES
     {
-        printf("Found Length %d %s\n", state->contents_length,
+        printf("Found Length %lu %s\n", state->contents_length,
                state->indefinite ? "indefinite" : "");
     }
 #endif
 
     /**
      * The maximum length for a child element should be constrained to the
      * length remaining in the first definite length element in the ancestor
      * stack. If there is no definite length element in the ancestor stack,
@@ -2712,26 +2712,25 @@ dump_states(SEC_ASN1DecoderContext *cx)
 
     for (; state; state = state->child) {
         int i;
         for (i = 0; i < state->depth; i++) {
             printf("  ");
         }
 
         i = formatKind(state->theTemplate->kind, kindBuf);
-        printf("%s: tmpl %08x, kind%s",
+        printf("%s: tmpl kind %s",
                (state == cx->current) ? "STATE" : "State",
-               state->theTemplate,
                kindBuf);
         printf(" %s", (state->place >= 0 && state->place <= notInUse) ? place_names[state->place] : "(undefined)");
         if (!i)
-            printf(", expect 0x%02x",
+            printf(", expect 0x%02lx",
                    state->expect_tag_number | state->expect_tag_modifiers);
 
-        printf("%s%s%s %d\n",
+        printf("%s%s%s %lu\n",
                state->indefinite ? ", indef" : "",
                state->missing ? ", miss" : "",
                state->endofcontents ? ", EOC" : "",
                state->pending);
     }
 
     return;
 }
@@ -2749,17 +2748,17 @@ SEC_ASN1DecoderUpdate(SEC_ASN1DecoderCon
     if (cx->status == needBytes)
         cx->status = keepGoing;
 
     while (cx->status == keepGoing) {
         state = cx->current;
         what = SEC_ASN1_Contents;
         consumed = 0;
 #ifdef DEBUG_ASN1D_STATES
-        printf("\nPLACE = %s, next byte = 0x%02x, %08x[%d]\n",
+        printf("\nPLACE = %s, next byte = 0x%02x, %p[%lu]\n",
                (state->place >= 0 && state->place <= notInUse) ? place_names[state->place] : "(undefined)",
                len ? (unsigned int)((unsigned char *)buf)[consumed] : 0,
                buf, consumed);
         dump_states(cx);
 #endif /* DEBUG_ASN1D_STATES */
         switch (state->place) {
             case beforeIdentifier:
                 consumed = sec_asn1d_parse_identifier(state, buf, len);
@@ -2972,17 +2971,17 @@ SEC_ASN1DecoderUpdate(SEC_ASN1DecoderCon
     return SECSuccess;
 }
 
 SECStatus
 SEC_ASN1DecoderFinish(SEC_ASN1DecoderContext *cx)
 {
     SECStatus rv;
 
-    if (cx->status == needBytes) {
+    if (!cx || cx->status == needBytes) {
         PORT_SetError(SEC_ERROR_BAD_DER);
         rv = SECFailure;
     } else {
         rv = SECSuccess;
     }
 
     /*
      * XXX anything else that needs to be finished?