Bug 1470914, land NSS beta snapshot 6349fa699c3b UPGRADE_NSS_RELEASE, r=me
authorKai Engert <kaie@kuix.de>
Wed, 15 Aug 2018 14:42:53 +0200
changeset 486790 de505fb3dfcdf779a76dc97b9f4da91138288307
parent 486789 0d27c68cfaf85c07e1059184f17354b358228dfe
child 486791 fde17a23425676f852797e25d93ca736d36f4291
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1470914
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1470914, land NSS beta snapshot 6349fa699c3b UPGRADE_NSS_RELEASE, r=me
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/gtests/nss_bogo_shim/config.json
security/nss/gtests/ssl_gtest/ssl_auth_unittest.cc
security/nss/gtests/ssl_gtest/ssl_dhe_unittest.cc
security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc
security/nss/lib/ssl/ssl3con.c
security/nss/lib/ssl/sslimpl.h
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-01d970fe9048
+6349fa699c3b
--- 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/gtests/nss_bogo_shim/config.json
+++ b/security/nss/gtests/nss_bogo_shim/config.json
@@ -10,18 +10,16 @@
         "Draft-Downgrade-Server":"Boring implements a draft downgrade sentinel used for measurements.",
         "FilterExtraAlgorithms":"NSS doesn't allow sending unsupported signature algorithms",
         "SendBogusAlertType":"Unexpected TLS alerts should abort connections (Bug 1438263)",
         "VerifyPreferences-Ed25519":"Add Ed25519 support (Bug 1325335)",
         "Ed25519DefaultDisable*":"Add Ed25519 support (Bug 1325335)",
         "ServerCipherFilter*":"Add Ed25519 support (Bug 1325335)",
         "GarbageCertificate*":"Send bad_certificate alert when certificate parsing fails (Bug 1441565)",
         "SupportedVersionSelection-TLS12":"Should maybe reject TLS 1.2 in SH.supported_versions (Bug 1438266)",
-        "WrongMessageType-TLS13-EncryptedExtensions":"wrong boring expectedLocalError",
-        "TrailingMessageData-TLS13-EncryptedExtensions":"wrong boring expectedLocalError",
         "Resume-Server-BinderWrongLength":"Alert disagreement (Bug 1317633)",
         "Resume-Server-NoPSKBinder":"Alert disagreement (Bug 1317633)",
         "CheckRecordVersion-TLS*":"Bug 1317634",
         "GREASE-Server-TLS13":"BoringSSL GREASEs without a flag, but we ignore it",
         "TLS13-ExpectNoSessionTicketOnBadKEMode-Server":"Bug in NSS. Don't send ticket when not permitted by KE modes (Bug 1317635)",
         "*KeyUpdate*":"KeyUpdate Unimplemented",
         "ClientAuth-NoFallback-TLS13":"Disagreement about alerts. Bug 1294975",
         "SendWarningAlerts-TLS13":"NSS needs to trigger on warning alerts",
@@ -40,22 +38,24 @@
         "TLS12-AES128-GCM-client":"Bug 1292895",
         "*TLS12-AES128-GCM-LargeRecord*":"Bug 1292895",
         "Renegotiate-Client-Forbidden-1":"Bug 1292898",
         "Renegotiate-Server-Forbidden":"NSS doesn't disable renegotiation by default",
         "Renegotiate-Client-NoIgnore":"NSS doesn't disable renegotiation by default",
         "StrayHelloRequest*":"NSS doesn't disable renegotiation by default",
         "NoSupportedCurves-TLS13":"wanted SSL_ERROR_NO_CYPHER_OVERLAP, got missing extension error",
         "FragmentedClientVersion":"received a malformed Client Hello handshake message",
