Add a per-SSL TLS 1.3 downgrade enforcement option and improve tests.

Due to non-compliant middleboxes, it is possible we'll need to do some
surgery to this mechanism. Making it per-SSL is a little more flexible
and also eases some tests in Chromium until we get its SSL_CTX usage
fixed up.

Also fix up BoringSSL tests. We forgot to test it at TLS 1.0 and use the
-expect-tls13-downgrade flag.

Bug: 226
Change-Id: Ib39227e74e2d6f5e1fbc1ebcc091e751471b3cdc
Reviewed-on: https://boringssl-review.googlesource.com/c/32424
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

include/openssl/ssl.h

@@ -3667,6 +3667,10 @@ OPENSSL_EXPORT void SSL_CTX_set_false_start_allowed_without_alpn(SSL_CTX *ctx,
 OPENSSL_EXPORT void SSL_CTX_set_ignore_tls13_downgrade(SSL_CTX *ctx,
                                                        int ignore);
 
+// SSL_set_ignore_tls13_downgrade configures whether |ssl| ignores the downgrade
+// signal in the server's random value.
+OPENSSL_EXPORT void SSL_set_ignore_tls13_downgrade(SSL *ssl, int ignore);
+
 // SSL_is_tls13_downgrade returns one if the TLS 1.3 anti-downgrade
 // mechanism would have aborted |ssl|'s handshake and zero otherwise.
 OPENSSL_EXPORT int SSL_is_tls13_downgrade(const SSL *ssl);

ssl/handshake_client.cc

@@ -600,7 +600,7 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
             .subspan(SSL3_RANDOM_SIZE - sizeof(kTLS13DowngradeRandom));
     if (suffix == kTLS12DowngradeRandom || suffix == kTLS13DowngradeRandom) {
       ssl->s3->tls13_downgrade = true;
-      if (!ssl->ctx->ignore_tls13_downgrade) {
+      if (!hs->config->ignore_tls13_downgrade) {
         OPENSSL_PUT_ERROR(SSL, SSL_R_TLS13_DOWNGRADE);
         ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
         return ssl_hs_error;

ssl/internal.h

@@ -2456,6 +2456,10 @@ struct SSL_CONFIG {
   // shed_handshake_config indicates that the handshake config (this object!)
   // should be freed after the handshake completes.
   bool shed_handshake_config : 1;
+
+  // ignore_tls13_downgrade is whether the connection should continue when the
+  // server random signals a downgrade.
+  bool ignore_tls13_downgrade:1;
 };
 
 // From RFC 8446, used in determining PSK modes.

ssl/ssl_lib.cc

@@ -693,6 +693,7 @@ SSL *SSL_new(SSL_CTX *ctx) {
       ctx->signed_cert_timestamps_enabled;
   ssl->config->ocsp_stapling_enabled = ctx->ocsp_stapling_enabled;
   ssl->config->handoff = ctx->handoff;
+  ssl->config->ignore_tls13_downgrade = ctx->ignore_tls13_downgrade;
 
   if (!ssl->method->ssl_new(ssl.get()) ||
       !ssl->ctx->x509_method->ssl_new(ssl->s3->hs.get())) {
@@ -709,7 +710,8 @@ SSL_CONFIG::SSL_CONFIG(SSL *ssl_arg)
       channel_id_enabled(false),
       retain_only_sha256_of_client_certs(false),
       handoff(false),
-      shed_handshake_config(false) {
+      shed_handshake_config(false),
+      ignore_tls13_downgrade(false) {
   assert(ssl);
 }
 
@@ -2642,6 +2644,13 @@ void SSL_CTX_set_ignore_tls13_downgrade(SSL_CTX *ctx, int ignore) {
   ctx->ignore_tls13_downgrade = !!ignore;
 }
 
+void SSL_set_ignore_tls13_downgrade(SSL *ssl, int ignore) {
+  if (!ssl->config) {
+    return;
+  }
+  ssl->config->ignore_tls13_downgrade = !!ignore;
+}
+
 void SSL_set_shed_handshake_config(SSL *ssl, int enable) {
   if (!ssl->config) {
     return;

ssl/test/runner/runner.go

@@ -5891,59 +5891,65 @@ func addVersionNegotiationTests() {
 	})
 
 	// Test TLS 1.3's downgrade signal.
-	testCases = append(testCases, testCase{
-		name: "Downgrade-TLS12-Client",
-		config: Config{
-			Bugs: ProtocolBugs{
-				NegotiateVersion: VersionTLS12,
-			},
-		},
-		tls13Variant:       TLS13RFC,
-		expectedVersion:    VersionTLS12,
-		shouldFail:         true,
-		expectedError:      ":TLS13_DOWNGRADE:",
-		expectedLocalError: "remote error: illegal parameter",
-	})
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "Downgrade-TLS12-Server",
-		config: Config{
-			Bugs: ProtocolBugs{
-				SendSupportedVersions: []uint16{VersionTLS12},
+	var downgradeTests = []struct {
+		name            string
+		version         uint16
+		clientShimError string
+	}{
+		{"TLS12", VersionTLS12, "tls: downgrade from TLS 1.3 detected"},
+		{"TLS11", VersionTLS11, "tls: downgrade from TLS 1.2 detected"},
+		// TLS 1.0 does not have a dedicated value.
+		{"TLS10", VersionTLS10, "tls: downgrade from TLS 1.2 detected"},
+	}
+
+	for _, test := range downgradeTests {
+		// The client should enforce the downgrade sentinel.
+		testCases = append(testCases, testCase{
+			name: "Downgrade-" + test.name + "-Client",
+			config: Config{
+				Bugs: ProtocolBugs{
+					NegotiateVersion: test.version,
+				},
 			},
-		},
-		tls13Variant:       TLS13RFC,
-		expectedVersion:    VersionTLS12,
-		shouldFail:         true,
-		expectedLocalError: "tls: downgrade from TLS 1.3 detected",
-	})
+			tls13Variant:       TLS13RFC,
+			expectedVersion:    test.version,
+			shouldFail:         true,
+			expectedError:      ":TLS13_DOWNGRADE:",
+			expectedLocalError: "remote error: illegal parameter",
+		})
 
-	testCases = append(testCases, testCase{
-		name: "Downgrade-TLS11-Client",
-		config: Config{
-			Bugs: ProtocolBugs{
-				NegotiateVersion: VersionTLS11,
+		// The client should ignore the downgrade sentinel if
+		// configured.
+		testCases = append(testCases, testCase{
+			name: "Downgrade-" + test.name + "-Client-Ignore",
+			config: Config{
+				Bugs: ProtocolBugs{
+					NegotiateVersion: test.version,
+				},
 			},
-		},
-		tls13Variant:       TLS13RFC,
-		expectedVersion:    VersionTLS11,
-		shouldFail:         true,
-		expectedError:      ":TLS13_DOWNGRADE:",
-		expectedLocalError: "remote error: illegal parameter",
-	})
-	testCases = append(testCases, testCase{
-		testType: serverTest,
-		name:     "Downgrade-TLS11-Server",
-		config: Config{
-			Bugs: ProtocolBugs{
-				SendSupportedVersions: []uint16{VersionTLS11},
+			tls13Variant:    TLS13RFC,
+			expectedVersion: test.version,
+			flags: []string{
+				"-ignore-tls13-downgrade",
+				"-expect-tls13-downgrade",
 			},
-		},
-		tls13Variant:       TLS13RFC,
-		expectedVersion:    VersionTLS11,
-		shouldFail:         true,
-		expectedLocalError: "tls: downgrade from TLS 1.2 detected",
-	})
+		})
+
+		// The server should emit the downgrade signal.
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			name:     "Downgrade-" + test.name + "-Server",
+			config: Config{
+				Bugs: ProtocolBugs{
+					SendSupportedVersions: []uint16{test.version},
+				},
+			},
+			tls13Variant:       TLS13RFC,
+			expectedVersion:    test.version,
+			shouldFail:         true,
+			expectedLocalError: test.clientShimError,
+		})
+	}
 
 	// Test that the draft TLS 1.3 variants don't trigger the downgrade logic.
 	testCases = append(testCases, testCase{