Remove SSL{_CTX}_set_aes_hw_override_for_testing
Allowing the AES hardware-based selection of default TLS 1.3 cipher
suites to change at runtime complicates the configuration of TLS 1.3
cipher suites. Remove this seemingly unused mechanism to simplify the
implementation of a more robust/flexible configuration mechanism that
should supersede this anyway.
Update-Note: This removes SSL{_CTX}_set_aes_hw_override_for_testing.
Bug: 545123692
Change-Id: I601141b7aa5cb9825512825c4739626e6a6a6964
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/101567
Auto-Submit: Lily Chen <chlily@google.com>
Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 3b48c2a..f0b3e43 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -6857,19 +6857,6 @@
const SSL *ssl, Span<const uint8_t> *out_read_traffic_secret,
Span<const uint8_t> *out_write_traffic_secret);
-// SSL_CTX_set_aes_hw_override_for_testing sets `override_value` to
-// override checking for aes hardware support for testing. If `override_value`
-// is set to true, the library will behave as if aes hardware support is
-// present. If it is set to false, the library will behave as if aes hardware
-// support is not present.
-OPENSSL_EXPORT void SSL_CTX_set_aes_hw_override_for_testing(
- SSL_CTX *ctx, bool override_value);
-
-// SSL_set_aes_hw_override_for_testing acts the same as
-// `SSL_CTX_set_aes_override_for_testing` but only configures a single `SSL*`.
-OPENSSL_EXPORT void SSL_set_aes_hw_override_for_testing(SSL *ssl,
- bool override_value);
-
BSSL_NAMESPACE_END
} // extern C++
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc
index 0cf1fc3..cc75a74 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -621,8 +621,7 @@
static bool select_ech_cipher_suite(const EVP_HPKE_KDF **out_kdf,
const EVP_HPKE_AEAD **out_aead,
- Span<const uint8_t> cipher_suites,
- const bool has_aes_hardware) {
+ Span<const uint8_t> cipher_suites) {
const EVP_HPKE_AEAD *aead = nullptr;
CBS cbs = cipher_suites;
while (CBS_len(&cbs) != 0) {
@@ -638,7 +637,7 @@
continue;
}
if (aead == nullptr ||
- (!has_aes_hardware && aead_id == EVP_HPKE_CHACHA20_POLY1305)) {
+ (!EVP_has_aes_hardware() && aead_id == EVP_HPKE_CHACHA20_POLY1305)) {
aead = candidate;
}
}
@@ -684,10 +683,7 @@
const EVP_HPKE_AEAD *aead;
if (supported && //
ech_config.kem_id == EVP_HPKE_DHKEM_X25519_HKDF_SHA256 &&
- select_ech_cipher_suite(&kdf, &aead, ech_config.cipher_suites,
- hs->ssl->config->aes_hw_override
- ? hs->ssl->config->aes_hw_override_value
- : EVP_has_aes_hardware())) {
+ select_ech_cipher_suite(&kdf, &aead, ech_config.cipher_suites)) {
ScopedCBB info;
static const uint8_t kInfoLabel[] = "tls ech"; // includes trailing NUL
if (!CBB_init(info.get(), sizeof(kInfoLabel) + ech_config.raw.size()) ||
@@ -745,11 +741,9 @@
}
const uint16_t kdf_id = EVP_HPKE_HKDF_SHA256;
- const bool has_aes_hw = hs->ssl->config->aes_hw_override
- ? hs->ssl->config->aes_hw_override_value
- : EVP_has_aes_hardware();
- const EVP_HPKE_AEAD *aead =
- has_aes_hw ? EVP_hpke_aes_128_gcm() : EVP_hpke_chacha20_poly1305();
+ const EVP_HPKE_AEAD *aead = EVP_has_aes_hardware()
+ ? EVP_hpke_aes_128_gcm()
+ : EVP_hpke_chacha20_poly1305();
static_assert(ssl_grease_ech_config_id < sizeof(hs->grease_seed),
"hs->grease_seed is too small");
uint8_t config_id = hs->grease_seed[ssl_grease_ech_config_id];
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index a4a53cd..c0ec883 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -127,14 +127,12 @@
SSL_CIPHER_CHACHA20_POLY1305_SHA256,
};
- const bool has_aes_hw = ssl->config->aes_hw_override
- ? ssl->config->aes_hw_override_value
- : EVP_has_aes_hardware();
const bssl::Span<const uint16_t> ciphers =
ssl->config->compliance_policy == ssl_compliance_policy_cnsa_202407
? bssl::Span<const uint16_t>(kCiphersCNSA)
- : (has_aes_hw ? bssl::Span<const uint16_t>(kCiphersAESHardware)
- : bssl::Span<const uint16_t>(kCiphersNoAESHardware));
+ : (EVP_has_aes_hardware()
+ ? bssl::Span<const uint16_t>(kCiphersAESHardware)
+ : bssl::Span<const uint16_t>(kCiphersNoAESHardware));
for (auto cipher : ciphers) {
if (!ssl_add_tls13_cipher(&child, cipher,
diff --git a/ssl/internal.h b/ssl/internal.h
index 96f6d3d..b60f5a1 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -325,11 +325,10 @@
// newly-allocated `SSLCipherPreferenceList` containing the result. It returns
// true on success and false on failure. If `strict` is true, nonsense will be
// rejected. If false, nonsense will be silently ignored. An empty result is
-// considered an error regardless of `strict`. `has_aes_hw` indicates if the
-// list should be ordered based on having support for AES in hardware or not.
+// considered an error regardless of `strict`. The resulting list will be
+// ordered based on having support for AES in hardware or not.
bool ssl_create_cipher_list(UniquePtr<SSLCipherPreferenceList> *out_cipher_list,
- const bool has_aes_hw, const char *rule_str,
- bool strict);
+ const char *rule_str, bool strict);
// ssl_cipher_auth_mask_for_key returns the mask of cipher `algorithm_auth`
// values suitable for use with `key` in TLS 1.2 and below. `sign_ok` indicates
@@ -3580,15 +3579,6 @@
// permute_extensions is whether to permute extensions when sending messages.
bool permute_extensions : 1;
- // aes_hw_override if set indicates we should override checking for aes
- // hardware support, and use the value in aes_hw_override_value instead.
- bool aes_hw_override : 1;
-
- // aes_hw_override_value is used for testing to indicate the support or lack
- // of support for AES hw. The value is only considered if `aes_hw_override` is
- // true.
- bool aes_hw_override_value : 1;
-
// alps_use_new_codepoint if set indicates we use new ALPS extension codepoint
// to negotiate and convey application settings.
bool alps_use_new_codepoint : 1;
@@ -4248,15 +4238,6 @@
// If enable_early_data is true, early data can be sent and accepted.
bool enable_early_data : 1;
- // aes_hw_override if set indicates we should override checking for AES
- // hardware support, and use the value in aes_hw_override_value instead.
- bool aes_hw_override : 1;
-
- // aes_hw_override_value is used for testing to indicate the support or lack
- // of support for AES hardware. The value is only considered if
- // `aes_hw_override` is true.
- bool aes_hw_override_value : 1;
-
// resumption_across_names_enabled indicates whether a TLS 1.3 server should
// signal its sessions may be resumed across names in the server certificate.
bool resumption_across_names_enabled : 1;
diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc
index 7799d79..e32bec5 100644
--- a/ssl/ssl_cipher.cc
+++ b/ssl/ssl_cipher.cc
@@ -21,6 +21,7 @@
#include <iterator>
+#include <openssl/aead.h>
#include <openssl/err.h>
#include <openssl/md5.h>
#include <openssl/mem.h>
@@ -1010,8 +1011,7 @@
}
bool ssl_create_cipher_list(UniquePtr<SSLCipherPreferenceList> *out_cipher_list,
- const bool has_aes_hw, const char *rule_str,
- bool strict) {
+ const char *rule_str, bool strict) {
// Return with error if nothing to do.
if (rule_str == nullptr || out_cipher_list == nullptr) {
return false;
@@ -1065,6 +1065,7 @@
// TODO(crbug.com/boringssl/29): We should also set up equipreference groups
// as a server.
size_t num = 0;
+ const bool has_aes_hw = EVP_has_aes_hardware();
if (has_aes_hw) {
for (uint16_t id : kAESCiphers) {
co_list[num++].cipher = SSL_get_cipher_by_value(id);
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index a0c746b..101ca7e 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -351,19 +351,6 @@
return true;
}
-void SSL_CTX_set_aes_hw_override_for_testing(SSL_CTX *ctx,
- bool override_value) {
- auto *ctx_impl = FromOpaque(ctx);
- ctx_impl->aes_hw_override = true;
- ctx_impl->aes_hw_override_value = override_value;
-}
-
-void SSL_set_aes_hw_override_for_testing(SSL *ssl, bool override_value) {
- auto *ssl_impl = FromOpaque(ssl);
- ssl_impl->config->aes_hw_override = true;
- ssl_impl->config->aes_hw_override_value = override_value;
-}
-
BSSL_NAMESPACE_END
using namespace bssl;
@@ -398,8 +385,6 @@
false_start_allowed_without_alpn(false),
handoff(false),
enable_early_data(false),
- aes_hw_override(false),
- aes_hw_override_value(false),
resumption_across_names_enabled(false) {
CRYPTO_new_ex_data(&ex_data);
}
@@ -527,8 +512,6 @@
ssl->config->retain_only_sha256_of_client_certs =
ctx_impl->retain_only_sha256_of_client_certs;
ssl->config->permute_extensions = ctx_impl->permute_extensions;
- ssl->config->aes_hw_override = ctx_impl->aes_hw_override;
- ssl->config->aes_hw_override_value = ctx_impl->aes_hw_override_value;
ssl->config->compliance_policy = ctx_impl->compliance_policy;
if (!ssl->config->supported_group_list.CopyFrom(
@@ -2209,20 +2192,13 @@
int SSL_CTX_set_cipher_list(SSL_CTX *ctx, const char *str) {
auto *ctx_impl = FromOpaque(ctx);
- const bool has_aes_hw = ctx_impl->aes_hw_override
- ? ctx_impl->aes_hw_override_value
- : EVP_has_aes_hardware();
- return ssl_create_cipher_list(&ctx_impl->cipher_list, has_aes_hw, str,
+ return ssl_create_cipher_list(&ctx_impl->cipher_list, str,
false /* not strict */);
}
int SSL_CTX_set_strict_cipher_list(SSL_CTX *ctx, const char *str) {
auto *ctx_impl = FromOpaque(ctx);
- const bool has_aes_hw = ctx_impl->aes_hw_override
- ? ctx_impl->aes_hw_override_value
- : EVP_has_aes_hardware();
- return ssl_create_cipher_list(&ctx_impl->cipher_list, has_aes_hw, str,
- true /* strict */);
+ return ssl_create_cipher_list(&ctx_impl->cipher_list, str, true /* strict */);
}
int SSL_set_cipher_list(SSL *ssl, const char *str) {
@@ -2230,10 +2206,7 @@
if (!ssl_impl->config) {
return 0;
}
- const bool has_aes_hw = ssl_impl->config->aes_hw_override
- ? ssl_impl->config->aes_hw_override_value
- : EVP_has_aes_hardware();
- return ssl_create_cipher_list(&ssl_impl->config->cipher_list, has_aes_hw, str,
+ return ssl_create_cipher_list(&ssl_impl->config->cipher_list, str,
false /* not strict */);
}
@@ -2242,10 +2215,7 @@
if (!ssl_impl->config) {
return 0;
}
- const bool has_aes_hw = ssl_impl->config->aes_hw_override
- ? ssl_impl->config->aes_hw_override_value
- : EVP_has_aes_hardware();
- return ssl_create_cipher_list(&ssl_impl->config->cipher_list, has_aes_hw, str,
+ return ssl_create_cipher_list(&ssl_impl->config->cipher_list, str,
true /* strict */);
}
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 94de72d..6d2031f 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -144,11 +144,8 @@
const uint16_t version = ssl_protocol_version(ssl);
- return ssl_choose_tls13_cipher(cipher_suites,
- ssl->config->aes_hw_override
- ? ssl->config->aes_hw_override_value
- : EVP_has_aes_hardware(),
- version, ssl->config->compliance_policy);
+ return ssl_choose_tls13_cipher(cipher_suites, EVP_has_aes_hardware(), version,
+ ssl->config->compliance_policy);
}
static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) {
