From afa867be8f6dce3e416b8e5f54013cbf970b5609 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Wed, 16 Jun 2021 14:22:43 -0400
Subject: [PATCH] runner: Test that clients actually use renewed tickets.

Some tests run three connections, resuming a renewed ticket.
Particularly the way TLS 1.2 ticket renewal works, the client logic
could accidentally report the old session up to the application. Our
runner tests would not currently notice (though one of the tests in
ssl_tests does).

Make runner tests also check this by cycling ticket keys between
connection attempts. This also makes newSessionsOnResume apply even if
the test did not specify a resumeConfig.

Change-Id: I95375c01adf6ad62de65ecf8aed3c286a0571875
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48131
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
---
 ssl/test/runner/common.go |  4 ++++
 ssl/test/runner/runner.go | 26 +++++++++++++++++++++-----
 ssl/test/runner/ticket.go |  9 +++++++--
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 645b96402..767f8da29 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -1867,6 +1867,10 @@ type ProtocolBugs struct {
 	// CompatModeWithQUIC, if true, enables TLS 1.3 compatibility mode
 	// when running over QUIC.
 	CompatModeWithQUIC bool
+
+	// EncryptSessionTicketKey, if non-nil, is the ticket key to use when
+	// encrypting tickets.
+	EncryptSessionTicketKey *[32]byte
 }
 
 func (c *Config) serverInit() {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index f9678506c..e07621de2 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1360,19 +1360,35 @@ func doExchanges(test *testCase, shim *shimProcess, resumeCount int, transcripts
 		return err
 	}
 
+	nextTicketKey := config.SessionTicketKey
 	for i := 0; i < resumeCount; i++ {
 		var resumeConfig Config
 		if test.resumeConfig != nil {
 			resumeConfig = *test.resumeConfig
-			if !test.newSessionsOnResume {
-				resumeConfig.SessionTicketKey = config.SessionTicketKey
-				resumeConfig.ClientSessionCache = config.ClientSessionCache
-				resumeConfig.ServerSessionCache = config.ServerSessionCache
-			}
 			resumeConfig.Rand = config.Rand
 		} else {
 			resumeConfig = config
 		}
+
+		if test.newSessionsOnResume {
+			resumeConfig.ClientSessionCache = nil
+			resumeConfig.ServerSessionCache = nil
+			if _, err := resumeConfig.rand().Read(resumeConfig.SessionTicketKey[:]); err != nil {
+				return err
+			}
+		} else {
+			resumeConfig.ClientSessionCache = config.ClientSessionCache
+			resumeConfig.ServerSessionCache = config.ServerSessionCache
+			// Rotate the ticket keys between each connection, with each connection
+			// encrypting with next connection's keys. This ensures that we test
+			// the renewed sessions.
+			resumeConfig.SessionTicketKey = nextTicketKey
+			if _, err := resumeConfig.rand().Read(nextTicketKey[:]); err != nil {
+				return err
+			}
+			resumeConfig.Bugs.EncryptSessionTicketKey = &nextTicketKey
+		}
+
 		var connResume net.Conn
 		connResume, err = shim.accept()
 		if err != nil {
diff --git a/ssl/test/runner/ticket.go b/ssl/test/runner/ticket.go
index 347edb5db..46a6b3579 100644
--- a/ssl/test/runner/ticket.go
+++ b/ssl/test/runner/ticket.go
@@ -145,6 +145,11 @@ func (s *sessionState) unmarshal(data []byte) bool {
 }
 
 func (c *Conn) encryptTicket(state *sessionState) ([]byte, error) {
+	key := c.config.SessionTicketKey[:]
+	if c.config.Bugs.EncryptSessionTicketKey != nil {
+		key = c.config.Bugs.EncryptSessionTicketKey[:]
+	}
+
 	serialized := state.marshal()
 	encrypted := make([]byte, aes.BlockSize+len(serialized)+sha256.Size)
 	iv := encrypted[:aes.BlockSize]
@@ -153,13 +158,13 @@ func (c *Conn) encryptTicket(state *sessionState) ([]byte, error) {
 	if _, err := io.ReadFull(c.config.rand(), iv); err != nil {
 		return nil, err
 	}
-	block, err := aes.NewCipher(c.config.SessionTicketKey[:16])
+	block, err := aes.NewCipher(key[:16])
 	if err != nil {
 		return nil, errors.New("tls: failed to create cipher while encrypting ticket: " + err.Error())
 	}
 	cipher.NewCTR(block, iv).XORKeyStream(encrypted[aes.BlockSize:], serialized)
 
-	mac := hmac.New(sha256.New, c.config.SessionTicketKey[16:32])
+	mac := hmac.New(sha256.New, key[16:32])
 	mac.Write(encrypted[:len(encrypted)-sha256.Size])
 	mac.Sum(macBytes[:0])