Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
bc111ba
[tls] SslHandshakerImpl now takes an int argument
ambuc Aug 14, 2020
e3e8dbc
[tls] Move ssl_handshaker_impl to its own target
ambuc Aug 14, 2020
5b718f4
SslHandshakerImpl construction is now a method of ContextConfig.
ambuc Aug 14, 2020
779758a
[api] Add custom_listener_handshaker
ambuc Aug 14, 2020
80a4d12
SslHandshakerImpl construction is now pluggable
ambuc Aug 14, 2020
a81686e
Add SslHandshakerImplForTest class and new :handshaker_test
ambuc Aug 14, 2020
ae2dd4e
[tls] Add requireCertificates() restriction
ambuc Aug 14, 2020
de92887
[misc] fix spelling issue
ambuc Aug 17, 2020
11d01e3
[tls] Initialize |len| in handshaker_test.
ambuc Aug 17, 2020
cf1ddd0
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 20, 2020
60d94a5
[tls] Expose connection(), not connectionState().
ambuc Aug 20, 2020
e12eadc
[tls] lowercase test helper methods
ambuc Aug 20, 2020
770c5b0
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 21, 2020
62874c2
[tls] requireCertificates should guard all certificate checks in Serv…
ambuc Aug 21, 2020
399b473
[misc] Rename custom_listener_handshaker to custom_handshaker
ambuc Aug 21, 2020
5637755
[tls] Add HandshakerRequirements struct to enable/disable SSL calls i…
ambuc Aug 21, 2020
3547b6b
[misc] Put SSL_CTX methods in backticks to pass spell check.
ambuc Aug 24, 2020
a981e60
[misc] Reword a sentence to pass check_spelling.
ambuc Aug 24, 2020
9263258
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 24, 2020
90454cc
[tls] Fix build failure for :handshaker_test.
ambuc Aug 24, 2020
355de2e
[tls] Fix release failures in :handshaker_test.
ambuc Aug 25, 2020
f976151
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 25, 2020
23a2685
[tls] Fix clang_tidy failures in :handshaker_test.
ambuc Aug 25, 2020
e9442e7
[tls] Redefine HandshakerCapabilities interface.
ambuc Aug 26, 2020
231399e
[tls] Fix typo in handshaker_interface
ambuc Aug 26, 2020
3c4644c
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 26, 2020
f0dfb8b
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 27, 2020
d787de2
[tls] Fix ifdef BORINGSSL_FIPS declaration
ambuc Aug 28, 2020
f6d91d6
[tls] s/verifies_certificates/verifies_peer_certificates/g
ambuc Aug 28, 2020
3af7c51
[tls] Collapse sets_timeout and handles_session_tickets into handles_…
ambuc Aug 28, 2020
7df8347
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Aug 28, 2020
a625798
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Sep 8, 2020
9023817
[misc] Run fix_format.
ambuc Sep 8, 2020
c28d2fa
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Sep 10, 2020
16e366a
[tls] Better capability-protect cert loading
ambuc Sep 10, 2020
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: 5 additions & 1 deletion api/envoy/extensions/transport_sockets/tls/v3/tls.proto
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ message DownstreamTlsContext {
}

