import NSS tip revs: bug1594965, bug1593167, bug1603027, bug1600144, bug1590001, bug1603257, bug1605545, bug1513586

security/nss/cmd/modutil/install-ds.c

@@ -889,8 +889,6 @@ Pk11Install_Platform_Generate(Pk11Install_Platform* _this,
                 errStr = Pk11Install_PlatformName_Generate(&_this->equivName,
                                                            subval->string);
                 if (errStr) {
-                    tmp = PR_smprintf("%s: %s",
-                                      Pk11Install_PlatformName_GetString(&_this->name), errStr);
                     tmp = PR_smprintf("%s: %s",
                                       Pk11Install_PlatformName_GetString(&_this->name), errStr);
                     PR_smprintf_free(errStr);

security/nss/gtests/pk11_gtest/pk11_chacha20poly1305_unittest.cc

@@ -261,13 +261,16 @@ TEST_F(Pkcs11ChaCha20Poly1305Test, GenerateXor) {
   ScopedPK11SymKey key(PK11_KeyGen(slot.get(), kMech, nullptr, 32, nullptr));
   EXPECT_TRUE(!!key);
 
-  SECItem ctrNonceItem = {siBuffer, toUcharPtr(kCtrNonce),
+  std::vector<uint8_t> iv(16);
-                          static_cast<unsigned int>(sizeof(kCtrNonce))};
+  SECStatus rv = PK11_GenerateRandomOnSlot(slot.get(), iv.data(), iv.size());
+  EXPECT_EQ(SECSuccess, rv);
+
+  SECItem ctrNonceItem = {siBuffer, toUcharPtr(iv.data()),
+                          static_cast<unsigned int>(iv.size())};
   uint8_t encrypted[sizeof(kData)];
   unsigned int encrypted_len = 88;  // This should be overwritten.
-  SECStatus rv =
+  rv = PK11_Encrypt(key.get(), kMechXor, &ctrNonceItem, encrypted,
-      PK11_Encrypt(key.get(), kMechXor, &ctrNonceItem, encrypted,
+                    &encrypted_len, sizeof(encrypted), kData, sizeof(kData));
-                   &encrypted_len, sizeof(encrypted), kData, sizeof(kData));
   ASSERT_EQ(SECSuccess, rv);
   ASSERT_EQ(sizeof(kData), static_cast<size_t>(encrypted_len));
 }

security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc

@@ -1055,6 +1055,41 @@ TEST_F(TlsConnectTest, Select12AfterHelloRetryRequest) {
   EXPECT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
 }
 
+// This class increments the low byte of the first Handshake.message_seq
+// field in every handshake record.
+class MessageSeqIncrementer : public TlsRecordFilter {
+ public:
+  MessageSeqIncrementer(const std::shared_ptr<TlsAgent>& a)
+      : TlsRecordFilter(a) {}
+
+ protected:
+  PacketFilter::Action FilterRecord(const TlsRecordHeader& header,
+                                    const DataBuffer& data,
+                                    DataBuffer* changed) override {
+    if (header.content_type() != ssl_ct_handshake) {
+      return KEEP;
+    }
+
+    *changed = data;
+    // struct { uint8 msg_type; uint24 length; uint16 message_seq; ... }
+    // Handshake;
+    changed->data()[5]++;
+    EXPECT_NE(0, changed->data()[5]);  // Check for overflow.
+    return CHANGE;
+  }
+};
+
+// A server that receives a ClientHello with message_seq == 1
+// assumes that this is after a stateless HelloRetryRequest.
+// However, it should reject the ClientHello if it lacks a cookie.
+TEST_F(TlsConnectDatagram13, MessageSeq1ClientHello) {
+  EnsureTlsSetup();
+  MakeTlsFilter<MessageSeqIncrementer>(client_);
+  ConnectExpectAlert(server_, kTlsAlertMissingExtension);
+  EXPECT_EQ(SSL_ERROR_MISSING_COOKIE_EXTENSION, server_->error_code());
+  EXPECT_EQ(SSL_ERROR_MISSING_EXTENSION_ALERT, client_->error_code());
+}
+
 class HelloRetryRequestAgentTest : public TlsAgentTestClient {
  protected:
   void SetUp() override {
@@ -1178,6 +1213,114 @@ TEST_P(TlsConnectStreamPre13, HrrRandomOnTls10) {
   server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
+TEST_F(TlsConnectStreamTls13, HrrThenTls12) {
+  StartConnect();
+  size_t cb_called = 0;
+  EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(),
+                                                      RetryHello, &cb_called));
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+
+  client_->Handshake();  // Send CH (1.3)
+  server_->Handshake();  // Send HRR.
+  EXPECT_EQ(1U, cb_called);
+
+  // Replace the client with a new TLS 1.2 client. Don't call Init(), since
+  // it will artifically limit the server's vrange.
+  client_.reset(
+      new TlsAgent(client_->name(), TlsAgent::CLIENT, ssl_variant_stream));
+  client_->SetPeer(server_);
+  server_->SetPeer(client_);
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+
+  client_->StartConnect();
+  client_->Handshake();  // Send CH (1.2)
+  ExpectAlert(server_, kTlsAlertProtocolVersion);
+  server_->Handshake();
+  server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
+  client_->Handshake();
+  client_->CheckErrorCode(SSL_ERROR_PROTOCOL_VERSION_ALERT);
+}
+
+TEST_F(TlsConnectStreamTls13, ZeroRttHrrThenTls12) {
+  SetupForZeroRtt();
+
+  client_->Set0RttEnabled(true);
+  size_t cb_called = 0;
+  EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(),
+                                                      RetryHello, &cb_called));
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+
+  client_->Handshake();  // Send CH (1.3)
+  ZeroRttSendReceive(true, false);
+  server_->Handshake();  // Send HRR.
+  EXPECT_EQ(1U, cb_called);
+
+  // Replace the client with a new TLS 1.2 client. Don't call Init(), since
+  // it will artifically limit the server's vrange.
+  client_.reset(
+      new TlsAgent(client_->name(), TlsAgent::CLIENT, ssl_variant_stream));
+  client_->SetPeer(server_);
+  server_->SetPeer(client_);
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+
+  client_->StartConnect();
+  client_->Handshake();  // Send CH (1.2)
+  ExpectAlert(server_, kTlsAlertProtocolVersion);
+  server_->Handshake();
+  server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
+  client_->Handshake();
+  client_->CheckErrorCode(SSL_ERROR_PROTOCOL_VERSION_ALERT);
+
+  // Try to write something
+  server_->Handshake();
+  client_->ExpectReadWriteError();
+  client_->SendData(1);
+  uint8_t buf[1];
+  EXPECT_EQ(-1, PR_Read(server_->ssl_fd(), buf, sizeof(buf)));
+  EXPECT_EQ(SSL_ERROR_HANDSHAKE_FAILED, PR_GetError());
+}
+
+TEST_F(TlsConnectStreamTls13, HrrThenTls12SupportedVersions) {
+  SetupForZeroRtt();
+  client_->Set0RttEnabled(true);
+  size_t cb_called = 0;
+  EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(),
+                                                      RetryHello, &cb_called));
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+
+  client_->Handshake();  // Send CH (1.3)
+  ZeroRttSendReceive(true, false);
+  server_->Handshake();  // Send HRR.
+  EXPECT_EQ(1U, cb_called);
+
+  // Replace the client with a new TLS 1.2 client. Don't call Init(), since
+  // it will artifically limit the server's vrange.
+  client_.reset(
+      new TlsAgent(client_->name(), TlsAgent::CLIENT, ssl_variant_stream));
+  client_->SetPeer(server_);
+  server_->SetPeer(client_);
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+  // Negotiate via supported_versions
+  static const uint8_t tls12[] = {0x02, 0x03, 0x03};
+  auto replacer = MakeTlsFilter<TlsExtensionInjector>(
+      client_, ssl_tls13_supported_versions_xtn,
+      DataBuffer(tls12, sizeof(tls12)));
+
+  client_->StartConnect();
+  client_->Handshake();  // Send CH (1.2)
+  ExpectAlert(server_, kTlsAlertProtocolVersion);
+  server_->Handshake();
+  server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
+  client_->Handshake();
+  client_->CheckErrorCode(SSL_ERROR_PROTOCOL_VERSION_ALERT);
+}
+
 INSTANTIATE_TEST_CASE_P(HelloRetryRequestAgentTests, HelloRetryRequestAgentTest,
                         ::testing::Combine(TlsConnectTestBase::kTlsVariantsAll,
                                            TlsConnectTestBase::kTlsV13));

