Bug 1340841 - Ticket lifetime duration is in seconds, r=ekr

--HG--
extra : source : 16f9b6c70772563079ee310ffbab91c5bcf16980
extra : amend_source : 57fc0d2054ab4061a670e05ca861d7c35bd56888

gtests/ssl_gtest/libssl_internals.c

@@ -367,3 +367,7 @@ SECStatus SSLInt_UsingShortHeaders(PRFileDesc *fd, PRBool *result) {
 
   return SECSuccess;
 }
+
+void SSLInt_SetTicketLifetime(uint32_t lifetime) {
+  ssl_ticket_lifetime = lifetime;
+}

gtests/ssl_gtest/libssl_internals.h

@@ -49,5 +49,6 @@ SSLCipherAlgorithm SSLInt_CipherSpecToAlgorithm(PRBool isServer,
 unsigned char *SSLInt_CipherSpecToIv(PRBool isServer, ssl3CipherSpec *spec);
 SECStatus SSLInt_EnableShortHeaders(PRFileDesc *fd);
 SECStatus SSLInt_UsingShortHeaders(PRFileDesc *fd, PRBool *result);
+void SSLInt_SetTicketLifetime(uint32_t lifetime);
 
 #endif  // ndef libssl_internals_h_

gtests/ssl_gtest/ssl_resumption_unittest.cc

@@ -201,6 +201,63 @@ TEST_P(TlsConnectGeneric, ConnectResumeClientBothTicketServerTicketForget) {
   SendReceive();
 }
 
+TEST_P(TlsConnectGeneric, ConnectWithExpiredTicketAtClient) {
+  SSLInt_SetTicketLifetime(1);  // one second
+  // This causes a ticket resumption.
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  Connect();
+  SendReceive();
+
+  WAIT_(false, 1000);
+
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  ExpectResumption(RESUME_NONE);
+
+  // TLS 1.3 uses the pre-shared key extension instead.
+  SSLExtensionType xtn = (version_ >= SSL_LIBRARY_VERSION_TLS_1_3)
+                             ? ssl_tls13_pre_shared_key_xtn
+                             : ssl_session_ticket_xtn;
+  auto capture = std::make_shared<TlsExtensionCapture>(xtn);
+  client_->SetPacketFilter(capture);
+  Connect();
+
+  if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    EXPECT_FALSE(capture->captured());
+  } else {
+    EXPECT_TRUE(capture->captured());
+    EXPECT_EQ(0U, capture->extension().len());
+  }
+}
+
+TEST_P(TlsConnectGeneric, ConnectWithExpiredTicketAtServer) {
+  SSLInt_SetTicketLifetime(1);  // one second
+  // This causes a ticket resumption.
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  Connect();
+  SendReceive();
+
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  ExpectResumption(RESUME_NONE);
+
+  SSLExtensionType xtn = (version_ >= SSL_LIBRARY_VERSION_TLS_1_3)
+                             ? ssl_tls13_pre_shared_key_xtn
+                             : ssl_session_ticket_xtn;
+  auto capture = std::make_shared<TlsExtensionCapture>(xtn);
+  client_->SetPacketFilter(capture);
+  client_->StartConnect();
+  server_->StartConnect();
+  client_->Handshake();
+  EXPECT_TRUE(capture->captured());
+  EXPECT_LT(0U, capture->extension().len());
+
+  WAIT_(false, 1000);  // Let the ticket expire on the server.
+
+  Handshake();
+  CheckConnected();
+}
+
 // This callback switches out the "server" cert used on the server with
 // the "client" certificate, which should be the same type.
 static int32_t SwitchCertificates(TlsAgent* agent, const SECItem* srvNameArr,

gtests/ssl_gtest/tls_connect.cc

@@ -174,6 +174,7 @@ void TlsConnectTestBase::ClearServerCache() {
 void TlsConnectTestBase::SetUp() {
   SSL_ConfigServerSessionIDCache(1024, 0, 0, g_working_dir_path.c_str());
   SSLInt_ClearSessionTicketKey();
+  SSLInt_SetTicketLifetime(10);
   ClearStats();
   Init();
 }

lib/ssl/ssl3con.c

@@ -10281,7 +10281,7 @@ ssl3_SendNewSessionTicket(sslSocket *ss)
         goto loser;
 
     /* This is a fixed value. */
-    rv = ssl3_AppendHandshakeNumber(ss, TLS_EX_SESS_TICKET_LIFETIME_HINT, 4);
+    rv = ssl3_AppendHandshakeNumber(ss, ssl_ticket_lifetime, 4);
     if (rv != SECSuccess)
         goto loser;
 

lib/ssl/ssl3exthandle.c

@@ -850,6 +850,9 @@ ssl3_ClientHandleStatusRequestXtn(const sslSocket *ss, TLSExtensionData *xtnData
     return SECSuccess;
 }
 
+PRUint32 ssl_ticket_lifetime = 2 * 24 * 60 * 60; /* 2 days in seconds */
+#define TLS_EX_SESS_TICKET_VERSION (0x0103)
+
 /*
  * Called from ssl3_SendNewSessionTicket, tls13_SendNewSessionTicket
  */
@@ -1567,8 +1570,7 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data)
      * memory since the ticket is of no use.
      */
     if (parsed_session_ticket->timestamp != 0 &&
-        parsed_session_ticket->timestamp +
-                TLS_EX_SESS_TICKET_LIFETIME_HINT >
+        parsed_session_ticket->timestamp + ssl_ticket_lifetime >
             ssl_Time()) {
 
         sid = ssl3_NewSessionID(ss, PR_TRUE);
@@ -1623,6 +1625,7 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data)
                 SECITEM_FreeItem(&sid->u.ssl3.srvName, PR_FALSE);
             }
             sid->u.ssl3.srvName = parsed_session_ticket->srvName;
