Fix TLS13SessionID-TLS13 test. The check was happening in code that only ran at TLS 1.2, so we weren't testing anything. Additionally check the resumption case. While we do handle it correctly, we only manage it due to the weird OpenSSL quirk we've carried over from TLS 1.2 tickets where we synthesize a session ID for TLS 1.3 tickets. (Is that still needed?) That's subtle enough to warrant a test, and some other implementations reuse our test suite, so it's worth the coverage there. Change-Id: I83cc355bd853097ec6edcd2cc40b06b19e3b00e5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45324 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 85561fc..35e9933 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -349,6 +349,10 @@ } } + if len(hs.clientHello.sessionID) == 0 && c.config.Bugs.ExpectClientHelloSessionID { + return errors.New("tls: expected non-empty session ID from client") + } + applyBugsToClientHello(hs.clientHello, config) return nil @@ -1250,10 +1254,6 @@ copy(hs.hello.random[len(hs.hello.random)-8:], downgradeJDK11) } - if len(hs.clientHello.sessionID) == 0 && c.config.Bugs.ExpectClientHelloSessionID { - return false, errors.New("tls: expected non-empty session ID from client") - } - foundCompression := false // We only support null compression, so check that the client offered it. for _, compression := range hs.clientHello.compressionMethods {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index e3931d7..07c0aa6 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -13419,7 +13419,9 @@ }, }) - // Test that the client sends a fake session ID in TLS 1.3. + // Test that the client sends a fake session ID in TLS 1.3. We cover both + // normal and resumption handshakes to capture interactions with the + // session resumption path. testCases = append(testCases, testCase{ testType: clientTest, name: "TLS13SessionID-TLS13", @@ -13429,6 +13431,7 @@ ExpectClientHelloSessionID: true, }, }, + resumeSession: true, }) // Test that the client omits the fake session ID when the max version is TLS 1.2 and below.