Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ behavior_changes:
The previous flag for specifying FIPS builds (ie ``--define=boringssl=fips``) will no longer work and
has been replaced by ``--config=boringssl-fips``. This change will allow us to better support custom
SSL libraries, and will allow FIPS-compliant Envoy to be built with the imminent switch to Bazel bzlmod.
- area: tls
change: |
Set
:ref:`enforce_rsa_key_usage <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.enforce_rsa_key_usage>`
to true by default. The handshake will fail if the keyUsage extension is present and incompatible with the TLS usage. In
the next version of Envoy, this option will be removed and the enforcing behavior will always be used.

minor_behavior_changes:
# *Changes that may cause incompatibilities for some users, but should not for most*
Expand Down
21 changes: 20 additions & 1 deletion source/common/tls/context_config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,27 @@ ClientContextConfigImpl::ClientContextConfigImpl(
FIPS_mode() ? DEFAULT_CURVES_FIPS : DEFAULT_CURVES, factory_context, creation_status),
server_name_indication_(config.sni()), auto_host_sni_(config.auto_host_sni()),
allow_renegotiation_(config.allow_renegotiation()),
enforce_rsa_key_usage_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, enforce_rsa_key_usage, false)),
enforce_rsa_key_usage_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, enforce_rsa_key_usage, true)),
max_session_keys_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_session_keys, 1)) {

if (!config.has_enforce_rsa_key_usage()) {
ENVOY_LOG(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we shouldn't warn in this case, and just rely on the release note. With this warning, there's no way to launch envoy without some warning logged: either this message if it's unset, a log of using a deprecated feature if it is set and is true, and a log of using a deprecated feature and the below log message if its set and false. If we remove this case and only warn for !enforce_rsa_key_usage_ then people who are using the new behavior don't get a warning, which seems fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. Thanks for suggestions!

warn,
"The 'enforce_rsa_key_usage' option is not configured, its default value is changed to "
"true, and the config option will be deprecated in the next version. The handshake will "
"fail "
"if the keyUsage extension is present and incompatible with the "
"TLS usage. Please update the certificates to be compliant.");
} else if (!enforce_rsa_key_usage_) {
ENVOY_LOG(
warn,
"The 'enforce_rsa_key_usage' option is set to false, which disables the enforcement of RSA "
"key usage. This option will be deprecated in the next version. The handshake will fail "
"if the keyUsage extension is present and incompatible with the "
"TLS usage. Please update the certificates to be compliant.");
factory_context.serverFactoryContext().runtime().countDeprecatedFeatureUse();
}

// BoringSSL treats this as a C string, so embedded NULL characters will not
// be handled correctly.
if (server_name_indication_.find('\0') != std::string::npos) {
Expand Down
3 changes: 2 additions & 1 deletion source/common/tls/context_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ struct CertificateValidationContextConfigProviderSharedPtrWithName {
Secret::CertificateValidationContextConfigProviderSharedPtr provider_;
};

class ContextConfigImpl : public virtual Ssl::ContextConfig {
class ContextConfigImpl : public virtual Ssl::ContextConfig,
public Logger::Loggable<Logger::Id::misc> {
public:
// Ssl::ContextConfig
const std::string& alpnProtocols() const override { return alpn_protocols_; }
Expand Down
2 changes: 0 additions & 2 deletions test/common/tls/ssl_socket_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7906,8 +7906,6 @@ TEST_P(SslSocketTest, RsaKeyUsageVerificationEnforcementOn) {

envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context;

// Enable the rsa_key_usage enforcement.
client_tls_context.mutable_enforce_rsa_key_usage()->set_value(true);
TestUtilOptionsV2 test_options(listener, client_tls_context, /*expect_success=*/false, version_,
/*skip_server_failure_reason_check=*/true);
// Client connection is failed with key_usage_mismatch.
Expand Down
Loading