security/nss/gtests/ssl_gtest/ssl_version_unittest.cc

@@ -102,6 +102,61 @@ TEST_F(TlsConnectTest, TestDisableDowngradeDetection) {
   server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
 }
 
+typedef std::tuple<SSLProtocolVariant,
+                   uint16_t,  // client version
+                   uint16_t>  // server version
+    TlsDowngradeProfile;
+
+class TlsDowngradeTest
+    : public TlsConnectTestBase,
+      public ::testing::WithParamInterface<TlsDowngradeProfile> {
+ public:
+  TlsDowngradeTest()
+      : TlsConnectTestBase(std::get<0>(GetParam()), std::get<1>(GetParam())),
+        c_ver(std::get<1>(GetParam())),
+        s_ver(std::get<2>(GetParam())) {}
+
+ protected:
+  const uint16_t c_ver;
+  const uint16_t s_ver;
+};
+
+TEST_P(TlsDowngradeTest, TlsDowngradeSentinelTest) {
+  static const uint8_t tls12_downgrade_random[] = {0x44, 0x4F, 0x57, 0x4E,
+                                                   0x47, 0x52, 0x44, 0x01};
+  static const uint8_t tls1_downgrade_random[] = {0x44, 0x4F, 0x57, 0x4E,
+                                                  0x47, 0x52, 0x44, 0x00};
+  static const size_t kRandomLen = 32;
+
+  if (c_ver > s_ver) {
+    return;
+  }
+
+  client_->SetVersionRange(c_ver, c_ver);
+  server_->SetVersionRange(c_ver, s_ver);
+
+  auto sh = MakeTlsFilter<TlsHandshakeRecorder>(server_, ssl_hs_server_hello);
+  Connect();
+  ASSERT_TRUE(sh->buffer().len() > (kRandomLen + 2));
+
+  const uint8_t* downgrade_sentinel =
+      sh->buffer().data() + 2 + kRandomLen - sizeof(tls1_downgrade_random);
+  if (c_ver < s_ver) {
+    if (c_ver == SSL_LIBRARY_VERSION_TLS_1_2) {
+      EXPECT_EQ(0, memcmp(downgrade_sentinel, tls12_downgrade_random,
+                          sizeof(tls12_downgrade_random)));
+    } else {
+      EXPECT_EQ(0, memcmp(downgrade_sentinel, tls1_downgrade_random,
+                          sizeof(tls1_downgrade_random)));
+    }
+  } else {
+    EXPECT_NE(0, memcmp(downgrade_sentinel, tls12_downgrade_random,
+                        sizeof(tls12_downgrade_random)));
+    EXPECT_NE(0, memcmp(downgrade_sentinel, tls1_downgrade_random,
+                        sizeof(tls1_downgrade_random)));
+  }
+}
+
 // TLS 1.1 clients do not check the random values, so we should
 // instead get a handshake failure alert from the server.
 TEST_F(TlsConnectTest, TestDowngradeDetectionToTls10) {
@@ -280,4 +335,10 @@ TEST_F(TlsConnectStreamTls13, Ssl30ClientHelloWithSupportedVersions) {
   ConnectExpectAlert(server_, kTlsAlertProtocolVersion);
 }
 
+INSTANTIATE_TEST_CASE_P(
+    TlsDowngradeSentinelTest, TlsDowngradeTest,
+    ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,
+                       TlsConnectTestBase::kTlsVAll,
+                       TlsConnectTestBase::kTlsV12Plus));
+
 }  // namespace nss_test

