Fix DTLS cross-version resumption tests

For now, just have runner tolerate the PSK extension changing across
HVR. It's possible we'll want to change this, but I think we can resolve
that separately.

The tests required an additional fix to deal with a syntactic
impossibility in DTLS. (Also QUIC, if TLS 1.2 in QUIC existed.)

Bug: 42290594
Change-Id: Ia06975104933e049bcd2c62aa8a29fd74b20f474
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73827
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>

ssl/test/runner/handshake_server.go

diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 7f890fd..2198ffd 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -323,7 +323,12 @@
 		if !bytes.Equal(newClientHello.cookie, helloVerifyRequest.cookie) {
 			return errors.New("dtls: invalid cookie")
 		}
-		if err := checkClientHellosEqual(hs.clientHello.raw, newClientHello.raw, c.isDTLS, nil); err != nil {
+		// Allow the client to recompute the pre_shared_key extension. It is
+		// possible we'll want to remove this and instead require the client
+		// keep it unchanged. (Either by remembering it or hashing the
+		// ClientHello funny.) See discussion at
+		// https://mailarchive.ietf.org/arch/msg/tls/HSTgV2voS9tn03oasGmBAdwazWA/
+		if err := checkClientHellosEqual(hs.clientHello.raw, newClientHello.raw, c.isDTLS, []uint16{extensionPreSharedKey}); err != nil {
 			return err
 		}
 		hs.clientHello = newClientHello

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 84fc93e..25651f6 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -9245,13 +9245,6 @@
 				suffix := "-" + sessionVers.name + "-" + resumeVers.name
 				suffix += "-" + protocol.String()
 
-				// TODO(crbug.com/42290594): Attempting to resume a DTLS 1.3 session
-				// when the new connection is an older version is currently broken.
-				// The current implementation recomputes the PSK binder value for the
-				// second ClientHello (sent in response to HelloVerifyRequest), but
-				// the runner expects all extension values to stay byte-for-byte the
-				// same (a possible interpretation of RFC 6347 section 4.2.1).
-				isBadDTLSResumption := protocol == dtls && sessionVers.version == VersionTLS13 && resumeVers.version < VersionTLS13
 				if sessionVers.version == resumeVers.version {
 					testCases = append(testCases, testCase{
 						protocol:      protocol,
@@ -9270,7 +9263,37 @@
 							version: resumeVers.version,
 						},
 					})
-				} else if !isBadDTLSResumption {
+				} else if protocol != tls && sessionVers.version >= VersionTLS13 && resumeVers.version < VersionTLS13 {
+					// In TLS 1.2 and below, the server indicates resumption by echoing
+					// the client's session ID, which is impossible if the client did
+					// not send a session ID. If the client offers a TLS 1.3 session, it
+					// only fills in session ID in TLS (not DTLS or QUIC) for middlebox
+					// compatibility mode. So, instead, test that the session ID was
+					// empty and it was indeed impossible to hit this path
+					testCases = append(testCases, testCase{
+						protocol:      protocol,
+						name:          "Resume-Client-Impossible" + suffix,
+						resumeSession: true,
+						config: Config{
+							MaxVersion: sessionVers.version,
+						},
+						expectations: connectionExpectations{
+							version: sessionVers.version,
+						},
+						resumeConfig: &Config{
+							MaxVersion: resumeVers.version,
+							Bugs: ProtocolBugs{
+								ExpectNoSessionID: true,
+							},
+						},
+						resumeExpectations: &connectionExpectations{
+							version: resumeVers.version,
+						},
+						expectResumeRejected: true,
+					})
+				} else {
+					// Test that the client rejects ServerHellos which resume
+					// sessions at inconsistent versions.
 					expectedError := ":OLD_SESSION_VERSION_NOT_RETURNED:"
 					if sessionVers.version < VersionTLS13 && resumeVers.version >= VersionTLS13 {
 						// The server will "resume" the session by sending pre_shared_key,
@@ -9278,6 +9301,7 @@
 						// should reject this because the extension was not allowed at all.
 						expectedError = ":UNEXPECTED_EXTENSION:"
 					}
+
 					testCases = append(testCases, testCase{
 						protocol:      protocol,
 						name:          "Resume-Client-Mismatch" + suffix,
@@ -9302,27 +9326,25 @@
 					})
 				}
 
-				if !isBadDTLSResumption {
-					testCases = append(testCases, testCase{
-						protocol:      protocol,
-						name:          "Resume-Client-NoResume" + suffix,
-						resumeSession: true,
-						config: Config{
-							MaxVersion: sessionVers.version,
-						},
-						expectations: connectionExpectations{
-							version: sessionVers.version,
-						},
-						resumeConfig: &Config{
-							MaxVersion: resumeVers.version,
-						},
-						newSessionsOnResume:  true,
-						expectResumeRejected: true,
-						resumeExpectations: &connectionExpectations{
-							version: resumeVers.version,
-						},
-					})
-				}
+				testCases = append(testCases, testCase{
+					protocol:      protocol,
+					name:          "Resume-Client-NoResume" + suffix,
+					resumeSession: true,
+					config: Config{
+						MaxVersion: sessionVers.version,
+					},
+					expectations: connectionExpectations{
+						version: sessionVers.version,
+					},
+					resumeConfig: &Config{
+						MaxVersion: resumeVers.version,
+					},
+					newSessionsOnResume:  true,
+					expectResumeRejected: true,
+					resumeExpectations: &connectionExpectations{
+						version: resumeVers.version,
+					},
+				})
 
 				testCases = append(testCases, testCase{
 					protocol:      protocol,