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;
