From 2c07469dc8ceae3f4f1aac29b633e3eb56c8f3d5 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 2 Apr 2024 11:02:28 -0700 Subject: [PATCH 01/13] feat: add TLS verify peer option --- .../config/shared/built/http_properties.hpp | 17 +++++++++++++- libs/common/src/config/http_properties.cpp | 23 ++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index d077e18f4..06324d16b 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -9,11 +9,21 @@ namespace launchdarkly::config::shared::built { class HttpProperties final { public: + class TLS final { + public: + TLS(bool verify_peer); + [[nodiscard]] bool VerifyPeer() const; + + private: + bool verify_peer_; + }; + HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds read_timeout, std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, - std::map base_headers); + std::map base_headers, + TLS tls); [[nodiscard]] std::chrono::milliseconds ConnectTimeout() const; [[nodiscard]] std::chrono::milliseconds ReadTimeout() const; @@ -22,16 +32,21 @@ class HttpProperties final { [[nodiscard]] std::chrono::milliseconds ResponseTimeout() const; [[nodiscard]] std::map const& BaseHeaders() const; + [[nodiscard]] class TLS const& TLS() const; + private: std::chrono::milliseconds connect_timeout_; std::chrono::milliseconds read_timeout_; std::chrono::milliseconds write_timeout_; std::chrono::milliseconds response_timeout_; std::map base_headers_; + class TLS tls_; // TODO: Proxy. }; bool operator==(HttpProperties const& lhs, HttpProperties const& rhs); +bool operator==(class HttpProperties::TLS const& lhs, + class HttpProperties::TLS const& rhs); } // namespace launchdarkly::config::shared::built diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index 905db4292..df0509511 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -4,16 +4,24 @@ namespace launchdarkly::config::shared::built { +HttpProperties::TLS::TLS(bool verify_peer) : verify_peer_(verify_peer) {} + +bool HttpProperties::TLS::VerifyPeer() const { + return verify_peer_; +} + HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds read_timeout, std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, - std::map base_headers) + std::map base_headers, + class HttpProperties::TLS tls) : connect_timeout_(connect_timeout), read_timeout_(read_timeout), write_timeout_(write_timeout), response_timeout_(response_timeout), - base_headers_(std::move(base_headers)) {} + base_headers_(std::move(base_headers)), + tls_(std::move(tls)) {} std::chrono::milliseconds HttpProperties::ConnectTimeout() const { return connect_timeout_; @@ -35,11 +43,20 @@ std::map const& HttpProperties::BaseHeaders() const { return base_headers_; } +class HttpProperties::TLS HttpProperties::TLS() const& { + return tls_; +} + bool operator==(HttpProperties const& lhs, HttpProperties const& rhs) { return lhs.ReadTimeout() == rhs.ReadTimeout() && lhs.WriteTimeout() == rhs.WriteTimeout() && lhs.ConnectTimeout() == rhs.ConnectTimeout() && - lhs.BaseHeaders() == rhs.BaseHeaders(); + lhs.BaseHeaders() == rhs.BaseHeaders() && lhs.TLS() == rhs.TLS(); +} + +bool operator==(class HttpProperties::TLS const& lhs, + class HttpProperties::TLS const& rhs) { + return lhs.VerifyPeer() == rhs.VerifyPeer(); } } // namespace launchdarkly::config::shared::built From aabc15fbafd42a6b412c43969316bd3b9c27ba12 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 3 Apr 2024 17:10:32 -0700 Subject: [PATCH 02/13] adding builders --- .../client_side/bindings/c/config/builder.h | 46 +++++++++++++++++- libs/client-sdk/src/bindings/c/builder.cpp | 33 +++++++++++++ libs/client-sdk/tests/client_config_test.cpp | 15 ++++++ .../include/launchdarkly/config/client.hpp | 1 + .../builders/http_properties_builder.hpp | 47 +++++++++++++++++++ .../config/shared/built/http_properties.hpp | 26 +++++----- .../launchdarkly/config/shared/defaults.hpp | 22 ++++++--- libs/common/src/config/http_properties.cpp | 15 +++--- .../src/config/http_properties_builder.cpp | 36 ++++++++++++-- 9 files changed, 210 insertions(+), 31 deletions(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index d1f92375e..79d4b91c0 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -21,6 +21,8 @@ typedef struct _LDClientConfigBuilder* LDClientConfigBuilder; typedef struct _LDDataSourceStreamBuilder* LDDataSourceStreamBuilder; typedef struct _LDDataSourcePollBuilder* LDDataSourcePollBuilder; typedef struct _LDPersistenceCustomBuilder* LDPersistenceCustomBuilder; +typedef struct _LDClientHttpPropertiesTlsBuilder* + LDClientHttpPropertiesTlsBuilder; typedef void (*SetFn)(char const* storage_namespace, char const* key, @@ -333,7 +335,6 @@ LD_EXPORT(void) LDDataSourceStreamBuilder_Free(LDDataSourceStreamBuilder b); * * @return New builder for Polling method. */ - LD_EXPORT(LDDataSourcePollBuilder) LDDataSourcePollBuilder_New(); @@ -390,6 +391,49 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, char const* key, char const* value); +/** + * Sets the TLS options builder. The builder is consumed; do not free it. + * @param b Client config builder. Must not be NULL. + * @param tls_builder The TLS options builder. Must not be NULL. + */ +LD_EXPORT(void) +LDClientConfigBuilder_HttpProperties_Tls( + LDClientConfigBuilder b, + LDClientHttpPropertiesTlsBuilder tls_builder); + +/** + * Creates a new TLS options builder for the HttpProperties builder. + * + * If not passed into the HttpProperties + * builder, must be manually freed with LDClientHttpPropertiesTlsBuilder_Free. + * + * @return New builder for TLS options. + */ +LD_EXPORT(LDClientHttpPropertiesTlsBuilder) +LDClientHttpPropertiesTlsBuilder_New(void); + +/** + * Frees a TLS options builder. Do not call if the builder was consumed by + * the HttpProperties builder. + * + * @param b Builder to free. + */ +LD_EXPORT(void) +LDClientHttpPropertiesTlsBuilder_Free(LDClientHttpPropertiesTlsBuilder b); + +/** + * Enables peer certificate verification. Enabled by default. + * + * Disabling peer verification is not recommended unless a specific + * use-case calls for it. + * + * @param b Client config builder. Must not be NULL. + * @param verify_peer True to verify peer, false otherwise. + */ +LD_EXPORT(void) +LDClientHttpPropertiesTlsBuilder_VerifyPeer(LDClientHttpPropertiesTlsBuilder b, + bool verify_peer); + /** * Disables the default SDK logging. * @param b Client config builder. Must not be NULL. diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index 0259ba83b..a59200f23 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -41,6 +41,11 @@ using namespace launchdarkly::client_side; #define FROM_CUSTOM_PERSISTENCE_BUILDER(ptr) \ (reinterpret_cast(ptr)) +#define TO_TLS_BUILDER(ptr) (reinterpret_cast(ptr)) + +#define FROM_TLS_BUILDER(ptr) \ + (reinterpret_cast(ptr)) + class PersistenceImplementationWrapper : public IPersistence { public: explicit PersistenceImplementationWrapper(LDPersistence impl) @@ -306,6 +311,34 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, TO_BUILDER(b)->HttpProperties().Header(key, value); } +LD_EXPORT(void) +LDClientConfigBuilder_HttpProperties_Tls( + LDClientConfigBuilder b, + LDClientHttpPropertiesTlsBuilder tls_builder) { + LD_ASSERT_NOT_NULL(b); + LD_ASSERT_NOT_NULL(tls_builder); + + TO_BUILDER(b)->HttpProperties().Tls(*TO_TLS_BUILDER(tls_builder)); +} + +LD_EXPORT(void) +LDClientHttpPropertiesTlsBuilder_VerifyPeer(LDClientHttpPropertiesTlsBuilder b, + bool verify_peer) { + LD_ASSERT_NOT_NULL(b); + + TO_TLS_BUILDER(b)->VerifyPeer(verify_peer); +} + +LD_EXPORT(LDClientHttpPropertiesTlsBuilder) +LDClientHttpPropertiesTlsBuilder_New(void) { + return FROM_TLS_BUILDER(new TlsBuilder()); +} + +LD_EXPORT(void) +LDClientHttpPropertiesTlsBuilder_Free(LDClientHttpPropertiesTlsBuilder b) { + delete TO_TLS_BUILDER(b); +} + LD_EXPORT(void) LDClientConfigBuilder_Logging_Disable(LDClientConfigBuilder b) { LD_ASSERT_NOT_NULL(b); diff --git a/libs/client-sdk/tests/client_config_test.cpp b/libs/client-sdk/tests/client_config_test.cpp index f9b923afe..d6eb93947 100644 --- a/libs/client-sdk/tests/client_config_test.cpp +++ b/libs/client-sdk/tests/client_config_test.cpp @@ -65,14 +65,29 @@ TEST(ClientConfigBindings, AllConfigs) { LDDataSourceStreamBuilder_InitialReconnectDelayMs(stream_builder, 500); LDClientConfigBuilder_DataSource_MethodStream(builder, stream_builder); + LDDataSourceStreamBuilder stream_builder2 = LDDataSourceStreamBuilder_New(); + LDDataSourceStreamBuilder_Free(stream_builder2); + LDDataSourcePollBuilder poll_builder = LDDataSourcePollBuilder_New(); LDDataSourcePollBuilder_IntervalS(poll_builder, 10); LDClientConfigBuilder_DataSource_MethodPoll(builder, poll_builder); + LDDataSourcePollBuilder poll_builder2 = LDDataSourcePollBuilder_New(); + LDDataSourcePollBuilder_Free(poll_builder2); + LDClientConfigBuilder_HttpProperties_Header(builder, "foo", "bar"); LDClientConfigBuilder_HttpProperties_WrapperName(builder, "wrapper"); LDClientConfigBuilder_HttpProperties_WrapperVersion(builder, "v1.2.3"); + LDClientHttpPropertiesTlsBuilder tls_builder = + LDClientHttpPropertiesTlsBuilder_New(); + LDClientHttpPropertiesTlsBuilder_VerifyPeer(tls_builder, false); + LDClientConfigBuilder_HttpProperties_Tls(builder, tls_builder); + + LDClientHttpPropertiesTlsBuilder tls_builder2 = + LDClientHttpPropertiesTlsBuilder_New(); + LDClientHttpPropertiesTlsBuilder_Free(tls_builder2); + LDClientConfigBuilder_Logging_Disable(builder); LDLoggingBasicBuilder log_builder = LDLoggingBasicBuilder_New(); diff --git a/libs/common/include/launchdarkly/config/client.hpp b/libs/common/include/launchdarkly/config/client.hpp index 516c4b85b..dd288a82a 100644 --- a/libs/common/include/launchdarkly/config/client.hpp +++ b/libs/common/include/launchdarkly/config/client.hpp @@ -22,6 +22,7 @@ using HttpPropertiesBuilder = using DataSourceBuilder = config::shared::builders::DataSourceBuilder; using LoggingBuilder = config::shared::builders::LoggingBuilder; using PersistenceBuilder = config::shared::builders::PersistenceBuilder; +using TlsBuilder = config::shared::builders::TlsBuilder; using Config = config::Config; diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index 2ccdef725..853424d73 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -10,6 +10,45 @@ namespace launchdarkly::config::shared::builders { +/** + * Class used for building TLS options used within HttpProperties. + * @tparam SDK THe SDK type to build options for. This affects the default + * values of the built options. + */ +template +class TlsBuilder { + public: + /** + * Construct a new TlsBuilder. The builder will use the default + * properties based on the SDK type. Setting a property will override + * the default value. + */ + TlsBuilder(); + + /** + * Create a TLS builder from an initial set of options. + * This can be useful when extending a set of options for a request. + * + * @param tls The TLS options to start with. + */ + TlsBuilder(built::TlsOptions const& tls); + + /** + * Whether the remote peer's certificates should be verified. + * @param verify_peer True to verify peer, false otherwise. + * @return A reference to this builder. + */ + TlsBuilder& VerifyPeer(bool verify_peer); + + /** + * Builds the TLS options. + * @return The built options. + */ + [[nodiscard]] built::TlsOptions Build() const; + + private: + bool verify_peer_; +}; /** * Class used for building a set of HttpProperties. * @tparam SDK The SDK type to build properties for. This affects the default @@ -116,6 +155,13 @@ class HttpPropertiesBuilder { HttpPropertiesBuilder& Header(std::string key, std::optional value); + /** + * Sets the builder for TLS properties. + * @param builder The TLS property builder. + * @return A reference to this builder. + */ + HttpPropertiesBuilder& Tls(TlsBuilder builder); + /** * Build a set of HttpProperties. * @return The built properties. @@ -130,6 +176,7 @@ class HttpPropertiesBuilder { std::string wrapper_name_; std::string wrapper_version_; std::map base_headers_; + TlsBuilder tls_; }; } // namespace launchdarkly::config::shared::builders diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index 06324d16b..f7334f340 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -7,23 +7,24 @@ namespace launchdarkly::config::shared::built { -class HttpProperties final { +class TlsOptions final { public: - class TLS final { - public: - TLS(bool verify_peer); - [[nodiscard]] bool VerifyPeer() const; + TlsOptions(bool verify_peer); + TlsOptions(); + [[nodiscard]] bool VerifyPeer() const; - private: - bool verify_peer_; - }; + private: + bool verify_peer_; +}; +class HttpProperties final { + public: HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds read_timeout, std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::map base_headers, - TLS tls); + TlsOptions tls); [[nodiscard]] std::chrono::milliseconds ConnectTimeout() const; [[nodiscard]] std::chrono::milliseconds ReadTimeout() const; @@ -32,7 +33,7 @@ class HttpProperties final { [[nodiscard]] std::chrono::milliseconds ResponseTimeout() const; [[nodiscard]] std::map const& BaseHeaders() const; - [[nodiscard]] class TLS const& TLS() const; + [[nodiscard]] TlsOptions const& Tls() const; private: std::chrono::milliseconds connect_timeout_; @@ -40,13 +41,12 @@ class HttpProperties final { std::chrono::milliseconds write_timeout_; std::chrono::milliseconds response_timeout_; std::map base_headers_; - class TLS tls_; + TlsOptions tls_; // TODO: Proxy. }; bool operator==(HttpProperties const& lhs, HttpProperties const& rhs); -bool operator==(class HttpProperties::TLS const& lhs, - class HttpProperties::TLS const& rhs); +bool operator==(TlsOptions const& lhs, TlsOptions const& rhs); } // namespace launchdarkly::config::shared::built diff --git a/libs/common/include/launchdarkly/config/shared/defaults.hpp b/libs/common/include/launchdarkly/config/shared/defaults.hpp index 9b7814360..0995eaa52 100644 --- a/libs/common/include/launchdarkly/config/shared/defaults.hpp +++ b/libs/common/include/launchdarkly/config/shared/defaults.hpp @@ -49,10 +49,15 @@ struct Defaults { std::nullopt}; } + static auto TLS() -> shared::built::TlsOptions { return {}; } + static auto HttpProperties() -> shared::built::HttpProperties { - return {std::chrono::seconds{10}, std::chrono::seconds{10}, - std::chrono::seconds{10}, std::chrono::seconds{10}, - std::map()}; + return {std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::map(), + TLS()}; } static auto StreamingConfig() -> shared::built::StreamingConfig { @@ -92,10 +97,15 @@ struct Defaults { 1000}; } + static auto TLS() -> shared::built::TlsOptions { return {}; } + static auto HttpProperties() -> built::HttpProperties { - return {std::chrono::seconds{10}, std::chrono::seconds{10}, - std::chrono::seconds{10}, std::chrono::seconds{10}, - std::map()}; + return {std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::chrono::seconds{10}, + std::map(), + TLS()}; } static auto StreamingConfig() -> built::StreamingConfig { diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index df0509511..fa52651ae 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -4,9 +4,11 @@ namespace launchdarkly::config::shared::built { -HttpProperties::TLS::TLS(bool verify_peer) : verify_peer_(verify_peer) {} +TlsOptions::TlsOptions(bool verify_peer) : verify_peer_(verify_peer) {} -bool HttpProperties::TLS::VerifyPeer() const { +TlsOptions::TlsOptions() : TlsOptions(true) {} + +bool TlsOptions::VerifyPeer() const { return verify_peer_; } @@ -15,7 +17,7 @@ HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout, std::chrono::milliseconds write_timeout, std::chrono::milliseconds response_timeout, std::map base_headers, - class HttpProperties::TLS tls) + TlsOptions tls) : connect_timeout_(connect_timeout), read_timeout_(read_timeout), write_timeout_(write_timeout), @@ -43,7 +45,7 @@ std::map const& HttpProperties::BaseHeaders() const { return base_headers_; } -class HttpProperties::TLS HttpProperties::TLS() const& { +TlsOptions const& HttpProperties::Tls() const { return tls_; } @@ -51,11 +53,10 @@ bool operator==(HttpProperties const& lhs, HttpProperties const& rhs) { return lhs.ReadTimeout() == rhs.ReadTimeout() && lhs.WriteTimeout() == rhs.WriteTimeout() && lhs.ConnectTimeout() == rhs.ConnectTimeout() && - lhs.BaseHeaders() == rhs.BaseHeaders() && lhs.TLS() == rhs.TLS(); + lhs.BaseHeaders() == rhs.BaseHeaders() && lhs.Tls() == rhs.Tls(); } -bool operator==(class HttpProperties::TLS const& lhs, - class HttpProperties::TLS const& rhs) { +bool operator==(TlsOptions const& lhs, TlsOptions const& rhs) { return lhs.VerifyPeer() == rhs.VerifyPeer(); } diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 7fad6195f..8c96bbb26 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -6,6 +6,25 @@ namespace launchdarkly::config::shared::builders { +template +TlsBuilder::TlsBuilder() : TlsBuilder(shared::Defaults::TLS()) {} + +template +TlsBuilder::TlsBuilder(built::TlsOptions const& tls) { + verify_peer_ = tls.VerifyPeer(); +} + +template +TlsBuilder& TlsBuilder::VerifyPeer(bool verify_peer) { + verify_peer_ = verify_peer; + return *this; +} + +template +built::TlsOptions TlsBuilder::Build() const { + return {verify_peer_}; +} + template HttpPropertiesBuilder::HttpPropertiesBuilder() : HttpPropertiesBuilder(shared::Defaults::HttpProperties()) {} @@ -18,6 +37,7 @@ HttpPropertiesBuilder::HttpPropertiesBuilder( write_timeout_ = properties.WriteTimeout(); response_timeout_ = properties.ResponseTimeout(); base_headers_ = properties.BaseHeaders(); + tls_ = properties.Tls(); } template @@ -81,19 +101,27 @@ HttpPropertiesBuilder& HttpPropertiesBuilder::Header( return *this; } +template +HttpPropertiesBuilder& HttpPropertiesBuilder::Tls( + TlsBuilder builder) { + tls_ = std::move(builder); + return *this; +} + template built::HttpProperties HttpPropertiesBuilder::Build() const { if (!wrapper_name_.empty()) { std::map headers_with_wrapper(base_headers_); headers_with_wrapper["X-LaunchDarkly-Wrapper"] = wrapper_name_ + "/" + wrapper_version_; - return {connect_timeout_, read_timeout_, write_timeout_, - response_timeout_, headers_with_wrapper}; + return {connect_timeout_, read_timeout_, write_timeout_, + response_timeout_, headers_with_wrapper, tls_.Build()}; } - return {connect_timeout_, read_timeout_, write_timeout_, response_timeout_, - base_headers_}; + return {connect_timeout_, read_timeout_, write_timeout_, + response_timeout_, base_headers_, tls_.Build()}; } +template class TlsBuilder; template class HttpPropertiesBuilder; template class HttpPropertiesBuilder; } // namespace launchdarkly::config::shared::builders From 505b63ea6d5c0394ed9ec1e86329b22d2f7e24c7 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 3 Apr 2024 17:41:07 -0700 Subject: [PATCH 03/13] feat: add peer verification to builder --- .../include/launchdarkly/sse/client.hpp | 9 +++++++++ libs/server-sent-events/src/client.cpp | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/libs/server-sent-events/include/launchdarkly/sse/client.hpp b/libs/server-sent-events/include/launchdarkly/sse/client.hpp index 9b9e191c6..ec9a83130 100644 --- a/libs/server-sent-events/include/launchdarkly/sse/client.hpp +++ b/libs/server-sent-events/include/launchdarkly/sse/client.hpp @@ -131,6 +131,14 @@ class Builder { */ Builder& errors(ErrorCallback callback); + /** + * If connecting to a TLS endpoint, whether to verify the remote + * peer's certificates. + * @param verify_peer True to verify the peer, false otherwise. + * @return Reference to this builder. + */ + Builder& verify_peer(bool verify_peer); + /** * Builds a Client. The shared pointer is necessary to extend the lifetime * of the Client to encompass each asynchronous operation that it performs. @@ -147,6 +155,7 @@ class Builder { std::optional write_timeout_; std::optional connect_timeout_; std::optional initial_reconnect_delay_; + bool verify_peer_; LogCallback logging_cb_; EventReceiver receiver_; ErrorCallback error_cb_; diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index c3c051365..c5c520adf 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -561,6 +561,11 @@ Builder& Builder::errors(ErrorCallback callback) { return *this; } +Builder& Builder::verify_peer(bool verify_peer) { + verify_peer_ = verify_peer; + return *this; +} + std::shared_ptr Builder::build() { auto uri_components = boost::urls::parse_uri(url_); if (!uri_components) { @@ -607,6 +612,10 @@ std::shared_ptr Builder::build() { if (uri_components->scheme_id() == boost::urls::scheme::https) { ssl = launchdarkly::foxy::make_ssl_ctx(ssl::context::tlsv12_client); ssl->set_default_verify_paths(); + if (!verify_peer_) { + ssl->set_verify_mode(ssl::context::verify_none); + logging_cb_("SSL peer verification is disabled"); + } } return std::make_shared( From 6dc225f6241f4739a8d51ba05b7d5ac452157136 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 3 Apr 2024 17:41:27 -0700 Subject: [PATCH 04/13] refactor: use sse client peer verification option --- libs/client-sdk/src/data_sources/streaming_data_source.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/client-sdk/src/data_sources/streaming_data_source.cpp b/libs/client-sdk/src/data_sources/streaming_data_source.cpp index fd8c4e29a..2d4d0eea4 100644 --- a/libs/client-sdk/src/data_sources/streaming_data_source.cpp +++ b/libs/client-sdk/src/data_sources/streaming_data_source.cpp @@ -127,6 +127,8 @@ void StreamingDataSource::Start() { client_builder.header(header.first, header.second); } + client_builder.verify_peer(http_config_.Tls().VerifyPeer()); + auto weak_self = weak_from_this(); client_builder.receiver([weak_self](launchdarkly::sse::Event const& event) { From 4cdac192604a1f087cc3da2e5ec52917364d625f Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 Apr 2024 16:28:28 -0700 Subject: [PATCH 05/13] refactor: move verify_peer member --- libs/server-sent-events/include/launchdarkly/sse/client.hpp | 2 +- libs/server-sent-events/src/client.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libs/server-sent-events/include/launchdarkly/sse/client.hpp b/libs/server-sent-events/include/launchdarkly/sse/client.hpp index ec9a83130..382c4114f 100644 --- a/libs/server-sent-events/include/launchdarkly/sse/client.hpp +++ b/libs/server-sent-events/include/launchdarkly/sse/client.hpp @@ -155,10 +155,10 @@ class Builder { std::optional write_timeout_; std::optional connect_timeout_; std::optional initial_reconnect_delay_; - bool verify_peer_; LogCallback logging_cb_; EventReceiver receiver_; ErrorCallback error_cb_; + bool verify_peer_; }; /** diff --git a/libs/server-sent-events/src/client.cpp b/libs/server-sent-events/src/client.cpp index c5c520adf..5a5ada1ff 100644 --- a/libs/server-sent-events/src/client.cpp +++ b/libs/server-sent-events/src/client.cpp @@ -503,7 +503,8 @@ Builder::Builder(net::any_io_executor ctx, std::string url) initial_reconnect_delay_{std::nullopt}, logging_cb_([](auto msg) {}), receiver_([](launchdarkly::sse::Event const&) {}), - error_cb_([](auto err) {}) { + error_cb_([](auto err) {}), + verify_peer_(true) { request_.version(11); request_.set(http::field::user_agent, kDefaultUserAgent); request_.method(http::verb::get); @@ -614,7 +615,7 @@ std::shared_ptr Builder::build() { ssl->set_default_verify_paths(); if (!verify_peer_) { ssl->set_verify_mode(ssl::context::verify_none); - logging_cb_("SSL peer verification is disabled"); + logging_cb_("SSL peer verification disabled"); } } From 6fc1b9b89b8cab13c65af2d8ed3fa76ddff238c1 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 Apr 2024 16:56:09 -0700 Subject: [PATCH 06/13] feat: asio requester now takes peer verification option --- .../include/launchdarkly/events/detail/request_worker.hpp | 2 ++ .../include/launchdarkly/events/detail/worker_pool.hpp | 3 +++ .../include/launchdarkly/network/asio_requester.hpp | 3 ++- libs/internal/src/events/asio_event_processor.cpp | 1 + libs/internal/src/events/request_worker.cpp | 3 ++- libs/internal/src/events/worker_pool.cpp | 7 ++++++- 6 files changed, 16 insertions(+), 3 deletions(-) diff --git a/libs/internal/include/launchdarkly/events/detail/request_worker.hpp b/libs/internal/include/launchdarkly/events/detail/request_worker.hpp index 9046d49eb..0abea6c87 100644 --- a/libs/internal/include/launchdarkly/events/detail/request_worker.hpp +++ b/libs/internal/include/launchdarkly/events/detail/request_worker.hpp @@ -94,12 +94,14 @@ class RequestWorker { * @param retry_after How long to wait after a recoverable failure before * trying to deliver events again. * @param id Unique identifier for the flush worker (used for logging). + * @param mode SSL peer verification mode. * @param logger Logger. */ RequestWorker(boost::asio::any_io_executor io, std::chrono::milliseconds retry_after, std::size_t id, std::optional date_header_locale, + ssl::verify_mode verify_mode, Logger& logger); /** diff --git a/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp b/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp index a6a08441d..3f96b07ad 100644 --- a/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp +++ b/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp @@ -29,11 +29,14 @@ class WorkerPool { * @param pool_size How many workers to make available. * @param delivery_retry_delay How long a worker should wait after a failed * delivery before trying again. + * @param verify_mode True if the SSL peer should be verified, false + * otherwise. * @param logger Logger. */ WorkerPool(boost::asio::any_io_executor io, std::size_t pool_size, std::chrono::milliseconds delivery_retry_delay, + bool verify_peer, Logger& logger); /** diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index 59da54c2b..a4a2950b6 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -255,11 +255,12 @@ class AsioRequester { * must be accounted for. */ public: - AsioRequester(net::any_io_executor ctx) + AsioRequester(net::any_io_executor ctx, boost::asio::ssl::verify_mode mode) : ctx_(std::move(ctx)), ssl_ctx_(std::make_shared( launchdarkly::foxy::make_ssl_ctx(ssl::context::tlsv12_client))) { ssl_ctx_->set_default_verify_paths(); + ssl_ctx_->set_verify_mode(mode); } template diff --git a/libs/internal/src/events/asio_event_processor.cpp b/libs/internal/src/events/asio_event_processor.cpp index e9cf2b525..dad62ee19 100644 --- a/libs/internal/src/events/asio_event_processor.cpp +++ b/libs/internal/src/events/asio_event_processor.cpp @@ -47,6 +47,7 @@ AsioEventProcessor::AsioEventProcessor( workers_(io_, events_config.FlushWorkers(), events_config.DeliveryRetryDelay(), + http_properties.Tls().VerifyPeer(), logger), inbox_capacity_(events_config.Capacity()), inbox_size_(0), diff --git a/libs/internal/src/events/request_worker.cpp b/libs/internal/src/events/request_worker.cpp index b39090294..844909562 100644 --- a/libs/internal/src/events/request_worker.cpp +++ b/libs/internal/src/events/request_worker.cpp @@ -7,11 +7,12 @@ RequestWorker::RequestWorker(boost::asio::any_io_executor io, std::chrono::milliseconds retry_after, std::size_t id, std::optional date_header_locale, + ssl::verify_mode verify_mode, Logger& logger) : timer_(std::move(io)), retry_delay_(retry_after), state_(State::Idle), - requester_(timer_.get_executor()), + requester_(timer_.get_executor(), verify_mode), batch_(std::nullopt), tag_("flush-worker[" + std::to_string(id) + "]: "), date_header_locale_(std::move(date_header_locale)), diff --git a/libs/internal/src/events/worker_pool.cpp b/libs/internal/src/events/worker_pool.cpp index caa4a2b70..aaf479472 100644 --- a/libs/internal/src/events/worker_pool.cpp +++ b/libs/internal/src/events/worker_pool.cpp @@ -22,6 +22,7 @@ std::optional GetLocale(std::string const& locale, WorkerPool::WorkerPool(boost::asio::any_io_executor io, std::size_t pool_size, std::chrono::milliseconds delivery_retry_delay, + bool verify_peer, Logger& logger) : io_(io), workers_() { // The en_US.utf-8 locale is used whenever a date is parsed from the HTTP @@ -33,9 +34,13 @@ WorkerPool::WorkerPool(boost::asio::any_io_executor io, std::optional date_header_locale = GetLocale("en_US.utf-8", "event-processor", logger); + ssl::verify_mode verify_mode = + (verify_peer ? ssl::verify_peer : ssl::verify_none); + for (std::size_t i = 0; i < pool_size; i++) { workers_.emplace_back(std::make_unique( - io_, delivery_retry_delay, i, date_header_locale, logger)); + io_, delivery_retry_delay, i, date_header_locale, verify_mode, + logger)); } } From c66b50d48c8a0562f8adcb98ded149841a216bf9 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 Apr 2024 16:56:48 -0700 Subject: [PATCH 07/13] refactor: update polling mode to pass in peer verification config --- libs/client-sdk/src/data_sources/polling_data_source.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libs/client-sdk/src/data_sources/polling_data_source.cpp b/libs/client-sdk/src/data_sources/polling_data_source.cpp index 4f685c2b2..def05f28a 100644 --- a/libs/client-sdk/src/data_sources/polling_data_source.cpp +++ b/libs/client-sdk/src/data_sources/polling_data_source.cpp @@ -74,7 +74,9 @@ PollingDataSource::PollingDataSource( status_manager_(status_manager), data_source_handler_( DataSourceEventHandler(context, handler, logger, status_manager_)), - requester_(ioc), + requester_(ioc, + http_properties.Tls().VerifyPeer() ? ssl::verify_peer + : ssl::verify_none), timer_(ioc), polling_interval_( std::get< @@ -88,6 +90,9 @@ PollingDataSource::PollingDataSource( auto const& polling_config = std::get< config::shared::built::PollingConfig>( data_source_config.method); + if (!http_properties.Tls().VerifyPeer()) { + LD_LOG(logger_, LogLevel::kDebug) << "SSL peer verification disabled"; + } if (polling_interval_ < polling_config.min_polling_interval) { LD_LOG(logger_, LogLevel::kWarn) << "Polling interval too frequent, defaulting to " From a9773fdef098ab173bc182ea8f8c0d43d83fd1b8 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 Apr 2024 17:00:36 -0700 Subject: [PATCH 08/13] docs: update docs on VerifyPeer C binding --- .../launchdarkly/client_side/bindings/c/config/builder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index 79d4b91c0..1e26f4147 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -422,7 +422,7 @@ LD_EXPORT(void) LDClientHttpPropertiesTlsBuilder_Free(LDClientHttpPropertiesTlsBuilder b); /** - * Enables peer certificate verification. Enabled by default. + * Configures TLS peer certificate verification. Enabled by default. * * Disabling peer verification is not recommended unless a specific * use-case calls for it. From d242c2be72d4506138bfc98db54328dee545da69 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 26 Apr 2024 13:11:17 -0700 Subject: [PATCH 09/13] update internal --- .../events/detail/request_worker.hpp | 15 +++++++++------ .../launchdarkly/events/detail/worker_pool.hpp | 11 ++++++++--- .../launchdarkly/network/asio_requester.hpp | 9 +++++++-- .../src/events/asio_event_processor.cpp | 2 +- libs/internal/src/events/request_worker.cpp | 13 +++++++------ libs/internal/src/events/worker_pool.cpp | 17 +++++++++++++---- 6 files changed, 45 insertions(+), 22 deletions(-) diff --git a/libs/internal/include/launchdarkly/events/detail/request_worker.hpp b/libs/internal/include/launchdarkly/events/detail/request_worker.hpp index 0abea6c87..ea582b409 100644 --- a/libs/internal/include/launchdarkly/events/detail/request_worker.hpp +++ b/libs/internal/include/launchdarkly/events/detail/request_worker.hpp @@ -9,6 +9,8 @@ #include #include +#include + #include namespace launchdarkly::events::detail { @@ -97,12 +99,13 @@ class RequestWorker { * @param mode SSL peer verification mode. * @param logger Logger. */ - RequestWorker(boost::asio::any_io_executor io, - std::chrono::milliseconds retry_after, - std::size_t id, - std::optional date_header_locale, - ssl::verify_mode verify_mode, - Logger& logger); + RequestWorker( + boost::asio::any_io_executor io, + std::chrono::milliseconds retry_after, + std::size_t id, + std::optional date_header_locale, + enum config::shared::built::TlsOptions::VerifyMode verify_mode, + Logger& logger); /** * Returns true if the worker is available for delivery. diff --git a/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp b/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp index 3f96b07ad..088f9a9a1 100644 --- a/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp +++ b/libs/internal/include/launchdarkly/events/detail/worker_pool.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -23,20 +24,24 @@ namespace launchdarkly::events::detail { */ class WorkerPool { public: + WorkerPool(boost::asio::any_io_executor io, + std::size_t pool_size, + std::chrono::milliseconds delivery_retry_delay, + Logger& logger); + /** * Constructs a new WorkerPool. * @param io The executor used for all workers. * @param pool_size How many workers to make available. * @param delivery_retry_delay How long a worker should wait after a failed * delivery before trying again. - * @param verify_mode True if the SSL peer should be verified, false - * otherwise. + * @param verify_mode The TLS verification mode. * @param logger Logger. */ WorkerPool(boost::asio::any_io_executor io, std::size_t pool_size, std::chrono::milliseconds delivery_retry_delay, - bool verify_peer, + enum config::shared::built::TlsOptions::VerifyMode verify_mode, Logger& logger); /** diff --git a/libs/internal/include/launchdarkly/network/asio_requester.hpp b/libs/internal/include/launchdarkly/network/asio_requester.hpp index a4a2950b6..a09214ce2 100644 --- a/libs/internal/include/launchdarkly/network/asio_requester.hpp +++ b/libs/internal/include/launchdarkly/network/asio_requester.hpp @@ -2,6 +2,7 @@ #include "http_requester.hpp" +#include #include #include @@ -29,6 +30,8 @@ using tcp = boost::asio::ip::tcp; namespace launchdarkly::network { +using VerifyMode = enum config::shared::built::TlsOptions::VerifyMode; + static unsigned char const kRedirectLimit = 20; static bool IsAbsolute(std::string_view str) { @@ -255,12 +258,14 @@ class AsioRequester { * must be accounted for. */ public: - AsioRequester(net::any_io_executor ctx, boost::asio::ssl::verify_mode mode) + AsioRequester(net::any_io_executor ctx, VerifyMode verify_mode) : ctx_(std::move(ctx)), ssl_ctx_(std::make_shared( launchdarkly::foxy::make_ssl_ctx(ssl::context::tlsv12_client))) { ssl_ctx_->set_default_verify_paths(); - ssl_ctx_->set_verify_mode(mode); + ssl_ctx_->set_verify_mode(verify_mode == VerifyMode::kVerifyPeer + ? ssl::verify_peer + : ssl::verify_none); } template diff --git a/libs/internal/src/events/asio_event_processor.cpp b/libs/internal/src/events/asio_event_processor.cpp index dad62ee19..b27b70adf 100644 --- a/libs/internal/src/events/asio_event_processor.cpp +++ b/libs/internal/src/events/asio_event_processor.cpp @@ -47,7 +47,7 @@ AsioEventProcessor::AsioEventProcessor( workers_(io_, events_config.FlushWorkers(), events_config.DeliveryRetryDelay(), - http_properties.Tls().VerifyPeer(), + http_properties.Tls().VerifyMode(), logger), inbox_capacity_(events_config.Capacity()), inbox_size_(0), diff --git a/libs/internal/src/events/request_worker.cpp b/libs/internal/src/events/request_worker.cpp index 844909562..e5437777c 100644 --- a/libs/internal/src/events/request_worker.cpp +++ b/libs/internal/src/events/request_worker.cpp @@ -3,12 +3,13 @@ namespace launchdarkly::events::detail { -RequestWorker::RequestWorker(boost::asio::any_io_executor io, - std::chrono::milliseconds retry_after, - std::size_t id, - std::optional date_header_locale, - ssl::verify_mode verify_mode, - Logger& logger) +RequestWorker::RequestWorker( + boost::asio::any_io_executor io, + std::chrono::milliseconds retry_after, + std::size_t id, + std::optional date_header_locale, + enum config::shared::built::TlsOptions::VerifyMode verify_mode, + Logger& logger) : timer_(std::move(io)), retry_delay_(retry_after), state_(State::Idle), diff --git a/libs/internal/src/events/worker_pool.cpp b/libs/internal/src/events/worker_pool.cpp index aaf479472..8cb1962a9 100644 --- a/libs/internal/src/events/worker_pool.cpp +++ b/libs/internal/src/events/worker_pool.cpp @@ -5,6 +5,8 @@ namespace launchdarkly::events::detail { +using namespace launchdarkly::config::shared::built; + std::optional GetLocale(std::string const& locale, std::string const& tag, Logger& logger) { @@ -22,7 +24,17 @@ std::optional GetLocale(std::string const& locale, WorkerPool::WorkerPool(boost::asio::any_io_executor io, std::size_t pool_size, std::chrono::milliseconds delivery_retry_delay, - bool verify_peer, + Logger& logger) + : WorkerPool(std::move(io), + pool_size, + delivery_retry_delay, + TlsOptions::VerifyMode::kVerifyPeer, + logger) {} + +WorkerPool::WorkerPool(boost::asio::any_io_executor io, + std::size_t pool_size, + std::chrono::milliseconds delivery_retry_delay, + enum TlsOptions::VerifyMode verify_mode, Logger& logger) : io_(io), workers_() { // The en_US.utf-8 locale is used whenever a date is parsed from the HTTP @@ -34,9 +46,6 @@ WorkerPool::WorkerPool(boost::asio::any_io_executor io, std::optional date_header_locale = GetLocale("en_US.utf-8", "event-processor", logger); - ssl::verify_mode verify_mode = - (verify_peer ? ssl::verify_peer : ssl::verify_none); - for (std::size_t i = 0; i < pool_size; i++) { workers_.emplace_back(std::make_unique( io_, delivery_retry_delay, i, date_header_locale, verify_mode, From a112e5b6b954fcba57e0fabe4fe302b577ced957 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 26 Apr 2024 13:11:24 -0700 Subject: [PATCH 10/13] update common --- .../shared/builders/http_properties_builder.hpp | 2 +- .../config/shared/built/http_properties.hpp | 7 ++++--- libs/common/src/config/http_properties.cpp | 11 ++++++----- libs/common/src/config/http_properties_builder.cpp | 7 ++++--- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index 853424d73..7af786617 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -47,7 +47,7 @@ class TlsBuilder { [[nodiscard]] built::TlsOptions Build() const; private: - bool verify_peer_; + enum built::TlsOptions::VerifyMode verify_mode_; }; /** * Class used for building a set of HttpProperties. diff --git a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp index f7334f340..51dc62f9d 100644 --- a/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp +++ b/libs/common/include/launchdarkly/config/shared/built/http_properties.hpp @@ -9,12 +9,13 @@ namespace launchdarkly::config::shared::built { class TlsOptions final { public: - TlsOptions(bool verify_peer); + enum class VerifyMode { kVerifyPeer, kVerifyNone }; + TlsOptions(VerifyMode verify_mode); TlsOptions(); - [[nodiscard]] bool VerifyPeer() const; + [[nodiscard]] VerifyMode VerifyMode() const; private: - bool verify_peer_; + enum VerifyMode verify_mode_; }; class HttpProperties final { diff --git a/libs/common/src/config/http_properties.cpp b/libs/common/src/config/http_properties.cpp index fa52651ae..66f40cf71 100644 --- a/libs/common/src/config/http_properties.cpp +++ b/libs/common/src/config/http_properties.cpp @@ -4,12 +4,13 @@ namespace launchdarkly::config::shared::built { -TlsOptions::TlsOptions(bool verify_peer) : verify_peer_(verify_peer) {} +TlsOptions::TlsOptions(enum TlsOptions::VerifyMode verify_mode) + : verify_mode_(verify_mode) {} -TlsOptions::TlsOptions() : TlsOptions(true) {} +TlsOptions::TlsOptions() : TlsOptions(TlsOptions::VerifyMode::kVerifyPeer) {} -bool TlsOptions::VerifyPeer() const { - return verify_peer_; +enum TlsOptions::VerifyMode TlsOptions::VerifyMode() const { + return verify_mode_; } HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout, @@ -57,7 +58,7 @@ bool operator==(HttpProperties const& lhs, HttpProperties const& rhs) { } bool operator==(TlsOptions const& lhs, TlsOptions const& rhs) { - return lhs.VerifyPeer() == rhs.VerifyPeer(); + return lhs.VerifyMode() == rhs.VerifyMode(); } } // namespace launchdarkly::config::shared::built diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index 8c96bbb26..aa7ff271d 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -11,18 +11,19 @@ TlsBuilder::TlsBuilder() : TlsBuilder(shared::Defaults::TLS()) {} template TlsBuilder::TlsBuilder(built::TlsOptions const& tls) { - verify_peer_ = tls.VerifyPeer(); + verify_mode_ = tls.VerifyMode(); } template TlsBuilder& TlsBuilder::VerifyPeer(bool verify_peer) { - verify_peer_ = verify_peer; + verify_mode_ = verify_peer ? built::TlsOptions::VerifyMode::kVerifyPeer + : built::TlsOptions::VerifyMode::kVerifyNone; return *this; } template built::TlsOptions TlsBuilder::Build() const { - return {verify_peer_}; + return {verify_mode_}; } template From 6c5113bed52d564a49c9b077ebdcae587836e6f0 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 26 Apr 2024 13:11:53 -0700 Subject: [PATCH 11/13] update client & server to plumb verify mode option internally --- libs/client-sdk/src/data_sources/polling_data_source.cpp | 7 +++---- libs/client-sdk/src/data_sources/streaming_data_source.cpp | 4 +++- .../sources/polling/polling_data_source.cpp | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libs/client-sdk/src/data_sources/polling_data_source.cpp b/libs/client-sdk/src/data_sources/polling_data_source.cpp index def05f28a..de2a479c2 100644 --- a/libs/client-sdk/src/data_sources/polling_data_source.cpp +++ b/libs/client-sdk/src/data_sources/polling_data_source.cpp @@ -74,9 +74,7 @@ PollingDataSource::PollingDataSource( status_manager_(status_manager), data_source_handler_( DataSourceEventHandler(context, handler, logger, status_manager_)), - requester_(ioc, - http_properties.Tls().VerifyPeer() ? ssl::verify_peer - : ssl::verify_none), + requester_(ioc, http_properties.Tls().VerifyMode()), timer_(ioc), polling_interval_( std::get< @@ -90,7 +88,8 @@ PollingDataSource::PollingDataSource( auto const& polling_config = std::get< config::shared::built::PollingConfig>( data_source_config.method); - if (!http_properties.Tls().VerifyPeer()) { + if (http_properties.Tls().VerifyMode() == + config::shared::built::TlsOptions::VerifyMode::kVerifyNone) { LD_LOG(logger_, LogLevel::kDebug) << "SSL peer verification disabled"; } if (polling_interval_ < polling_config.min_polling_interval) { diff --git a/libs/client-sdk/src/data_sources/streaming_data_source.cpp b/libs/client-sdk/src/data_sources/streaming_data_source.cpp index 2d4d0eea4..6ffb25059 100644 --- a/libs/client-sdk/src/data_sources/streaming_data_source.cpp +++ b/libs/client-sdk/src/data_sources/streaming_data_source.cpp @@ -127,7 +127,9 @@ void StreamingDataSource::Start() { client_builder.header(header.first, header.second); } - client_builder.verify_peer(http_config_.Tls().VerifyPeer()); + client_builder.verify_peer( + http_config_.Tls().VerifyMode() == + config::shared::built::TlsOptions::VerifyMode::kVerifyPeer); auto weak_self = weak_from_this(); diff --git a/libs/server-sdk/src/data_systems/background_sync/sources/polling/polling_data_source.cpp b/libs/server-sdk/src/data_systems/background_sync/sources/polling/polling_data_source.cpp index f88eb5cc9..5fcd7aa09 100644 --- a/libs/server-sdk/src/data_systems/background_sync/sources/polling/polling_data_source.cpp +++ b/libs/server-sdk/src/data_systems/background_sync/sources/polling/polling_data_source.cpp @@ -52,7 +52,7 @@ PollingDataSource::PollingDataSource( config::built::HttpProperties const& http_properties) : logger_(logger), status_manager_(status_manager), - requester_(ioc), + requester_(ioc, http_properties.Tls().VerifyMode()), polling_interval_(data_source_config.poll_interval), request_(MakeRequest(data_source_config, endpoints, http_properties)), timer_(ioc), From 69e0a2ffaffb537508c55f4b461be182d203ed03 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 26 Apr 2024 13:17:57 -0700 Subject: [PATCH 12/13] free tls builder automatically when passed to HttpProperties builder in c binding --- libs/client-sdk/src/bindings/c/builder.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index a59200f23..5fef588a4 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -319,6 +319,8 @@ LDClientConfigBuilder_HttpProperties_Tls( LD_ASSERT_NOT_NULL(tls_builder); TO_BUILDER(b)->HttpProperties().Tls(*TO_TLS_BUILDER(tls_builder)); + + LDClientHttpPropertiesTlsBuilder_Free(tls_builder); } LD_EXPORT(void) From 8df929f295dd434e74186c4b3b652d348b9991f2 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Thu, 9 May 2024 10:50:12 -0700 Subject: [PATCH 13/13] rename VerifyPeer to SkipVerifyPeer --- .../client_side/bindings/c/config/builder.h | 10 ++++++---- libs/client-sdk/src/bindings/c/builder.cpp | 7 ++++--- libs/client-sdk/tests/client_config_test.cpp | 2 +- .../config/shared/builders/http_properties_builder.hpp | 6 +++--- libs/common/src/config/http_properties_builder.cpp | 7 ++++--- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index 1e26f4147..1eb87040d 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -422,17 +422,19 @@ LD_EXPORT(void) LDClientHttpPropertiesTlsBuilder_Free(LDClientHttpPropertiesTlsBuilder b); /** - * Configures TLS peer certificate verification. Enabled by default. + * Configures TLS peer certificate verification. Peer verification + * is enabled by default. * * Disabling peer verification is not recommended unless a specific * use-case calls for it. * * @param b Client config builder. Must not be NULL. - * @param verify_peer True to verify peer, false otherwise. + * @param skip_verify_peer False to skip verification. */ LD_EXPORT(void) -LDClientHttpPropertiesTlsBuilder_VerifyPeer(LDClientHttpPropertiesTlsBuilder b, - bool verify_peer); +LDClientHttpPropertiesTlsBuilder_SkipVerifyPeer( + LDClientHttpPropertiesTlsBuilder b, + bool skip_verify_peer); /** * Disables the default SDK logging. diff --git a/libs/client-sdk/src/bindings/c/builder.cpp b/libs/client-sdk/src/bindings/c/builder.cpp index 5fef588a4..f1071a73d 100644 --- a/libs/client-sdk/src/bindings/c/builder.cpp +++ b/libs/client-sdk/src/bindings/c/builder.cpp @@ -324,11 +324,12 @@ LDClientConfigBuilder_HttpProperties_Tls( } LD_EXPORT(void) -LDClientHttpPropertiesTlsBuilder_VerifyPeer(LDClientHttpPropertiesTlsBuilder b, - bool verify_peer) { +LDClientHttpPropertiesTlsBuilder_SkipVerifyPeer( + LDClientHttpPropertiesTlsBuilder b, + bool skip_verify_peer) { LD_ASSERT_NOT_NULL(b); - TO_TLS_BUILDER(b)->VerifyPeer(verify_peer); + TO_TLS_BUILDER(b)->SkipVerifyPeer(skip_verify_peer); } LD_EXPORT(LDClientHttpPropertiesTlsBuilder) diff --git a/libs/client-sdk/tests/client_config_test.cpp b/libs/client-sdk/tests/client_config_test.cpp index d6eb93947..c62357541 100644 --- a/libs/client-sdk/tests/client_config_test.cpp +++ b/libs/client-sdk/tests/client_config_test.cpp @@ -81,7 +81,7 @@ TEST(ClientConfigBindings, AllConfigs) { LDClientHttpPropertiesTlsBuilder tls_builder = LDClientHttpPropertiesTlsBuilder_New(); - LDClientHttpPropertiesTlsBuilder_VerifyPeer(tls_builder, false); + LDClientHttpPropertiesTlsBuilder_SkipVerifyPeer(tls_builder, false); LDClientConfigBuilder_HttpProperties_Tls(builder, tls_builder); LDClientHttpPropertiesTlsBuilder tls_builder2 = diff --git a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp index 7af786617..b46bd5deb 100644 --- a/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp +++ b/libs/common/include/launchdarkly/config/shared/builders/http_properties_builder.hpp @@ -34,11 +34,11 @@ class TlsBuilder { TlsBuilder(built::TlsOptions const& tls); /** - * Whether the remote peer's certificates should be verified. - * @param verify_peer True to verify peer, false otherwise. + * Whether to skip verifying the remote peer's certificates. + * @param verify_peer True to skip verification, false to verify. * @return A reference to this builder. */ - TlsBuilder& VerifyPeer(bool verify_peer); + TlsBuilder& SkipVerifyPeer(bool skip_verify_peer); /** * Builds the TLS options. diff --git a/libs/common/src/config/http_properties_builder.cpp b/libs/common/src/config/http_properties_builder.cpp index aa7ff271d..7fb9d6f28 100644 --- a/libs/common/src/config/http_properties_builder.cpp +++ b/libs/common/src/config/http_properties_builder.cpp @@ -15,9 +15,10 @@ TlsBuilder::TlsBuilder(built::TlsOptions const& tls) { } template -TlsBuilder& TlsBuilder::VerifyPeer(bool verify_peer) { - verify_mode_ = verify_peer ? built::TlsOptions::VerifyMode::kVerifyPeer - : built::TlsOptions::VerifyMode::kVerifyNone; +TlsBuilder& TlsBuilder::SkipVerifyPeer(bool skip_verify_peer) { + verify_mode_ = skip_verify_peer + ? built::TlsOptions::VerifyMode::kVerifyNone + : built::TlsOptions::VerifyMode::kVerifyPeer; return *this; }