Bug 1341002 - Don't assert when receiving a certificate_verify as the…

… first message r=franziskus,mt

Differential Revision: https://nss-review.dev.mozaws.net/D225

--HG--
extra : amend_source : f27186a71e57c0022d0904f3f8ff01caf580963c

gtests/ssl_gtest/ssl_agent_unittest.cc

@@ -67,11 +67,9 @@ TEST_P(TlsAgentTest, EarlyCertificateVerify) {
                  SSL_ERROR_RX_UNEXPECTED_CERT_VERIFY);
 }
 
-TEST_P(TlsAgentTestClient, CannedHello) {
+TEST_P(TlsAgentTestClient13, CannedHello) {
   DataBuffer buffer;
   EnsureInit();
-  agent_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_3,
-                          SSL_LIBRARY_VERSION_TLS_1_3);
   DataBuffer server_hello;
   MakeHandshakeMessage(kTlsHandshakeServerHello, kCannedTls13ServerHello,
                        sizeof(kCannedTls13ServerHello), &server_hello);
@@ -80,7 +78,7 @@ TEST_P(TlsAgentTestClient, CannedHello) {
   ProcessMessage(buffer, TlsAgent::STATE_CONNECTING);
 }
 
-TEST_P(TlsAgentTestClient, EncryptedExtensionsInClear) {
+TEST_P(TlsAgentTestClient13, EncryptedExtensionsInClear) {
   DataBuffer server_hello;
   MakeHandshakeMessage(kTlsHandshakeServerHello, kCannedTls13ServerHello,
                        sizeof(kCannedTls13ServerHello), &server_hello);
@@ -92,8 +90,6 @@ TEST_P(TlsAgentTestClient, EncryptedExtensionsInClear) {
   MakeRecord(kTlsHandshakeType, SSL_LIBRARY_VERSION_TLS_1_3,
              server_hello.data(), server_hello.len(), &buffer);
   EnsureInit();
-  agent_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_3,
-                          SSL_LIBRARY_VERSION_TLS_1_3);
   ProcessMessage(buffer, TlsAgent::STATE_ERROR,
                  SSL_ERROR_RX_UNEXPECTED_HANDSHAKE);
 }
@@ -201,10 +197,14 @@ TEST_F(TlsAgentStreamTestServer, Set0RttOptionClientHelloThenRead) {
   ProcessMessage(buffer, TlsAgent::STATE_ERROR, SSL_ERROR_BAD_MAC_READ);
 }
 
-INSTANTIATE_TEST_CASE_P(
-    AgentTests, TlsAgentTest,
-    ::testing::Combine(TlsAgentTestBase::kTlsRolesAll,
-                       TlsConnectTestBase::kTlsModesStream));
+INSTANTIATE_TEST_CASE_P(AgentTests, TlsAgentTest,
+                        ::testing::Combine(TlsAgentTestBase::kTlsRolesAll,
+                                           TlsConnectTestBase::kTlsModesStream,
+                                           TlsConnectTestBase::kTlsVAll));
 INSTANTIATE_TEST_CASE_P(ClientTests, TlsAgentTestClient,
-                        TlsConnectTestBase::kTlsModesAll);
+                        ::testing::Combine(TlsConnectTestBase::kTlsModesAll,
+                                           TlsConnectTestBase::kTlsVAll));
+INSTANTIATE_TEST_CASE_P(ClientTests13, TlsAgentTestClient13,
+                        ::testing::Combine(TlsConnectTestBase::kTlsModesAll,
+                                           TlsConnectTestBase::kTlsV13));
 }  // namespace nss_test

gtests/ssl_gtest/ssl_hrr_unittest.cc

@@ -268,8 +268,6 @@ class HelloRetryRequestAgentTest : public TlsAgentTestClient {
   void SetUp() override {
     TlsAgentTestClient::SetUp();
     EnsureInit();
-    agent_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_3,
-                            SSL_LIBRARY_VERSION_TLS_1_3);
     agent_->StartConnect();
   }
 
