Bug 1396525 - put keaGroup and sigScheme in the cache so we can put i…

…t in SSLChannelInfo, r=mt

Summary: This adds originalKeaGroup and resumed fields to the SSLChannelInfo, which provide information about the key exchange group of the original TLS handshake when the session is resumed, and information whether a session is resumed or not.
To do this this patch adds the keaGroup and sigScheme to the session cache and the session ticket.

Reviewers: mt

Reviewed By: mt

Bug #: 1396525

Differential Revision: https://phabricator.services.mozilla.com/D29

--HG--
extra : amend_source : 4c1d4898d9c53ee0668c9d1fb2d2ae05fdb85384

gtests/ssl_gtest/ssl_dhe_unittest.cc

@@ -536,7 +536,8 @@ TEST_P(TlsConnectTls13, ResumeFfdhe) {
   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
   Connect();
   SendReceive();  // Need to read so that we absorb the session ticket.
-  CheckKeys(ssl_kea_dh, ssl_auth_rsa_sign);
+  CheckKeys(ssl_kea_dh, ssl_grp_ffdhe_2048, ssl_auth_rsa_sign,
+            ssl_sig_rsa_pss_sha256);
 
   Reset();
   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
@@ -549,7 +550,8 @@ TEST_P(TlsConnectTls13, ResumeFfdhe) {
   server_->SetPacketFilter(serverCapture);
   ExpectResumption(RESUME_TICKET);
   Connect();
-  CheckKeys(ssl_kea_dh, ssl_grp_ffdhe_2048, ssl_auth_rsa_sign, ssl_sig_none);
+  CheckKeys(ssl_kea_dh, ssl_grp_ffdhe_2048, ssl_auth_rsa_sign,
+            ssl_sig_rsa_pss_sha256);
   ASSERT_LT(0UL, clientCapture->extension().len());
   ASSERT_LT(0UL, serverCapture->extension().len());
 }

gtests/ssl_gtest/ssl_resumption_unittest.cc

@@ -396,7 +396,8 @@ TEST_P(TlsConnectTls13, TestTls13ResumeDifferentGroup) {
   client_->ConfigNamedGroups(kFFDHEGroups);
   server_->ConfigNamedGroups(kFFDHEGroups);
   Connect();
-  CheckKeys(ssl_kea_dh, ssl_grp_ffdhe_2048, ssl_auth_rsa_sign, ssl_sig_none);
+  CheckKeys(ssl_kea_dh, ssl_grp_ffdhe_2048, ssl_auth_rsa_sign,
+            ssl_sig_rsa_pss_sha256);
 }
 
 // We need to enable different cipher suites at different times in the following
@@ -604,7 +605,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionTwice) {
   Connect();
   SendReceive();
   CheckKeys(ssl_kea_ecdh, ssl_grp_ec_curve25519, ssl_auth_rsa_sign,
-            ssl_sig_none);
+            ssl_sig_rsa_pss_sha256);
   // The filter will go away when we reset, so save the captured extension.
   DataBuffer initialTicket(c1->extension());
   ASSERT_LT(0U, initialTicket.len());
@@ -622,7 +623,7 @@ TEST_F(TlsConnectTest, TestTls13ResumptionTwice) {
   Connect();
   SendReceive();
   CheckKeys(ssl_kea_ecdh, ssl_grp_ec_curve25519, ssl_auth_rsa_sign,
-            ssl_sig_none);
+            ssl_sig_rsa_pss_sha256);
   ASSERT_LT(0U, c2->extension().len());
 
   ScopedCERTCertificate cert2(SSL_PeerCertificate(client_->ssl_fd()));
@@ -723,4 +724,61 @@ TEST_F(TlsConnectTest, TestTls13ResumptionForcedDowngrade) {
   server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
 }
 
