Bug 1606927 - land NSS 9e0d34a6cf91 UPGRADE_NSS_RELEASE, r=jcj

2020-02-18  Kevin Jacobs  <kjacobs@mozilla.com>

	* gtests/ssl_gtest/ssl_extension_unittest.cc,
	gtests/ssl_gtest/ssl_version_unittest.cc, lib/ssl/dtlscon.c,
	lib/ssl/tls13con.c, lib/ssl/tls13con.h, lib/ssl/tls13exthandle.c:
	Bug 1615208 - Send DTLS version numbers in DTLS 1.3
	supported_versions extension r=mt

	This patch modifies `supported_versions` encodings to reflect DTLS
	versions when DTLS1.3 is use. Previously, a DTLS1.3 CH would include
	`[0x7f1e, 0x303, 0x302]` instead of the expected `[0x7f1e, 0xfefd,
	0xfeff]`, causing compatibility issues.

	[9e0d34a6cf91] [tip]

2020-02-12  Mikael Urankar  <mikael.urankar@gmail.com>

	* lib/freebl/Makefile, lib/freebl/freebl.gyp:
	Bug 1612177 - Set -march=armv7 when compiling gcm-arm32-neon, in
	order to enable NEON code generation.
	[4413841bd26d]

2020-02-14  Dmitry Baryshkov  <dbaryshkov@gmail.com>

	* gtests/freebl_gtest/blake2b_unittest.cc, lib/freebl/blake2b.c:
	Bug 1431940 - remove dereference before NULL check in BLAKE2B code.
	r=kjacobs

	[5e661906698f]

2020-02-12  Kevin Jacobs  <kjacobs@mozilla.com>

	* gtests/ssl_gtest/ssl_resumption_unittest.cc, lib/ssl/sslnonce.c:
	Bug 1614870 - Free sid->peerID before reallocating in
	ssl_DecodeResumptionToken. r=mt

	This patch adds a missing `PORT_Free()` when reallocating
	`sid->PeerID`, and adds a test for a non-empty PeerID.

	[1eb4e00b016e]

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

--HG--
extra : moz-landing-system : lando

security/nss/TAG-INFO

@@ -1 +1 @@
-735ed2e47040
+9e0d34a6cf91

security/nss/coreconf/coreconf.dep

@@ -10,3 +10,4 @@
  */
 
 #error "Do not include this header file."
+

security/nss/gtests/freebl_gtest/blake2b_unittest.cc

@@ -113,6 +113,18 @@ TEST_F(Blake2BTests, ContextTest2) {
       << "BLAKE2B_End failed!";
 }
 
+TEST_F(Blake2BTests, NullContextTest) {
+  SECStatus rv = BLAKE2B_Begin(nullptr);
+  ASSERT_EQ(SECFailure, rv);
+
+  rv = BLAKE2B_Update(nullptr, kat_data.data(), 128);
+  ASSERT_EQ(SECFailure, rv);
+
+  std::vector<uint8_t> digest(BLAKE2B512_LENGTH);
+  rv = BLAKE2B_End(nullptr, digest.data(), nullptr, BLAKE2B512_LENGTH);
+  ASSERT_EQ(SECFailure, rv);
+}
+
 TEST_F(Blake2BTests, CloneTest) {
   ScopedBLAKE2BContext ctx(BLAKE2B_NewContext());
   ScopedBLAKE2BContext cloned_ctx(BLAKE2B_NewContext());

security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc

@@ -189,8 +189,27 @@ class TlsExtensionTest13
   }
 
   void ConnectWithReplacementVersionList(uint16_t version) {
-    DataBuffer versions_buf;
+    // Convert the version encoding for DTLS, if needed.
+    if (variant_ == ssl_variant_datagram) {
+      switch (version) {
+#ifdef DTLS_1_3_DRAFT_VERSION
+        case SSL_LIBRARY_VERSION_TLS_1_3:
+          version = 0x7f00 | DTLS_1_3_DRAFT_VERSION;
+          break;
+#endif
+        case SSL_LIBRARY_VERSION_TLS_1_2:
+          version = SSL_LIBRARY_VERSION_DTLS_1_2_WIRE;
+          break;
+        case SSL_LIBRARY_VERSION_TLS_1_1:
+          /* TLS_1_1 maps to DTLS_1_0, see sslproto.h. */
+          version = SSL_LIBRARY_VERSION_DTLS_1_0_WIRE;
+          break;
+        default:
+          PORT_Assert(0);
+      }
+    }
 
+    DataBuffer versions_buf;
     size_t index = versions_buf.Write(0, 2, 1);
     versions_buf.Write(index, version, 2);
     MakeTlsFilter<TlsExtensionReplacer>(

security/nss/gtests/ssl_gtest/ssl_resumption_unittest.cc

@@ -838,7 +838,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionDuplicateNST) {
 
   // Clear the session ticket keys to invalidate the old ticket.
   SSLInt_ClearSelfEncryptKey();
-  SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0);
+  EXPECT_EQ(SECSuccess, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0));
 
   SendReceive();  // Need to read so that we absorb the session tickets.
   CheckKeys();