-        "UnofferedExtension-Client-TLS13":"wrong boring expectedLocalError",
-        "UnknownExtension-Client-TLS13":"wrong boring expectedLocalError",
-        "WrongMessageType-TLS13-CertificateRequest":"wrong boring expectedLocalError",
-        "WrongMessageType-TLS13-ServerCertificateVerify":"wrong boring expectedLocalError",
-        "WrongMessageType-TLS13-ServerCertificate":"wrong boring expectedLocalError",
-        "WrongMessageType-TLS13-ServerFinished":"wrong boring expectedLocalError",
+        "WrongMessageType-TLS13-EncryptedExtensions":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "TrailingMessageData-TLS13-EncryptedExtensions":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "UnofferedExtension-Client-TLS13":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "UnknownExtension-Client-TLS13":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "WrongMessageType-TLS13-CertificateRequest":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "WrongMessageType-TLS13-ServerCertificateVerify":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "WrongMessageType-TLS13-ServerCertificate":"Boring expects CCS (Bugs 1481209, 1304603)",
+        "WrongMessageType-TLS13-ServerFinished":"Boring expects CCS (Bugs 1481209, 1304603)",
         "TrailingMessageData-*": "Bug 1304575",
         "DuplicateKeyShares":"Bug 1304578",
         "Resume-Server-TLS13-TLS13":"Bug 1314351",
         "SkipEarlyData-Interleaved":"Bug 1336916",
         "ECDSAKeyUsage-TLS1*":"Bug 1338194",
         "PointFormat-Client-MissingUncompressed":"We ignore ec_point_formats extensions sent by servers.",
         "SkipEarlyData-SecondClientHelloEarlyData":"Boring doesn't reject early_data in the 2nd CH but fails later with bad_record_mac.",
         "SkipEarlyData-*TooMuchData":"Bug 1339373",
--- a/security/nss/gtests/ssl_gtest/ssl_auth_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_auth_unittest.cc
@@ -192,16 +192,91 @@ TEST_P(TlsConnectTls12, ClientAuthBigRsa
   client_->SetupClientAuth();
   server_->RequestClientAuth(true);
   Connect();
   CheckKeys();
   CheckSigScheme(capture_cert_verify, 0, server_, ssl_sig_rsa_pss_rsae_sha256,
                  2048);
 }
 
