Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: add ability to disable SSL peer verification #391

Closed
wants to merge 13 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -333,7 +335,6 @@ LD_EXPORT(void) LDDataSourceStreamBuilder_Free(LDDataSourceStreamBuilder b);
*
* @return New builder for Polling method.
*/

LD_EXPORT(LDDataSourcePollBuilder)
LDDataSourcePollBuilder_New();

Expand Down Expand Up @@ -390,6 +391,51 @@ 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);

/**
* 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 skip_verify_peer False to skip verification.
*/
LD_EXPORT(void)
LDClientHttpPropertiesTlsBuilder_SkipVerifyPeer(
LDClientHttpPropertiesTlsBuilder b,
bool skip_verify_peer);

/**
* Disables the default SDK logging.
* @param b Client config builder. Must not be NULL.
Expand Down
36 changes: 36 additions & 0 deletions libs/client-sdk/src/bindings/c/builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ using namespace launchdarkly::client_side;
#define FROM_CUSTOM_PERSISTENCE_BUILDER(ptr) \
(reinterpret_cast<LDPersistenceCustomBuilder>(ptr))

#define TO_TLS_BUILDER(ptr) (reinterpret_cast<TlsBuilder*>(ptr))

#define FROM_TLS_BUILDER(ptr) \
(reinterpret_cast<LDClientHttpPropertiesTlsBuilder>(ptr))

class PersistenceImplementationWrapper : public IPersistence {
public:
explicit PersistenceImplementationWrapper(LDPersistence impl)
Expand Down Expand Up @@ -306,6 +311,37 @@ 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));

LDClientHttpPropertiesTlsBuilder_Free(tls_builder);
}

LD_EXPORT(void)
LDClientHttpPropertiesTlsBuilder_SkipVerifyPeer(
LDClientHttpPropertiesTlsBuilder b,
bool skip_verify_peer) {
LD_ASSERT_NOT_NULL(b);

TO_TLS_BUILDER(b)->SkipVerifyPeer(skip_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);
Expand Down
6 changes: 5 additions & 1 deletion libs/client-sdk/src/data_sources/polling_data_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
config::shared::built::DataSourceConfig<config::shared::ClientSDK> const&
data_source_config,
config::shared::built::HttpProperties const& http_properties,
boost::asio::any_io_executor ioc,

Check warning on line 67 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:67:34 [performance-unnecessary-value-param]

the parameter 'ioc' is copied for each invocation but only used as a const reference; consider making it a const reference
Context const& context,
IDataSourceUpdateSink& handler,
DataSourceStatusManager& status_manager,
Expand All @@ -74,7 +74,7 @@
status_manager_(status_manager),
data_source_handler_(
DataSourceEventHandler(context, handler, logger, status_manager_)),
requester_(ioc),
requester_(ioc, http_properties.Tls().VerifyMode()),
timer_(ioc),
polling_interval_(
std::get<
Expand All @@ -88,6 +88,10 @@
auto const& polling_config = std::get<
config::shared::built::PollingConfig<config::shared::ClientSDK>>(
data_source_config.method);
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) {
LD_LOG(logger_, LogLevel::kWarn)
<< "Polling interval too frequent, defaulting to "
Expand All @@ -111,7 +115,7 @@
});
}

void PollingDataSource::HandlePollResult(network::HttpResult res) {

Check warning on line 118 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:118:62 [performance-unnecessary-value-param]

the parameter 'res' is copied for each invocation but only used as a const reference; consider making it a const reference
auto header_etag = res.Headers().find("etag");
bool has_etag = header_etag != res.Headers().end();

Expand Down Expand Up @@ -141,13 +145,13 @@
status_manager_.SetState(
DataSourceStatus::DataSourceState::kInterrupted,
DataSourceStatus::ErrorInfo::ErrorKind::kNetworkError,
res.ErrorMessage() ? *res.ErrorMessage() : "unknown error");

Check warning on line 148 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:148:35 [bugprone-unchecked-optional-access]

unchecked access to optional value
LD_LOG(logger_, LogLevel::kWarn)
<< "Polling for feature flag updates failed: "
<< (res.ErrorMessage() ? *res.ErrorMessage() : "unknown error");

Check warning on line 151 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:151:39 [bugprone-unchecked-optional-access]

unchecked access to optional value
} else if (res.Status() == 200) {

Check warning on line 152 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:152:32 [cppcoreguidelines-avoid-magic-numbers

200 is a magic number; consider replacing it with a named constant
data_source_handler_.HandleMessage("put", res.Body().value());

Check warning on line 153 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:153:51 [bugprone-unchecked-optional-access]

unchecked access to optional value
} else if (res.Status() == 304) {

Check warning on line 154 in libs/client-sdk/src/data_sources/polling_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/polling_data_source.cpp:154:32 [cppcoreguidelines-avoid-magic-numbers

304 is a magic number; consider replacing it with a named constant
// This should be handled ahead of here, but if we get a 304,
// and it didn't have an etag, we still don't want to try to
// parse the body.
Expand Down
4 changes: 4 additions & 0 deletions libs/client-sdk/src/data_sources/streaming_data_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@

StreamingDataSource::StreamingDataSource(
config::shared::built::ServiceEndpoints const& endpoints,
config::shared::built::DataSourceConfig<config::shared::ClientSDK> const&

Check warning on line 39 in libs/client-sdk/src/data_sources/streaming_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/streaming_data_source.cpp:39:5 [modernize-pass-by-value]

pass by value and use std::move
data_source_config,
config::shared::built::HttpProperties const& http_properties,

Check warning on line 41 in libs/client-sdk/src/data_sources/streaming_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/streaming_data_source.cpp:41:5 [modernize-pass-by-value]

pass by value and use std::move
boost::asio::any_io_executor ioc,

Check warning on line 42 in libs/client-sdk/src/data_sources/streaming_data_source.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/src/data_sources/streaming_data_source.cpp:42:5 [modernize-pass-by-value]

pass by value and use std::move
Context context,
IDataSourceUpdateSink& handler,
DataSourceStatusManager& status_manager,
Expand Down Expand Up @@ -127,6 +127,10 @@
client_builder.header(header.first, header.second);
}

client_builder.verify_peer(
http_config_.Tls().VerifyMode() ==
config::shared::built::TlsOptions::VerifyMode::kVerifyPeer);

auto weak_self = weak_from_this();

client_builder.receiver([weak_self](launchdarkly::sse::Event const& event) {
Expand Down
15 changes: 15 additions & 0 deletions libs/client-sdk/tests/client_config_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_SkipVerifyPeer(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();
Expand Down
1 change: 1 addition & 0 deletions libs/common/include/launchdarkly/config/client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ using HttpPropertiesBuilder =
using DataSourceBuilder = config::shared::builders::DataSourceBuilder<SDK>;
using LoggingBuilder = config::shared::builders::LoggingBuilder;
using PersistenceBuilder = config::shared::builders::PersistenceBuilder<SDK>;
using TlsBuilder = config::shared::builders::TlsBuilder<SDK>;

using Config = config::Config<SDK>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename SDK>
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 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& SkipVerifyPeer(bool skip_verify_peer);

/**
* Builds the TLS options.
* @return The built options.
*/
[[nodiscard]] built::TlsOptions Build() const;