@@ -1005,7 +1005,8 @@ TEST_F(TlsConnectStreamTls13, ExternalResumptionUseSecondTicket) {
     state->invoked++;
     return SECSuccess;
   };
-  SSL_SetResumptionTokenCallback(client_->ssl_fd(), cb, &ticket_state);
+  EXPECT_EQ(SECSuccess, SSL_SetResumptionTokenCallback(client_->ssl_fd(), cb,
+                                                       &ticket_state));
 
   Connect();
   EXPECT_EQ(SECSuccess, SSL_SendSessionTicket(server_->ssl_fd(), nullptr, 0));
@@ -1446,4 +1447,34 @@ TEST_F(TlsConnectStreamTls13, ExternalTokenAfterHrr) {
   SendReceive();
 }
 
+TEST_F(TlsConnectStreamTls13, ExternalTokenWithPeerId) {
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  EXPECT_EQ(SECSuccess, SSL_SetSockPeerID(client_->ssl_fd(), "testPeerId"));
+  std::vector<uint8_t> ticket_state;
+  auto cb = [](PRFileDesc* fd, const PRUint8* ticket, unsigned int ticket_len,
+               void* arg) -> SECStatus {
+    EXPECT_NE(0U, ticket_len);
+    EXPECT_NE(nullptr, ticket);
+    auto ticket_state_ = reinterpret_cast<std::vector<uint8_t>*>(arg);
+    ticket_state_->assign(ticket, ticket + ticket_len);
+    return SECSuccess;
+  };
+  EXPECT_EQ(SECSuccess, SSL_SetResumptionTokenCallback(client_->ssl_fd(), cb,
+                                                       &ticket_state));
+
+  Connect();
+  SendReceive();
+  EXPECT_NE(0U, ticket_state.size());
+
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  EXPECT_EQ(SECSuccess, SSL_SetSockPeerID(client_->ssl_fd(), "testPeerId"));
+  client_->SetResumptionToken(ticket_state);
+  ASSERT_TRUE(client_->MaybeSetResumptionToken());
+  ExpectResumption(RESUME_TICKET);
+  Connect();
+  SendReceive();
+}
+
 }  // namespace nss_test

security/nss/gtests/ssl_gtest/ssl_version_unittest.cc

@@ -335,6 +335,48 @@ TEST_F(TlsConnectStreamTls13, Ssl30ClientHelloWithSupportedVersions) {
   ConnectExpectAlert(server_, kTlsAlertProtocolVersion);
 }
 
