Unwind SSL_set_enforce_rsa_key_usage The last remaining user (Envoy) recently removed this flag. Finally unwind this, after it was enabled by default three years ago. Update-Note: SSL_set_enforce_rsa_key_usage and SSL_was_key_usage_invalid are now removed. The last callers of these functions are now gone. Bug: 42290387 Change-Id: Iae7965e7699e51394699a579020b318d0874348f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/97507 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/prefix_symbols.h b/include/openssl/prefix_symbols.h index 5f91388..abfe812 100644 --- a/include/openssl/prefix_symbols.h +++ b/include/openssl/prefix_symbols.h
@@ -2356,7 +2356,6 @@ #pragma redefine_extname SSL_set_custom_verify BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_custom_verify) #pragma redefine_extname SSL_set_early_data_enabled BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_early_data_enabled) #pragma redefine_extname SSL_set_enable_ech_grease BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_enable_ech_grease) -#pragma redefine_extname SSL_set_enforce_rsa_key_usage BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_enforce_rsa_key_usage) #pragma redefine_extname SSL_set_ex_data BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_ex_data) #pragma redefine_extname SSL_set_fd BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_fd) #pragma redefine_extname SSL_set_handshake_hints BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_handshake_hints) @@ -2433,7 +2432,6 @@ #pragma redefine_extname SSL_used_hello_retry_request BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_used_hello_retry_request) #pragma redefine_extname SSL_version BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_version) #pragma redefine_extname SSL_want BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_want) -#pragma redefine_extname SSL_was_key_usage_invalid BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_was_key_usage_invalid) #pragma redefine_extname SSL_write BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_write) #pragma redefine_extname SSLeay BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSLeay) #pragma redefine_extname SSLeay_version BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSLeay_version) @@ -5475,7 +5473,6 @@ #define SSL_set_custom_verify BORINGSSL_ADD_PREFIX(SSL_set_custom_verify) #define SSL_set_early_data_enabled BORINGSSL_ADD_PREFIX(SSL_set_early_data_enabled) #define SSL_set_enable_ech_grease BORINGSSL_ADD_PREFIX(SSL_set_enable_ech_grease) -#define SSL_set_enforce_rsa_key_usage BORINGSSL_ADD_PREFIX(SSL_set_enforce_rsa_key_usage) #define SSL_set_ex_data BORINGSSL_ADD_PREFIX(SSL_set_ex_data) #define SSL_set_fd BORINGSSL_ADD_PREFIX(SSL_set_fd) #define SSL_set_handshake_hints BORINGSSL_ADD_PREFIX(SSL_set_handshake_hints) @@ -5552,7 +5549,6 @@ #define SSL_used_hello_retry_request BORINGSSL_ADD_PREFIX(SSL_used_hello_retry_request) #define SSL_version BORINGSSL_ADD_PREFIX(SSL_version) #define SSL_want BORINGSSL_ADD_PREFIX(SSL_want) -#define SSL_was_key_usage_invalid BORINGSSL_ADD_PREFIX(SSL_was_key_usage_invalid) #define SSL_write BORINGSSL_ADD_PREFIX(SSL_write) #define SSLeay BORINGSSL_ADD_PREFIX(SSLeay) #define SSLeay_version BORINGSSL_ADD_PREFIX(SSLeay_version)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index b73232f..ff27b47 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -5348,24 +5348,6 @@ // respected on clients. OPENSSL_EXPORT void SSL_CTX_set_reverify_on_resume(SSL_CTX *ctx, int enabled); -// SSL_set_enforce_rsa_key_usage configures whether, when `ssl` is a client -// negotiating TLS 1.2 or below, the keyUsage extension of RSA leaf server -// certificates will be checked for consistency with the TLS usage. In all other -// cases, this check is always enabled. -// -// This parameter may be set late; it will not be read until after the -// certificate verification callback. -OPENSSL_EXPORT void SSL_set_enforce_rsa_key_usage(SSL *ssl, int enabled); - -// SSL_was_key_usage_invalid returns one if `ssl`'s handshake succeeded despite -// using TLS parameters which were incompatible with the leaf certificate's -// keyUsage extension. Otherwise, it returns zero. -// -// If `SSL_set_enforce_rsa_key_usage` is enabled or not applicable, this -// function will always return zero because key usages will be consistently -// checked. -OPENSSL_EXPORT int SSL_was_key_usage_invalid(const SSL *ssl); - // SSL_ST_* are possible values for `SSL_state`, the bitmasks that make them up, // and some historical values for compatibility. Only `SSL_ST_INIT` and // `SSL_ST_OK` are ever returned.
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index 1a37903..af70197 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -1400,23 +1400,13 @@ CBS leaf_cbs; CRYPTO_BUFFER_init_CBS(leaf, &leaf_cbs); - // Check the key usage matches the cipher suite. We do this unconditionally - // for non-RSA certificates. In particular, it's needed to distinguish ECDH - // certificates, which we do not support, from ECDSA certificates. - // Historically, we have not checked RSA key usages, so it is controlled by - // a flag for now. See https://crbug.com/795089. - // Key usage is only checked for X.509 certs. (RPKs have no keyUsage to - // enforce.) + // Check the key usage matches the cipher suite. Key usage is only checked + // for X.509 certs. (RPKs have no keyUsage to enforce.) ssl_key_usage_t intended_use = (alg_k & SSL_kRSA) ? key_usage_encipherment : key_usage_digital_signature; if (!ssl_cert_check_key_usage(&leaf_cbs, intended_use)) { - if (hs->config->enforce_rsa_key_usage || - EVP_PKEY_id(hs->peer_pubkey.get()) != EVP_PKEY_RSA) { - return ssl_hs_error; - } - ERR_clear_error(); - ssl->s3->was_key_usage_invalid = true; + return ssl_hs_error; } }
diff --git a/ssl/internal.h b/ssl/internal.h index 867d5c4..5cd13e1 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -2927,11 +2927,6 @@ // HelloRetryRequest message. bool used_hello_retry_request : 1; - // was_key_usage_invalid is whether the handshake succeeded despite using a - // TLS mode which was incompatible with the leaf certificate's keyUsage - // extension. - bool was_key_usage_invalid : 1; - // server_sent_requested_padding is true iff a client requested padding // through the server padding extension, and the server sent back the // requested amount of padding. @@ -3501,11 +3496,6 @@ // that we'll accept Channel IDs from clients. It is ignored on the client. bool channel_id_enabled : 1; - // If enforce_rsa_key_usage is true, the handshake will fail if the - // keyUsage extension is present and incompatible with the TLS usage. - // This field is not read until after certificate verification. - bool enforce_rsa_key_usage : 1; - // retain_only_sha256_of_client_certs is true if we should compute the SHA256 // hash of the peer's certificate and then discard it to save memory and // session space. Only effective on the server side.
diff --git a/ssl/s3_lib.cc b/ssl/s3_lib.cc index 6f76f76..a58242a 100644 --- a/ssl/s3_lib.cc +++ b/ssl/s3_lib.cc
@@ -45,7 +45,6 @@ alert_dispatch(false), renegotiate_pending(false), used_hello_retry_request(false), - was_key_usage_invalid(false), server_sent_requested_padding(false) {} SSL3_STATE::~SSL3_STATE() {}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 96cce25..f5f7df0 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -581,7 +581,6 @@ signed_cert_timestamps_enabled(false), ocsp_stapling_enabled(false), channel_id_enabled(false), - enforce_rsa_key_usage(true), retain_only_sha256_of_client_certs(false), handoff(false), shed_handshake_config(false), @@ -2956,17 +2955,6 @@ FromOpaque(ctx)->reverify_on_resume = !!enabled; } -void SSL_set_enforce_rsa_key_usage(SSL *ssl, int enabled) { - if (!ssl->config) { - return; - } - ssl->config->enforce_rsa_key_usage = !!enabled; -} - -int SSL_was_key_usage_invalid(const SSL *ssl) { - return ssl->s3->was_key_usage_invalid; -} - void SSL_set_renegotiate_mode(SSL *ssl, enum ssl_renegotiate_mode_t mode) { ssl->renegotiate_mode = mode;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index daf166b..2eefbde 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -716,12 +716,6 @@ return false; } - if (config->expect_key_usage_invalid != !!SSL_was_key_usage_invalid(ssl)) { - fprintf(stderr, "X.509 key usage was %svalid, but wanted opposite.\n", - SSL_was_key_usage_invalid(ssl) ? "in" : ""); - return false; - } - if (const auto &expected = config->expect_peer_certificate_type; expected.has_value()) { const uint8_t negotiated = SSL_get_peer_cert_type(ssl);
diff --git a/ssl/test/runner/key_usage_tests.go b/ssl/test/runner/key_usage_tests.go index dab468b..c955506 100644 --- a/ssl/test/runner/key_usage_tests.go +++ b/ssl/test/runner/key_usage_tests.go
@@ -131,51 +131,9 @@ shouldFail: true, expectedError: ":KEY_USAGE_BIT_INCORRECT:", }) - - // In 1.2 and below, we should not enforce without the enforce-rsa-key-usage flag. - testCases = append(testCases, testCase{ - testType: clientTest, - name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-Unenforced-" + ver.name, - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - Credential: &dsCert, - CipherSuites: encSuites, - }, - flags: []string{"-expect-key-usage-invalid", "-ignore-rsa-key-usage"}, - }) - - testCases = append(testCases, testCase{ - testType: clientTest, - name: "RSAKeyUsage-Client-WantEncipherment-GotSignature-Unenforced-" + ver.name, - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - Credential: &encCert, - CipherSuites: dsSuites, - }, - flags: []string{"-expect-key-usage-invalid", "-ignore-rsa-key-usage"}, - }) } - if ver.version >= VersionTLS13 { - // In 1.3 and above, we enforce keyUsage even when disabled. - testCases = append(testCases, testCase{ - testType: clientTest, - name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-AlwaysEnforced-" + ver.name, - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - Credential: &encCert, - CipherSuites: dsSuites, - }, - flags: []string{"-ignore-rsa-key-usage"}, - shouldFail: true, - expectedError: ":KEY_USAGE_BIT_INCORRECT:", - }) - } - - // The server only uses signatures and always enforces it. + // The server expects signatures from the client and always enforces it. testCases = append(testCases, testCase{ testType: serverTest, name: "RSAKeyUsage-Server-WantSignature-GotEncipherment-" + ver.name,
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index f89a250..4f3e790 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -538,9 +538,6 @@ IntFlag("-install-one-cert-compression-alg", &TestConfig::install_one_cert_compression_alg), BoolFlag("-reverify-on-resume", &TestConfig::reverify_on_resume), - BoolFlag("-ignore-rsa-key-usage", &TestConfig::ignore_rsa_key_usage), - BoolFlag("-expect-key-usage-invalid", - &TestConfig::expect_key_usage_invalid), BoolFlag("-is-handshaker-supported", &TestConfig::is_handshaker_supported), BoolFlag("-handshaker-resume", &TestConfig::handshaker_resume), @@ -2428,9 +2425,6 @@ if (reverify_on_resume) { SSL_CTX_set_reverify_on_resume(ssl_ctx, 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); }
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index cd526f0..fea9848 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -226,8 +226,6 @@ bool install_cert_compression_algs = false; int install_one_cert_compression_alg = 0; bool reverify_on_resume = false; - bool ignore_rsa_key_usage = false; - bool expect_key_usage_invalid = false; bool is_handshaker_supported = false; bool handshaker_resume = false; std::string handshaker_path;