Bug 1261677 - Carry over certificate and cipher suite on resumption, r=ekr

4 files changed, 25 insertions, 3 deletions

diff --git a/external_tests/ssl_gtest/ssl_loopback_unittest.cc b/external_tests/ssl_gtest/ssl_loopback_unittest.cc
index 86ae2a8c6..60a7f81eb 100644
--- a/external_tests/ssl_gtest/ssl_loopback_unittest.cc
+++ b/external_tests/ssl_gtest/ssl_loopback_unittest.cc
@@ -956,6 +956,8 @@ TEST_F(TlsConnectTest, TestTls13ResumptionTwice) {
   Connect();
   SendReceive(); // Need to read so that we absorb the session ticket.
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa);
+  uint16_t original_suite;
+  EXPECT_TRUE(client_->cipher_suite(&original_suite));
 
   ResetRsa();
   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
@@ -971,6 +973,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionTwice) {
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa);
   DataBuffer psk1(c1->extension());
   ASSERT_GE(psk1.len(), 0UL);
+  ASSERT_TRUE(!!client_->peer_cert());
 
   ResetRsa();
   ClearStats();
@@ -987,6 +990,15 @@ TEST_F(TlsConnectTest, TestTls13ResumptionTwice) {
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa);
   DataBuffer psk2(c2->extension());
   ASSERT_GE(psk2.len(), 0UL);
+  ASSERT_TRUE(!!client_->peer_cert());
+
+  // Check that the cipher suite is reported the same on both sides, though in
+  // TLS 1.3 resumption actually negotiates a different cipher suite.
+  uint16_t resumed_suite;
+  EXPECT_TRUE(server_->cipher_suite(&resumed_suite));
+  EXPECT_EQ(original_suite, resumed_suite);
+  EXPECT_TRUE(client_->cipher_suite(&resumed_suite));
+  EXPECT_EQ(original_suite, resumed_suite);
 
   // TODO(ekr@rtfm.com): This will change when we fix bug 1257047.
   ASSERT_EQ(psk1, psk2);
diff --git a/external_tests/ssl_gtest/tls_agent.h b/external_tests/ssl_gtest/tls_agent.h
index 1c824d6e6..2dbe8a334 100644
--- a/external_tests/ssl_gtest/tls_agent.h
+++ b/external_tests/ssl_gtest/tls_agent.h
@@ -120,6 +120,10 @@ class TlsAgent : public PollTarget {
 
   SSLKEAType kea() const { return kea_; }
 
+  const CERTCertificate* peer_cert() const {
+    return SSL_PeerCertificate(ssl_fd_);
+  }
+
   const char* state_str() const { return state_str(state()); }
 
   const char* state_str(State state) const { return states[state]; }
diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h
index d456eba48..f97196238 100644
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -873,7 +873,7 @@ typedef struct SSL3HandshakeStateStr {
     /* message for message type and header length */
     SSL3HandshakeType msg_type;
     unsigned long msg_len;
-    PRBool isResuming;      /* are we resuming a session */
+    PRBool isResuming; /* we are resuming (not used in TLS 1.3) */
     PRBool usedStepDownKey; /* we did a server key exchange. */
     PRBool sendingSCSV;     /* instead of empty RI */
     sslBuffer msgState;     /* current state for handshake messages*/
diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c
index af6efa6a5..3260a061c 100644
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -527,7 +527,6 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
 
         SSL_AtomicIncrementLong(& ssl3stats->hch_sid_cache_hits);
         SSL_AtomicIncrementLong(& ssl3stats->hch_sid_stateless_resumes);
-        ss->ssl3.hs.isResuming = PR_TRUE;
 
         tls13_RestoreCipherInfo(ss, sid);
 
@@ -596,7 +595,6 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
             goto loser;
         }
         ss->sec.ci.sid = sid;
-        ss->ssl3.hs.isResuming = PR_FALSE;
     }
 
     ssl_GetXmitBufLock(ss);
@@ -1007,6 +1005,9 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
         }
 
         tls13_RestoreCipherInfo(ss, sid);
+        if (sid->peerCert) {
+            ss->sec.peerCert = CERT_DupCertificate(sid->peerCert);
+        }
 
         SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_cache_hits);
         SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_stateless_resumes);
@@ -1051,7 +1052,12 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
         FATAL_ERROR(ss, PORT_GetError(), internal_error);
         return SECFailure;
     }
+    if (isPSK && ss->sec.peerCert) {
+        sid->peerCert = CERT_DupCertificate(ss->sec.peerCert);
+    }
     sid->version = ss->version;
+    sid->u.ssl3.cipherSuite = ss->ssl3.hs.origCipherSuite;
+
     rv = tls13_HandleServerKeyShare(ss);
     if (rv != SECSuccess) {
         return SECFailure;