// TLS context shared by both client and server TLS contexts.
// [#next-free-field: 13]
// [#next-free-field: 14]
message CommonTlsContext {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.auth.CommonTlsContext";

Expand Down Expand Up @@ -238,4 +238,8 @@ message CommonTlsContext {
//
// There is no default for this parameter. If empty, Envoy will not expose ALPN.
repeated string alpn_protocols = 4;

// Custom TLS handshaker. If empty, defaults to native TLS handshaking
// behavior.
config.core.v3.TypedExtensionConfig custom_handshaker = 13;
}
6 changes: 5 additions & 1 deletion api/envoy/extensions/transport_sockets/tls/v4alpha/tls.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions include/envoy/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ envoy_cc_library(
hdrs = ["context_config.h"],
deps = [
":certificate_validation_context_config_interface",
":handshaker_interface",
":tls_certificate_config_interface",
],
)
Expand Down Expand Up @@ -81,7 +82,10 @@ envoy_cc_library(
hdrs = ["handshaker.h"],
external_deps = ["ssl"],
deps = [
"//include/envoy/api:api_interface",
"//include/envoy/config:typed_config_interface",
"//include/envoy/network:connection_interface",
"//include/envoy/network:post_io_action_interface",
"//include/envoy/protobuf:message_validator_interface",
],
)
11 changes: 11 additions & 0 deletions include/envoy/ssl/context_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "envoy/common/pure.h"
#include "envoy/ssl/certificate_validation_context_config.h"
#include "envoy/ssl/handshaker.h"
#include "envoy/ssl/tls_certificate_config.h"

#include "absl/types/optional.h"
Expand Down Expand Up @@ -73,6 +74,16 @@ class ContextConfig {
* @param callback callback that is executed by context config.
*/
virtual void setSecretUpdateCallback(std::function<void()> callback) PURE;

/**
* @return a callback which can be used to create Handshaker instances.
*/
virtual HandshakerFactoryCb createHandshaker() const PURE;

/**
* @return the set of capabilities for handshaker instances created by this context.
*/
virtual HandshakerCapabilities capabilities() const PURE;
};

class ClientContextConfig : public virtual ContextConfig {
Expand Down
72 changes: 70 additions & 2 deletions include/envoy/ssl/handshaker.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
#pragma once

#include "envoy/api/api.h"
#include "envoy/config/typed_config.h"
#include "envoy/network/connection.h"
#include "envoy/network/post_io_action.h"
#include "envoy/protobuf/message_validator.h"

#include "openssl/ssl.h"

Expand All @@ -13,9 +16,9 @@ class HandshakeCallbacks {
virtual ~HandshakeCallbacks() = default;

/**
* @return the connection state.
* @return the connection.
*/
virtual Network::Connection::State connectionState() const PURE;
virtual Network::Connection& connection() const PURE;

/**
* A callback which will be executed at most once upon successful completion
Expand Down Expand Up @@ -43,5 +46,70 @@ class Handshaker {
virtual Network::PostIoAction doHandshake() PURE;
};

using HandshakerSharedPtr = std::shared_ptr<Handshaker>;
using HandshakerFactoryCb =
std::function<HandshakerSharedPtr(bssl::UniquePtr<SSL>, int, HandshakeCallbacks*)>;

class HandshakerFactoryContext {
public:
virtual ~HandshakerFactoryContext() = default;

/**
* @return reference to the Api object
*/
virtual Api::Api& api() PURE;

/**
* The list of supported protocols exposed via ALPN, from ContextConfig.
*/
virtual absl::string_view alpnProtocols() const PURE;
};

struct HandshakerCapabilities {
// Whether or not a handshaker implementation provides certificates itself.
bool provides_certificates = false;

// Whether or not a handshaker implementation verifies certificates itself.
bool verifies_peer_certificates = false;

// Whether or not a handshaker implementation handles session resumption
// itself.
bool handles_session_resumption = false;

// Whether or not a handshaker implementation provides its own list of ciphers
// and curves.
bool provides_ciphers_and_curves = false;

// Whether or not a handshaker implementation handles ALPN selection.
bool handles_alpn_selection = false;

// Should return true if this handshaker is FIPS-compliant.
// Envoy will fail to compile if this returns true and `--define=boringssl=fips`.
bool is_fips_compliant = true;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

This is way too fine-grained, IMHO (it should be expressed as a list of custom handshaker capabilities, e.g. provides_certificates, handles_session_resumption, handles_peer_verification, rather than a list of BoringSSL functions to call, otherwise this will be extremely sensitive to any changes).

But looking at this now, I think we should move all those configuration settings to the handshaker, otherwise it has no way to learn about them (e.g. which cipher suites it should use, etc.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it makes sense to move these configurations to the handshaker, since no handshaker object exists during the instantiation of (for example) ServerContextConfigImpl, which needs to know which of these features it needs to fulfill and which of these features it can leave to the handshaker itself. The current design, which exposes these features as a part of the factory which creates the handshaker, solves that problem.

I agree that these permissions are too fine-grained and brittle. On that note, I've renamed this HandshakerCapabilities and reversed the polarity of the booleans; handshaker implementations now get to declare that they have the capability to handle feature Foo natively; otherwise Envoy will handle it as normal. The default implementation obviously has none of these capabilities, and defers to Envoy.

I have also renamed and condensed these capabilities somewhat to make them more general. PTAL.


class HandshakerFactory : public Config::TypedFactory {
public:
/**
* @returns a callback to create a Handshaker. Accepts the |config| and
* |validation_visitor| for early validation. This virtual base doesn't
* perform MessageUtil::downcastAndValidate, but an implementation should.
*/
virtual HandshakerFactoryCb
createHandshakerCb(const Protobuf::Message& message,
HandshakerFactoryContext& handshaker_factory_context,
ProtobufMessage::ValidationVisitor& validation_visitor) PURE;

std::string category() const override { return "envoy.tls_handshakers"; }

/**
* Implementations should return a struct with their capabilities. See
* HandshakerCapabilities above. For any capability a Handshaker
* implementation explicitly declares, Envoy will not also configure that SSL
* capability.
*/
virtual HandshakerCapabilities capabilities() const PURE;
};

} // namespace Ssl
} // namespace Envoy
26 changes: 26 additions & 0 deletions source/extensions/transport_sockets/tls/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,30 @@ envoy_cc_extension(
],
)

envoy_cc_library(
name = "ssl_handshaker_lib",
srcs = ["ssl_handshaker.cc"],
hdrs = ["ssl_handshaker.h"],
external_deps = ["ssl"],
deps = [
":context_lib",
":utility_lib",
"//include/envoy/network:connection_interface",
"//include/envoy/network:transport_socket_interface",
"//include/envoy/ssl:handshaker_interface",
"//include/envoy/ssl:ssl_socket_extended_info_interface",
"//include/envoy/ssl:ssl_socket_state",
"//include/envoy/ssl/private_key:private_key_callbacks_interface",
"//include/envoy/ssl/private_key:private_key_interface",
"//include/envoy/stats:stats_macros",
"//source/common/common:assert_lib",
"//source/common/common:empty_string",
"//source/common/common:minimal_logger_lib",
"//source/common/common:thread_annotations",
"//source/common/http:headers_lib",
],
)

envoy_cc_library(
name = "io_handle_bio_lib",
srcs = ["io_handle_bio.cc"],
Expand Down Expand Up @@ -56,6 +80,7 @@ envoy_cc_library(
":context_config_lib",
":context_lib",
":io_handle_bio_lib",
":ssl_handshaker_lib",
":utility_lib",
"//include/envoy/network:connection_interface",
"//include/envoy/network:transport_socket_interface",
Expand Down Expand Up @@ -83,6 +108,7 @@ envoy_cc_library(
# TLS is core functionality.
visibility = ["//visibility:public"],
deps = [
":ssl_handshaker_lib",
"//include/envoy/secret:secret_callbacks_interface",
"//include/envoy/secret:secret_provider_interface",
"//include/envoy/server:transport_socket_config_interface",
Expand Down
39 changes: 33 additions & 6 deletions source/extensions/transport_sockets/tls/context_config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "common/secret/sds_api.h"
#include "common/ssl/certificate_validation_context_config_impl.h"

#include "extensions/transport_sockets/tls/ssl_handshaker.h"

#include "openssl/ssl.h"

namespace Envoy {
Expand Down Expand Up @@ -213,6 +215,25 @@ ContextConfigImpl::ContextConfigImpl(
}
}
}

HandshakerFactoryContextImpl handshaker_factory_context(api_, alpn_protocols_);
Ssl::HandshakerFactory* handshaker_factory;
if (config.has_custom_handshaker()) {
// If a custom handshaker is configured, derive the factory from the config.
const auto& handshaker_config = config.custom_handshaker();
handshaker_factory =
&Config::Utility::getAndCheckFactory<Ssl::HandshakerFactory>(handshaker_config);
handshaker_factory_cb_ = handshaker_factory->createHandshakerCb(
handshaker_config.typed_config(), handshaker_factory_context,
factory_context.messageValidationVisitor());
} else {
// Otherwise, derive the config from the default factory.
handshaker_factory = HandshakerFactoryImpl::getDefaultHandshakerFactory();
handshaker_factory_cb_ = handshaker_factory->createHandshakerCb(
*handshaker_factory->createEmptyConfigProto(), handshaker_factory_context,
factory_context.messageValidationVisitor());
}
capabilities_ = handshaker_factory->capabilities();
}

Ssl::CertificateValidationContextConfigPtr ContextConfigImpl::getCombinedValidationContextConfig(
Expand Down Expand Up @@ -270,6 +291,10 @@ void ContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback)
}
}

Ssl::HandshakerFactoryCb ContextConfigImpl::createHandshaker() const {
return handshaker_factory_cb_;
}

ContextConfigImpl::~ContextConfigImpl() {
if (tc_update_callback_handle_) {
tc_update_callback_handle_->remove();
Expand Down Expand Up @@ -400,12 +425,14 @@ ServerContextConfigImpl::ServerContextConfigImpl(
}
}

if ((config.common_tls_context().tls_certificates().size() +
config.common_tls_context().tls_certificate_sds_secret_configs().size()) == 0) {
throw EnvoyException("No TLS certificates found for server context");
} else if (!config.common_tls_context().tls_certificates().empty() &&
!config.common_tls_context().tls_certificate_sds_secret_configs().empty()) {
throw EnvoyException("SDS and non-SDS TLS certificates may not be mixed in server contexts");
if (!capabilities().provides_certificates) {
if ((config.common_tls_context().tls_certificates().size() +
config.common_tls_context().tls_certificate_sds_secret_configs().size()) == 0) {
throw EnvoyException("No TLS certificates found for server context");
} else if (!config.common_tls_context().tls_certificates().empty() &&
!config.common_tls_context().tls_certificate_sds_secret_configs().empty()) {
throw EnvoyException("SDS and non-SDS TLS certificates may not be mixed in server contexts");
}
}

if (config.has_session_timeout()) {
Expand Down
5 changes: 5 additions & 0 deletions source/extensions/transport_sockets/tls/context_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
}

void setSecretUpdateCallback(std::function<void()> callback) override;
Ssl::HandshakerFactoryCb createHandshaker() const override;
Ssl::HandshakerCapabilities capabilities() const override { return capabilities_; }

Ssl::CertificateValidationContextConfigPtr getCombinedValidationContextConfig(
const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext&
Expand Down Expand Up @@ -94,6 +96,9 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
Envoy::Common::CallbackHandle* cvc_validation_callback_handle_{};
const unsigned min_protocol_version_;
const unsigned max_protocol_version_;

Ssl::HandshakerFactoryCb handshaker_factory_cb_;
Ssl::HandshakerCapabilities capabilities_;
};

class ClientContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::ClientContextConfig {
Expand Down
Loading