Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 api/envoy/config/core/v3/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,19 @@ message UpstreamHttpProtocolOptions {
// header when :ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>`
// is set, as seen by the :ref:`router filter <config_http_filters_router>`.
// Does nothing if a filter before the http router filter sets the corresponding metadata.
//
// See :ref:`SNI configuration <start_quick_start_securing_sni_client>` for details on how this
// interacts with other validation options.
bool auto_sni = 1;

// Automatic validate upstream presented certificate for new upstream connections based on the
// downstream HTTP host/authority header or any other arbitrary header when :ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>`
// is set, as seen by the :ref:`router filter <config_http_filters_router>`.
// This field is intended to be set with ``auto_sni`` field.
// Does nothing if a filter before the http router filter sets the corresponding metadata.
//
// See :ref:`validation configuration <start_quick_start_securing_validation>` for how this interacts with
// other validation options.
bool auto_san_validation = 2;

// An optional alternative to the host/authority header to be used for setting the SNI value.
Expand Down
22 changes: 21 additions & 1 deletion api/envoy/extensions/transport_sockets/tls/v3/tls.proto
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// [#extension: envoy.transport_sockets.tls]
// The TLS contexts below provide the transport socket configuration for upstream/downstream TLS.

// [#next-free-field: 6]
// [#next-free-field: 8]
message UpstreamTlsContext {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.auth.UpstreamTlsContext";
Expand All @@ -42,6 +42,26 @@ message UpstreamTlsContext {
// SNI string to use when creating TLS backend connections.
string sni = 2 [(validate.rules).string = {max_bytes: 255}];

// If true, replaces the SNI for the connection with the hostname of the upstream host, if
// the hostname is known due to either a DNS cluster type or the
// :ref:`hostname <envoy_v3_api_field_config.endpoint.v3.Endpoint.hostname>` is set on
// the host.
//
// See :ref:`SNI configuration <start_quick_start_securing_sni_client>` for details on how this
// interacts with other validation options.
bool auto_host_sni = 6;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How is this intended to relate to the auto_sni here:
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#config-core-v3-upstreamhttpprotocoloptions
And ditto for auto_san_validation vs auto_sni_san_validation?


// If true, replace any Subject Alternative Name validations with a validation for a DNS SAN matching
// the SNI value sent. Note that the validation will be against the actual requested SNI, regardless of how it
// is configured.
//
// For the common case where an SNI value is sent and it is expected that the server certificate contains a SAN
// matching that SNI value, this option will do the correct SAN validation.
//
// See :ref:`validation configuration <start_quick_start_securing_validation>` for how this interacts with
// other validation options.
bool auto_sni_san_validation = 7;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think I quite understand the semantics here. Perhaps we could have an example?


// If true, server-initiated TLS renegotiation will be allowed.
//
// .. attention::
Expand Down
10 changes: 10 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ new_features:
- area: tls
change: |
Added support for P-384 and P-521 curves for TLS server certificates.
- area: tls
change: |
Added an :ref:`option
<envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_host_sni>` to change the upstream
SNI to the configured hostname for the upstream.
- area: tls
change: |
Added an :ref:`option
<envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_sni_san_validation>` to validate
the upstream server certificate SANs against the actual SNI value sent, regardless of the method of configuring SNI.
- area: xds
change: |
Added support for ADS replacement by invoking ``xdsManager().setAdsConfigSource()`` with a new config source.
Expand Down
9 changes: 1 addition & 8 deletions docs/root/faq/configuration/sni.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,4 @@ The following is a YAML example of the above requirement.
How do I configure SNI for clusters?
====================================

For clusters, a fixed SNI can be set in :ref:`sni <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.sni>`.
To derive SNI from a downstream HTTP header like, ``host`` or ``:authority``, turn on
:ref:`auto_sni <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_sni>` to override the fixed SNI in
:ref:`UpstreamTlsContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.UpstreamTlsContext>`. A custom header other than the ``host`` or ``:authority`` can also be supplied using the optional
:ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>` field.
If upstream will present certificates with the hostname in SAN, turn on
:ref:`auto_san_validation <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_san_validation>` too.
It still needs a trust CA in validation context in :ref:`UpstreamTlsContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.UpstreamTlsContext>` for trust anchor.
See :ref:`SNI configuration <start_quick_start_securing_sni_client>` and :ref:`validation configuration <start_quick_start_securing_validation>`.
8 changes: 8 additions & 0 deletions docs/root/start/quick-start/_include/envoy-demo-tls-sni.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,11 @@ static_resources:
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
sni: www.envoyproxy.io
common_tls_context:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this because I didn't want an example to not have a validation_context. That makes it a dangerously-insecure example.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

probably we can use 1 config file and just highlight different parts - i can follow up with this

validation_context:
trusted_ca:
filename: certs/cacert.pem
match_typed_subject_alt_names:
- san_type: DNS
matcher:
exact: www.envoyproxy.io
30 changes: 25 additions & 5 deletions docs/root/start/quick-start/securing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,22 @@ Firstly, you can ensure that the certificates are from a mutually trusted certif
:emphasize-lines: 8-11
:caption: :download:`envoy-demo-tls-validation.yaml <_include/envoy-demo-tls-validation.yaml>`

You can also ensure that the "Subject Alternative Names" for the cerficate match.
You should also ensure that the Subject Alternative Names (SANs) for the certificate match.

This is commonly used by web certificates (X.509) to identify the domain or domains that a
certificate is valid for.

For the most common case where the certificate should have a SAN matching the :ref:`SNI <start_quick_start_securing_sni_client>`
which was sent, you can enable :ref:`auto_sni_san_validation <envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_sni_san_validation>`
and omit :ref:`match_typed_subject_alt_names <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.match_typed_subject_alt_names>`
in the validation context.

To validate that a certificate has a SAN matching the downstream request ``host`` or ``:authority`` header, you can enable
:ref:`auto_san_validation <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_san_validation>`.

When multiple validation options are configured, ``auto_san_validation`` has the highest priority, followed by ``auto_sni_san_validation``,
followed by ``match_typed_subject_alt_names``.

.. literalinclude:: _include/envoy-demo-tls-validation.yaml
:language: yaml
:linenos:
Expand All @@ -97,10 +108,6 @@ certificate is valid for.
:emphasize-lines: 6-7, 10-11
:caption: :download:`envoy-demo-tls-validation.yaml <_include/envoy-demo-tls-validation.yaml>`

.. note::

If the "Subject Alternative Names" for a certificate are for a wildcard domain, eg ``*.example.com``,
Copy link
Copy Markdown
Member Author

@ggreenway ggreenway Nov 7, 2024

Choose a reason for hiding this comment

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

This note didn't make sense; Envoy will correctly validate an expected SAN of test.example.com against a certificate with *.example.com (Envoy considers that a match)

Copy link
Copy Markdown
Member

@phlax phlax Nov 7, 2024

Choose a reason for hiding this comment

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

it was quite a while ago when i added this - but i thought i added it after testing it out

Copy link
Copy Markdown
Member Author

@ggreenway ggreenway Nov 7, 2024

Choose a reason for hiding this comment

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

It's documented in match_typed_subject_alt_names; this definitely works

// For example if the certificate has "\*.example.com" as DNS SAN entry, to allow only "api.example.com",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hmm - not sure - but im wondering if there are some circumstances where it doesnt work

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If there are, it's a bug and we should fix it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if i get some time ill try and retry the steps i might have taken

this is what you should use when matching with ``match_typed_subject_alt_names``.

.. note::

Expand Down Expand Up @@ -204,3 +211,16 @@ When connecting to an Envoy endpoint that is protected by ``SNI``, this must mat
:ref:`server_names <envoy_v3_api_field_config.listener.v3.FilterChainMatch.server_names>` set in the endpoint's
:ref:`filter_chain_match <envoy_v3_api_msg_config.listener.v3.FilterChainMatch>`, as
:ref:`described above <start_quick_start_securing_sni>`.

To derive SNI from a downstream HTTP header ``host`` or ``:authority``, turn on
:ref:`auto_sni <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.auto_sni>` to override the fixed SNI in
:ref:`UpstreamTlsContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.UpstreamTlsContext>`. A custom header other than
the ``host`` or ``:authority`` can also be supplied using the optional
:ref:`override_auto_sni_header <envoy_v3_api_field_config.core.v3.UpstreamHttpProtocolOptions.override_auto_sni_header>` field.

To derive SNI from the host Envoy is connecting to, turn on :ref:`auto_host_sni
<envoy_v3_api_field_extensions.transport_sockets.tls.v3.UpstreamTlsContext.auto_host_sni>`, which will use the hostname
of the upstream endpoint.

When multiple options are configured, ``auto_sni`` has the highest priority, followed by ``auto_host_sni``, followed by
the fixed ``sni``.
2 changes: 2 additions & 0 deletions envoy/common/optref.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ template <class T> struct OptRef {
*/
bool has_value() const { return ptr_ != nullptr; }

T& value_or(T& other) const { return has_value() ? *ptr_ : other; }

/**
* @return true if the object has a value.
*/
Expand Down
12 changes: 12 additions & 0 deletions envoy/ssl/certificate_validation_context_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ class CertificateValidationContextConfig {
* @return the max depth used when verifying the certificate-chain
*/
virtual absl::optional<uint32_t> maxVerifyDepth() const PURE;

/**
* @return true if the SAN validation rules should be replaced with a rule to validate that the
* certificate matches the transmitted SNI.
*/
virtual bool autoSniSanMatch() const PURE;

// SECURITY NOTE
//
// When adding or changing this interface, it is likely that a change is needed to
// `DefaultCertValidator::updateDigestForSessionId` in
// `source/common/tls/cert_validator/default_validator.cc`.
};

using CertificateValidationContextConfigPtr = std::unique_ptr<CertificateValidationContextConfig>;
Expand Down
13 changes: 13 additions & 0 deletions envoy/ssl/context_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,19 @@ class ClientContextConfig : public virtual ContextConfig {
*/
virtual const std::string& serverNameIndication() const PURE;

/**
* If true, replaces the SNI for the connection with the hostname of the upstream host, if
* the hostname is known.
*/
virtual bool autoHostServerNameIndication() const PURE;

/**
* If true, replace any Subject Alternative Name validations with a validation for a DNS SAN
* matching the SNI value sent. Note that the validation will be against the actual requested SNI,
* regardless of how it is configured.
*/
virtual bool autoSniSanMatch() const PURE;

/**
* @return true if server-initiated TLS renegotiation will be allowed.
*/
Expand Down
14 changes: 14 additions & 0 deletions source/common/common/matchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher {
}
}

// Helper to create an exact matcher in contexts where there is no factory context available.
// This is a static member instead of constructor so that it can be named for clarity of what it
// produces.
static StringMatcherImpl createExactMatcher(absl::string_view match) {
return StringMatcherImpl(match);
}

// StringMatcher
bool match(const absl::string_view value) const override {
switch (matcher_.match_pattern_case()) {
Expand Down Expand Up @@ -165,6 +172,13 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher {
}

private:
StringMatcherImpl(absl::string_view exact_match)
: matcher_([&]() -> StringMatcherType {
StringMatcherType cfg;
cfg.set_exact(exact_match);
return cfg;
}()) {}

const StringMatcherType matcher_;
Regex::CompiledMatcherPtr regex_;
std::string lowercase_contains_match_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static const std::string INLINE_STRING = "<inline>";

CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl(
const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config,
Api::Api& api)
bool auto_sni_san_match, Api::Api& api)
: ca_cert_(THROW_OR_RETURN_VALUE(
THROW_OR_RETURN_VALUE(Config::DataSource::read(config.trusted_ca(), true, api),
std::string),
Expand Down Expand Up @@ -47,14 +47,15 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl(
api_(api), only_verify_leaf_cert_crl_(config.only_verify_leaf_cert_crl()),
max_verify_depth_(config.has_max_verify_depth()
? absl::optional<uint32_t>(config.max_verify_depth().value())
: absl::nullopt) {}
: absl::nullopt),
auto_sni_san_match_(auto_sni_san_match) {}

absl::StatusOr<std::unique_ptr<CertificateValidationContextConfigImpl>>
CertificateValidationContextConfigImpl::create(
const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& context,
Api::Api& api) {
bool auto_sni_san_match, Api::Api& api) {
auto config = std::unique_ptr<CertificateValidationContextConfigImpl>(
new CertificateValidationContextConfigImpl(context, api));
new CertificateValidationContextConfigImpl(context, auto_sni_san_match, api));
absl::Status status = config->initialize();
if (status.ok()) {
return config;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte
// Create a CertificateValidationContextConfigImpl or return an error status.
static absl::StatusOr<std::unique_ptr<CertificateValidationContextConfigImpl>>
create(const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& context,
Api::Api& api);
bool auto_sni_san_match, Api::Api& api);

absl::Status initialize();

Expand Down Expand Up @@ -58,10 +58,12 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte

absl::optional<uint32_t> maxVerifyDepth() const override { return max_verify_depth_; }

bool autoSniSanMatch() const override { return auto_sni_san_match_; }

protected:
CertificateValidationContextConfigImpl(
const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext& config,
Api::Api& api);
bool auto_sni_san_match, Api::Api& api);

private:
static std::vector<envoy::extensions::transport_sockets::tls::v3::SubjectAltNameMatcher>
Expand All @@ -82,6 +84,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte
Api::Api& api_;
const bool only_verify_leaf_cert_crl_;
absl::optional<uint32_t> max_verify_depth_;
const bool auto_sni_san_match_;
};

} // namespace Ssl
Expand Down
52 changes: 41 additions & 11 deletions source/common/tls/cert_validator/default_validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ namespace Tls {
DefaultCertValidator::DefaultCertValidator(
const Envoy::Ssl::CertificateValidationContextConfig* config, SslStats& stats,
Server::Configuration::CommonFactoryContext& context)
: config_(config), stats_(stats), context_(context) {
: config_(config), stats_(stats), context_(context),
auto_sni_san_match_(config_ != nullptr ? config_->autoSniSanMatch() : false) {
if (config_ != nullptr) {
allow_untrusted_certificate_ = config_->trustChainVerification() ==
envoy::extensions::transport_sockets::tls::v3::
Expand Down Expand Up @@ -118,6 +119,12 @@ absl::StatusOr<int> DefaultCertValidator::initializeSslContexts(std::vector<SSL_
}
}

// Disallow insecure configuration.
if (config_ != nullptr && config_->autoSniSanMatch() && !verify_trusted_ca_) {
return absl::InvalidArgumentError(
"'auto_sni_san_validation' was configured without configuring a trusted CA");
}

if (config_ != nullptr && !config_->certificateRevocationList().empty()) {
bssl::UniquePtr<BIO> bio(
BIO_new_mem_buf(const_cast<char*>(config_->certificateRevocationList().data()),
Expand Down Expand Up @@ -193,15 +200,26 @@ absl::StatusOr<int> DefaultCertValidator::initializeSslContexts(std::vector<SSL_
}

bool DefaultCertValidator::verifyCertAndUpdateStatus(
X509* leaf_cert, const Network::TransportSocketOptions* transport_socket_options,
X509* leaf_cert, absl::string_view sni,
const Network::TransportSocketOptions* transport_socket_options,
Envoy::Ssl::ClientValidationStatus& detailed_status, std::string* error_details,
uint8_t* out_alert) {
Envoy::Ssl::ClientValidationStatus validated =
verifyCertificate(leaf_cert,
transport_socket_options != nullptr
? transport_socket_options->verifySubjectAltNameListOverride()
: std::vector<std::string>{},
subject_alt_name_matchers_, error_details, out_alert);

std::vector<SanMatcherPtr> match_sni_san;
OptRef<const std::vector<std::string>> verify_san_override;
OptRef<const std::vector<SanMatcherPtr>> match_san_override;
if (transport_socket_options != nullptr &&
!transport_socket_options->verifySubjectAltNameListOverride().empty()) {
// TODO(ggreenway): this validation should be part of `match_sni_san` so that the type is
// validated as a DNS SAN, but this change will require a runtime flag for the behavior change.
verify_san_override = transport_socket_options->verifySubjectAltNameListOverride();
} else if (auto_sni_san_match_ && !sni.empty()) {
match_sni_san.push_back(std::make_unique<StringSanMatcher>(sni));
match_san_override = match_sni_san;
}
Envoy::Ssl::ClientValidationStatus validated = verifyCertificate(
leaf_cert, verify_san_override.value_or(std::vector<std::string>()),
match_san_override.value_or(subject_alt_name_matchers_), error_details, out_alert);

if (detailed_status == Envoy::Ssl::ClientValidationStatus::NotValidated ||
validated != Envoy::Ssl::ClientValidationStatus::NotValidated) {
Expand Down Expand Up @@ -281,7 +299,7 @@ ValidationResults DefaultCertValidator::doVerifyCertChain(
STACK_OF(X509)& cert_chain, Ssl::ValidateResultCallbackPtr /*callback*/,
const Network::TransportSocketOptionsConstSharedPtr& transport_socket_options, SSL_CTX& ssl_ctx,
const CertValidator::ExtraValidationContext& /*validation_context*/, bool is_server,
absl::string_view /*host_name*/) {
absl::string_view host_name) {
if (sk_X509_num(&cert_chain) == 0) {
stats_.fail_verify_error_.inc();
const char* error = "verify cert failed: empty cert chain";
Expand Down Expand Up @@ -332,8 +350,9 @@ ValidationResults DefaultCertValidator::doVerifyCertChain(
}
std::string error_details;
uint8_t tls_alert = SSL_AD_CERTIFICATE_UNKNOWN;
const bool succeeded = verifyCertAndUpdateStatus(leaf_cert, transport_socket_options.get(),
detailed_status, &error_details, &tls_alert);
const bool succeeded =
verifyCertAndUpdateStatus(leaf_cert, host_name, transport_socket_options.get(),
detailed_status, &error_details, &tls_alert);
return succeeded ? ValidationResults{ValidationResults::ValidationStatus::Successful,
detailed_status, absl::nullopt, absl::nullopt}
: ValidationResults{ValidationResults::ValidationStatus::Failed, detailed_status,
Expand Down Expand Up @@ -475,6 +494,17 @@ void DefaultCertValidator::updateDigestForSessionId(bssl::ScopedEVP_MD_CTX& md,
auto only_leaf_crl = config_->onlyVerifyLeafCertificateCrl();
rc = EVP_DigestUpdate(md.get(), &only_leaf_crl, sizeof(only_leaf_crl));
RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or(""));

auto max_verify_depth_opt = config_->maxVerifyDepth();
if (max_verify_depth_opt.has_value()) {
auto max_verify_depth = *max_verify_depth_opt;
rc = EVP_DigestUpdate(md.get(), &max_verify_depth, sizeof(max_verify_depth));
RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or(""));
}

bool auto_sni_san_match = config_->autoSniSanMatch();
rc = EVP_DigestUpdate(md.get(), &auto_sni_san_match, sizeof(auto_sni_san_match));
RELEASE_ASSERT(rc == 1, Utility::getLastCryptoError().value_or(""));
}
}

Expand Down
Loading