@@ -354,7 +352,8 @@ TEST_P(HelloRetryRequestAgentTest, HandleHelloRetryRequestCookie) {
 }
 
 INSTANTIATE_TEST_CASE_P(HelloRetryRequestAgentTests, HelloRetryRequestAgentTest,
-                        TlsConnectTestBase::kTlsModesAll);
+                        ::testing::Combine(TlsConnectTestBase::kTlsModesAll,
+                                           TlsConnectTestBase::kTlsV13));
 #ifndef NSS_DISABLE_TLS_1_3
 INSTANTIATE_TEST_CASE_P(HelloRetryRequestKeyExchangeTests, TlsKeyExchange13,
                         ::testing::Combine(TlsConnectTestBase::kTlsModesAll,

gtests/ssl_gtest/tls_agent.cc

@@ -906,6 +906,9 @@ void TlsAgentTestBase::Reset(const std::string& server_name) {
   agent_.reset(
       new TlsAgent(role_ == TlsAgent::CLIENT ? TlsAgent::kClient : server_name,
                    role_, mode_));
+  if (version_) {
+    agent_->SetVersionRange(version_, version_);
+  }
   agent_->adapter()->SetPeer(sink_adapter_);
   agent_->StartConnect();
 }

gtests/ssl_gtest/tls_agent.h

@@ -379,10 +379,11 @@ class TlsAgentTestBase : public ::testing::Test {
  public:
   static ::testing::internal::ParamGenerator<std::string> kTlsRolesAll;
 
-  TlsAgentTestBase(TlsAgent::Role role, Mode mode)
+  TlsAgentTestBase(TlsAgent::Role role, Mode mode, uint16_t version = 0)
       : agent_(nullptr),
         role_(role),
         mode_(mode),
+        version_(version),
         sink_adapter_(new DummyPrSocket("sink", mode)) {}
   virtual ~TlsAgentTestBase() {}
 
@@ -421,26 +422,32 @@ class TlsAgentTestBase : public ::testing::Test {
   std::unique_ptr<TlsAgent> agent_;
   TlsAgent::Role role_;
   Mode mode_;
+  uint16_t version_;
   // This adapter is here just to accept packets from this agent.
   std::shared_ptr<DummyPrSocket> sink_adapter_;
 };
 
 class TlsAgentTest : public TlsAgentTestBase,
                      public ::testing::WithParamInterface<
-                         std::tuple<std::string, std::string>> {
+                         std::tuple<std::string, std::string, uint16_t>> {
  public:
   TlsAgentTest()
       : TlsAgentTestBase(ToRole(std::get<0>(GetParam())),
-                         ToMode(std::get<1>(GetParam()))) {}
+                         ToMode(std::get<1>(GetParam())),
+                         std::get<2>(GetParam())) {}
 };
 
-class TlsAgentTestClient : public TlsAgentTestBase,
-                           public ::testing::WithParamInterface<std::string> {
+class TlsAgentTestClient
+    : public TlsAgentTestBase,
+      public ::testing::WithParamInterface<std::tuple<std::string, uint16_t>> {
  public:
   TlsAgentTestClient()
-      : TlsAgentTestBase(TlsAgent::CLIENT, ToMode(GetParam())) {}
+      : TlsAgentTestBase(TlsAgent::CLIENT, ToMode(std::get<0>(GetParam())),
+                         std::get<1>(GetParam())) {}
 };
 
+class TlsAgentTestClient13 : public TlsAgentTestClient {};
+
 class TlsAgentStreamTestClient : public TlsAgentTestBase {
  public:
   TlsAgentStreamTestClient() : TlsAgentTestBase(TlsAgent::CLIENT, STREAM) {}

lib/ssl/ssl3con.c

@@ -9794,17 +9794,15 @@ ssl3_HandleCertificateVerify(sslSocket *ss, SSL3Opaque *b, PRUint32 length,
     PORT_Assert(ss->opt.noLocks || ssl_HaveRecvBufLock(ss));
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
 
-    /* TLS 1.3 is handled by tls13_HandleCertificateVerify */
-    PORT_Assert(ss->ssl3.prSpec->version <= SSL_LIBRARY_VERSION_TLS_1_2);
-
-    isTLS = (PRBool)(ss->ssl3.prSpec->version > SSL_LIBRARY_VERSION_3_0);
-
     if (ss->ssl3.hs.ws != wait_cert_verify) {
         desc = unexpected_message;
         errCode = SSL_ERROR_RX_UNEXPECTED_CERT_VERIFY;
         goto alert_loser;
     }
 
+    /* TLS 1.3 is handled by tls13_HandleCertificateVerify */
+    PORT_Assert(ss->ssl3.prSpec->version <= SSL_LIBRARY_VERSION_TLS_1_2);
+
     if (!hashes) {
         PORT_Assert(0);
         desc = internal_error;
@@ -9852,6 +9850,8 @@ ssl3_HandleCertificateVerify(sslSocket *ss, SSL3Opaque *b, PRUint32 length,
         goto loser; /* malformed. */
     }
 
+    isTLS = (PRBool)(ss->ssl3.prSpec->version > SSL_LIBRARY_VERSION_3_0);
+
     /* XXX verify that the key & kea match */
     rv = ssl3_VerifySignedHashes(ss, sigScheme, hashesForVerify, &signed_hash);
     if (rv != SECSuccess) {