From e7e36aae253be9d054b5df108ac9fea42e1a0557 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Mon, 8 Aug 2016 12:39:41 -0400
Subject: [PATCH] Test that switching versions on renego is illegal.

We handle this correctly but never wrote a test for it. Noticed this in
chatting about the second ClientHello.version bug workaround with Eric
Rescorla.

Change-Id: I09bc6c995d07c0f2c9936031b52c3c639ed3695e
Reviewed-on: https://boringssl-review.googlesource.com/9154
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
---
 ssl/test/runner/common.go           |  4 ++++
 ssl/test/runner/handshake_server.go |  2 ++
 ssl/test/runner/runner.go           | 24 ++++++++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index aca308c5..b6befe42 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -606,6 +606,10 @@ type ProtocolBugs struct {
 	// peer.
 	NegotiateVersion uint16
 
+	// NegotiateVersionOnRenego, if non-zero, causes the server to negotiate
+	// the specified TLS version on renegotiation rather than retaining it.
+	NegotiateVersionOnRenego uint16
+
 	// ExpectFalseStart causes the server to, on full handshakes,
 	// expect the peer to False Start; the server Finished message
 	// isn't sent until we receive an application data record
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index c2b28f21..fe860f8f 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -222,6 +222,8 @@ func (hs *serverHandshakeState) readClientHello() error {
 
 	if config.Bugs.NegotiateVersion != 0 {
 		c.vers = config.Bugs.NegotiateVersion
+	} else if c.haveVers && config.Bugs.NegotiateVersionOnRenego != 0 {
+		c.vers = config.Bugs.NegotiateVersionOnRenego
 	} else {
 		c.vers, ok = config.mutualVersion(hs.clientHello.vers, c.isDTLS)
 		if !ok {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ee4694bc..635600f4 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5026,6 +5026,9 @@ func addRenegotiationTests() {
 			"-expect-total-renegotiations", "1",
 		},
 	})
+
+	// Test that the server may switch ciphers on renegotiation without
+	// problems.
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-Client-SwitchCiphers",
 		renegotiate: 1,
@@ -5052,6 +5055,27 @@ func addRenegotiationTests() {
 			"-expect-total-renegotiations", "1",
 		},
 	})
+
+	// Test that the server may not switch versions on renegotiation.
+	testCases = append(testCases, testCase{
+		name: "Renegotiate-Client-SwitchVersion",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			// Pick a cipher which exists at both versions.
+			CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+			Bugs: ProtocolBugs{
+				NegotiateVersionOnRenego: VersionTLS11,
+			},
+		},
+		renegotiate: 1,
+		flags: []string{
+			"-renegotiate-freely",
+			"-expect-total-renegotiations", "1",
+		},
+		shouldFail:    true,
+		expectedError: ":WRONG_SSL_VERSION:",
+	})
+
 	testCases = append(testCases, testCase{
 		name:        "Renegotiate-SameClientVersion",
 		renegotiate: 1,