+TEST_P(TlsConnectGeneric, ReConnectTicket) {
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  server_->EnableSingleCipher(ChooseOneCipher(version_));
+  Connect();
+  SendReceive();
+  CheckKeys(ssl_kea_ecdh, ssl_grp_ec_curve25519, ssl_auth_rsa_sign,
+            ssl_sig_rsa_pss_sha256);
+  // Resume
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  ExpectResumption(RESUME_TICKET);
+  Connect();
+  // Only the client knows this.
+  CheckKeysResumption(ssl_kea_ecdh, ssl_grp_none, ssl_grp_ec_curve25519,
+                      ssl_auth_rsa_sign, ssl_sig_rsa_pss_sha256);
+}
+
+TEST_P(TlsConnectGenericPre13, ReConnectCache) {
+  ConfigureSessionCache(RESUME_SESSIONID, RESUME_SESSIONID);
+  server_->EnableSingleCipher(ChooseOneCipher(version_));
+  Connect();
+  SendReceive();
+  CheckKeys(ssl_kea_ecdh, ssl_grp_ec_curve25519, ssl_auth_rsa_sign,
+            ssl_sig_rsa_pss_sha256);
+  // Resume
+  Reset();
+  ExpectResumption(RESUME_SESSIONID);
+  Connect();
+  CheckKeysResumption(ssl_kea_ecdh, ssl_grp_none, ssl_grp_ec_curve25519,
+                      ssl_auth_rsa_sign, ssl_sig_rsa_pss_sha256);
+}
+
+TEST_P(TlsConnectGeneric, ReConnectAgainTicket) {
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  server_->EnableSingleCipher(ChooseOneCipher(version_));
+  Connect();
+  SendReceive();
+  CheckKeys(ssl_kea_ecdh, ssl_grp_ec_curve25519, ssl_auth_rsa_sign,
+            ssl_sig_rsa_pss_sha256);
+  // Resume
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  ExpectResumption(RESUME_TICKET);
+  Connect();
+  // Only the client knows this.
+  CheckKeysResumption(ssl_kea_ecdh, ssl_grp_none, ssl_grp_ec_curve25519,
+                      ssl_auth_rsa_sign, ssl_sig_rsa_pss_sha256);
+  // Resume connection again
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  ExpectResumption(RESUME_TICKET, 2);
+  Connect();
+  // Only the client knows this.
+  CheckKeysResumption(ssl_kea_ecdh, ssl_grp_none, ssl_grp_ec_curve25519,
+                      ssl_auth_rsa_sign, ssl_sig_rsa_pss_sha256);
+}
+
 }  // namespace nss_test

gtests/ssl_gtest/tls_agent.cc

@@ -495,6 +495,12 @@ void TlsAgent::CheckKEA(SSLKEAType kea_type, SSLNamedGroup kea_group,
   }
 }
 
