Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tls] Add a custom listener handshaker for TLS. #12075

Closed
wants to merge 42 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
647e23c
[tls] Add a custom listener handshaker for TLS.
ambuc Jul 14, 2020
a497d44
[misc] run fix_format
ambuc Jul 14, 2020
df9dfb2
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Jul 15, 2020
22f2003
[misc] run fix_format
ambuc Jul 15, 2020
a403972
[ssl] Rename some HandshakerCallbacks methods.
ambuc Jul 20, 2020
59d5905
[tls] Rename HandOff/HandBack.
ambuc Jul 20, 2020
2168d8b
[misc] run fix_format
ambuc Jul 20, 2020
44b31b8
[misc] remove comment formatting on PemPasswordCallback, breaking che…
ambuc Jul 20, 2020
78656cc
[misc] fix typo in handshaker_test
ambuc Jul 20, 2020
2240f09
[tls] omit rwflag argument entirely
ambuc Jul 20, 2020
e6de2a1
[ssl] Remove unnecessary ctor and max_proto_version
ambuc Jul 20, 2020
bff00b6
[ssl] initialize() takes a reference
ambuc Jul 20, 2020
1c2a650
Handshakers now raise Connected as part of OnSuccessCb.
ambuc Jul 20, 2020
aaae5bf
[tls] HandshakerFactory is of category 'envoy.tls_handshakers'.
ambuc Jul 20, 2020
1266425
[tls] More specific documentation on DoHandshake.
ambuc Jul 20, 2020
d9db724
[tls] Factory should hold the config message, not context config
ambuc Jul 21, 2020
48a840e
[tls] Introduce a default HandshakerFactoryImpl
ambuc Jul 21, 2020
32c5095
[tls] requireCertificates as a method on the factory, not the handsha…
ambuc Jul 21, 2020
3b94f43
[tls] Add test for HandshakerWithOutOfProcessComponent
ambuc Jul 21, 2020
08e4183
[tls] Clarify guidance on doHandshake() handling nullptrs
ambuc Jul 21, 2020
0dc6bde
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Jul 21, 2020
480e433
[misc] Run fix_format.
ambuc Jul 21, 2020
61b7d5c
[tls] Move SSL into Handshaker and remove extra HandshakerCallbacks m…
ambuc Jul 22, 2020
508ea7a
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Jul 22, 2020
2e21ee8
[tls] Remove ::initialize() method
ambuc Jul 22, 2020
51ace82
[tls] Add test with example HandshakerImpl demonstrating special case…
ambuc Jul 23, 2020
5dbb557
Merge branch 'master' of github.com:envoyproxy/envoy into custom-hand…
ambuc Jul 23, 2020
4e3af18
[tls] handshaker_factory_ must be a reseatable reference
ambuc Jul 24, 2020
78e3f2f
[tls] handshaker_test.cc calls SSL_set_cert_cb in ctor
ambuc Jul 24, 2020
e4b22fd
[api] Remove typed_config stutter
ambuc Jul 24, 2020
6725f9c
Resolved merge conflicts
ambuc Jul 27, 2020
fad305b
[tls] Refactor HandshakerFactory with callback to allow for early val…
ambuc Jul 28, 2020
36c4d83
[tls] More clarity around Handshaker interface documentation.
ambuc Jul 28, 2020
7c3020d
[misc] Add bssl, rbio, wbio to dictionary.
ambuc Jul 28, 2020
cb39b1a
[misc] Run fix_format.
ambuc Jul 28, 2020
ed5f209
[tls] Clean up initialization order inside context_config_impl
ambuc Jul 29, 2020
2c7d3c8
[tls] Rename methods to onSuccessCb/onFailureCb
ambuc Jul 29, 2020
2d5ec2f
[tls] dedup HandshakerMaker with HandshakerFactoryCb
ambuc Jul 29, 2020
f32acbc
[misc] Remove wbio, rbio, bssl from dictionary
ambuc Jul 29, 2020
18dc2c6
[tls] HandshakerPtr is now a shared_ptr; SslSocketInfo retains access…
ambuc Jul 29, 2020
c7c40b5
Move HandshakerCallbacks into setCallbacks()
ambuc Jul 29, 2020
f711e26
[misc] Rename HandshakerPtr to HandshakerSharedPtr
ambuc Jul 29, 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
1 change: 1 addition & 0 deletions include/envoy/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ envoy_cc_library(
":socket_state",
"//include/envoy/config:typed_config_interface",
"//include/envoy/network:transport_socket_interface",
"//include/envoy/protobuf:message_validator_interface",
],
)