security/nss/gtests/ssl_gtest/tls_esni_unittest.cc

@@ -314,12 +314,20 @@ TEST_P(TlsConnectTls13, ConnectEsniHrr) {
       MakeTlsFilter<TlsExtensionCapture>(client_, ssl_server_name_xtn);
   auto filter2 =
       MakeTlsFilter<TlsExtensionCapture>(client_, ssl_server_name_xtn, true);
+  auto efilter =
+      MakeTlsFilter<TlsExtensionCapture>(client_, ssl_tls13_encrypted_sni_xtn);
+  auto efilter2 = MakeTlsFilter<TlsExtensionCapture>(
+      client_, ssl_tls13_encrypted_sni_xtn, true);
+
   client_->SetFilter(std::make_shared<ChainedPacketFilter>(
-      ChainedPacketFilterInit({filter, filter2})));
+      ChainedPacketFilterInit({filter, filter2, efilter, efilter2})));
   server_->SetSniCallback(SniCallback);
   Connect();
   CheckSniExtension(filter->extension());
   CheckSniExtension(filter2->extension());
+  ASSERT_TRUE(efilter->captured());
+  ASSERT_TRUE(efilter2->captured());
+  ASSERT_NE(efilter->extension(), efilter2->extension());
   EXPECT_NE(0UL, hrr_capture->buffer().len());
 }
 

