From a5fd6eda3af28ca473aee4e6a50e318d0d090e79 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Sat, 15 Sep 2018 04:00:32 +0000 Subject: [PATCH 01/13] tls: add support for client-side session resumption. *Risk Level*: Low *Testing*: bazel test //test/... *Docs Changes*: Added *Release Notes*: Added Signed-off-by: Piotr Sikora --- api/envoy/api/v2/auth/cert.proto | 6 ++++ include/envoy/ssl/context_config.h | 5 ++++ source/common/ssl/BUILD | 5 +++- source/common/ssl/context_config_impl.cc | 3 +- source/common/ssl/context_config_impl.h | 2 ++ source/common/ssl/context_impl.cc | 36 +++++++++++++++++++++++- source/common/ssl/context_impl.h | 7 +++++ 7 files changed, 61 insertions(+), 3 deletions(-) diff --git a/api/envoy/api/v2/auth/cert.proto b/api/envoy/api/v2/auth/cert.proto index 297d3bbe73c86..755e2dd720b16 100644 --- a/api/envoy/api/v2/auth/cert.proto +++ b/api/envoy/api/v2/auth/cert.proto @@ -279,6 +279,12 @@ message UpstreamTlsContext { // // TLS renegotiation is considered insecure and shouldn't be used unless absolutely necessary. bool allow_renegotiation = 3; + + // Maximum number of session keys (Pre-Shared Keys for TLSv1.3+, Session IDs and Session Tickets + // for TLSv1.2 and older) to store for the purpose of session resumption. + // + // Defaults to 1, setting this to 0 disables session resumption. + google.protobuf.UInt32Value max_session_keys = 4; } message DownstreamTlsContext { diff --git a/include/envoy/ssl/context_config.h b/include/envoy/ssl/context_config.h index 7b30b50c82837..a811817c9cf04 100644 --- a/include/envoy/ssl/context_config.h +++ b/include/envoy/ssl/context_config.h @@ -86,6 +86,11 @@ class ClientContextConfig : public virtual ContextConfig { * @return true if server-initiated TLS renegotiation will be allowed. */ virtual bool allowRenegotiation() const PURE; + + /** + * @return The maximum number of session keys to store. + */ + virtual size_t maxSessionKeys() const PURE; }; typedef std::unique_ptr ClientContextConfigPtr; diff --git a/source/common/ssl/BUILD b/source/common/ssl/BUILD index fc896665aec00..675fd25be5928 100644 --- a/source/common/ssl/BUILD +++ b/source/common/ssl/BUILD @@ -63,7 +63,10 @@ envoy_cc_library( "context_impl.h", "context_manager_impl.h", ], - external_deps = ["ssl"], + external_deps = [ + "abseil_synchronization", + "ssl", + ], deps = [ ":utility_lib", "//include/envoy/runtime:runtime_interface", diff --git a/source/common/ssl/context_config_impl.cc b/source/common/ssl/context_config_impl.cc index 8c80ecdb426c7..51f704fee1f72 100644 --- a/source/common/ssl/context_config_impl.cc +++ b/source/common/ssl/context_config_impl.cc @@ -141,7 +141,8 @@ ClientContextConfigImpl::ClientContextConfigImpl( const envoy::api::v2::auth::UpstreamTlsContext& config, Server::Configuration::TransportSocketFactoryContext& factory_context) : ContextConfigImpl(config.common_tls_context(), factory_context), - server_name_indication_(config.sni()), allow_renegotiation_(config.allow_renegotiation()) { + server_name_indication_(config.sni()), allow_renegotiation_(config.allow_renegotiation()), + max_session_keys_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_session_keys, 1)) { // 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) { diff --git a/source/common/ssl/context_config_impl.h b/source/common/ssl/context_config_impl.h index 7304108ff3aac..3191c085e8c05 100644 --- a/source/common/ssl/context_config_impl.h +++ b/source/common/ssl/context_config_impl.h @@ -102,10 +102,12 @@ class ClientContextConfigImpl : public ContextConfigImpl, public ClientContextCo // Ssl::ClientContextConfig const std::string& serverNameIndication() const override { return server_name_indication_; } bool allowRenegotiation() const override { return allow_renegotiation_; } + size_t maxSessionKeys() const override { return max_session_keys_; } private: const std::string server_name_indication_; const bool allow_renegotiation_; + const size_t max_session_keys_; }; class ServerContextConfigImpl : public ContextConfigImpl, public ServerContextConfig { diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc index 29f6937f3a5f2..12885df72e497 100644 --- a/source/common/ssl/context_impl.cc +++ b/source/common/ssl/context_impl.cc @@ -483,12 +483,24 @@ CertificateDetailsPtr ContextImpl::certificateDetails(X509* cert, const std::str ClientContextImpl::ClientContextImpl(Stats::Scope& scope, const ClientContextConfig& config) : ContextImpl(scope, config), server_name_indication_(config.serverNameIndication()), - allow_renegotiation_(config.allowRenegotiation()) { + allow_renegotiation_(config.allowRenegotiation()), + max_session_keys_(config.maxSessionKeys()) { if (!parsed_alpn_protocols_.empty()) { int rc = SSL_CTX_set_alpn_protos(ctx_.get(), &parsed_alpn_protocols_[0], parsed_alpn_protocols_.size()); RELEASE_ASSERT(rc == 0, ""); } + + if (max_session_keys_ > 0) { + SSL_CTX_set_session_cache_mode(ctx_.get(), SSL_SESS_CACHE_CLIENT); + SSL_CTX_sess_set_new_cb(ctx_.get(), [](SSL* ssl, SSL_SESSION* session) -> int { + ContextImpl* context_impl = + static_cast(SSL_CTX_get_ex_data(SSL_get_SSL_CTX(ssl), sslContextIndex())); + ClientContextImpl* client_context_impl = dynamic_cast(context_impl); + RELEASE_ASSERT(client_context_impl != nullptr, ""); // for Coverity + return client_context_impl->newSessionKey(session); + }); + } } bssl::UniquePtr ClientContextImpl::newSsl() const { @@ -503,9 +515,31 @@ bssl::UniquePtr ClientContextImpl::newSsl() const { SSL_set_renegotiate_mode(ssl_con.get(), ssl_renegotiate_freely); } + if (max_session_keys_ > 0) { + absl::WriterMutexLock l(&session_keys_mu_); + if (!session_keys_.empty()) { + SSL_SESSION* session = session_keys_.front().get(); + SSL_set_session(ssl_con.get(), session); + // Remove single-use (TLS v1.3) session key after first use. + if (SSL_SESSION_should_be_single_use(session)) { + session_keys_.pop_front(); + } + } + } + return ssl_con; } +int ClientContextImpl::newSessionKey(SSL_SESSION* session) { + absl::WriterMutexLock l(&session_keys_mu_); + // Evict oldest entries. + while (session_keys_.size() >= max_session_keys_) { + session_keys_.pop_back(); + } + session_keys_.push_front(bssl::UniquePtr(session)); + return 1; // Tell BoringSSL that we took ownership of the session. +} + ServerContextImpl::ServerContextImpl(Stats::Scope& scope, const ServerContextConfig& config, const std::vector& server_names, Runtime::Loader& runtime) diff --git a/source/common/ssl/context_impl.h b/source/common/ssl/context_impl.h index 2096045222328..a8458f4451b9c 100644 --- a/source/common/ssl/context_impl.h +++ b/source/common/ssl/context_impl.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -12,6 +13,7 @@ #include "common/ssl/context_impl.h" #include "common/ssl/context_manager_impl.h" +#include "absl/synchronization/mutex.h" #include "openssl/ssl.h" namespace Envoy { @@ -144,8 +146,13 @@ class ClientContextImpl : public ContextImpl, public ClientContext { bssl::UniquePtr newSsl() const override; private: + int newSessionKey(SSL_SESSION* session); + const std::string server_name_indication_; const bool allow_renegotiation_; + const size_t max_session_keys_; + mutable absl::Mutex session_keys_mu_; + mutable std::deque> session_keys_ GUARDED_BY(session_keys_mu_); }; class ServerContextImpl : public ContextImpl, public ServerContext { From ea184755e3d53bbc63ed79c523d809afe8ae95e4 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 19 Oct 2018 23:02:24 +0000 Subject: [PATCH 02/13] review: add tests. Signed-off-by: Piotr Sikora --- test/common/ssl/ssl_socket_test.cc | 185 +++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index d1f1f5aa6743e..8134d93235376 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -2411,6 +2411,191 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { EXPECT_EQ(0UL, stats_store.counter("ssl.session_reused").value()); } +void testClientSessionResumption(const std::string& server_ctx_yaml, + const std::string& client_ctx_yaml, bool expect_reuse, + const Network::Address::IpVersion version) { + Runtime::MockLoader runtime; + testing::NiceMock factory_context; + ContextManagerImpl manager(runtime); + + envoy::api::v2::auth::DownstreamTlsContext server_ctx_proto; + MessageUtil::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), server_ctx_proto); + auto server_cfg = std::make_unique(server_ctx_proto, factory_context); + Stats::IsolatedStoreImpl server_stats_store; + ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, + server_stats_store, std::vector{}); + + Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version), nullptr, + true); + NiceMock callbacks; + Network::MockConnectionHandler connection_handler; + DangerousDeprecatedTestTime test_time; + Event::DispatcherImpl dispatcher(test_time.timeSystem()); + Network::ListenerPtr listener = dispatcher.createListener(socket, callbacks, true, false); + + Network::ConnectionPtr server_connection; + Network::MockConnectionCallbacks server_connection_callbacks; + EXPECT_CALL(callbacks, onAccept_(_, _)) + .WillRepeatedly(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void { + Network::ConnectionPtr new_connection = dispatcher.createServerConnection( + std::move(socket), server_ssl_socket_factory.createTransportSocket()); + callbacks.onNewConnection(std::move(new_connection)); + })); + EXPECT_CALL(callbacks, onNewConnection_(_)) + .WillRepeatedly(Invoke([&](Network::ConnectionPtr& conn) -> void { + server_connection = std::move(conn); + server_connection->addConnectionCallbacks(server_connection_callbacks); + })); + + envoy::api::v2::auth::UpstreamTlsContext client_ctx_proto; + MessageUtil::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), client_ctx_proto); + auto client_cfg = std::make_unique(client_ctx_proto, factory_context); + Stats::IsolatedStoreImpl client_stats_store; + ClientSslSocketFactory client_ssl_socket_factory(std::move(client_cfg), manager, + client_stats_store); + Network::ClientConnectionPtr client_connection = dispatcher.createClientConnection( + socket.localAddress(), Network::Address::InstanceConstSharedPtr(), + client_ssl_socket_factory.createTransportSocket(), nullptr); + + Network::MockConnectionCallbacks client_connection_callbacks; + client_connection->addConnectionCallbacks(client_connection_callbacks); + client_connection->connect(); + + size_t connect_count = 0; + auto connectSecondTime = [&]() { + if (++connect_count == 2) { + server_connection->close(Network::ConnectionCloseType::NoFlush); + } + }; + + size_t close_count = 0; + auto closeSecondTime = [&]() { + if (++close_count == 2) { + dispatcher.exit(); + } + }; + + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + + dispatcher.run(Event::Dispatcher::RunType::Block); + + EXPECT_EQ(0UL, server_stats_store.counter("ssl.session_reused").value()); + EXPECT_EQ(0UL, client_stats_store.counter("ssl.session_reused").value()); + + connect_count = 0; + close_count = 0; + + client_connection = dispatcher.createClientConnection( + socket.localAddress(), Network::Address::InstanceConstSharedPtr(), + client_ssl_socket_factory.createTransportSocket(), nullptr); + client_connection->addConnectionCallbacks(client_connection_callbacks); + client_connection->connect(); + + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + + dispatcher.run(Event::Dispatcher::RunType::Block); + + EXPECT_EQ(expect_reuse ? 1UL : 0UL, server_stats_store.counter("ssl.session_reused").value()); + EXPECT_EQ(expect_reuse ? 1UL : 0UL, client_stats_store.counter("ssl.session_reused").value()); +} + +TEST_P(SslSocketTest, ClientSessionResumptionDefault) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" +)EOF"; + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: +)EOF"; + + testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); +} + +TEST_P(SslSocketTest, ClientSessionResumptionDisabled) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" +)EOF"; + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: + max_session_keys: 0 +)EOF"; + + testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, false, GetParam()); +} + +TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls12) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + tls_minimum_protocol_version: TLSv1_0 + tls_maximum_protocol_version: TLSv1_2 + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" +)EOF"; + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + tls_minimum_protocol_version: TLSv1_0 + tls_maximum_protocol_version: TLSv1_2 + max_session_keys: 2 +)EOF"; + + testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); +} + +TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls13) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + tls_minimum_protocol_version: TLSv1_3 + tls_maximum_protocol_version: TLSv1_3 + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" +)EOF"; + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + tls_minimum_protocol_version: TLSv1_3 + tls_maximum_protocol_version: TLSv1_3 + max_session_keys: 2 +)EOF"; + + testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); +} + TEST_P(SslSocketTest, SslError) { Stats::IsolatedStoreImpl stats_store; Runtime::MockLoader runtime; From 8512db478a75d23db4cd0a9e524d9c87d6d7bf19 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Wed, 14 Nov 2018 19:04:42 +0000 Subject: [PATCH 03/13] review: de-constify newSsl. Signed-off-by: Piotr Sikora --- source/common/ssl/context_impl.cc | 6 ++---- source/common/ssl/context_impl.h | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc index f724a67a6f2b7..8b0761dcf921d 100644 --- a/source/common/ssl/context_impl.cc +++ b/source/common/ssl/context_impl.cc @@ -275,9 +275,7 @@ std::vector ContextImpl::parseAlpnProtocols(const std::string& alpn_pro return out; } -bssl::UniquePtr ContextImpl::newSsl() const { - return bssl::UniquePtr(SSL_new(ctx_.get())); -} +bssl::UniquePtr ContextImpl::newSsl() { return bssl::UniquePtr(SSL_new(ctx_.get())); } int ContextImpl::ignoreCertificateExpirationCallback(int ok, X509_STORE_CTX* ctx) { if (!ok) { @@ -510,7 +508,7 @@ ClientContextImpl::ClientContextImpl(Stats::Scope& scope, const ClientContextCon } } -bssl::UniquePtr ClientContextImpl::newSsl() const { +bssl::UniquePtr ClientContextImpl::newSsl() { bssl::UniquePtr ssl_con(ContextImpl::newSsl()); if (!server_name_indication_.empty()) { diff --git a/source/common/ssl/context_impl.h b/source/common/ssl/context_impl.h index dd43570749be3..6621193abcbc9 100644 --- a/source/common/ssl/context_impl.h +++ b/source/common/ssl/context_impl.h @@ -43,7 +43,7 @@ struct SslStats { class ContextImpl : public virtual Context { public: - virtual bssl::UniquePtr newSsl() const; + virtual bssl::UniquePtr newSsl(); /** * Logs successful TLS handshake and updates stats. @@ -144,7 +144,7 @@ class ClientContextImpl : public ContextImpl, public ClientContext { ClientContextImpl(Stats::Scope& scope, const ClientContextConfig& config, TimeSource& time_source); - bssl::UniquePtr newSsl() const override; + bssl::UniquePtr newSsl() override; private: int newSessionKey(SSL_SESSION* session); @@ -152,8 +152,8 @@ class ClientContextImpl : public ContextImpl, public ClientContext { const std::string server_name_indication_; const bool allow_renegotiation_; const size_t max_session_keys_; - mutable absl::Mutex session_keys_mu_; - mutable std::deque> session_keys_ GUARDED_BY(session_keys_mu_); + absl::Mutex session_keys_mu_; + std::deque> session_keys_ GUARDED_BY(session_keys_mu_); }; class ServerContextImpl : public ContextImpl, public ServerContext { From 9aea0b4d5488a6bd55b69af3f748eacae26004ae Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Sat, 17 Nov 2018 00:46:21 +0000 Subject: [PATCH 04/13] review: add comments about LIFO. Signed-off-by: Piotr Sikora --- source/common/ssl/context_impl.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc index 8b0761dcf921d..a53feb8d9cd56 100644 --- a/source/common/ssl/context_impl.cc +++ b/source/common/ssl/context_impl.cc @@ -523,6 +523,8 @@ bssl::UniquePtr ClientContextImpl::newSsl() { if (max_session_keys_ > 0) { absl::WriterMutexLock l(&session_keys_mu_); if (!session_keys_.empty()) { + // Use the most recently stored session key, since it has the highest + // probability of still being recognized/accepted by the server. SSL_SESSION* session = session_keys_.front().get(); SSL_set_session(ssl_con.get(), session); // Remove single-use (TLS v1.3) session key after first use. @@ -541,6 +543,7 @@ int ClientContextImpl::newSessionKey(SSL_SESSION* session) { while (session_keys_.size() >= max_session_keys_) { session_keys_.pop_back(); } + // Add new session key at the front of the queue, so that it's used first. session_keys_.push_front(bssl::UniquePtr(session)); return 1; // Tell BoringSSL that we took ownership of the session. } From fd556b86a1dcdaaf792bdd54e6f3c0134e6dc182 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Sat, 17 Nov 2018 00:46:46 +0000 Subject: [PATCH 05/13] review: use write/write locks only when necessary. Signed-off-by: Piotr Sikora --- source/common/ssl/context_impl.cc | 35 +++++++++++++++++++++++-------- source/common/ssl/context_impl.h | 1 + 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc index a53feb8d9cd56..db07156d431e3 100644 --- a/source/common/ssl/context_impl.cc +++ b/source/common/ssl/context_impl.cc @@ -521,15 +521,27 @@ bssl::UniquePtr ClientContextImpl::newSsl() { } if (max_session_keys_ > 0) { - absl::WriterMutexLock l(&session_keys_mu_); - if (!session_keys_.empty()) { - // Use the most recently stored session key, since it has the highest - // probability of still being recognized/accepted by the server. - SSL_SESSION* session = session_keys_.front().get(); - SSL_set_session(ssl_con.get(), session); - // Remove single-use (TLS v1.3) session key after first use. - if (SSL_SESSION_should_be_single_use(session)) { - session_keys_.pop_front(); + if (session_keys_single_use_) { + // Stored single-use session keys, use write/write locks. + absl::WriterMutexLock l(&session_keys_mu_); + if (!session_keys_.empty()) { + // Use the most recently stored session key, since it has the highest + // probability of still being recognized/accepted by the server. + SSL_SESSION* session = session_keys_.front().get(); + SSL_set_session(ssl_con.get(), session); + // Remove single-use session key (TLS 1.3) after first use. + if (SSL_SESSION_should_be_single_use(session)) { + session_keys_.pop_front(); + } + } + } else { + // Never stored single-use session keys, use read/write locks. + absl::ReaderMutexLock l(&session_keys_mu_); + if (!session_keys_.empty()) { + // Use the most recently stored session key, since it has the highest + // probability of still being recognized/accepted by the server. + SSL_SESSION* session = session_keys_.front().get(); + SSL_set_session(ssl_con.get(), session); } } } @@ -538,6 +550,11 @@ bssl::UniquePtr ClientContextImpl::newSsl() { } int ClientContextImpl::newSessionKey(SSL_SESSION* session) { + // In case we ever store single-use session key (TLS 1.3), + // we need to switch to using write/write locks. + if (SSL_SESSION_should_be_single_use(session)) { + session_keys_single_use_ = true; + } absl::WriterMutexLock l(&session_keys_mu_); // Evict oldest entries. while (session_keys_.size() >= max_session_keys_) { diff --git a/source/common/ssl/context_impl.h b/source/common/ssl/context_impl.h index 6621193abcbc9..f6e51f7f84ed2 100644 --- a/source/common/ssl/context_impl.h +++ b/source/common/ssl/context_impl.h @@ -154,6 +154,7 @@ class ClientContextImpl : public ContextImpl, public ClientContext { const size_t max_session_keys_; absl::Mutex session_keys_mu_; std::deque> session_keys_ GUARDED_BY(session_keys_mu_); + bool session_keys_single_use_{false}; }; class ServerContextImpl : public ContextImpl, public ServerContext { From e21ab8028664a8cff561b71a512f6f57338d7e44 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Nov 2018 01:06:47 +0000 Subject: [PATCH 06/13] review: add one-line comments to describe test cases. Signed-off-by: Piotr Sikora --- test/common/ssl/ssl_socket_test.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index d1b1b42e096c4..caef41447aca2 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -2563,6 +2563,7 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, EXPECT_EQ(expect_reuse ? 1UL : 0UL, client_stats_store.counter("ssl.session_reused").value()); } +// Test client session resumption using default settings (should be enabled). TEST_P(SslSocketTest, ClientSessionResumptionDefault) { const std::string server_ctx_yaml = R"EOF( common_tls_context: @@ -2580,6 +2581,7 @@ TEST_P(SslSocketTest, ClientSessionResumptionDefault) { testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); } +// Make sure client session resumption is not happening when it's disabled. TEST_P(SslSocketTest, ClientSessionResumptionDisabled) { const std::string server_ctx_yaml = R"EOF( common_tls_context: @@ -2598,6 +2600,7 @@ TEST_P(SslSocketTest, ClientSessionResumptionDisabled) { testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, false, GetParam()); } +// Test client session resumption with TLS 1.0-1.2. TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls12) { const std::string server_ctx_yaml = R"EOF( common_tls_context: @@ -2622,6 +2625,7 @@ TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls12) { testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); } +// Test client session resumption with TLS 1.3 (it's different than in older versions of TLS). TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls13) { const std::string server_ctx_yaml = R"EOF( common_tls_context: From e4a754bacb04e220170cfca46128cbc169944413 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Nov 2018 01:09:34 +0000 Subject: [PATCH 07/13] review: use Event::SimulatedTimeSystem. Signed-off-by: Piotr Sikora --- test/common/ssl/ssl_socket_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index caef41447aca2..fc8394b039af5 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -2479,8 +2479,7 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, true); NiceMock callbacks; Network::MockConnectionHandler connection_handler; - DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl dispatcher(test_time.timeSystem()); + Event::DispatcherImpl dispatcher(time_system); Network::ListenerPtr listener = dispatcher.createListener(socket, callbacks, true, false); Network::ConnectionPtr server_connection; From 5f315404416ebae24ded37da24bbec7a8119889c Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Nov 2018 01:10:49 +0000 Subject: [PATCH 08/13] review: rename lambdas and use explicit capture list. Signed-off-by: Piotr Sikora --- test/common/ssl/ssl_socket_test.cc | 75 +++++++++++++++++------------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index fc8394b039af5..0becc0a635126 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -100,9 +100,12 @@ void testUtil(const std::string& client_ctx_yaml, const std::string& server_ctx_ client_connection->addConnectionCallbacks(client_connection_callbacks); client_connection->connect(); - size_t counter = 0; - auto stopSecondTime = [&]() { - if (++counter == 2) { + size_t connect_count = 0; + auto connect_second_time = [&connect_count, &dispatcher, &server_connection, &client_connection, + expected_digest, expected_uri, expected_local_uri, + expected_serial_number, expected_subject, expected_local_subject, + expected_peer_cert]() { + if (++connect_count == 2) { if (!expected_digest.empty()) { // Assert twice to ensure a cached value is returned and still valid. EXPECT_EQ(expected_digest, server_connection->ssl()->sha256PeerCertificateDigest()); @@ -131,24 +134,26 @@ void testUtil(const std::string& client_ctx_yaml, const std::string& server_ctx_ dispatcher.exit(); } }; - auto closeSecondTime = [&]() { - if (++counter == 2) { + + size_t close_count = 0; + auto close_second_time = [&close_count, &dispatcher]() { + if (++close_count == 2) { dispatcher.exit(); } }; if (expect_success) { EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { stopSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { stopSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); } else { EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); } dispatcher.run(Event::Dispatcher::RunType::Block); @@ -236,9 +241,12 @@ const std::string testUtilV2( client_connection->addConnectionCallbacks(client_connection_callbacks); client_connection->connect(); - size_t counter = 0; - auto stopSecondTime = [&]() { - if (++counter == 2) { + size_t connect_count = 0; + auto connect_second_time = [&connect_count, &dispatcher, &server_connection, &client_connection, + &new_session, expected_server_cert_digest, expected_alpn_protocol, + expected_client_cert_uri, expected_protocol_version, + expected_requested_server_name]() { + if (++connect_count == 2) { if (!expected_server_cert_digest.empty()) { EXPECT_EQ(expected_server_cert_digest, client_connection->ssl()->sha256PeerCertificateDigest()); @@ -283,27 +291,29 @@ const std::string testUtilV2( dispatcher.exit(); } }; - auto closeSecondTime = [&]() { - if (++counter == 2) { + + size_t close_count = 0; + auto close_second_time = [&close_count, &dispatcher]() { + if (++close_count == 2) { dispatcher.exit(); } }; if (expect_success) { EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { stopSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { EXPECT_EQ(expected_requested_server_name, server_connection->requestedServerName()); - stopSecondTime(); + connect_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); } else { EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); } dispatcher.run(Event::Dispatcher::RunType::Block); @@ -2037,8 +2047,9 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, // Different tests have different order of whether client or server gets Connected event // first, so always wait until both have happened. - unsigned connect_count = 0; - auto stopSecondTime = [&]() { + size_t connect_count = 0; + auto connect_second_time = [&connect_count, &dispatcher, &server_connection, + &client_connection]() { connect_count++; if (connect_count == 2) { client_connection->close(Network::ConnectionCloseType::NoFlush); @@ -2048,9 +2059,9 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, }; EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { stopSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { stopSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); @@ -2511,27 +2522,27 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, client_connection->connect(); size_t connect_count = 0; - auto connectSecondTime = [&]() { + auto connect_second_time = [&connect_count, &server_connection]() { if (++connect_count == 2) { server_connection->close(Network::ConnectionCloseType::NoFlush); } }; size_t close_count = 0; - auto closeSecondTime = [&]() { + auto close_second_time = [&close_count, &dispatcher]() { if (++close_count == 2) { dispatcher.exit(); } }; EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); dispatcher.run(Event::Dispatcher::RunType::Block); @@ -2548,13 +2559,13 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, client_connection->connect(); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connectSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { closeSecondTime(); })); + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); dispatcher.run(Event::Dispatcher::RunType::Block); From 4aeefe4f3ffde4ab5f07be20c3fe4829a8bf338d Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Nov 2018 01:46:41 +0000 Subject: [PATCH 09/13] review: use InSequence. Signed-off-by: Piotr Sikora --- test/common/ssl/ssl_socket_test.cc | 108 +++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 21 deletions(-) diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index 0becc0a635126..70299e50b815c 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -35,6 +35,7 @@ using testing::_; using testing::DoAll; +using testing::InSequence; using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -2475,6 +2476,8 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { void testClientSessionResumption(const std::string& server_ctx_yaml, const std::string& client_ctx_yaml, bool expect_reuse, const Network::Address::IpVersion version) { + InSequence s; + testing::NiceMock factory_context; Event::SimulatedTimeSystem time_system; ContextManagerImpl manager(time_system); @@ -2495,17 +2498,6 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, Network::ConnectionPtr server_connection; Network::MockConnectionCallbacks server_connection_callbacks; - EXPECT_CALL(callbacks, onAccept_(_, _)) - .WillRepeatedly(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void { - Network::ConnectionPtr new_connection = dispatcher.createServerConnection( - std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr)); - callbacks.onNewConnection(std::move(new_connection)); - })); - EXPECT_CALL(callbacks, onNewConnection_(_)) - .WillRepeatedly(Invoke([&](Network::ConnectionPtr& conn) -> void { - server_connection = std::move(conn); - server_connection->addConnectionCallbacks(server_connection_callbacks); - })); envoy::api::v2::auth::UpstreamTlsContext client_ctx_proto; MessageUtil::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), client_ctx_proto); @@ -2535,10 +2527,37 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, } }; - EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); - EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + // WillRepeatedly doesn't work with InSequence. + EXPECT_CALL(callbacks, onAccept_(_, _)) + .WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void { + Network::ConnectionPtr new_connection = dispatcher.createServerConnection( + std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr)); + callbacks.onNewConnection(std::move(new_connection)); + })); + EXPECT_CALL(callbacks, onNewConnection_(_)) + .WillOnce(Invoke([&](Network::ConnectionPtr& conn) -> void { + server_connection = std::move(conn); + server_connection->addConnectionCallbacks(server_connection_callbacks); + })); + + const bool expect_tls13 = + client_ctx_proto.common_tls_context().tls_params().tls_maximum_protocol_version() == + envoy::api::v2::auth::TlsParameters::TLSv1_3 && + server_ctx_proto.common_tls_context().tls_params().tls_maximum_protocol_version() == + envoy::api::v2::auth::TlsParameters::TLSv1_3; + + // The order of "Connected" events depends on the version of the TLS protocol (1.3 or older). + if (expect_tls13) { + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + } else { + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + } EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) @@ -2558,10 +2577,32 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, client_connection->addConnectionCallbacks(client_connection_callbacks); client_connection->connect(); - EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); - EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) - .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + // WillRepeatedly doesn't work with InSequence. + EXPECT_CALL(callbacks, onAccept_(_, _)) + .WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void { + Network::ConnectionPtr new_connection = dispatcher.createServerConnection( + std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr)); + callbacks.onNewConnection(std::move(new_connection)); + })); + EXPECT_CALL(callbacks, onNewConnection_(_)) + .WillOnce(Invoke([&](Network::ConnectionPtr& conn) -> void { + server_connection = std::move(conn); + server_connection->addConnectionCallbacks(server_connection_callbacks); + })); + + // The order of "Connected" events depends on the version of the TLS protocol (1.3 or older), + // and whether or not the session was successfully resumed. + if (expect_tls13 || expect_reuse) { + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + } else { + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { connect_second_time(); })); + } EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { close_second_time(); })); EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) @@ -2591,8 +2632,8 @@ TEST_P(SslSocketTest, ClientSessionResumptionDefault) { testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); } -// Make sure client session resumption is not happening when it's disabled. -TEST_P(SslSocketTest, ClientSessionResumptionDisabled) { +// Make sure client session resumption is not happening with TLS 1.0-1.2 when it's disabled. +TEST_P(SslSocketTest, ClientSessionResumptionDisabledTls12) { const std::string server_ctx_yaml = R"EOF( common_tls_context: tls_certificates: @@ -2635,6 +2676,31 @@ TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls12) { testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, true, GetParam()); } +// Make sure client session resumption is not happening with TLS 1.3 when it's disabled. +TEST_P(SslSocketTest, ClientSessionResumptionDisabledTls13) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + tls_minimum_protocol_version: TLSv1_3 + tls_maximum_protocol_version: TLSv1_3 + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" +)EOF"; + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: + tls_params: + tls_minimum_protocol_version: TLSv1_3 + tls_maximum_protocol_version: TLSv1_3 + max_session_keys: 0 +)EOF"; + + testClientSessionResumption(server_ctx_yaml, client_ctx_yaml, false, GetParam()); +} + // Test client session resumption with TLS 1.3 (it's different than in older versions of TLS). TEST_P(SslSocketTest, ClientSessionResumptionEnabledTls13) { const std::string server_ctx_yaml = R"EOF( From a09a3198873303decdb564db0f87ff40781f6b13 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Nov 2018 04:16:43 +0000 Subject: [PATCH 10/13] review: fix format. Signed-off-by: Piotr Sikora --- source/common/ssl/context_impl.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc index a0977a6ff0b47..6fd8e3230320b 100644 --- a/source/common/ssl/context_impl.cc +++ b/source/common/ssl/context_impl.cc @@ -510,8 +510,7 @@ ClientContextImpl::ClientContextImpl(Stats::Scope& scope, const ClientContextCon } } -bssl::UniquePtr -ClientContextImpl::newSsl(absl::optional override_server_name) { +bssl::UniquePtr ClientContextImpl::newSsl(absl::optional override_server_name) { bssl::UniquePtr ssl_con(ContextImpl::newSsl(absl::nullopt)); std::string server_name_indication = From 63f2da9a7e8a83942ac4815f09439932d3dcf471 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 23 Nov 2018 22:39:29 +0000 Subject: [PATCH 11/13] review: Kick CI. Signed-off-by: Piotr Sikora From 8eb40282d8afc8560ff60b2a35686026c07832ea Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Sat, 24 Nov 2018 00:00:08 +0000 Subject: [PATCH 12/13] review: Kick CI. Signed-off-by: Piotr Sikora From e8da10819812f537898a40043f3f86b6ec0b0277 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Mon, 26 Nov 2018 18:47:06 +0000 Subject: [PATCH 13/13] review: revert ridiculous lambda captures. Signed-off-by: Piotr Sikora --- test/common/ssl/ssl_socket_test.cc | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index 70299e50b815c..257baec1d025e 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -102,10 +102,7 @@ void testUtil(const std::string& client_ctx_yaml, const std::string& server_ctx_ client_connection->connect(); size_t connect_count = 0; - auto connect_second_time = [&connect_count, &dispatcher, &server_connection, &client_connection, - expected_digest, expected_uri, expected_local_uri, - expected_serial_number, expected_subject, expected_local_subject, - expected_peer_cert]() { + auto connect_second_time = [&]() { if (++connect_count == 2) { if (!expected_digest.empty()) { // Assert twice to ensure a cached value is returned and still valid. @@ -243,10 +240,7 @@ const std::string testUtilV2( client_connection->connect(); size_t connect_count = 0; - auto connect_second_time = [&connect_count, &dispatcher, &server_connection, &client_connection, - &new_session, expected_server_cert_digest, expected_alpn_protocol, - expected_client_cert_uri, expected_protocol_version, - expected_requested_server_name]() { + auto connect_second_time = [&]() { if (++connect_count == 2) { if (!expected_server_cert_digest.empty()) { EXPECT_EQ(expected_server_cert_digest,