Send unsupported_extension on unexpected ServerHello extensions.

We were sending decode_error, but the spec explicitly says (RFC 5246):

   unsupported_extension
      sent by clients that receive an extended server hello containing
      an extension that they did not put in the corresponding client
      hello.  This message is always fatal.

Also add a test for this when it's a known but unoffered extension. We
actually end up putting these in different codepaths now due to the
custom extensions stuff.

Thanks to Eric Rescorla for pointing this out.

Change-Id: If6c8033d4cfe69ef8af5678b873b25e0dbadfc4f
Reviewed-on: https://boringssl-review.googlesource.com/9061
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>

5 files changed, 45 insertions, 6 deletions

diff --git a/ssl/custom_extensions.c b/ssl/custom_extensions.c
index c94543d8..4a9baa88 100644
--- a/ssl/custom_extensions.c
+++ b/ssl/custom_extensions.c
@@ -139,7 +139,7 @@ int custom_ext_parse_serverhello(SSL *ssl, int *out_alert, uint16_t value,
       !(ssl->s3->tmp.custom_extensions.sent & (1u << index))) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
     ERR_add_error_dataf("extension: %u", (unsigned)value);
-    *out_alert = SSL_AD_DECODE_ERROR;
+    *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
     return 0;
   }
 
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 6b0c16ab..e35a38c8 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2642,7 +2642,7 @@ static int ssl_scan_serverhello_tlsext(SSL *ssl, CBS *cbs, int *out_alert) {
       /* If the extension was never sent then it is illegal. */
       OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
       ERR_add_error_dataf("extension :%u", (unsigned)type);
-      *out_alert = SSL_AD_DECODE_ERROR;
+      *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
       return 0;
     }
 
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index dfd5b30d..e9a3fdb5 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -866,6 +866,11 @@ type ProtocolBugs struct {
 	// ALPN and NPN in the same connetion.
 	NegotiateALPNAndNPN bool
 
+	// SendALPN, if non-empty, causes the server to send the specified
+	// string in the ALPN extension regardless of whether the client
+	// advertised it.
+	SendALPN string
+
 	// SendEmptySessionTicket, if true, causes the server to send an empty
 	// session ticket.
 	SendEmptySessionTicket bool
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index ff8005bd..4d8b5c16 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -851,6 +851,10 @@ func (hs *serverHandshakeState) processClientExtensions(serverExtensions *server
 		}
 	}
 
+	if len(c.config.Bugs.SendALPN) > 0 {
+		serverExtensions.alpnProtocol = c.config.Bugs.SendALPN
+	}
+
 	if c.vers < VersionTLS13 || config.Bugs.NegotiateNPNAtAllVersions {
 		if len(hs.clientHello.alpnProtocols) == 0 || c.config.Bugs.NegotiateALPNAndNPN {
 			// Although sending an empty NPN extension is reasonable, Firefox has
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index b45e42ae..5e049b11 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -6440,8 +6440,9 @@ func addCustomExtensionTests() {
 				CustomExtension: expectedContents,
 			},
 		},
-		shouldFail:    true,
-		expectedError: ":UNEXPECTED_EXTENSION:",
+		shouldFail:         true,
+		expectedError:      ":UNEXPECTED_EXTENSION:",
+		expectedLocalError: "remote error: unsupported extension",
 	})
 	testCases = append(testCases, testCase{
 		testType: clientTest,
@@ -6452,8 +6453,37 @@ func addCustomExtensionTests() {
 				CustomExtension: expectedContents,
 			},
 		},
-		shouldFail:    true,
-		expectedError: ":UNEXPECTED_EXTENSION:",
+		shouldFail:         true,
+		expectedError:      ":UNEXPECTED_EXTENSION:",
+		expectedLocalError: "remote error: unsupported extension",
+	})
+
+	// Test a known but unoffered extension from the server.
+	testCases = append(testCases, testCase{
+		testType: clientTest,
+		name:     "UnofferedExtension-Client",
+		config: Config{
+			MaxVersion: VersionTLS12,
+			Bugs: ProtocolBugs{
+				SendALPN: "alpn",
+			},
+		},
+		shouldFail:         true,
+		expectedError:      ":UNEXPECTED_EXTENSION:",
+		expectedLocalError: "remote error: unsupported extension",
+	})
+	testCases = append(testCases, testCase{
+		testType: clientTest,
+		name:     "UnofferedExtension-Client-TLS13",
+		config: Config{
+			MaxVersion: VersionTLS13,
+			Bugs: ProtocolBugs{
+				SendALPN: "alpn",
+			},
+		},
+		shouldFail:         true,
+		expectedError:      ":UNEXPECTED_EXTENSION:",
+		expectedLocalError: "remote error: unsupported extension",
 	})
 }