+// Verify the client sends only DTLS versions in supported_versions
+TEST_F(DtlsConnectTest, DtlsSupportedVersionsEncoding) {
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  auto capture = MakeTlsFilter<TlsExtensionCapture>(
+      client_, ssl_tls13_supported_versions_xtn);
+  Connect();
+
+  ASSERT_EQ(7U, capture->extension().len());
+  uint32_t version = 0;
+  ASSERT_TRUE(capture->extension().Read(1, 2, &version));
+  EXPECT_EQ(0x7f00 | DTLS_1_3_DRAFT_VERSION, static_cast<int>(version));
+  ASSERT_TRUE(capture->extension().Read(3, 2, &version));
+  EXPECT_EQ(SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, static_cast<int>(version));
+  ASSERT_TRUE(capture->extension().Read(5, 2, &version));
+  EXPECT_EQ(SSL_LIBRARY_VERSION_DTLS_1_0_WIRE, static_cast<int>(version));
+}
+
+// Verify the client sends only TLS versions in supported_versions
+TEST_F(TlsConnectTest, TlsSupportedVersionsEncoding) {
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  auto capture = MakeTlsFilter<TlsExtensionCapture>(
+      client_, ssl_tls13_supported_versions_xtn);
+  Connect();
+
+  ASSERT_EQ(9U, capture->extension().len());
+  uint32_t version = 0;
+  ASSERT_TRUE(capture->extension().Read(1, 2, &version));
+  EXPECT_EQ(SSL_LIBRARY_VERSION_TLS_1_3, static_cast<int>(version));
+  ASSERT_TRUE(capture->extension().Read(3, 2, &version));
+  EXPECT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, static_cast<int>(version));
+  ASSERT_TRUE(capture->extension().Read(5, 2, &version));
+  EXPECT_EQ(SSL_LIBRARY_VERSION_TLS_1_1, static_cast<int>(version));
+  ASSERT_TRUE(capture->extension().Read(7, 2, &version));
+  EXPECT_EQ(SSL_LIBRARY_VERSION_TLS_1_0, static_cast<int>(version));
+}
+
 INSTANTIATE_TEST_CASE_P(
     TlsDowngradeSentinelTest, TlsDowngradeTest,
     ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,

security/nss/lib/freebl/Makefile

@@ -770,7 +770,7 @@ ifeq ($(CPU_ARCH),arm)
 # Confusingly, __SOFTFP__ is the name of the define for the softfloat ABI, not for the softfp ABI.
 USES_SOFTFLOAT_ABI := $(shell $(CC) -o - -E -dM - $(CFLAGS) < /dev/null | grep __SOFTFP__ > /dev/null && echo 1)
 $(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a -mfpu=crypto-neon-fp-armv8$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
-$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
+$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -march=armv7 -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
 endif
 ifeq ($(CPU_ARCH),aarch64)
 $(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a+crypto

security/nss/lib/freebl/blake2b.c

@@ -147,9 +147,8 @@ static SECStatus
 blake2b_Begin(BLAKE2BContext* ctx, uint8_t outlen, const uint8_t* key,
               size_t keylen)
 {
-    PORT_Assert(ctx != NULL);
     if (!ctx) {
-        goto failure;
+        goto failure_noclean;
     }
     if (outlen == 0 || outlen > BLAKE2B512_LENGTH) {
         goto failure;
@@ -181,6 +180,7 @@ blake2b_Begin(BLAKE2BContext* ctx, uint8_t outlen, const uint8_t* key,
 
 failure:
     PORT_Memset(ctx, 0, sizeof(*ctx));
+failure_noclean:
     PORT_SetError(SEC_ERROR_INVALID_ARGS);
     return SECFailure;
 }
@@ -218,17 +218,11 @@ SECStatus
 BLAKE2B_Update(BLAKE2BContext* ctx, const unsigned char* in,
                unsigned int inlen)
 {
-    size_t left = ctx->buflen;
-    size_t fill = BLAKE2B_BLOCK_LENGTH - left;
-
     /* Nothing to do if there's nothing. */
     if (inlen == 0) {
         return SECSuccess;
     }
 
-    PORT_Assert(ctx != NULL);
-    PORT_Assert(in != NULL);
-    PORT_Assert(left <= BLAKE2B_BLOCK_LENGTH);
     if (!ctx || !in) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return SECFailure;
@@ -240,6 +234,10 @@ BLAKE2B_Update(BLAKE2BContext* ctx, const unsigned char* in,
         return SECFailure;
     }
 
+    size_t left = ctx->buflen;
+    PORT_Assert(left <= BLAKE2B_BLOCK_LENGTH);
+    size_t fill = BLAKE2B_BLOCK_LENGTH - left;
+
     if (inlen > fill) {
         if (ctx->buflen) {
             /* There's some remaining data in ctx->buf that we have to prepend

security/nss/lib/freebl/freebl.gyp

@@ -158,6 +158,7 @@
         '<(DEPTH)/exports.gyp:nss_exports'
       ],
       'cflags': [
+        '-march=armv7',
         '-mfpu=neon',
         '<@(softfp_cflags)',
       ],

security/nss/lib/ssl/dtlscon.c

@@ -53,7 +53,7 @@ static const ssl3CipherSuite nonDTLSSuites[] = {
  * TLS             DTLS
  * 1.1 (0302)      1.0 (feff)
  * 1.2 (0303)      1.2 (fefd)
- * 1.3 (0304)      1.3 (fefc)
+ * 1.3 (0304)      1.3 (0304)
  */
 SSL3ProtocolVersion
 dtls_TLSVersionToDTLSVersion(SSL3ProtocolVersion tlsv)
@@ -68,7 +68,7 @@ dtls_TLSVersionToDTLSVersion(SSL3ProtocolVersion tlsv)
         return SSL_LIBRARY_VERSION_DTLS_1_3_WIRE;
     }
 
-    /* Anything other than TLS 1.1 or 1.2 is an error, so return
+    /* Anything else is an error, so return
      * the invalid version 0xffff. */
     return 0xffff;
 }

security/nss/lib/ssl/sslnonce.c

@@ -537,6 +537,9 @@ ssl_DecodeResumptionToken(sslSessionID *sid, const PRUint8 *encodedToken,
     }
     if (readerBuffer.len) {
         PORT_Assert(readerBuffer.buf);
+        if (sid->peerID) {
+            PORT_Free((void *)sid->peerID);
+        }
         sid->peerID = PORT_Strdup((const char *)readerBuffer.buf);
     }
 

security/nss/lib/ssl/tls13con.c

@@ -5803,14 +5803,26 @@ tls13_HandleEarlyApplicationData(sslSocket *ss, sslBuffer *origBuf)
 }
 
 PRUint16
-tls13_EncodeDraftVersion(SSL3ProtocolVersion version, SSLProtocolVariant variant)
+tls13_EncodeVersion(SSL3ProtocolVersion version, SSLProtocolVariant variant)
 {
+    if (variant == ssl_variant_datagram) {
+        /* TODO: When DTLS 1.3 is out of draft, replace this with
+         * dtls_TLSVersionToDTLSVersion(). */
+        switch (version) {
 #ifdef DTLS_1_3_DRAFT_VERSION
-    if (version == SSL_LIBRARY_VERSION_TLS_1_3 &&
+            case SSL_LIBRARY_VERSION_TLS_1_3:
-        variant == ssl_variant_datagram) {
+                return 0x7f00 | DTLS_1_3_DRAFT_VERSION;
-        return 0x7f00 | DTLS_1_3_DRAFT_VERSION;
-    }
 #endif
+            case SSL_LIBRARY_VERSION_TLS_1_2:
+                return SSL_LIBRARY_VERSION_DTLS_1_2_WIRE;
+            case SSL_LIBRARY_VERSION_TLS_1_1:
+                /* TLS_1_1 maps to DTLS_1_0, see sslproto.h. */
+                return SSL_LIBRARY_VERSION_DTLS_1_0_WIRE;
+            default:
+                PORT_Assert(0);
+        }
+    }
+    /* Stream-variant encodings do not change. */
     return (PRUint16)version;
 }
 
@@ -5840,8 +5852,8 @@ tls13_ClientReadSupportedVersion(sslSocket *ss)
         return SECFailure;
     }
 
-    if (temp != tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3,
+    if (temp != tls13_EncodeVersion(SSL_LIBRARY_VERSION_TLS_1_3,
-                                         ss->protocolVariant)) {
+                                    ss->protocolVariant)) {
         /* You cannot negotiate < TLS 1.3 with supported_versions. */
         FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO, illegal_parameter);
         return SECFailure;
@@ -5880,7 +5892,7 @@ tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supportedVersions)
             return SECFailure;
         }
 
-        PRUint16 wire = tls13_EncodeDraftVersion(version, ss->protocolVariant);
+        PRUint16 wire = tls13_EncodeVersion(version, ss->protocolVariant);
         unsigned long offset;
 
         for (offset = 0; offset < versions.len; offset += 2) {

security/nss/lib/ssl/tls13con.h

@@ -109,8 +109,8 @@ SECStatus tls13_ProtectRecord(sslSocket *ss,
 PRInt32 tls13_Read0RttData(sslSocket *ss, PRUint8 *buf, PRInt32 len);
 SECStatus tls13_HandleEarlyApplicationData(sslSocket *ss, sslBuffer *origBuf);
 PRBool tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid);
-PRUint16 tls13_EncodeDraftVersion(SSL3ProtocolVersion version,
+PRUint16 tls13_EncodeVersion(SSL3ProtocolVersion version,
-                                  SSLProtocolVariant variant);
+                             SSLProtocolVariant variant);
 SECStatus tls13_ClientReadSupportedVersion(sslSocket *ss);
 SECStatus tls13_NegotiateVersion(sslSocket *ss,
                                  const TLSExtension *supported_versions);

security/nss/lib/ssl/tls13exthandle.c

@@ -789,8 +789,8 @@ tls13_ClientSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD
     }
 
     for (version = ss->vrange.max; version >= ss->vrange.min; --version) {
-        PRUint16 wire = tls13_EncodeDraftVersion(version,
+        PRUint16 wire = tls13_EncodeVersion(version,
-                                                 ss->protocolVariant);
+                                            ss->protocolVariant);
         rv = sslBuffer_AppendNumber(buf, wire, 2);
         if (rv != SECSuccess) {
             return SECFailure;
@@ -819,8 +819,8 @@ tls13_ServerSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD
     SSL_TRC(3, ("%d: TLS13[%d]: server send supported_versions extension",
                 SSL_GETPID(), ss->fd));
 
-    PRUint16 ver = tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3,
+    PRUint16 ver = tls13_EncodeVersion(SSL_LIBRARY_VERSION_TLS_1_3,
-                                            ss->protocolVariant);
+                                       ss->protocolVariant);
     rv = sslBuffer_AppendNumber(buf, ver, 2);
     if (rv != SECSuccess) {
         return SECFailure;