Add SSL_was_key_usage_invalid. This function reports when security-critical checks on the X.509 key usage extension would have failed, but were skipped due to the temporary exception in SSL_set_enforce_rsa_key_usage. This function is meant to aid deployments as they work through enabling this. Change-Id: Ice0359879c0a6cbe55bf0cb81a63685506883123 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55465 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 6c8eba0..26e8f91 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -4327,12 +4327,24 @@ // 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 the keyUsage extension of -// RSA leaf certificates will be checked for consistency with the TLS -// usage. This parameter may be set late; it will not be read until after the +// 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 0a41ffe..b9b3f27 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -1390,11 +1390,13 @@ ssl_key_usage_t intended_use = (alg_k & SSL_kRSA) ? key_usage_encipherment : key_usage_digital_signature; - if (hs->config->enforce_rsa_key_usage || - EVP_PKEY_id(hs->peer_pubkey.get()) != EVP_PKEY_RSA) { - if (!ssl_cert_check_key_usage(&leaf_cbs, intended_use)) { + 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; } }
diff --git a/ssl/internal.h b/ssl/internal.h index 4d9ab49..456fa7a 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -2763,6 +2763,11 @@ // 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; + // hs_buf is the buffer of handshake data to process. UniquePtr<BUF_MEM> hs_buf;
diff --git a/ssl/s3_lib.cc b/ssl/s3_lib.cc index 2adf386..8e8a034 100644 --- a/ssl/s3_lib.cc +++ b/ssl/s3_lib.cc
@@ -178,7 +178,8 @@ early_data_accepted(false), alert_dispatch(false), renegotiate_pending(false), - used_hello_retry_request(false) {} + used_hello_retry_request(false), + was_key_usage_invalid(false) {} SSL3_STATE::~SSL3_STATE() {}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index cfd1862..0f0f5b1 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -2807,6 +2807,10 @@ 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 3e12a0f..684c254 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -666,6 +666,12 @@ 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; + } + // Test that handshake hints correctly skipped the expected operations. if (config->handshake_hints && !config->allow_hint_mismatch) { const TestState *state = GetTestState(ssl);
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index b66ae52..a30dba0 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -15727,24 +15727,26 @@ // 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, + name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-Unenforced-" + ver.name, config: Config{ MinVersion: ver.version, MaxVersion: ver.version, Certificates: []Certificate{dsCert}, CipherSuites: encSuites, }, + flags: []string{"-expect-key-usage-invalid"}, }) testCases = append(testCases, testCase{ testType: clientTest, - name: "RSAKeyUsage-Client-WantEncipherment-GotSignature-Unenforced" + ver.name, + name: "RSAKeyUsage-Client-WantEncipherment-GotSignature-Unenforced-" + ver.name, config: Config{ MinVersion: ver.version, MaxVersion: ver.version, Certificates: []Certificate{encCert}, CipherSuites: dsSuites, }, + flags: []string{"-expect-key-usage-invalid"}, }) } @@ -15752,7 +15754,7 @@ // In 1.3 and above, we enforce keyUsage even without the flag. testCases = append(testCases, testCase{ testType: clientTest, - name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-Enforced" + ver.name, + name: "RSAKeyUsage-Client-WantSignature-GotEncipherment-Enforced-" + ver.name, config: Config{ MinVersion: ver.version, MaxVersion: ver.version,
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 2671370..465c616 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -365,6 +365,8 @@ &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("-expect-key-usage-invalid", + &TestConfig::expect_key_usage_invalid), BoolFlag("-is-handshaker-supported", &TestConfig::is_handshaker_supported), BoolFlag("-handshaker-resume", &TestConfig::handshaker_resume),
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 1a21ac1..5608dab 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -178,6 +178,7 @@ int install_one_cert_compression_alg = 0; bool reverify_on_resume = false; bool enforce_rsa_key_usage = false; + bool expect_key_usage_invalid = false; bool is_handshaker_supported = false; bool handshaker_resume = false; std::string handshaker_path;