security/nss/lib/pki/pki3hack.c

@@ -921,14 +921,28 @@ stan_GetCERTCertificate(NSSCertificate *c, PRBool forceUpdate)
     }
     if (!cc->nssCertificate || forceUpdate) {
         fill_CERTCertificateFields(c, cc, forceUpdate);
-    } else if (CERT_GetCertTrust(cc, &certTrust) != SECSuccess &&
+    } else if (CERT_GetCertTrust(cc, &certTrust) != SECSuccess) {
-               !c->object.cryptoContext) {
+        CERTCertTrust *trust;
-        /* if it's a perm cert, it might have been stored before the
+        if (!c->object.cryptoContext) {
-         * trust, so look for the trust again.  But a temp cert can be
+            /* If it's a perm cert, it might have been stored before the
-         * ignored.
+             * trust, so look for the trust again.
-         */
+             */
-        CERTCertTrust *trust = NULL;
+            trust = nssTrust_GetCERTCertTrustForCert(c, cc);
-        trust = nssTrust_GetCERTCertTrustForCert(c, cc);
+        } else {
+            /* If it's a temp cert, it might have been stored before the
+             * builtin trust module is loaded, so look for the trust
+             * again, but don't set the empty trust if it is not found.
+             */
+            NSSTrust *t = nssTrustDomain_FindTrustForCertificate(c->object.cryptoContext->td, c);
+            if (!t) {
+                goto loser;
+            }
+            trust = cert_trust_from_stan_trust(t, cc->arena);
+            nssTrust_Destroy(t);
+            if (!trust) {
+                goto loser;
+            }
+        }
 
         CERT_LockCertTrust(cc);
         cc->trust = trust;

security/nss/lib/softoken/pkcs11c.c

@@ -1238,7 +1238,7 @@ sftk_CryptInit(CK_SESSION_HANDLE hSession, CK_MECHANISM_PTR pMechanism,
             PRUint8 *param = pMechanism->pParameter;
             int i = 0;
             for (; i < 4; ++i) {
-                ctx->counter |= param[i] << (i * 8);
+                ctx->counter |= (PRUint32)param[i] << (i * 8);
             }
             memcpy(ctx->nonce, param + 4, 12);
             context->cipherInfo = ctx;

security/nss/lib/ssl/dtlscon.c

@@ -343,6 +343,7 @@ dtls_HandleHandshake(sslSocket *ss, DTLSEpoch epoch, sslSequenceNumber seqNum,
             SSL_TRC(5, ("%d: DTLS[%d]: Received apparent 2nd ClientHello",
                         SSL_GETPID(), ss->fd));
             ss->ssl3.hs.recvMessageSeq = 1;
+            ss->ssl3.hs.helloRetry = PR_TRUE;
         }
 
         /* There are three ways we could not be ready for this packet.

security/nss/lib/ssl/ssl3con.c

@@ -394,12 +394,12 @@ static const SSLCipher2Mech alg2Mech[] = {
     { ssl_calg_chacha20, CKM_NSS_CHACHA20_POLY1305 },
 };
 
-const PRUint8 tls13_downgrade_random[] = { 0x44, 0x4F, 0x57, 0x4E,
-                                           0x47, 0x52, 0x44, 0x01 };
 const PRUint8 tls12_downgrade_random[] = { 0x44, 0x4F, 0x57, 0x4E,
-                                           0x47, 0x52, 0x44, 0x00 };
+                                           0x47, 0x52, 0x44, 0x01 };
-PR_STATIC_ASSERT(sizeof(tls13_downgrade_random) ==
+const PRUint8 tls1_downgrade_random[] = { 0x44, 0x4F, 0x57, 0x4E,
-                 sizeof(tls13_downgrade_random));
+                                          0x47, 0x52, 0x44, 0x00 };
+PR_STATIC_ASSERT(sizeof(tls12_downgrade_random) ==
+                 sizeof(tls1_downgrade_random));
 
 /* The ECCWrappedKeyInfo structure defines how various pieces of
  * information are laid out within wrappedSymmetricWrappingkey
@@ -6713,13 +6713,13 @@ ssl_CheckServerRandom(sslSocket *ss)
         /* Both sections use the same sentinel region. */
         PRUint8 *downgrade_sentinel =
             ss->ssl3.hs.server_random +
-            SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random);
+            SSL3_RANDOM_LENGTH - sizeof(tls12_downgrade_random);
         if (!PORT_Memcmp(downgrade_sentinel,
-                         tls13_downgrade_random,
-                         sizeof(tls13_downgrade_random)) ||
-            !PORT_Memcmp(downgrade_sentinel,
                          tls12_downgrade_random,
-                         sizeof(tls12_downgrade_random))) {
+                         sizeof(tls12_downgrade_random)) ||
+            !PORT_Memcmp(downgrade_sentinel,
+                         tls1_downgrade_random,
+                         sizeof(tls1_downgrade_random))) {
             return SECFailure;
         }
     }