+// Replaces the signature scheme in a CertificateVerify message.
+class TlsReplaceSignatureSchemeFilter : public TlsHandshakeFilter {
+ public:
+  TlsReplaceSignatureSchemeFilter(const std::shared_ptr<TlsAgent>& a,
+                                  SSLSignatureScheme scheme)
+      : TlsHandshakeFilter(a, {kTlsHandshakeCertificateVerify}),
+        scheme_(scheme) {
+    EnableDecryption();
+  }
+
+ protected:
+  virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
+                                               const DataBuffer& input,
+                                               DataBuffer* output) {
+    *output = input;
+    output->Write(0, scheme_, 2);
+    return CHANGE;
+  }
+
+ private:
+  SSLSignatureScheme scheme_;
+};
+
+// Check if CertificateVerify signed with rsa_pss_rsae_* is properly
+// rejected when the certificate is RSA-PSS.
+//
+// This only works under TLS 1.2, because PSS doesn't work with TLS
+// 1.0 or TLS 1.1 and the TLS 1.3 1-RTT handshake is partially
+// successful at the client side.
+TEST_P(TlsConnectTls12, ClientAuthInconsistentRsaeSignatureScheme) {
+  static const SSLSignatureScheme kSignatureSchemePss[] = {
+      ssl_sig_rsa_pss_pss_sha256, ssl_sig_rsa_pss_rsae_sha256};
+
+  Reset(TlsAgent::kServerRsa, "rsa_pss");
+  client_->SetSignatureSchemes(kSignatureSchemePss,
+                               PR_ARRAY_SIZE(kSignatureSchemePss));
+  server_->SetSignatureSchemes(kSignatureSchemePss,
+                               PR_ARRAY_SIZE(kSignatureSchemePss));
+  client_->SetupClientAuth();
+  server_->RequestClientAuth(true);
+
+  EnsureTlsSetup();
+
+  MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(client_,
+                                                 ssl_sig_rsa_pss_rsae_sha256);
+
+  ConnectExpectAlert(server_, kTlsAlertIllegalParameter);
+}
+
+// Check if CertificateVerify signed with rsa_pss_pss_* is properly
+// rejected when the certificate is RSA.
+//
+// This only works under TLS 1.2, because PSS doesn't work with TLS
+// 1.0 or TLS 1.1 and the TLS 1.3 1-RTT handshake is partially
+// successful at the client side.
+TEST_P(TlsConnectTls12, ClientAuthInconsistentPssSignatureScheme) {
+  static const SSLSignatureScheme kSignatureSchemePss[] = {
+      ssl_sig_rsa_pss_rsae_sha256, ssl_sig_rsa_pss_pss_sha256};
+
+  Reset(TlsAgent::kServerRsa, "rsa");
+  client_->SetSignatureSchemes(kSignatureSchemePss,
+                               PR_ARRAY_SIZE(kSignatureSchemePss));
+  server_->SetSignatureSchemes(kSignatureSchemePss,
+                               PR_ARRAY_SIZE(kSignatureSchemePss));
+  client_->SetupClientAuth();
+  server_->RequestClientAuth(true);
+
+  EnsureTlsSetup();
+
+  MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(client_,
+                                                 ssl_sig_rsa_pss_pss_sha256);
+
+  ConnectExpectAlert(server_, kTlsAlertIllegalParameter);
+}
+
 class TlsZeroCertificateRequestSigAlgsFilter : public TlsHandshakeFilter {
  public:
   TlsZeroCertificateRequestSigAlgsFilter(const std::shared_ptr<TlsAgent>& a)
       : TlsHandshakeFilter(a, {kTlsHandshakeCertificateRequest}) {}
   virtual PacketFilter::Action FilterHandshake(
       const TlsHandshakeFilter::HandshakeHeader& header,
       const DataBuffer& input, DataBuffer* output) {
     TlsParser parser(input);
@@ -405,39 +480,16 @@ TEST_P(TlsConnectTls13, SignatureAlgorit
 // only fails when the Finished is checked.
 TEST_P(TlsConnectTls12, SignatureAlgorithmDrop) {
   MakeTlsFilter<TlsExtensionDropper>(client_, ssl_signature_algorithms_xtn);
   ConnectExpectAlert(server_, kTlsAlertDecryptError);
   client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
   server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
 }
 
-// Replaces the signature scheme in a TLS 1.3 CertificateVerify message.
-class TlsReplaceSignatureSchemeFilter : public TlsHandshakeFilter {
- public:
-  TlsReplaceSignatureSchemeFilter(const std::shared_ptr<TlsAgent>& a,
-                                  SSLSignatureScheme scheme)
-      : TlsHandshakeFilter(a, {kTlsHandshakeCertificateVerify}),
-        scheme_(scheme) {
-    EnableDecryption();
-  }
-
- protected:
-  virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
-                                               const DataBuffer& input,
-                                               DataBuffer* output) {
-    *output = input;
-    output->Write(0, scheme_, 2);
-    return CHANGE;
-  }
-
- private:
-  SSLSignatureScheme scheme_;
-};
-
 TEST_P(TlsConnectTls13, UnsupportedSignatureSchemeAlert) {
   EnsureTlsSetup();
   MakeTlsFilter<TlsReplaceSignatureSchemeFilter>(server_, ssl_sig_none);
 
   ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
   server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
   client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CERT_VERIFY);
 }
--- a/security/nss/gtests/ssl_gtest/ssl_dhe_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_dhe_unittest.cc
@@ -638,9 +638,48 @@ TEST_P(TlsConnectGenericPre13, InvalidDE
 
   MakeTlsFilter<TlsDheSkeChangeSignature>(server_, version_, kBogusDheSignature,
                                           sizeof(kBogusDheSignature));
 
   ConnectExpectAlert(client_, kTlsAlertDecryptError);
   client_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
 }
 
+// Replace SignatureAndHashAlgorithm of a SKE.
+class DHEServerKEXSigAlgReplacer : public TlsHandshakeFilter {
+ public:
+  DHEServerKEXSigAlgReplacer(const std::shared_ptr<TlsAgent>& server,
+                             SSLSignatureScheme sig_scheme)
+      : TlsHandshakeFilter(server, {kTlsHandshakeServerKeyExchange}),
+        sig_scheme_(sig_scheme) {}
+
+ protected:
+  virtual PacketFilter::Action FilterHandshake(const HandshakeHeader& header,
+                                               const DataBuffer& input,
+                                               DataBuffer* output) {
+    *output = input;
+
+    uint32_t len;
+    uint32_t idx = 0;
+    EXPECT_TRUE(output->Read(idx, 2, &len));
+    idx += 2 + len;
+    EXPECT_TRUE(output->Read(idx, 2, &len));
+    idx += 2 + len;
+    EXPECT_TRUE(output->Read(idx, 2, &len));
+    idx += 2 + len;
+    output->Write(idx, sig_scheme_, 2);
+
+    return CHANGE;
+  }
+
+ private:
+  SSLSignatureScheme sig_scheme_;
+};
+
+TEST_P(TlsConnectTls12, ConnectInconsistentSigAlgDHE) {
+  EnableOnlyDheCiphers();
+
+  MakeTlsFilter<DHEServerKEXSigAlgReplacer>(server_,
+                                            ssl_sig_ecdsa_secp256r1_sha256);
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+}
+
 }  // namespace nss_test
