Bug 1295163 - Limit time-variation when accepting 0-RTT, r=ekr

5 files changed, 68 insertions, 10 deletions

diff --git a/gtests/ssl_gtest/libssl_internals.c b/gtests/ssl_gtest/libssl_internals.c
index 823d61cd1..1f2988db7 100644
--- a/gtests/ssl_gtest/libssl_internals.c
+++ b/gtests/ssl_gtest/libssl_internals.c
@@ -380,3 +380,14 @@ SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size) {
 
   return SECSuccess;
 }
+
+SECStatus SSLInt_SetTicketAgeTolerance(PRFileDesc *fd, PRUint16 tolerance) {
+  sslSocket *ss = ssl_FindSocket(fd);
+
+  if (!ss) {
+    return SECFailure;
+  }
+
+  ss->opt.ticketAgeTolerance = tolerance;
+  return SECSuccess;
+}
diff --git a/gtests/ssl_gtest/libssl_internals.h b/gtests/ssl_gtest/libssl_internals.h
index c1b484aa8..d0ba5d0a8 100644
--- a/gtests/ssl_gtest/libssl_internals.h
+++ b/gtests/ssl_gtest/libssl_internals.h
@@ -51,5 +51,6 @@ unsigned char *SSLInt_CipherSpecToIv(PRBool isServer, ssl3CipherSpec *spec);
 void SSLInt_SetTicketLifetime(uint32_t lifetime);
 void SSLInt_SetMaxEarlyDataSize(uint32_t size);
 SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size);
+SECStatus SSLInt_SetTicketAgeTolerance(PRFileDesc *fd, PRUint16 tolerance);
 
 #endif  // ndef libssl_internals_h_
diff --git a/gtests/ssl_gtest/ssl_0rtt_unittest.cc b/gtests/ssl_gtest/ssl_0rtt_unittest.cc
index 9b34665ce..9b66a7fa4 100644
--- a/gtests/ssl_gtest/ssl_0rtt_unittest.cc
+++ b/gtests/ssl_gtest/ssl_0rtt_unittest.cc
@@ -100,6 +100,59 @@ TEST_P(TlsConnectTls13, ZeroRttServerOnly) {
   CheckKeys();
 }
 
+// A small sleep after sending the ClientHello means that the ticket age that
+// arrives at the server is too low.  With a small tolerance for variation in
+// ticket age, the server then rejects early data.
+TEST_P(TlsConnectTls13, ZeroRttRejectOldTicket) {
+  SetupForZeroRtt();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+  SSLInt_SetTicketAgeTolerance(server_->ssl_fd(), 1);
+  ExpectResumption(RESUME_TICKET);
+  ZeroRttSendReceive(true, false, []() {
+    PR_Sleep(PR_MillisecondsToInterval(10));
+    return true;
+  });
+  Handshake();
+  ExpectEarlyDataAccepted(false);
+  CheckConnected();
+  SendReceive();
+}
+
+// In this test, we falsely inflate the estimate of the RTT by delaying the
+// ServerHello on the first handshake.  This results in the server estimating a
+// higher value of the ticket age than the client ultimately provides.  Add a
+// small tolerance for variation in ticket age and the ticket will appear to
+// arrive prematurely, causing the server to reject early data.
+TEST_P(TlsConnectTls13, ZeroRttRejectPrematureTicket) {
+  ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->Set0RttEnabled(true);
+  client_->StartConnect();
+  server_->StartConnect();
+  client_->Handshake();  // ClientHello
+  server_->Handshake();  // ServerHello
+  PR_Sleep(PR_MillisecondsToInterval(10));
+  Handshake();  // Remainder of handshake
+  CheckConnected();
+  SendReceive();
+  CheckKeys();
+
+  Reset();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+  SSLInt_SetTicketAgeTolerance(server_->ssl_fd(), 1);
+  ExpectResumption(RESUME_TICKET);
+  ExpectEarlyDataAccepted(false);
+
+  server_->StartConnect();
+  client_->StartConnect();
+  ZeroRttSendReceive(true, false);
+  Handshake();
+  CheckConnected();
+  SendReceive();
+}
+
 TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpn) {
   EnableAlpn();
   SetupForZeroRtt();
diff --git a/gtests/ssl_gtest/ssl_fuzz_unittest.cc b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
index 2a6e8e134..b9d8bdf89 100644
--- a/gtests/ssl_gtest/ssl_fuzz_unittest.cc
+++ b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
@@ -47,9 +47,9 @@ class TlsApplicationDataRecorder : public TlsRecordFilter {
 
 // Ensure that ssl_Time() returns a constant value.
 FUZZ_F(TlsFuzzTest, SSL_Time_Constant) {
-  PRUint32 now = ssl_Time();
+  PRUint32 now = ssl_TimeSec();
   PR_Sleep(PR_SecondsToInterval(2));
-  EXPECT_EQ(ssl_Time(), now);
+  EXPECT_EQ(ssl_TimeSec(), now);
 }
 
 // Check that due to the deterministic PRNG we derive
diff --git a/gtests/ssl_gtest/tls_connect.cc b/gtests/ssl_gtest/tls_connect.cc
index 890e8a5d6..403b0c290 100644
--- a/gtests/ssl_gtest/tls_connect.cc
+++ b/gtests/ssl_gtest/tls_connect.cc
@@ -552,20 +552,13 @@ void TlsConnectTestBase::SendReceive() {
 // Do a first connection so we can do 0-RTT on the second one.
 void TlsConnectTestBase::SetupForZeroRtt() {
   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
-  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
-                           SSL_LIBRARY_VERSION_TLS_1_3);
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
-                           SSL_LIBRARY_VERSION_TLS_1_3);
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
   server_->Set0RttEnabled(true);  // So we signal that we allow 0-RTT.
   Connect();
   SendReceive();  // Need to read so that we absorb the session ticket.
   CheckKeys();
 
   Reset();
-  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
-                           SSL_LIBRARY_VERSION_TLS_1_3);
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
-                           SSL_LIBRARY_VERSION_TLS_1_3);
   server_->StartConnect();
   client_->StartConnect();
 }