Expand Down
5 changes: 3 additions & 2 deletions include/envoy/ssl/context_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ class ContextConfig {
virtual Ssl::HandshakerPtr createHandshaker(bssl::UniquePtr<SSL> ssl) const PURE;

/**
* @return the handshaker factory for attribute evaluation.
* @return whether or not this context requires certificates for TLS
* handshakes.
*/
virtual const Ssl::HandshakerFactory& handshakerFactory() const PURE;
virtual bool requireCertificates() const PURE;
};

class ClientContextConfig : public virtual ContextConfig {
Expand Down
18 changes: 12 additions & 6 deletions include/envoy/ssl/handshaker.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "envoy/common/pure.h"
#include "envoy/config/typed_config.h"
#include "envoy/network/transport_socket.h"
#include "envoy/protobuf/message_validator.h"
#include "envoy/ssl/socket_state.h"

#include "openssl/ssl.h"
Expand Down Expand Up @@ -74,17 +75,22 @@ class HandshakerFactoryContext {
virtual absl::string_view alpnProtocols() const PURE;
};

using HandshakerFactoryCb = std::function<HandshakerPtr(bssl::UniquePtr<SSL>)>;

class HandshakerFactory : public Config::TypedFactory {
public:
virtual HandshakerPtr createHandshaker(bssl::UniquePtr<SSL> ssl,
HandshakerFactoryContext& context) PURE;

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

/**
* Set a config for use when creating handshakers.
* @returns a callback (of type HandshakerFactoryCb). Accepts the |config| and
* |validation_visitor| for early config validation. This virtual base doesn't
* perform MessageUtil::downcastAndValidate, but an implementation should.
*/
virtual void setConfig(ProtobufTypes::MessagePtr message) PURE;
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 true if the tls context accompanying this
Expand Down
25 changes: 17 additions & 8 deletions source/extensions/transport_sockets/tls/context_config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,15 @@ ContextConfigImpl::ContextConfigImpl(
default_min_protocol_version)),
max_protocol_version_(tlsVersionFromProto(config.tls_params().tls_maximum_protocol_version(),
default_max_protocol_version)),
handshaker_factory_(*HandshakerFactoryImpl::getDefaultHandshakerFactory()) {
handshaker_factory_cb_([&]() {
auto* factory = HandshakerFactoryImpl::getDefaultHandshakerFactory();
ambuc marked this conversation as resolved.
Show resolved Hide resolved
auto handshaker_factory_context = HandshakerFactoryContextImpl(api_, alpnProtocols());
ambuc marked this conversation as resolved.
Show resolved Hide resolved
return factory->createHandshakerCb(*factory->createEmptyConfigProto(),
handshaker_factory_context,
factory_context.messageValidationVisitor());
}()),
require_certificates_(
HandshakerFactoryImpl::getDefaultHandshakerFactory()->requireCertificates()) {
if (certificate_validation_context_provider_ != nullptr) {
if (default_cvc_) {
// We need to validate combined certificate validation context.
Expand Down Expand Up @@ -217,11 +225,13 @@ ContextConfigImpl::ContextConfigImpl(

if (config.has_custom_listener_handshaker()) {
auto& handshaker_config = config.custom_listener_handshaker();
handshaker_factory_ =
Ssl::HandshakerFactory& handshaker_factory =
Config::Utility::getAndCheckFactory<Ssl::HandshakerFactory>(handshaker_config);
handshaker_factory_.get().setConfig(Config::Utility::translateAnyToFactoryConfig(
handshaker_config.typed_config(), factory_context.messageValidationVisitor(),
handshaker_factory_.get()));
HandshakerFactoryContextImpl handshaker_factory_context(api_, alpnProtocols());
handshaker_factory_cb_ = handshaker_factory.createHandshakerCb(
handshaker_config.typed_config(), handshaker_factory_context,
factory_context.messageValidationVisitor());
require_certificates_ = handshaker_factory.requireCertificates();
}
}

Expand All @@ -235,8 +245,7 @@ Ssl::CertificateValidationContextConfigPtr ContextConfigImpl::getCombinedValidat
}

Ssl::HandshakerPtr ContextConfigImpl::createHandshaker(bssl::UniquePtr<SSL> ssl) const {
HandshakerFactoryContextImpl context(api_, alpnProtocols());
return handshaker_factory_.get().createHandshaker(std::move(ssl), context);
return handshaker_factory_cb_(std::move(ssl));
}

void ContextConfigImpl::setSecretUpdateCallback(std::function<void()> callback) {
Expand Down Expand Up @@ -419,7 +428,7 @@ ServerContextConfigImpl::ServerContextConfigImpl(

if ((config.common_tls_context().tls_certificates().size() +
config.common_tls_context().tls_certificate_sds_secret_configs().size()) == 0) {
if (handshakerFactory().requireCertificates()) {
if (requireCertificates()) {
throw EnvoyException("No TLS certificates found for server context");
}
} else if (!config.common_tls_context().tls_certificates().empty() &&
Expand Down
6 changes: 3 additions & 3 deletions source/extensions/transport_sockets/tls/context_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {

Ssl::HandshakerPtr createHandshaker(bssl::UniquePtr<SSL> ssl) const override;

const Ssl::HandshakerFactory& handshakerFactory() const override { return handshaker_factory_; }
bool requireCertificates() const override { return require_certificates_; }

protected:
ContextConfigImpl(const envoy::extensions::transport_sockets::tls::v3::CommonTlsContext& config,
Expand Down Expand Up @@ -101,8 +101,8 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
const unsigned min_protocol_version_;
const unsigned max_protocol_version_;

// This reference must be reseatable.
std::reference_wrapper<Ssl::HandshakerFactory> handshaker_factory_;
Ssl::HandshakerFactoryCb handshaker_factory_cb_;
bool require_certificates_;
};

class ClientContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::ClientContextConfig {
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/transport_sockets/tls/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c
ssl_versions_(stat_name_set_->add("ssl.versions")),
ssl_curves_(stat_name_set_->add("ssl.curves")),
ssl_sigalgs_(stat_name_set_->add("ssl.sigalgs")),
require_certificates_(config.handshakerFactory().requireCertificates()) {
require_certificates_(config.requireCertificates()) {
const auto tls_certificates = config.tlsCertificates();
tls_contexts_.resize(std::max(static_cast<size_t>(1), tls_certificates.size()));

Expand Down Expand Up @@ -985,7 +985,7 @@ ServerContextImpl::ServerContextImpl(Stats::Scope& scope,
const std::vector<std::string>& server_names,
TimeSource& time_source)
: ContextImpl(scope, config, time_source), session_ticket_keys_(config.sessionTicketKeys()) {
if (config.tlsCertificates().empty() && config.handshakerFactory().requireCertificates()) {
if (config.tlsCertificates().empty() && config.requireCertificates()) {
throw EnvoyException("Server TlsCertificates must have a certificate specified");
}

Expand Down
11 changes: 6 additions & 5 deletions source/extensions/transport_sockets/tls/handshaker_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ class HandshakerFactoryImpl : public Ssl::HandshakerFactory {
return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Struct()};
}

Ssl::HandshakerPtr createHandshaker(bssl::UniquePtr<SSL> ssl,
Ssl::HandshakerFactoryContext&) override {
return std::make_unique<HandshakerImpl>(std::move(ssl));
Ssl::HandshakerFactoryCb createHandshakerCb(const Protobuf::Message&,
Ssl::HandshakerFactoryContext&,
ProtobufMessage::ValidationVisitor&) override {
// The default HandshakerImpl doesn't take a config or use the HandshakerFactoryContext.
return
[](bssl::UniquePtr<SSL> ssl) { return std::make_unique<HandshakerImpl>(std::move(ssl)); };
}

void setConfig(ProtobufTypes::MessagePtr) override {}

bool requireCertificates() const override {
// The default HandshakerImpl does require certificates.
return true;
Expand Down
5 changes: 2 additions & 3 deletions test/extensions/transport_sockets/tls/handshaker_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,10 @@ class HandshakerImplForTest : public Ssl::Handshaker {
HandshakerImplForTest(bssl::UniquePtr<SSL> ssl, std::function<void()> requested_cert_cb)
: ssl_(std::move(ssl)), requested_cert_cb_(requested_cert_cb) {
SSL_set_cert_cb(
ssl_.get(), [](SSL*, void* arg) -> int { return *static_cast<bool*>(arg) ? 1 : -1; },
&cert_cb_ok_);
ssl_.get(), [](SSL*, void* arg) -> int { return *static_cast<bool*>(arg) ? 1 : -1; },
&cert_cb_ok_);
}


Network::PostIoAction doHandshake(Ssl::SocketState& state,
Ssl::HandshakerCallbacks& callbacks) override {
ASSERT(state != Ssl::SocketState::HandshakeComplete && state != Ssl::SocketState::ShutdownSent);
Copy link
Contributor

Choose a reason for hiding this comment

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

This for test doHandshake implementation looks too real. it seems like a copy of HandshakeImpl::doHandshake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It basically is. I specifically wanted to demonstrate special-case error handling under switch(SSL_get_error(...)). Should I add more comments to call out that only the SSL_ERROR_WANT_X509_LOOKUP block is different?

Expand Down