@@ -8491,20 +8491,24 @@ ssl_GenerateServerRandom(sslSocket *ss)
      */
     PRUint8 *downgradeSentinel =
         ss->ssl3.hs.server_random +
-        SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random);
+        SSL3_RANDOM_LENGTH - sizeof(tls12_downgrade_random);
 
-    switch (ss->vrange.max) {
+    if (ss->vrange.max >= SSL_LIBRARY_VERSION_TLS_1_2) {
-        case SSL_LIBRARY_VERSION_TLS_1_3:
+        switch (ss->version) {
-            PORT_Memcpy(downgradeSentinel,
+            case SSL_LIBRARY_VERSION_TLS_1_2:
-                        tls13_downgrade_random, sizeof(tls13_downgrade_random));
+                /* vrange.max > 1.2, since we didn't early exit above. */
-            break;
+                PORT_Memcpy(downgradeSentinel,
-        case SSL_LIBRARY_VERSION_TLS_1_2:
+                            tls12_downgrade_random, sizeof(tls12_downgrade_random));
-            PORT_Memcpy(downgradeSentinel,
+                break;
-                        tls12_downgrade_random, sizeof(tls12_downgrade_random));
+            case SSL_LIBRARY_VERSION_TLS_1_1:
-            break;
+            case SSL_LIBRARY_VERSION_TLS_1_0:
-        default:
+                PORT_Memcpy(downgradeSentinel,
-            /* Do not change random. */
+                            tls1_downgrade_random, sizeof(tls1_downgrade_random));
-            break;
+                break;
+            default:
+                /* Do not change random. */
+                break;
+        }
     }
 
     return SECSuccess;

security/nss/lib/ssl/tls13subcerts.c

@@ -7,6 +7,7 @@
 #include "nss.h"
 #include "pk11func.h"
 #include "secder.h"
+#include "sechash.h"
 #include "ssl.h"
 #include "sslproto.h"
 #include "sslimpl.h"
@@ -538,6 +539,15 @@ tls13_MakePssSpki(const SECKEYPublicKey *pub, SECOidTag hashOid)
         goto loser; /* Code already set. */
     }
 
+    /* Always include saltLength: all hashes are larger than 20. */
+    unsigned int saltLength = HASH_ResultLenByOidTag(hashOid);
+    PORT_Assert(saltLength > 20);
+    if (!SEC_ASN1EncodeInteger(arena, &params.saltLength, saltLength)) {
+        PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
+        goto loser;
+    }
+    /* Omit the trailerField always. */
+
     SECItem *algorithmItem =
         SEC_ASN1EncodeItem(arena, NULL, &params,
                            SEC_ASN1_GET(SECKEY_RSAPSSParamsTemplate));
@@ -752,6 +762,8 @@ SSLExp_DelegateCredential(const CERTCertificate *cert,
         goto loser;
     }
 
+    PRINT_BUF(20, (NULL, "delegated credential", dcBuf.buf, dcBuf.len));
+
     SECKEY_DestroySubjectPublicKeyInfo(spki);
     SECKEY_DestroyPrivateKey(tmpPriv);
     tls13_DestroyDelegatedCredential(dc);