--- a/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/security/nss/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -449,16 +449,35 @@ TEST_P(TlsExtensionTest12Plus, Signature
 
 TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsOddLength) {
   const uint8_t val[] = {0x00, 0x01, 0x04};
   DataBuffer extension(val, sizeof(val));
   ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
       client_, ssl_signature_algorithms_xtn, extension));
 }
 
+TEST_F(TlsExtensionTest13Stream, SignatureAlgorithmsPrecedingGarbage) {
+  // 31 unknown signature algorithms followed by sha-256, rsa
+  const uint8_t val[] = {
+      0x00, 0x40, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x04, 0x01};
+  DataBuffer extension(val, sizeof(val));
+  MakeTlsFilter<TlsExtensionReplacer>(client_, ssl_signature_algorithms_xtn,
+                                      extension);
+  client_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  server_->ExpectSendAlert(kTlsAlertBadRecordMac);
+  ConnectExpectFail();
+  client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
+  server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
+}
+
 TEST_P(TlsExtensionTestGeneric, NoSupportedGroups) {
   ClientHelloErrorTest(
       std::make_shared<TlsExtensionDropper>(client_, ssl_supported_groups_xtn),
       version_ < SSL_LIBRARY_VERSION_TLS_1_3 ? kTlsAlertDecryptError
                                              : kTlsAlertMissingExtension);
 }
 
 TEST_P(TlsExtensionTestGeneric, SupportedCurvesShort) {
--- a/security/nss/lib/ssl/ssl3con.c
+++ b/security/nss/lib/ssl/ssl3con.c
@@ -6956,22 +6956,22 @@ ssl_HandleDHServerKeyExchange(sslSocket 
     if (!ssl_IsValidDHEShare(&dh_p, &dh_Ys)) {
         errCode = SSL_ERROR_RX_MALFORMED_DHE_KEY_SHARE;
         goto alert_loser;
     }
 
     if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_2) {
         rv = ssl_ConsumeSignatureScheme(ss, &b, &length, &sigScheme);
         if (rv != SECSuccess) {
-            goto loser; /* malformed or unsupported. */
+            goto alert_loser; /* malformed or unsupported. */
         }
         rv = ssl_CheckSignatureSchemeConsistency(ss, sigScheme,
                                                  ss->sec.peerCert);
         if (rv != SECSuccess) {
-            goto loser;
+            goto alert_loser;
         }
         hashAlg = ssl_SignatureSchemeToHashType(sigScheme);
     } else {
         /* Use ssl_hash_none to represent the MD5+SHA1 combo. */
         hashAlg = ssl_hash_none;
         sigScheme = ssl_sig_none;
     }
     rv = ssl3_ConsumeHandshakeVariable(ss, &signature, 2, &b, &length);
@@ -7185,17 +7185,18 @@ SECStatus
 ssl_ParseSignatureSchemes(const sslSocket *ss, PLArenaPool *arena,
                           SSLSignatureScheme **schemesOut,
                           unsigned int *numSchemesOut,
                           unsigned char **b, unsigned int *len)
 {
     SECStatus rv;
     SECItem buf;
     SSLSignatureScheme *schemes = NULL;
-    unsigned int numSchemes = 0;
+    unsigned int numSupported = 0;
+    unsigned int numRemaining = 0;
     unsigned int max;
 
     rv = ssl3_ExtConsumeHandshakeVariable(ss, &buf, 2, b, len);
     if (rv != SECSuccess) {
         return SECFailure;
     }
     /* An odd-length value is invalid. */
     if ((buf.len & 1) != 0) {
@@ -7204,51 +7205,52 @@ ssl_ParseSignatureSchemes(const sslSocke
     }
 
     /* Let the caller decide whether to alert here. */
     if (buf.len == 0) {
         goto done;
     }
 
     /* Limit the number of schemes we read. */
-    max = PR_MIN(buf.len / 2, MAX_SIGNATURE_SCHEMES);
+    numRemaining = buf.len / 2;
+    max = PR_MIN(numRemaining, MAX_SIGNATURE_SCHEMES);
 
     if (arena) {
         schemes = PORT_ArenaZNewArray(arena, SSLSignatureScheme, max);
     } else {
         schemes = PORT_ZNewArray(SSLSignatureScheme, max);
     }
     if (!schemes) {
         ssl3_ExtSendAlert(ss, alert_fatal, internal_error);
         return SECFailure;
     }
 
-    for (; max; --max) {
+    for (; numRemaining && numSupported < MAX_SIGNATURE_SCHEMES; --numRemaining) {
         PRUint32 tmp;
         rv = ssl3_ExtConsumeHandshakeNumber(ss, &tmp, 2, &buf.data, &buf.len);
         if (rv != SECSuccess) {
             PORT_Assert(0);
             PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
             return SECFailure;
         }
         if (ssl_IsSupportedSignatureScheme((SSLSignatureScheme)tmp)) {
-            schemes[numSchemes++] = (SSLSignatureScheme)tmp;
-        }
-    }
-
-    if (!numSchemes) {
+            schemes[numSupported++] = (SSLSignatureScheme)tmp;
+        }
+    }
+
+    if (!numSupported) {
         if (!arena) {
             PORT_Free(schemes);
         }
         schemes = NULL;
     }
 
 done:
     *schemesOut = schemes;
-    *numSchemesOut = numSchemes;
+    *numSchemesOut = numSupported;
     return SECSuccess;
 }
 
 /* Called from ssl3_HandlePostHelloHandshakeMessage() when it has deciphered
  * a complete ssl3 Certificate Request message.
  * Caller must hold Handshake and RecvBuf locks.
  */
 static SECStatus
@@ -9565,17 +9567,17 @@ ssl3_HandleCertificateVerify(sslSocket *
         rv = ssl_ConsumeSignatureScheme(ss, &b, &length, &sigScheme);
         if (rv != SECSuccess) {
             goto loser; /* malformed or unsupported. */
         }
         rv = ssl_CheckSignatureSchemeConsistency(ss, sigScheme,
                                                  ss->sec.peerCert);
         if (rv != SECSuccess) {
             errCode = PORT_GetError();
-            desc = decrypt_error;
+            desc = illegal_parameter;
             goto alert_loser;
         }
 
         rv = ssl3_ComputeHandshakeHash(ss->ssl3.hs.messages.buf,
                                        ss->ssl3.hs.messages.len,
                                        ssl_SignatureSchemeToHashType(sigScheme),
                                        &hashes);
     } else {
--- a/security/nss/lib/ssl/sslimpl.h
+++ b/security/nss/lib/ssl/sslimpl.h
@@ -225,17 +225,17 @@ typedef struct {
 #endif
 } ssl3CipherSuiteCfg;
 
 #define ssl_V3_SUITES_IMPLEMENTED 71
 
 #define MAX_DTLS_SRTP_CIPHER_SUITES 4
 
 /* MAX_SIGNATURE_SCHEMES allows for all the values we support. */
-#define MAX_SIGNATURE_SCHEMES 15
+#define MAX_SIGNATURE_SCHEMES 18
 
 typedef struct sslOptionsStr {
     /* If SSL_SetNextProtoNego has been called, then this contains the
      * list of supported protocols. */
     SECItem nextProtoNego;
     PRUint16 recordSizeLimit;
 
     PRUint32 maxEarlyDataSize;