private:
enum built::TlsOptions::VerifyMode verify_mode_;
};
/**
* Class used for building a set of HttpProperties.
* @tparam SDK The SDK type to build properties for. This affects the default
Expand Down Expand Up @@ -116,6 +155,13 @@ class HttpPropertiesBuilder {
HttpPropertiesBuilder& Header(std::string key,
std::optional<std::string> value);

/**
* Sets the builder for TLS properties.
* @param builder The TLS property builder.
* @return A reference to this builder.
*/
HttpPropertiesBuilder& Tls(TlsBuilder<SDK> builder);

/**
* Build a set of HttpProperties.
* @return The built properties.
Expand All @@ -130,6 +176,7 @@ class HttpPropertiesBuilder {
std::string wrapper_name_;
std::string wrapper_version_;
std::map<std::string, std::string> base_headers_;
TlsBuilder<SDK> tls_;
};

} // namespace launchdarkly::config::shared::builders
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,25 @@

namespace launchdarkly::config::shared::built {

class TlsOptions final {
public:
enum class VerifyMode { kVerifyPeer, kVerifyNone };
TlsOptions(VerifyMode verify_mode);
TlsOptions();
[[nodiscard]] VerifyMode VerifyMode() const;

private:
enum VerifyMode verify_mode_;
};

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<std::string, std::string> base_headers);
std::map<std::string, std::string> base_headers,
TlsOptions tls);

[[nodiscard]] std::chrono::milliseconds ConnectTimeout() const;
[[nodiscard]] std::chrono::milliseconds ReadTimeout() const;
Expand All @@ -22,16 +34,20 @@ class HttpProperties final {
[[nodiscard]] std::chrono::milliseconds ResponseTimeout() const;
[[nodiscard]] std::map<std::string, std::string> const& BaseHeaders() const;

[[nodiscard]] TlsOptions 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<std::string, std::string> base_headers_;
TlsOptions tls_;

// TODO: Proxy.
};

bool operator==(HttpProperties const& lhs, HttpProperties const& rhs);
bool operator==(TlsOptions const& lhs, TlsOptions const& rhs);

} // namespace launchdarkly::config::shared::built
22 changes: 16 additions & 6 deletions libs/common/include/launchdarkly/config/shared/defaults.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,15 @@ struct Defaults<ClientSDK> {
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<std::string, std::string>()};
return {std::chrono::seconds{10},
std::chrono::seconds{10},
std::chrono::seconds{10},
std::chrono::seconds{10},
std::map<std::string, std::string>(),
TLS()};
}

static auto StreamingConfig() -> shared::built::StreamingConfig<ClientSDK> {
Expand Down Expand Up @@ -92,10 +97,15 @@ struct Defaults<ServerSDK> {
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<std::string, std::string>()};
return {std::chrono::seconds{10},
std::chrono::seconds{10},
std::chrono::seconds{10},
std::chrono::seconds{10},
std::map<std::string, std::string>(),
TLS()};
}

static auto StreamingConfig() -> built::StreamingConfig<ServerSDK> {
Expand Down
25 changes: 22 additions & 3 deletions libs/common/src/config/http_properties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,27 @@

namespace launchdarkly::config::shared::built {

TlsOptions::TlsOptions(enum TlsOptions::VerifyMode verify_mode)
: verify_mode_(verify_mode) {}

TlsOptions::TlsOptions() : TlsOptions(TlsOptions::VerifyMode::kVerifyPeer) {}

enum TlsOptions::VerifyMode TlsOptions::VerifyMode() const {
return verify_mode_;
}

HttpProperties::HttpProperties(std::chrono::milliseconds connect_timeout,
std::chrono::milliseconds read_timeout,
std::chrono::milliseconds write_timeout,
std::chrono::milliseconds response_timeout,
std::map<std::string, std::string> base_headers)
std::map<std::string, std::string> base_headers,
TlsOptions 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_;
Expand All @@ -35,11 +46,19 @@ std::map<std::string, std::string> const& HttpProperties::BaseHeaders() const {
return base_headers_;
}

TlsOptions const& 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==(TlsOptions const& lhs, TlsOptions const& rhs) {
return lhs.VerifyMode() == rhs.VerifyMode();
}

} // namespace launchdarkly::config::shared::built
Loading
Loading