Default SSL_set_enforce_rsa_key_usage to enabled.

This relands
https://boringssl-review.googlesource.com/c/boringssl/+/54606, which was
temporarily reverted.

Update-Note: By default, clients will now require RSA server
certificates used in TLS 1.2 and earlier to include the keyEncipherment
or digitalSignature bit. keyEncipherment is required if using RSA key
exchange. digitalSignature is required if using ECDHE_RSA key exchange.

If unsure, TLS RSA server signatures should include both, but some
deployments may wish to include only one if separating keys, or simply
disabling RSA key exchange. The latter is useful to mitigate either the
Bleichenbacher attack (from 1998, most recently resurfaced in 2017 as
ROBOT), or to strengthen TLS 1.3 downgrade protections, which is
particularly important for enterprise environments using client
certificates (aka "mTLS") because, prior to TLS 1.3, the TLS client
certificate flow was insufficiently encrypted or authenticated. Without
reflecting an RSA key exchange disable into key usage, and then the
client checking it, an attacker can spoof a CertificateRequest as coming
from some server.

This aligns with standard security requirements for using X.509
certificates, specified in RFC 5280, section 4.2.1.3, and reiterated in
TLS as early as TLS 1.0, RFC 2246, section 7.4.2, published 24 years ago
on January 1999. Constraints on usage of keys are important to mitigate
cross-protocol attacks, a class of cryptographic attacks that is
well-studied in the literature.

We already checked this for each of ECDSA, TLS 1.3, and servers
verifying client certificates, so this just fills in the remaining hole.
As a result, this change is also important for avoiding some weird
behaviors when configuration changes transition a server in or out of
this hole. (We've seen connection failures get misattributed to TLS 1.3
when it was really a certificate misconfiguration.)

Chrome has also enforced this for some time with publicly-trusted
certificates. As a temporary measure for callers that need more time,
the SSL_set_enforce_rsa_key_usage API, added to BoringSSL in January
2019, still exists where we need to turn this off.

Fixed: 519
Change-Id: I91bf2cfb04c92aec7875e640f90ba6f837146dc1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58805
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

ssl/ssl_lib.cc

@@ -703,7 +703,7 @@ SSL_CONFIG::SSL_CONFIG(SSL *ssl_arg)
       signed_cert_timestamps_enabled(false),
       ocsp_stapling_enabled(false),
       channel_id_enabled(false),
-      enforce_rsa_key_usage(false),
+      enforce_rsa_key_usage(true),
       retain_only_sha256_of_client_certs(false),
       handoff(false),
       shed_handshake_config(false),

ssl/test/runner/runner.go

@@ -15605,9 +15605,6 @@ func addRSAKeyUsageTests() {
 			},
 			shouldFail:    true,
 			expectedError: ":KEY_USAGE_BIT_INCORRECT:",
-			flags: []string{
-				"-enforce-rsa-key-usage",
-			},
 		})
 
 		testCases = append(testCases, testCase{
@@ -15619,9 +15616,6 @@ func addRSAKeyUsageTests() {
 				Certificates: []Certificate{dsCert},
 				CipherSuites: dsSuites,
 			},
-			flags: []string{
-				"-enforce-rsa-key-usage",
-			},
 		})
 
 		// TLS 1.3 removes the encipherment suites.
@@ -15635,9 +15629,6 @@ func addRSAKeyUsageTests() {
 					Certificates: []Certificate{encCert},
 					CipherSuites: encSuites,
 				},
-				flags: []string{
-					"-enforce-rsa-key-usage",
-				},
 			})
 
 			testCases = append(testCases, testCase{
@@ -15651,9 +15642,6 @@ func addRSAKeyUsageTests() {
 				},
 				shouldFail:    true,
 				expectedError: ":KEY_USAGE_BIT_INCORRECT:",
-				flags: []string{
-					"-enforce-rsa-key-usage",
-				},
 			})
 
 			// In 1.2 and below, we should not enforce without the enforce-rsa-key-usage flag.
@@ -15666,7 +15654,7 @@ func addRSAKeyUsageTests() {
 					Certificates: []Certificate{dsCert},
 					CipherSuites: encSuites,
 				},
-				flags: []string{"-expect-key-usage-invalid"},
+				flags: []string{"-expect-key-usage-invalid", "-ignore-rsa-key-usage"},
 			})
 
 			testCases = append(testCases, testCase{
@@ -15678,21 +15666,22 @@ func addRSAKeyUsageTests() {
 					Certificates: []Certificate{encCert},
 					CipherSuites: dsSuites,
 				},
-				flags: []string{"-expect-key-usage-invalid"},
+				flags: []string{"-expect-key-usage-invalid", "-ignore-rsa-key-usage"},
 			})
 		}
 
 		if ver.version >= VersionTLS13 {
-			// In 1.3 and above, we enforce keyUsage even without the flag.
+			// In 1.3 and above, we enforce keyUsage even when disabled.
 			testCases = append(testCases, testCase{
 				testType: clientTest,
-				name:     "RSAKeyUsage-Client-WantSignature-GotEncipherment-Enforced-" + ver.name,
+				name:     "RSAKeyUsage-Client-WantSignature-GotEncipherment-AlwaysEnforced-" + ver.name,
 				config: Config{
 					MinVersion:   ver.version,
 					MaxVersion:   ver.version,
 					Certificates: []Certificate{encCert},
 					CipherSuites: dsSuites,
 				},
+				flags:         []string{"-ignore-rsa-key-usage"},
 				shouldFail:    true,
 				expectedError: ":KEY_USAGE_BIT_INCORRECT:",
 			})

ssl/test/test_config.cc

@@ -366,7 +366,7 @@ std::vector<Flag> SortedFlags() {
       IntFlag("-install-one-cert-compression-alg",
               &TestConfig::install_one_cert_compression_alg),
       BoolFlag("-reverify-on-resume", &TestConfig::reverify_on_resume),
-      BoolFlag("-enforce-rsa-key-usage", &TestConfig::enforce_rsa_key_usage),
+      BoolFlag("-ignore-rsa-key-usage", &TestConfig::ignore_rsa_key_usage),
       BoolFlag("-expect-key-usage-invalid",
                &TestConfig::expect_key_usage_invalid),
       BoolFlag("-is-handshaker-supported",
@@ -1741,8 +1741,8 @@ bssl::UniquePtr<SSL> TestConfig::NewSSL(
   if (reverify_on_resume) {
     SSL_CTX_set_reverify_on_resume(ssl_ctx, 1);
   }
-  if (enforce_rsa_key_usage) {
-    SSL_set_enforce_rsa_key_usage(ssl.get(), 1);
+  if (ignore_rsa_key_usage) {
+    SSL_set_enforce_rsa_key_usage(ssl.get(), 0);
   }
   if (no_tls13) {
     SSL_set_options(ssl.get(), SSL_OP_NO_TLSv1_3);

ssl/test/test_config.h

@@ -177,7 +177,7 @@ struct TestConfig {
   bool install_cert_compression_algs = false;
   int install_one_cert_compression_alg = 0;
   bool reverify_on_resume = false;
-  bool enforce_rsa_key_usage = false;
+  bool ignore_rsa_key_usage = false;
   bool expect_key_usage_invalid = false;
   bool is_handshaker_supported = false;
   bool handshaker_resume = false;