Bug 1572593 - Reset advertised extensions in ssl_ConstructExtensions r=mt,kjacobs
authorJ.C. Jones <jjones@mozilla.com>
Wed, 14 Aug 2019 14:23:15 +0000
changeset 15254 b03ff661491e412bf42d15d2ebe6f6ea7d9e4b9c
parent 15253 9d1f5e71773d4e3146524096d74cb96c8df51abe
child 15255 f8926908be71c3e1b4fd112e655d603fcf0fdb32
push id3465
push userjjones@mozilla.com
push dateWed, 14 Aug 2019 17:05:17 +0000
reviewersmt, kjacobs
bugs1572593
Bug 1572593 - Reset advertised extensions in ssl_ConstructExtensions r=mt,kjacobs Reset the list of advertised extensions before sending a new set. This reverts the changes of https://hg.mozilla.org/projects/nss/rev/1ca362213631d6edc885b6b965b52ecffcf29afd Differential Revision: https://phabricator.services.mozilla.com/D41302
gtests/ssl_gtest/tls_agent.cc
lib/ssl/ssl3ext.c
--- a/gtests/ssl_gtest/tls_agent.cc
+++ b/gtests/ssl_gtest/tls_agent.cc
@@ -244,20 +244,16 @@ bool TlsAgent::EnsureTlsSetup(PRFileDesc
 
   SECStatus rv;
   if (!skip_version_checks_) {
     rv = SSL_VersionRangeSet(ssl_fd(), &vrange_);
     EXPECT_EQ(SECSuccess, rv);
     if (rv != SECSuccess) return false;
   }
 
-  ScopedCERTCertList anchors(CERT_NewCertList());
-  rv = SSL_SetTrustAnchors(ssl_fd(), anchors.get());
-  if (rv != SECSuccess) return false;
-
   if (role_ == SERVER) {
     EXPECT_TRUE(ConfigServerCert(name_, true));
 
     rv = SSL_SNISocketConfigHook(ssl_fd(), SniHook, this);
     EXPECT_EQ(SECSuccess, rv);
     if (rv != SECSuccess) return false;
 
     rv = SSL_SetMaxEarlyDataSize(ssl_fd(), 1024);
@@ -320,17 +316,17 @@ void TlsAgent::SetupClientAuth() {
   EXPECT_TRUE(EnsureTlsSetup());
   ASSERT_EQ(CLIENT, role_);
 
   EXPECT_EQ(SECSuccess,
             SSL_GetClientAuthDataHook(ssl_fd(), GetClientAuthDataHook,
                                       reinterpret_cast<void*>(this)));
 }
 
-void CheckCertReqAgainstDefaultCAs(const CERTDistNames* caNames) {
+static void CheckCertReqAgainstDefaultCAs(const CERTDistNames* caNames) {
   ScopedCERTDistNames expected(CERT_GetSSLCACerts(nullptr));
 
   ASSERT_EQ(expected->nnames, caNames->nnames);
 
   for (size_t i = 0; i < static_cast<size_t>(expected->nnames); ++i) {
     EXPECT_EQ(SECEqual,
               SECITEM_CompareItem(&(expected->names[i]), &(caNames->names[i])));
   }
@@ -339,18 +335,17 @@ void CheckCertReqAgainstDefaultCAs(const
 SECStatus TlsAgent::GetClientAuthDataHook(void* self, PRFileDesc* fd,
                                           CERTDistNames* caNames,
                                           CERTCertificate** clientCert,
                                           SECKEYPrivateKey** clientKey) {
   TlsAgent* agent = reinterpret_cast<TlsAgent*>(self);
   ScopedCERTCertificate peerCert(SSL_PeerCertificate(agent->ssl_fd()));
   EXPECT_TRUE(peerCert) << "Client should be able to see the server cert";
 
-  // See bug 1457716
-  // CheckCertReqAgainstDefaultCAs(caNames);
+  CheckCertReqAgainstDefaultCAs(caNames);
 
   ScopedCERTCertificate cert;
   ScopedSECKEYPrivateKey priv;
   if (!TlsAgent::LoadCertificate(agent->name(), &cert, &priv)) {
     return SECFailure;
   }
 
   *clientCert = cert.release();
--- a/lib/ssl/ssl3ext.c
+++ b/lib/ssl/ssl3ext.c
@@ -709,16 +709,19 @@ loser:
 SECStatus
 ssl_ConstructExtensions(sslSocket *ss, sslBuffer *buf, SSLHandshakeType message)
 {
     const sslExtensionBuilder *sender;
     SECStatus rv;
 
     PORT_Assert(buf->len == 0);
 
+    /* Clear out any extensions previously advertised */
+    ss->xtnData.numAdvertised = 0;
+
     switch (message) {
         case ssl_hs_client_hello:
             if (ss->vrange.max > SSL_LIBRARY_VERSION_3_0) {
                 sender = clientHelloSendersTLS;
             } else {
                 sender = clientHelloSendersSSL3;
             }
             break;