Bug 1315865 - Basic KeyUpdate handling, r=ekr

Summary: KeyUpdate for TLS (not DTLS).  Experimental API for triggering one.

Reviewers: ekr

7 files changed, 161 insertions, 5 deletions

diff --git a/gtests/ssl_gtest/libssl_internals.c b/gtests/ssl_gtest/libssl_internals.c
index 1b2b71028..887d85278 100644
--- a/gtests/ssl_gtest/libssl_internals.c
+++ b/gtests/ssl_gtest/libssl_internals.c
@@ -362,3 +362,17 @@ SECStatus SSLInt_SetSocketMaxEarlyDataSize(PRFileDesc *fd, uint32_t size) {
 void SSLInt_RolloverAntiReplay(void) {
   tls13_AntiReplayRollover(ssl_TimeUsec());
 }
+
+SECStatus SSLInt_GetEpochs(PRFileDesc *fd, PRUint16 *readEpoch,
+                           PRUint16 *writeEpoch) {
+  sslSocket *ss = ssl_FindSocket(fd);
+  if (!ss || !readEpoch || !writeEpoch) {
+    return SECFailure;
+  }
+
+  ssl_GetSpecReadLock(ss);
+  *readEpoch = ss->ssl3.crSpec->epoch;
+  *writeEpoch = ss->ssl3.cwSpec->epoch;
+  ssl_ReleaseSpecReadLock(ss);
+  return SECSuccess;
+}
diff --git a/gtests/ssl_gtest/libssl_internals.h b/gtests/ssl_gtest/libssl_internals.h
index e7ea9ef34..95d4afdaf 100644
--- a/gtests/ssl_gtest/libssl_internals.h
+++ b/gtests/ssl_gtest/libssl_internals.h
@@ -39,6 +39,8 @@ SECStatus SSLInt_AdvanceWriteSeqNum(PRFileDesc *fd, PRUint64 to);
 SECStatus SSLInt_AdvanceReadSeqNum(PRFileDesc *fd, PRUint64 to);
 SECStatus SSLInt_AdvanceWriteSeqByAWindow(PRFileDesc *fd, PRInt32 extra);
 SSLKEAType SSLInt_GetKEAType(SSLNamedGroup group);
+SECStatus SSLInt_GetEpochs(PRFileDesc *fd, PRUint16 *readEpoch,
+                           PRUint16 *writeEpoch);
 
 SECStatus SSLInt_SetCipherSpecChangeFunc(PRFileDesc *fd,
                                          sslCipherSpecChangedFunc func,
diff --git a/gtests/ssl_gtest/manifest.mn b/gtests/ssl_gtest/manifest.mn
index 86d6ddd6f..5d893bab3 100644
--- a/gtests/ssl_gtest/manifest.mn
+++ b/gtests/ssl_gtest/manifest.mn
@@ -32,6 +32,7 @@ CPPSRCS = \
       ssl_gtest.cc \
       ssl_hrr_unittest.cc \
       ssl_keylog_unittest.cc \
+      ssl_keyupdate_unittest.cc \
       ssl_loopback_unittest.cc \
       ssl_misc_unittest.cc \
       ssl_record_unittest.cc \
diff --git a/gtests/ssl_gtest/ssl_gtest.gyp b/gtests/ssl_gtest/ssl_gtest.gyp
index 03cad2e89..e2a8d830a 100644
--- a/gtests/ssl_gtest/ssl_gtest.gyp
+++ b/gtests/ssl_gtest/ssl_gtest.gyp
@@ -33,6 +33,7 @@
         'ssl_gtest.cc',
         'ssl_hrr_unittest.cc',
         'ssl_keylog_unittest.cc',
+        'ssl_keyupdate_unittest.cc',
         'ssl_loopback_unittest.cc',
         'ssl_misc_unittest.cc',
         'ssl_record_unittest.cc',
diff --git a/gtests/ssl_gtest/ssl_keyupdate_unittest.cc b/gtests/ssl_gtest/ssl_keyupdate_unittest.cc
new file mode 100644
index 000000000..c2d03aa48
--- /dev/null
+++ b/gtests/ssl_gtest/ssl_keyupdate_unittest.cc
@@ -0,0 +1,119 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "secerr.h"
+#include "ssl.h"
+#include "sslerr.h"
+#include "sslproto.h"
+
+extern "C" {
+// This is not something that should make you happy.
+#include "libssl_internals.h"
+}
+
+#include "gtest_utils.h"
+#include "scoped_ptrs.h"
+#include "tls_connect.h"
+#include "tls_filter.h"
+#include "tls_parser.h"
+
+namespace nss_test {
+
+// All stream only tests; DTLS isn't supported yet.
+
+TEST_F(TlsConnectTest, KeyUpdateClient) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(client_->ssl_fd(), PR_FALSE));
+  SendReceive(50);
+  SendReceive(60);
+  CheckEpochs(4, 3);
+}
+
+TEST_F(TlsConnectTest, KeyUpdateClientRequestUpdate) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(client_->ssl_fd(), PR_TRUE));
+  // SendReceive() only gives each peer one chance to read.  This isn't enough
+  // when the read on one side generates another handshake message.  A second
+  // read gives each peer an extra chance to consume the KeyUpdate.
+  SendReceive(50);
+  SendReceive(60);  // Cumulative count.
+  CheckEpochs(4, 4);
+}
+
+TEST_F(TlsConnectTest, KeyUpdateServer) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_FALSE));
+  SendReceive(50);
+  SendReceive(60);
+  CheckEpochs(3, 4);
+}
+
+TEST_F(TlsConnectTest, KeyUpdateServerRequestUpdate) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  SendReceive(50);
+  SendReceive(60);
+  CheckEpochs(4, 4);
+}
+
+TEST_F(TlsConnectTest, KeyUpdateConsecutiveRequests) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  SendReceive(50);
+  SendReceive(60);
+  // The server should have updated twice, but the client should have declined
+  // to respond to the second request from the server, since it doesn't send
+  // anything in between those two requests.
+  CheckEpochs(4, 5);
+}
+
+// Check that a local update can be immediately followed by a remotely triggered
+// update even if there is no use of the keys.
+TEST_F(TlsConnectTest, KeyUpdateLocalUpdateThenConsecutiveRequests) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  // This should trigger an update on the client.
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(client_->ssl_fd(), PR_FALSE));
+  // The client should update for the first request.
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  // ...but not the second.
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  SendReceive(50);
+  SendReceive(60);
+  // Both should have updated twice.
+  CheckEpochs(5, 5);
+}
+
+TEST_F(TlsConnectTest, KeyUpdateMultiple) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_FALSE));
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_FALSE));
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(client_->ssl_fd(), PR_FALSE));
+  SendReceive(50);
+  SendReceive(60);
+  CheckEpochs(5, 6);
+}
+
+// Both ask the other for an update, and both should react.
+TEST_F(TlsConnectTest, KeyUpdateBothRequest) {
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(client_->ssl_fd(), PR_TRUE));
+  EXPECT_EQ(SECSuccess, SSL_KeyUpdate(server_->ssl_fd(), PR_TRUE));
+  SendReceive(50);
+  SendReceive(60);
+  CheckEpochs(5, 5);
+}
+
+}  // namespace nss_test
diff --git a/gtests/ssl_gtest/tls_connect.cc b/gtests/ssl_gtest/tls_connect.cc
index c23dc3bd1..2eca87aea 100644
--- a/gtests/ssl_gtest/tls_connect.cc
+++ b/gtests/ssl_gtest/tls_connect.cc
@@ -165,6 +165,22 @@ void TlsConnectTestBase::CheckShares(
   EXPECT_EQ(shares.len(), i);
 }
 
