Reject the ECH extension in TLS 1.2 ServerHello.

The ECH server extension is defined for TLS 1.3 EncryptedExtensions, not
TLS 1.2 ServerHello.

Bug: 275
Change-Id: Ie6e76c238075d70e6a0694ec0192df07da3457d1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47910
Reviewed-by: Adam Langley <agl@google.com>

ssl/t1_lib.cc

diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 44c96e8..3553276 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -688,10 +688,19 @@
 
 static bool ext_ech_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
                                       CBS *contents) {
+  SSL *const ssl = hs->ssl;
   if (contents == NULL) {
     return true;
   }
 
+  // The ECH extension may not be sent in TLS 1.2 ServerHello, only TLS 1.3
+  // EncryptedExtension.
+  if (ssl_protocol_version(ssl) < TLS1_3_VERSION) {
+    *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+    OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
+    return false;
+  }
+
   // If the client only sent GREASE, we must check the extension syntactically.
   CBS ech_configs;
   if (!CBS_get_u16_length_prefixed(contents, &ech_configs) ||

ssl/test/runner/common.go

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 4e84a0e..ae07dc0 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -859,6 +859,10 @@
 	// retry configs.
 	SendECHRetryConfigs []byte
 
+	// SendECHRetryConfigsInTLS12ServerHello, if true, causes the ECH server to
+	// send retry configs in the TLS 1.2 ServerHello.
+	SendECHRetryConfigsInTLS12ServerHello bool
+
 	// SendInvalidECHIsInner, if not empty, causes the client to send the
 	// specified byte string in the ech_is_inner extension.
 	SendInvalidECHIsInner []byte

ssl/test/runner/handshake_server.go

diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index ef21c24..8622bfa 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -428,10 +428,6 @@
 		return errors.New("tls: expected client to send ClientECH")
 	}
 
-	if hs.clientHello.clientECH != nil && len(config.Bugs.SendECHRetryConfigs) > 0 {
-		encryptedExtensions.extensions.echRetryConfigs = config.Bugs.SendECHRetryConfigs
-	}
-
 	// Select the cipher suite.
 	var preferenceList, supportedList []uint16
 	if config.PreferServerCipherSuites {
@@ -1528,6 +1524,10 @@
 
 	serverExtensions.serverNameAck = c.config.Bugs.SendServerNameAck
 
+	if (c.vers >= VersionTLS13 || c.config.Bugs.SendECHRetryConfigsInTLS12ServerHello) && hs.clientHello.clientECH != nil && len(config.Bugs.SendECHRetryConfigs) > 0 {
+		serverExtensions.echRetryConfigs = config.Bugs.SendECHRetryConfigs
+	}
+
 	return nil
 }
 

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 176fdd9..2a861f3 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -16887,11 +16887,11 @@
 			0x05, 0x04, 0x03, 0x02, 0x01,
 		}
 
-		validAndInvalidConfigsBuilder := newByteBuilder()
-		validAndInvalidConfigsBody := validAndInvalidConfigsBuilder.addU16LengthPrefixed()
-		validAndInvalidConfigsBody.addBytes(MarshalECHConfig(&retryConfigValid))
-		validAndInvalidConfigsBody.addBytes(retryConfigUnsupportedVersion)
-		validAndInvalidConfigs := validAndInvalidConfigsBuilder.finish()
+		validAndUnsupportedConfigsBuilder := newByteBuilder()
+		validAndUnsupportedConfigsBody := validAndUnsupportedConfigsBuilder.addU16LengthPrefixed()
+		validAndUnsupportedConfigsBody.addBytes(MarshalECHConfig(&retryConfigValid))
+		validAndUnsupportedConfigsBody.addBytes(retryConfigUnsupportedVersion)
+		validAndUnsupportedConfigs := validAndUnsupportedConfigsBuilder.finish()
 
 		// Test that the client accepts a well-formed encrypted_client_hello
 		// extension in response to ECH GREASE. The response includes one ECHConfig
@@ -16905,15 +16905,37 @@
 				MaxVersion: VersionTLS13,
 				Bugs: ProtocolBugs{
 					ExpectClientECH: true,
-					// Include an additional well-formed ECHConfig with an invalid
-					// version. This ensures the client can iterate over the retry
-					// configs.
-					SendECHRetryConfigs: validAndInvalidConfigs,
+					// Include an additional well-formed ECHConfig with an
+					// unsupported version. This ensures the client can skip
+					// unsupported configs.
+					SendECHRetryConfigs: validAndUnsupportedConfigs,
 				},
 			},
 			flags: []string{"-enable-ech-grease"},
 		})
 
+		if protocol != quic {
+			// Test that the client rejects retry configs in TLS 1.2.
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				protocol: protocol,
+				name:     prefix + "ECH-GREASE-Client-TLS12-Retry-Configs",
+				config: Config{
+					MinVersion: VersionTLS12,
+					MaxVersion: VersionTLS12,
+					Bugs: ProtocolBugs{
+						ExpectClientECH:                       true,
+						SendECHRetryConfigs:                   validAndUnsupportedConfigs,
+						SendECHRetryConfigsInTLS12ServerHello: true,
+					},
+				},
+				flags:              []string{"-enable-ech-grease"},
+				shouldFail:         true,
+				expectedLocalError: "remote error: unsupported extension",
+				expectedError:      ":UNEXPECTED_EXTENSION:",
+			})
+		}
+
 		// Test that the client aborts with a decode_error alert when it receives a
 		// syntactically-invalid encrypted_client_hello extension from the server.
 		testCases = append(testCases, testCase{