+            parsed_session_ticket->srvName.data = NULL;
         }
         if (parsed_session_ticket->alpnSelection.data != NULL) {
             sid->u.ssl3.alpnSelection = parsed_session_ticket->alpnSelection;
@@ -1662,6 +1665,9 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, SECItem *data)
         if (parsed_session_ticket->alpnSelection.data) {
             SECITEM_FreeItem(&parsed_session_ticket->alpnSelection, PR_FALSE);
         }
+        if (parsed_session_ticket->srvName.data) {
+            SECITEM_FreeItem(&parsed_session_ticket->srvName, PR_FALSE);
+        }
         PORT_ZFree(parsed_session_ticket, sizeof(SessionTicket));
     }
 

lib/ssl/sslimpl.h

@@ -1221,16 +1221,12 @@ struct sslSocketStr {
     SSLProtocolVariant protocolVariant;
 };
 
-/* All the global data items declared here should be protected using the
-** ssl_global_data_lock, which is a reader/writer lock.
-*/
-extern NSSRWLock *ssl_global_data_lock;
 extern char ssl_debug;
 extern char ssl_trace;
 extern FILE *ssl_trace_iob;
 extern FILE *ssl_keylog_iob;
-extern PRUint32 ssl_sid_timeout;
 extern PRUint32 ssl3_sid_timeout;
+extern PRUint32 ssl_ticket_lifetime;
 
 extern const char *const ssl3_cipherName[];
 
@@ -1698,10 +1694,6 @@ SECStatus ssl_GetSessionTicketKeys(sslSocket *ss, unsigned char *keyName,
                                    PK11SymKey **encKey, PK11SymKey **macKey);
 void ssl_ResetSessionTicketKeys();
 
-/* Tell clients to consider tickets valid for this long. */
-#define TLS_EX_SESS_TICKET_LIFETIME_HINT (2 * 24 * 60 * 60) /* 2 days */
-#define TLS_EX_SESS_TICKET_VERSION (0x0103)
-
 extern SECStatus ssl3_ValidateNextProtoNego(const unsigned char *data,
                                             unsigned int length);
 

lib/ssl/sslnonce.c

@@ -19,7 +19,6 @@
 #include <time.h>
 #endif
 
-PRUint32 ssl_sid_timeout = 100;
 PRUint32 ssl3_sid_timeout = 86400L; /* 24 hours */
 
 static sslSessionID *cache = NULL;
@@ -471,7 +470,7 @@ ssl_TicketTimeValid(const NewSessionTicket *ticket)
     }
 
     endTime = ticket->received_timestamp +
-              (PRTime)(ticket->ticket_lifetime_hint * PR_USEC_PER_MSEC);
+              (PRTime)(ticket->ticket_lifetime_hint * PR_USEC_PER_SEC);
     return endTime > PR_Now();
 }
 

lib/ssl/tls13con.c

@@ -3808,7 +3808,7 @@ tls13_SendNewSessionTicket(sslSocket *ss)
         ticket.flags |= ticket_allow_early_data;
         max_early_data_size_len = 8; /* type + len + value. */
     }
-    ticket.ticket_lifetime_hint = TLS_EX_SESS_TICKET_LIFETIME_HINT;
+    ticket.ticket_lifetime_hint = ssl_ticket_lifetime;
 
     rv = ssl3_EncodeSessionTicket(ss, &ticket, &ticket_data);
     if (rv != SECSuccess)
@@ -3827,7 +3827,7 @@ tls13_SendNewSessionTicket(sslSocket *ss)
         goto loser;
 
     /* This is a fixed value. */
-    rv = ssl3_AppendHandshakeNumber(ss, TLS_EX_SESS_TICKET_LIFETIME_HINT, 4);
+    rv = ssl3_AppendHandshakeNumber(ss, ssl_ticket_lifetime, 4);
     if (rv != SECSuccess)
         goto loser;