Skip to content

Commit 8db119b

Browse files
authored
tls: refactor cipher_suites to be shared_ptr instead of copied strings (#44793)
Refactoring the cipher_suites in the TLS `ContextConfigImpl` object, so that if there are multiple TLS objects with the same `cipher_suites` these will be shared instead of copied. The PR introduces a SharedObjectPool of the cipher_suites strings, that the `ContextConfigImpl` have shared-pointers to. Signed-off-by: Adi Suissa-Peleg <adip@google.com>
1 parent 426db91 commit 8db119b

3 files changed

Lines changed: 44 additions & 4 deletions

File tree

source/common/tls/context_config_impl.cc

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "source/common/protobuf/message_validator_impl.h"
1414
#include "source/common/protobuf/utility.h"
1515
#include "source/common/secret/sds_api.h"
16+
#include "source/common/shared_pool/shared_pool.h"
1617
#include "source/common/ssl/certificate_validation_context_config_impl.h"
1718
#include "source/common/tls/ssl_handshaker.h"
1819

@@ -24,6 +25,8 @@ namespace Extensions {
2425
namespace TransportSockets {
2526
namespace Tls {
2627

28+
SINGLETON_MANAGER_REGISTRATION(cipher_suites_pool);
29+
2730
namespace {
2831

2932
std::string generateCertificateHash(const std::string& cert_data) {
@@ -179,6 +182,14 @@ compliancePolicyFromProto(
179182
}
180183
}
181184

185+
std::shared_ptr<SharedPool::ObjectSharedPool<std::string>>
186+
getCipherSuitesPool(Singleton::Manager& singleton_manager, Event::Dispatcher& dispatcher) {
187+
return singleton_manager.getTyped<SharedPool::ObjectSharedPool<std::string>>(
188+
SINGLETON_MANAGER_REGISTERED_NAME(cipher_suites_pool), [&dispatcher] {
189+
return std::make_shared<SharedPool::ObjectSharedPool<std::string>>(dispatcher);
190+
});
191+
}
192+
182193
} // namespace
183194

184195
ContextConfigImpl::ContextConfigImpl(
@@ -194,8 +205,12 @@ ContextConfigImpl::ContextConfigImpl(
194205
lifecycle_notifier_(factory_context.serverFactoryContext().lifecycleNotifier()),
195206
auto_sni_san_match_(auto_sni_san_match),
196207
alpn_protocols_(RepeatedPtrUtil::join(config.alpn_protocols(), ",")),
197-
cipher_suites_(StringUtil::nonEmptyStringOrDefault(
198-
RepeatedPtrUtil::join(config.tls_params().cipher_suites(), ":"), default_cipher_suites)),
208+
cipher_suites_(
209+
getCipherSuitesPool(factory_context.serverFactoryContext().singletonManager(),
210+
factory_context.serverFactoryContext().mainThreadDispatcher())
211+
->getObject(StringUtil::nonEmptyStringOrDefault(
212+
RepeatedPtrUtil::join(config.tls_params().cipher_suites(), ":"),
213+
default_cipher_suites))),
199214
ecdh_curves_(StringUtil::nonEmptyStringOrDefault(
200215
RepeatedPtrUtil::join(config.tls_params().ecdh_curves(), ":"), default_curves)),
201216
signature_algorithms_(RepeatedPtrUtil::join(config.tls_params().signature_algorithms(), ":")),

source/common/tls/context_config_impl.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <memory>
34
#include <string>
45
#include <vector>
56

@@ -35,7 +36,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig,
3536
public:
3637
// Ssl::ContextConfig
3738
const std::string& alpnProtocols() const override { return alpn_protocols_; }
38-
const std::string& cipherSuites() const override { return cipher_suites_; }
39+
const std::string& cipherSuites() const override { return *cipher_suites_; }
3940
const std::string& ecdhCurves() const override { return ecdh_curves_; }
4041
const std::string& signatureAlgorithms() const override { return signature_algorithms_; }
4142
absl::optional<envoy::extensions::transport_sockets::tls::v3::TlsParameters::CompliancePolicy>
@@ -104,7 +105,7 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig,
104105
unsigned default_version);
105106

106107
const std::string alpn_protocols_;
107-
const std::string cipher_suites_;
108+
const std::shared_ptr<const std::string> cipher_suites_;
108109
const std::string ecdh_curves_;
109110
const std::string signature_algorithms_;
110111

test/common/tls/context_impl_test.cc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,30 @@ TEST_F(SslContextImplTest, TestCipherSuites) {
148148
"ciphers were rejected when tried individually: BOGUS1-SHA256, BOGUS2-SHA");
149149
}
150150

151+
// Validates that multiple cipher-suites with the same contents are still equal
152+
// after dedup.
153+
TEST_F(SslContextImplTest, TestCipherSuitesDeduplication) {
154+
const std::string yaml = R"EOF(
155+
common_tls_context:
156+
tls_params:
157+
cipher_suites: "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256"
158+
)EOF";
159+
160+
envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context1;
161+
TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), tls_context1);
162+
auto cfg1 = *ClientContextConfigImpl::create(tls_context1, factory_context_);
163+
164+
envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context2;
165+
TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), tls_context2);
166+
auto cfg2 = *ClientContextConfigImpl::create(tls_context2, factory_context_);
167+
168+
EXPECT_EQ(cfg1->cipherSuites(), "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256");
169+
EXPECT_EQ(cfg2->cipherSuites(), "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256");
170+
171+
EXPECT_TRUE(manager_.createSslClientContext(*store_.rootScope(), *cfg1).ok());
172+
EXPECT_TRUE(manager_.createSslClientContext(*store_.rootScope(), *cfg2).ok());
173+
}
174+
151175
// Envoy's default cipher preference is server's.
152176
TEST_F(SslContextImplTest, TestServerCipherPreference) {
153177
const std::string yaml = R"EOF(

0 commit comments

Comments
 (0)