+void TlsAgent::CheckOriginalKEA(SSLNamedGroup kea_group) const {
+  if (kea_group != ssl_grp_ffdhe_custom) {
+    EXPECT_EQ(kea_group, info_.originalKeaGroup);
+  }
+}
+
 void TlsAgent::CheckAuthType(SSLAuthType auth_type,
                              SSLSignatureScheme sig_scheme) const {
   EXPECT_EQ(STATE_CONNECTED, state_);
@@ -720,6 +726,8 @@ void TlsAgent::Connected() {
   EXPECT_EQ(SECSuccess, rv);
   EXPECT_EQ(sizeof(info_), info_.length);
 
+  EXPECT_EQ(expect_resumption_, info_.resumed == PR_TRUE);
+
   // Preliminary values are exposed through callbacks during the handshake.
   // If either expected values were set or the callbacks were called, check
   // that the final values are correct.

gtests/ssl_gtest/tls_agent.h

@@ -95,6 +95,7 @@ class TlsAgent : public PollTarget {
   void StartConnect(PRFileDesc* model = nullptr);
   void CheckKEA(SSLKEAType kea_type, SSLNamedGroup group,
                 size_t kea_size = 0) const;
+  void CheckOriginalKEA(SSLNamedGroup kea_group) const;
   void CheckAuthType(SSLAuthType auth_type,
                      SSLSignatureScheme sig_scheme) const;
 

gtests/ssl_gtest/tls_connect.cc

@@ -113,6 +113,7 @@ TlsConnectTestBase::TlsConnectTestBase(SSLProtocolVariant variant,
       server_model_(nullptr),
       version_(version),
       expected_resumption_mode_(RESUME_NONE),
+      expected_resumptions_(0),
       session_ids_(),
       expect_extended_master_secret_(false),
       expect_early_data_accepted_(false),
@@ -220,12 +221,15 @@ void TlsConnectTestBase::Reset(const std::string& server_name,
   Init();
 }
 
-void TlsConnectTestBase::ExpectResumption(SessionResumptionMode expected) {
+void TlsConnectTestBase::ExpectResumption(SessionResumptionMode expected,
+                                          uint8_t num_resumptions) {
   expected_resumption_mode_ = expected;
   if (expected != RESUME_NONE) {
     client_->ExpectResumption();
     server_->ExpectResumption();
+    expected_resumptions_ = num_resumptions;
   }
+  EXPECT_EQ(expected_resumptions_ == 0, expected == RESUME_NONE);
 }
 
 void TlsConnectTestBase::EnsureTlsSetup() {
@@ -315,10 +319,12 @@ void TlsConnectTestBase::CheckConnected() {
 void TlsConnectTestBase::CheckKeys(SSLKEAType kea_type, SSLNamedGroup kea_group,
                                    SSLAuthType auth_type,
                                    SSLSignatureScheme sig_scheme) const {
-  client_->CheckKEA(kea_type, kea_group);
-  server_->CheckKEA(kea_type, kea_group);
-  client_->CheckAuthType(auth_type, sig_scheme);
+  if (kea_group != ssl_grp_none) {
+    client_->CheckKEA(kea_type, kea_group);
+    server_->CheckKEA(kea_type, kea_group);
+  }
   server_->CheckAuthType(auth_type, sig_scheme);
+  client_->CheckAuthType(auth_type, sig_scheme);
 }
 
 void TlsConnectTestBase::CheckKeys(SSLKEAType kea_type,
@@ -373,6 +379,17 @@ void TlsConnectTestBase::CheckKeys() const {
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign);
 }
 
+void TlsConnectTestBase::CheckKeysResumption(SSLKEAType kea_type,
+                                             SSLNamedGroup kea_group,
+                                             SSLNamedGroup original_kea_group,
+                                             SSLAuthType auth_type,
+                                             SSLSignatureScheme sig_scheme) {
+  CheckKeys(kea_type, kea_group, auth_type, sig_scheme);
+  EXPECT_TRUE(expected_resumption_mode_ != RESUME_NONE);
+  client_->CheckOriginalKEA(original_kea_group);
+  server_->CheckOriginalKEA(original_kea_group);
+}
+
 void TlsConnectTestBase::ConnectExpectFail() {
   server_->StartConnect();
   client_->StartConnect();
@@ -477,8 +494,8 @@ void TlsConnectTestBase::ConfigureSessionCache(SessionResumptionMode client,
 void TlsConnectTestBase::CheckResumption(SessionResumptionMode expected) {
   EXPECT_NE(RESUME_BOTH, expected);
 
-  int resume_count = expected ? 1 : 0;
-  int stateless_count = (expected & RESUME_TICKET) ? 1 : 0;
+  int resume_count = expected ? expected_resumptions_ : 0;
+  int stateless_count = (expected & RESUME_TICKET) ? expected_resumptions_ : 0;
 
   // Note: hch == server counter; hsh == client counter.
   SSL3Statistics* stats = SSL_GetStatistics();
@@ -491,7 +508,7 @@ void TlsConnectTestBase::CheckResumption(SessionResumptionMode expected) {
   if (expected != RESUME_NONE) {
     if (client_->version() < SSL_LIBRARY_VERSION_TLS_1_3) {
       // Check that the last two session ids match.
-      ASSERT_EQ(2U, session_ids_.size());
+      ASSERT_EQ(1U + expected_resumptions_, session_ids_.size());
       EXPECT_EQ(session_ids_[session_ids_.size() - 1],
                 session_ids_[session_ids_.size() - 2]);
     } else {

gtests/ssl_gtest/tls_connect.h

@@ -81,6 +81,11 @@ class TlsConnectTestBase : public ::testing::Test {
   void CheckKeys(SSLKEAType kea_type, SSLAuthType auth_type) const;
   // This version assumes defaults.
   void CheckKeys() const;
+  // Check that keys on resumed sessions.
+  void CheckKeysResumption(SSLKEAType kea_type, SSLNamedGroup kea_group,
+                           SSLNamedGroup original_kea_group,
+                           SSLAuthType auth_type,
+                           SSLSignatureScheme sig_scheme);
   void CheckGroups(const DataBuffer& groups,
                    std::function<void(SSLNamedGroup)> check_group);
   void CheckShares(const DataBuffer& shares,
@@ -89,7 +94,8 @@ class TlsConnectTestBase : public ::testing::Test {
   void ConfigureVersion(uint16_t version);
   void SetExpectedVersion(uint16_t version);
   // Expect resumption of a particular type.
-  void ExpectResumption(SessionResumptionMode expected);
+  void ExpectResumption(SessionResumptionMode expected,
+                        uint8_t num_resumed = 1);
   void DisableAllCiphers();
   void EnableOnlyStaticRsaCiphers();
   void EnableOnlyDheCiphers();
@@ -123,6 +129,7 @@ class TlsConnectTestBase : public ::testing::Test {
   std::unique_ptr<TlsAgent> server_model_;
   uint16_t version_;
   SessionResumptionMode expected_resumption_mode_;
+  uint8_t expected_resumptions_;
   std::vector<std::vector<uint8_t>> session_ids_;
 
   // A simple value of "a", "b".  Note that the preferred value of "a" is placed

lib/ssl/ssl3con.c

@@ -6909,6 +6909,8 @@ ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes,
             ss->sec.authKeyBits = sid->authKeyBits;
             ss->sec.keaType = sid->keaType;
             ss->sec.keaKeyBits = sid->keaKeyBits;
+            ss->sec.originalKeaGroup = ssl_LookupNamedGroup(sid->keaGroup);
+            ss->sec.signatureScheme = sid->sigScheme;
 
             if (sid->u.ssl3.keys.msIsWrapped) {
                 PK11SlotInfo *slot;
@@ -7919,6 +7921,7 @@ ssl3_NewSessionID(sslSocket *ss, PRBool is_server)
     sid->references = 1;
     sid->cached = never_cached;
     sid->version = ss->version;
+    sid->sigScheme = ssl_sig_none;
 
     sid->u.ssl3.keys.resumable = PR_TRUE;
     sid->u.ssl3.policy = SSL_ALLOWED;
@@ -8892,6 +8895,8 @@ ssl3_HandleClientHelloPart2(sslSocket *ss,
             ss->sec.authKeyBits = sid->authKeyBits;
             ss->sec.keaType = sid->keaType;
             ss->sec.keaKeyBits = sid->keaKeyBits;
+            ss->sec.originalKeaGroup = ssl_LookupNamedGroup(sid->keaGroup);
+            ss->sec.signatureScheme = sid->sigScheme;
 
             ss->sec.localCert =
                 CERT_DupCertificate(ss->sec.serverCert->serverCert);
@@ -8963,6 +8968,7 @@ ssl3_HandleClientHelloPart2(sslSocket *ss,
     }
 
     if (sid) { /* we had a sid, but it's no longer valid, free it */
+        ss->statelessResume = PR_FALSE;
         SSL_AtomicIncrementLong(&ssl3stats.hch_sid_cache_not_ok);
         ss->sec.uncache(sid);
         ssl_FreeSID(sid);
@@ -11536,6 +11542,12 @@ ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid)
     sid->authKeyBits = ss->sec.authKeyBits;
     sid->keaType = ss->sec.keaType;
     sid->keaKeyBits = ss->sec.keaKeyBits;
+    if (ss->sec.keaGroup) {
+        sid->keaGroup = ss->sec.keaGroup->name;
+    } else {
+        sid->keaGroup = ssl_grp_none;
+    }
+    sid->sigScheme = ss->sec.signatureScheme;
     sid->lastAccessTime = sid->creationTime = ssl_Time();
     sid->expirationTime = sid->creationTime + ssl3_sid_timeout;
     sid->localCert = CERT_DupCertificate(ss->sec.localCert);

lib/ssl/ssl3exthandle.c

@@ -802,7 +802,7 @@ ssl3_ClientHandleStatusRequestXtn(const sslSocket *ss, TLSExtensionData *xtnData
 }
 
 PRUint32 ssl_ticket_lifetime = 2 * 24 * 60 * 60; /* 2 days in seconds */
-#define TLS_EX_SESS_TICKET_VERSION (0x0105)
+#define TLS_EX_SESS_TICKET_VERSION (0x0106)
 
 /*
  * Called from ssl3_SendNewSessionTicket, tls13_SendNewSessionTicket
@@ -882,6 +882,7 @@ ssl3_EncodeSessionTicket(sslSocket *ss,
         + sizeof(ssl3CipherSuite)              /* ciphersuite */
         + 1                                    /* compression */
         + 10                                   /* cipher spec parameters */
+        + 8                                    /* kea group and sig scheme */
         + 1                                    /* certType arguments */
         + 1                                    /* SessionTicket.ms_is_wrapped */
         + 4                                    /* msWrapMech */
@@ -935,6 +936,19 @@ ssl3_EncodeSessionTicket(sslSocket *ss,
     if (rv != SECSuccess)
         goto loser;
     rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.keaKeyBits, 4);
+    if (rv != SECSuccess)
+        goto loser;
+    if (ss->sec.keaGroup) {
+        rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.keaGroup->name, 4);
+        if (rv != SECSuccess)
+            goto loser;
+    } else {
+        /* No kea group. Write 0 as invalid value. */
+        rv = ssl3_AppendNumberToItem(&plaintext, 0, 4);
+        if (rv != SECSuccess)
+            goto loser;
+    }
+    rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.signatureScheme, 4);
     if (rv != SECSuccess)
         goto loser;
 
@@ -1165,6 +1179,18 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket,
         return SECFailure;
     }
     parsedTicket->keaKeyBits = temp;
+    rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len);
+    if (rv != SECSuccess) {
+        PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
+        return SECFailure;
+    }
+    parsedTicket->originalKeaGroup = temp;
+    rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len);
+    if (rv != SECSuccess) {
+        PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
+        return SECFailure;
+    }
+    parsedTicket->signatureScheme = (SSLSignatureScheme)temp;
 
     /* Read the optional named curve. */
     rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 1, &buffer, &len);
@@ -1319,7 +1345,9 @@ ssl_CreateSIDFromTicket(sslSocket *ss, const SECItem *rawTicket,
     sid->authKeyBits = parsedTicket->authKeyBits;
     sid->keaType = parsedTicket->keaType;
     sid->keaKeyBits = parsedTicket->keaKeyBits;
+    sid->keaGroup = parsedTicket->originalKeaGroup;
     sid->namedCurve = parsedTicket->namedCurve;
+    sid->sigScheme = parsedTicket->signatureScheme;
 
     rv = SECITEM_CopyItem(NULL, &sid->u.ssl3.locked.sessionTicket.ticket,
                           rawTicket);

lib/ssl/sslimpl.h

@@ -553,6 +553,8 @@ struct sslSessionIDStr {
     PRUint32 authKeyBits;
     SSLKEAType keaType;
     PRUint32 keaKeyBits;
+    SSLNamedGroup keaGroup;
+    SSLSignatureScheme sigScheme;
 
     union {
         struct {
@@ -1002,6 +1004,8 @@ typedef struct SessionTicketStr {
     PRUint32 authKeyBits;
     SSLKEAType keaType;
     PRUint32 keaKeyBits;
+    SSLNamedGroup originalKeaGroup;
+    SSLSignatureScheme signatureScheme;
     const sslNamedGroupDef *namedCurve; /* For certificate lookup. */
 
     /*
@@ -1068,6 +1072,7 @@ struct sslSecurityInfoStr {
     SSLKEAType keaType;
     PRUint32 keaKeyBits;
     const sslNamedGroupDef *keaGroup;
+    const sslNamedGroupDef *originalKeaGroup;
     /* The selected certificate (for servers only). */
     const sslServerCert *serverCert;