+void TlsConnectTestBase::CheckEpochs(uint16_t client_epoch,
+                                     uint16_t server_epoch) const {
+  uint16_t read_epoch = 0;
+  uint16_t write_epoch = 0;
+
+  EXPECT_EQ(SECSuccess,
+            SSLInt_GetEpochs(client_->ssl_fd(), &read_epoch, &write_epoch));
+  EXPECT_EQ(server_epoch, read_epoch);
+  EXPECT_EQ(client_epoch, write_epoch);
+
+  EXPECT_EQ(SECSuccess,
+            SSLInt_GetEpochs(server_->ssl_fd(), &read_epoch, &write_epoch));
+  EXPECT_EQ(client_epoch, read_epoch);
+  EXPECT_EQ(server_epoch, write_epoch);
+}
+
 void TlsConnectTestBase::ClearStats() {
   // Clear statistics.
   SSL3Statistics* stats = SSL_GetStatistics();
@@ -593,10 +609,12 @@ void TlsConnectTestBase::CheckSrtp() const {
   server_->CheckSrtp();
 }
 
-void TlsConnectTestBase::SendReceive() {
-  client_->SendData(50);
-  server_->SendData(50);
-  Receive(50);
+void TlsConnectTestBase::SendReceive(size_t total) {
+  ASSERT_GT(total, client_->received_bytes());
+  ASSERT_GT(total, server_->received_bytes());
+  client_->SendData(total - server_->received_bytes());
+  server_->SendData(total - client_->received_bytes());
+  Receive(total);  // Receive() is cumulative
 }
 
 // Do a first connection so we can do 0-RTT on the second one.
diff --git a/gtests/ssl_gtest/tls_connect.h b/gtests/ssl_gtest/tls_connect.h
index c1f23c2f2..c650dda1d 100644
--- a/gtests/ssl_gtest/tls_connect.h
+++ b/gtests/ssl_gtest/tls_connect.h
@@ -94,6 +94,7 @@ class TlsConnectTestBase : public ::testing::Test {
                    std::function<void(SSLNamedGroup)> check_group);
   void CheckShares(const DataBuffer& shares,
                    std::function<void(SSLNamedGroup)> check_group);
+  void CheckEpochs(uint16_t client_epoch, uint16_t server_epoch) const;
 
   void ConfigureVersion(uint16_t version);
   void SetExpectedVersion(uint16_t version);
@@ -114,7 +115,7 @@ class TlsConnectTestBase : public ::testing::Test {
   void CheckAlpn(const std::string& val);
   void EnableSrtp();
   void CheckSrtp() const;
-  void SendReceive();
+  void SendReceive(size_t total = 50);
   void SetupForZeroRtt();
   void SetupForResume();
   void ZeroRttSendReceive(