commit 397c8e6fb6b493fae96328a41b1b94c6dd6f85d3
author David Benjamin <davidben@google.com> Fri Jul 08 14:14:36 2016 -0700
committer David Benjamin <davidben@google.com> Mon Jul 11 18:26:27 2016 +0000
tree 259ead65582ab188c3760175370d823e042a8dfa
parent 71dd6660e849e8f98129c997e2f52ae8991ec3d3

Forbid renegotiation in TLS 1.3.

Change-Id: I1b34acbbb5528e7e31595ee0cbce7618890f3955
Reviewed-on: https://boringssl-review.googlesource.com/8669
Reviewed-by: David Benjamin <davidben@google.com>

ssl/s3_both.c

diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index e67f6a4..b1947c9 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -574,12 +574,14 @@
   ssl_do_msg_callback(ssl, 0 /* read */, ssl->version, SSL3_RT_HANDSHAKE,
                       ssl->init_buf->data, ssl->init_buf->length);
 
+  /* Ignore stray HelloRequest messages. Per RFC 5246, section 7.4.1.1, the
+   * server may send HelloRequest at any time. */
   static const uint8_t kHelloRequest[4] = {SSL3_MT_HELLO_REQUEST, 0, 0, 0};
-  if (!ssl->server && ssl->init_buf->length == sizeof(kHelloRequest) &&
+  if (!ssl->server &&
+      (!ssl->s3->have_version ||
+       ssl3_protocol_version(ssl) < TLS1_3_VERSION) &&
+      ssl->init_buf->length == sizeof(kHelloRequest) &&
       memcmp(kHelloRequest, ssl->init_buf->data, sizeof(kHelloRequest)) == 0) {
-    /* The server may always send 'Hello Request' messages -- we are doing a
-     * handshake anyway now, so ignore them if their format is correct.  Does
-     * not count for 'Finished' MAC. */
     goto again;
   }
 

ssl/s3_pkt.c

diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 1bbed59..dec8288 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -353,6 +353,10 @@
 }
 
 static int ssl3_can_renegotiate(SSL *ssl) {
+  if (ssl->server || ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
+    return 0;
+  }
+
   switch (ssl->renegotiate_mode) {
     case ssl_renegotiate_never:
       return 0;

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index c6aa104..01f8a46 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4542,7 +4542,7 @@
 		},
 	})
 
-	// Stray HelloRequests during the handshake are ignored.
+	// Stray HelloRequests during the handshake are ignored in TLS 1.2.
 	testCases = append(testCases, testCase{
 		name: "StrayHelloRequest",
 		config: Config{
@@ -4563,7 +4563,32 @@
 		},
 	})
 
-	// TODO(davidben): Add a test that HelloRequests are illegal in TLS 1.3.
+	// Renegotiation is forbidden in TLS 1.3.
+	testCases = append(testCases, testCase{
+		name: "Renegotiate-Client-TLS13",
+		config: Config{
+			MaxVersion: VersionTLS13,
+		},
+		renegotiate: 1,
+		flags: []string{
+			"-renegotiate-freely",
+		},
+		shouldFail:    true,
+		expectedError: ":NO_RENEGOTIATION:",
+	})
+
+	// Stray HelloRequests during the handshake are forbidden in TLS 1.3.
+	testCases = append(testCases, testCase{
+		name: "StrayHelloRequest-TLS13",
+		config: Config{
+			MaxVersion: VersionTLS13,
+			Bugs: ProtocolBugs{
+				SendHelloRequestBeforeEveryHandshakeMessage: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_MESSAGE:",
+	})
 }
 
 func addDTLSReplayTests() {