From 0c79543e621dbdca4cc4b8d986b292e604f4a5fc Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Mon, 18 Dec 2023 12:46:50 +0100 Subject: [PATCH 01/14] Parses schema extraction remote config configuration --- .../listeners/asm_features_listener.cpp | 48 +++++- .../listeners/asm_features_listener.hpp | 3 + appsec/src/helper/service_config.hpp | 9 + .../listeners/asm_features_listener_test.cpp | 154 +++++++++++++++++- 4 files changed, 205 insertions(+), 9 deletions(-) diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp index edd8ae0f66..1eba7cc1df 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp @@ -9,18 +9,42 @@ #include "remote_config/exception.hpp" #include "utils.hpp" #include -#include -void dds::remote_config::asm_features_listener::on_update(const config &config) +void dds::remote_config::asm_features_listener::parse_api_security( + const rapidjson::Document &serialized_doc) { - rapidjson::Document serialized_doc; - if (!json_helper::get_json_base64_encoded_content( - config.contents, serialized_doc)) { - throw error_applying_config("Invalid config contents"); + auto api_security_itr = json_helper::get_field_of_type( + serialized_doc, "api_security", rapidjson::kObjectType); + + if (!api_security_itr) { + throw error_applying_config("Invalid config json encoded contents: " + "api_security key missing or invalid"); } + auto request_sample_rate_itr = + api_security_itr.value()->value.FindMember("request_sample_rate"); + if (request_sample_rate_itr == + api_security_itr.value()->value.MemberEnd()) { + throw error_applying_config("Invalid config json encoded contents: " + "request_sample_rate key missing"); + } + + if (request_sample_rate_itr->value.GetType() != rapidjson::kNumberType || + !request_sample_rate_itr->value.IsDouble()) { + throw error_applying_config("Invalid config json encoded contents: " + "request_sample_rate is not double"); + } + + service_config_->set_request_sample_rate( + request_sample_rate_itr->value.GetDouble()); +} + +void dds::remote_config::asm_features_listener::parse_asm( + const rapidjson::Document &serialized_doc) +{ auto asm_itr = json_helper::get_field_of_type( serialized_doc, "asm", rapidjson::kObjectType); + if (!asm_itr) { throw error_applying_config("Invalid config json encoded contents: " "asm key missing or invalid"); @@ -51,3 +75,15 @@ void dds::remote_config::asm_features_listener::on_update(const config &config) "Invalid config json encoded contents: enabled key invalid"); } } + +void dds::remote_config::asm_features_listener::on_update(const config &config) +{ + rapidjson::Document serialized_doc; + if (!json_helper::get_json_base64_encoded_content( + config.contents, serialized_doc)) { + throw error_applying_config("Invalid config contents"); + } + + parse_asm(serialized_doc); + parse_api_security(serialized_doc); +} diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp index ab91cc42be..f290cfbe6f 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp @@ -8,6 +8,7 @@ #include "config.hpp" #include "listener.hpp" #include "service_config.hpp" +#include namespace dds::remote_config { @@ -32,6 +33,8 @@ class asm_features_listener : public listener_base { void commit() override {} protected: + void parse_asm(const rapidjson::Document &serialized_doc); + void parse_api_security(const rapidjson::Document &serialized_doc); std::shared_ptr service_config_; }; diff --git a/appsec/src/helper/service_config.hpp b/appsec/src/helper/service_config.hpp index 475ddf8a37..cf70961155 100644 --- a/appsec/src/helper/service_config.hpp +++ b/appsec/src/helper/service_config.hpp @@ -19,9 +19,18 @@ struct service_config { void disable_asm() { asm_enabled = enable_asm_status::DISABLED; } void unset_asm() { asm_enabled = enable_asm_status::NOT_SET; } enable_asm_status get_asm_enabled_status() { return asm_enabled; } + double get_request_sample_rate() { return request_sample_rate; } + void set_request_sample_rate(double sample_rate) + { + if (sample_rate < 0 || sample_rate > 1) { + return; + } + request_sample_rate = sample_rate; + } protected: std::atomic asm_enabled = {enable_asm_status::NOT_SET}; + std::atomic request_sample_rate = 0.1; }; } // namespace dds diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index 5d1af34b73..8b11e202f0 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -25,9 +25,22 @@ remote_config::config get_config(const std::string &content, bool encode = true) ""}; } +remote_config::config get_config_with_status_and_sample_rate( + std::string status, double sample_rate) +{ + return get_config("{\"asm\":{\"enabled\":" + status + + "}, \"api_security\": { \"request_sample_rate\": " + + std::to_string(sample_rate) + "}}"); +} + remote_config::config get_config_with_status(std::string status) { - return get_config("{\"asm\":{\"enabled\":" + status + "}}"); + return get_config_with_status_and_sample_rate(status, 0.1); +} + +remote_config::config get_config_with_sample_rate(double sample_rate) +{ + return get_config_with_status_and_sample_rate("true", sample_rate); } remote_config::config get_enabled_config(bool as_string = true) @@ -222,7 +235,8 @@ TEST( "Invalid config json encoded contents: enabled key missing"; auto remote_config_service = std::make_shared(); remote_config::asm_features_listener listener(remote_config_service); - remote_config::config enabled_key_missing = get_config("{ \"asm\": {}}"); + remote_config::config enabled_key_missing = get_config( + "{ \"asm\": {}, \"api_security\": { \"request_sample_rate\": 0.1}}"); try { listener.on_update(enabled_key_missing); @@ -244,7 +258,8 @@ TEST(RemoteConfigAsmFeaturesListener, auto remote_config_service = std::make_shared(); remote_config::asm_features_listener listener(remote_config_service); remote_config::config enabled_key_invalid = - get_config("{ \"asm\": { \"enabled\": 123}}"); + get_config("{ \"asm\": { \"enabled\": 123}, \"api_security\": { " + "\"request_sample_rate\": 0.1}}"); try { listener.on_update(enabled_key_invalid); @@ -273,4 +288,137 @@ TEST(RemoteConfigAsmFeaturesListener, WhenListenerGetsUnapplyItGetsNotSet) remote_config_service->get_asm_enabled_status()); } +TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoApiSecurityPresent) +{ + std::string error_message = ""; + std::string expected_error_message = + "Invalid config json encoded contents: api_security key missing or " + "invalid"; + auto remote_config_service = std::make_shared(); + remote_config::asm_features_listener listener(remote_config_service); + remote_config::config payload = + get_config("{ \"asm\": { \"enabled\": true}}"); + + try { + listener.on_update(payload); + } catch (remote_config::error_applying_config &error) { + error_message = error.what(); + } + + EXPECT_EQ(0, error_message.compare(expected_error_message)); +} + +TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenApiSecurityHasWrongType) +{ + std::string error_message = ""; + std::string expected_error_message = + "Invalid config json encoded contents: api_security key missing or " + "invalid"; + auto remote_config_service = std::make_shared(); + remote_config::asm_features_listener listener(remote_config_service); + remote_config::config payload = + get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": 1234}"); + + try { + listener.on_update(payload); + } catch (remote_config::error_applying_config &error) { + error_message = error.what(); + } + + EXPECT_EQ(0, error_message.compare(expected_error_message)); +} + +TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoRequestSampleRatePresent) +{ + std::string error_message = ""; + std::string expected_error_message = + "Invalid config json encoded contents: request_sample_rate key missing"; + auto remote_config_service = std::make_shared(); + remote_config::asm_features_listener listener(remote_config_service); + remote_config::config payload = + get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": {}}"); + + try { + listener.on_update(payload); + } catch (remote_config::error_applying_config &error) { + error_message = error.what(); + } + + EXPECT_EQ(0, error_message.compare(expected_error_message)); +} + +TEST(RemoteConfigAsmFeaturesListener, + ThrowsErrorWhenRequestSampleRateHasWrongType) +{ + std::string error_message = ""; + std::string expected_error_message = + "Invalid config json encoded contents: request_sample_rate is not " + "double"; + auto remote_config_service = std::make_shared(); + remote_config::asm_features_listener listener(remote_config_service); + remote_config::config payload = + get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": { " + "\"request_sample_rate\": true}}"); + + try { + listener.on_update(payload); + } catch (remote_config::error_applying_config &error) { + error_message = error.what(); + } + + EXPECT_EQ(0, error_message.compare(expected_error_message)); +} + +TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) +{ + auto remote_config_service = std::make_shared(); + remote_config::asm_features_listener listener(remote_config_service); + + { // It parses floats + auto sample_rate = 0.12; + try { + listener.on_update(get_config_with_sample_rate(sample_rate)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ( + sample_rate, remote_config_service->get_request_sample_rate()); + } + + { // It parses integers + auto sample_rate = 0; + try { + listener.on_update(get_config_with_sample_rate(sample_rate)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ( + sample_rate, remote_config_service->get_request_sample_rate()); + } +} + +TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateLimits) +{ + auto remote_config_service = std::make_shared(); + remote_config::asm_features_listener listener(remote_config_service); + + { // Over 1, sets default 0.1 + try { + listener.on_update(get_config_with_sample_rate(2)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ(0.1, remote_config_service->get_request_sample_rate()); + } + + { // Below 0, sets default 0.1 + try { + listener.on_update(get_config_with_sample_rate(-2)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ(0.1, remote_config_service->get_request_sample_rate()); + } +} + } // namespace dds From 89a322819bbc20a42bd2cccd5cfd5f0083d3f3a8 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Tue, 19 Dec 2023 16:58:04 +0100 Subject: [PATCH 02/14] Add api security product to remote config --- .../helper/remote_config/listeners/asm_features_listener.hpp | 4 +++- appsec/src/helper/remote_config/protocol/client.hpp | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp index f290cfbe6f..6f5abdd6ed 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp @@ -26,7 +26,9 @@ class asm_features_listener : public listener_base { [[nodiscard]] std::unordered_map get_supported_products() override { - return {{"ASM_FEATURES", protocol::capabilities_e::ASM_ACTIVATION}}; + return {{"ASM_FEATURES", protocol::capabilities_e::ASM_ACTIVATION}, + {"ASM_API_SECURITY_SAMPLE_RATE", + protocol::capabilities_e::ASM_API_SECURITY_SAMPLE_RATE}}; } void init() override {} diff --git a/appsec/src/helper/remote_config/protocol/client.hpp b/appsec/src/helper/remote_config/protocol/client.hpp index 1e13d08a7c..107d4ef1f6 100644 --- a/appsec/src/helper/remote_config/protocol/client.hpp +++ b/appsec/src/helper/remote_config/protocol/client.hpp @@ -27,6 +27,7 @@ enum class capabilities_e : uint16_t { ASM_CUSTOM_RULES = 1 << 8, ASM_CUSTOM_BLOCKING_RESPONSE = 1 << 9, ASM_TRUSTED_IPS = 1 << 10, + ASM_API_SECURITY_SAMPLE_RATE = 1 << 11, }; constexpr capabilities_e operator|( From 52c651aaa30c86cfddbaa402aef28e1036f1e791 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Wed, 20 Dec 2023 16:36:56 +0100 Subject: [PATCH 03/14] Update sampler rate based on remote config --- appsec/src/helper/client.cpp | 2 + appsec/src/helper/sampler.hpp | 32 ++++++++---- appsec/src/helper/service.cpp | 1 + appsec/tests/helper/client_test.cpp | 73 ++++++++++++++++++++++++++++ appsec/tests/helper/sampler_test.cpp | 46 ++++++++++++++++++ appsec/tests/helper/service_test.cpp | 5 ++ 6 files changed, 149 insertions(+), 10 deletions(-) diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index d7f859e790..e949b132fe 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -441,6 +441,8 @@ bool client::handle_command(network::request_shutdown::request &command) auto sampler = service_->get_schema_sampler(); std::optional scope; if (sampler) { + sampler->set_sampler_rate( + service_->get_service_config()->get_request_sample_rate()); scope = sampler->get(); if (scope.has_value()) { parameter context_processor = parameter::map(); diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index 4a3dc2cc8c..909f7ba573 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -19,15 +19,7 @@ class sampler { public: sampler(double sample_rate) : sample_rate_(sample_rate) { - // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - if (sample_rate_ <= 0) { - sample_rate_ = 0; - } else if (sample_rate_ > 1) { - sample_rate_ = 1; - } else if (sample_rate_ < min_rate) { - sample_rate_ = min_rate; - } - // NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + set_sampler_rate(sample_rate); } class scope { public: @@ -82,9 +74,29 @@ class sampler { return result; } + void set_sampler_rate(double sampler_rate) + { + // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + if (sampler_rate <= 0) { + sampler_rate = 0; + } else if (sampler_rate > 1) { + sampler_rate = 1; + } else if (sampler_rate < min_rate) { + sampler_rate = min_rate; + } + // NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + + if (sampler_rate == sample_rate_) { + return; + } + + request_ = 1; + sample_rate_ = sampler_rate; + } + protected: unsigned request_{1}; - double sample_rate_; + double sample_rate_{0}; std::atomic concurrent_{false}; std::mutex mtx_; }; diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index 561675deb3..0b8c3f97a3 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -31,6 +31,7 @@ service::service(std::shared_ptr engine, } schema_sampler_ = std::make_shared(sample_rate); + service_config_->set_request_sample_rate(sample_rate); } service::ptr service::from_settings(service_identifier &&id, diff --git a/appsec/tests/helper/client_test.cpp b/appsec/tests/helper/client_test.cpp index 7a76029992..2bf6ab4162 100644 --- a/appsec/tests/helper/client_test.cpp +++ b/appsec/tests/helper/client_test.cpp @@ -2408,4 +2408,77 @@ TEST(ClientTest, SchemasOverTheLimitAreCompressed) } } +TEST(ClientTest, SchemaSamplerRateIsUpdatedFromService) +{ + auto smanager = std::make_shared(); + auto broker = new mock::broker(); + client c(smanager, std::unique_ptr(broker)); + + network::client_init::request msg = get_default_client_init_msg(); + msg.enabled_configuration = EXTENSION_CONFIGURATION_ENABLED; + msg.engine_settings.schema_extraction.enabled = true; + msg.engine_settings.schema_extraction.sample_rate = + 1; // All request should be get + + send_client_init(broker, c, std::move(msg)); + + { // First request + request_init(broker, c); + + // Request Shutdown + { + network::request_shutdown::request msg; + msg.data = parameter::map(); + msg.data.add("server.request.headers.no_cookies", + parameter::string("acunetix-product"sv)); + + network::request req(std::move(msg)); + + std::shared_ptr res; + EXPECT_CALL(*broker, recv(_)).WillOnce(Return(req)); + EXPECT_CALL(*broker, + send(testing::An< + const std::shared_ptr &>())) + .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); + + EXPECT_TRUE(c.run_request()); + auto msg_res = + dynamic_cast(res.get()); + EXPECT_FALSE(msg_res->meta.empty()); + EXPECT_GT(count_schemas(msg_res->meta), 0); + EXPECT_STREQ( + msg_res->meta["_dd.appsec.s.req.headers.no_cookies"].c_str(), + "[8]"); + } + } + // Setting this to zero means no more request get schema extraction + c.get_service()->get_service_config()->set_request_sample_rate(0); + { // First request + request_init(broker, c); + + // Request Shutdown + { + network::request_shutdown::request msg; + msg.data = parameter::map(); + msg.data.add("server.request.headers.no_cookies", + parameter::string("acunetix-product"sv)); + + network::request req(std::move(msg)); + + std::shared_ptr res; + EXPECT_CALL(*broker, recv(_)).WillOnce(Return(req)); + EXPECT_CALL(*broker, + send(testing::An< + const std::shared_ptr &>())) + .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); + + EXPECT_TRUE(c.run_request()); + auto msg_res = + dynamic_cast(res.get()); + EXPECT_FALSE(msg_res->meta.empty()); + EXPECT_EQ(count_schemas(msg_res->meta), 0); + } + } +} + } // namespace dds diff --git a/appsec/tests/helper/sampler_test.cpp b/appsec/tests/helper/sampler_test.cpp index eb2c90d8a8..b0c101b98d 100644 --- a/appsec/tests/helper/sampler_test.cpp +++ b/appsec/tests/helper/sampler_test.cpp @@ -16,6 +16,10 @@ class sampler : public dds::sampler { sampler(double sample_rate) : dds::sampler(sample_rate) {} void set_request(unsigned int i) { request_ = i; } auto get_request() { return request_; } + void set_sampler_rate(double sampler_rate) + { + dds::sampler::set_sampler_rate(sampler_rate); + } }; } // namespace mock @@ -202,6 +206,48 @@ TEST(SamplerTest, TestOverflow) EXPECT_EQ(1, s.get_request()); } +TEST(SamplerTest, ModifySamplerRate) +{ + { // New sampler rate reset requests + mock::sampler s(0.1); + s.get(); + EXPECT_EQ(2, s.get_request()); + s.set_sampler_rate(0.2); + EXPECT_EQ(1, s.get_request()); + } + { // Setting same sampler rate does do anything + mock::sampler s(0.1); + s.get(); + EXPECT_EQ(2, s.get_request()); + s.set_sampler_rate(0.1); + EXPECT_EQ(2, s.get_request()); + } + { // Over Zero: If given rate is invalid and gets defaulted to a value which + // is same as before, it does not change anything + mock::sampler s(3); + s.get(); + EXPECT_EQ(2, s.get_request()); + s.set_sampler_rate(4); + EXPECT_EQ(2, s.get_request()); + } + { // Below zero: If given rate is invalid and gets defaulted to a value + // which is same as before, it does not change anything + mock::sampler s(-3); + s.get(); + EXPECT_EQ(2, s.get_request()); + s.set_sampler_rate(-4); + EXPECT_EQ(2, s.get_request()); + } + { // Below min: If given rate is invalid and gets defaulted to a value which + // is same as before, it does not change anything + mock::sampler s(0.000001); + s.get(); + EXPECT_EQ(2, s.get_request()); + s.set_sampler_rate(0.000002); + EXPECT_EQ(2, s.get_request()); + } +} + TEST(ScopeTest, TestConcurrent) { std::atomic concurrent = false; diff --git a/appsec/tests/helper/service_test.cpp b/appsec/tests/helper/service_test.cpp index 5af59db440..afa7a950f2 100644 --- a/appsec/tests/helper/service_test.cpp +++ b/appsec/tests/helper/service_test.cpp @@ -57,6 +57,8 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto s = service( engine, service_config, nullptr, {true, all_requests_are_picked}); + EXPECT_EQ( + all_requests_are_picked, service_config->get_request_sample_rate()); EXPECT_TRUE(s.get_schema_sampler()->get().has_value()); } @@ -65,6 +67,8 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto s = service( engine, service_config, nullptr, {true, no_request_is_picked}); + EXPECT_EQ( + no_request_is_picked, service_config->get_request_sample_rate()); EXPECT_FALSE(s.get_schema_sampler()->get().has_value()); } @@ -74,6 +78,7 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto s = service(engine, service_config, nullptr, {schema_extraction_disabled, all_requests_are_picked}); + EXPECT_EQ(0, service_config->get_request_sample_rate()); EXPECT_FALSE(s.get_schema_sampler()->get().has_value()); } From c3a1f18be92f817abea844974e9cd5fcee7fe7db Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Thu, 21 Dec 2023 16:25:17 +0100 Subject: [PATCH 04/14] Asm features also parses api security updates --- .../helper/remote_config/client_handler.cpp | 5 +- .../listeners/asm_features_listener.cpp | 12 +- .../listeners/asm_features_listener.hpp | 24 +- .../listeners/asm_features_listener_test.cpp | 248 ++++++++++++------ 4 files changed, 202 insertions(+), 87 deletions(-) diff --git a/appsec/src/helper/remote_config/client_handler.cpp b/appsec/src/helper/remote_config/client_handler.cpp index c0e33ba829..58d5c2733e 100644 --- a/appsec/src/helper/remote_config/client_handler.cpp +++ b/appsec/src/helper/remote_config/client_handler.cpp @@ -47,10 +47,11 @@ client_handler::ptr client_handler::from_settings(service_identifier &&id, } std::vector listeners = {}; - if (dynamic_enablement) { + if (dynamic_enablement || eng_settings.schema_extraction.enabled) { listeners.emplace_back( std::make_shared( - service_config)); + service_config, dynamic_enablement, + eng_settings.schema_extraction.enabled)); } if (eng_settings.rules_file.empty()) { diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp index 1eba7cc1df..b848bf7317 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp @@ -60,15 +60,11 @@ void dds::remote_config::asm_features_listener::parse_asm( if (dd_tolower(enabled_itr->value.GetString()) == std::string("true")) { service_config_->enable_asm(); } else { - // This scenario should not happen since RC would remove the file - // when appsec should not be enabled service_config_->disable_asm(); } } else if (enabled_itr->value.GetType() == rapidjson::kTrueType) { service_config_->enable_asm(); } else if (enabled_itr->value.GetType() == rapidjson::kFalseType) { - // This scenario should not happen since RC would remove the file - // when appsec should not be enabled service_config_->disable_asm(); } else { throw error_applying_config( @@ -84,6 +80,10 @@ void dds::remote_config::asm_features_listener::on_update(const config &config) throw error_applying_config("Invalid config contents"); } - parse_asm(serialized_doc); - parse_api_security(serialized_doc); + if (dynamic_enablement_) { + parse_asm(serialized_doc); + } + if (api_security_enabled_) { + parse_api_security(serialized_doc); + } } diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp index 6f5abdd6ed..7fdcde93ac 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp @@ -15,8 +15,11 @@ namespace dds::remote_config { class asm_features_listener : public listener_base { public: explicit asm_features_listener( - std::shared_ptr service_config) - : service_config_(std::move(service_config)){}; + std::shared_ptr service_config, + bool dynamic_enablement, bool api_security_enabled) + : service_config_(std::move(service_config)), + dynamic_enablement_(dynamic_enablement), + api_security_enabled_(api_security_enabled){}; void on_update(const config &config) override; void on_unapply(const config & /*config*/) override { @@ -26,9 +29,18 @@ class asm_features_listener : public listener_base { [[nodiscard]] std::unordered_map get_supported_products() override { - return {{"ASM_FEATURES", protocol::capabilities_e::ASM_ACTIVATION}, - {"ASM_API_SECURITY_SAMPLE_RATE", - protocol::capabilities_e::ASM_API_SECURITY_SAMPLE_RATE}}; + std::unordered_map + supported; + + if (dynamic_enablement_) { + supported["ASM_FEATURES"] = + protocol::capabilities_e::ASM_ACTIVATION; + } + if (api_security_enabled_) { + supported["ASM_API_SECURITY_SAMPLE_RATE"] = + protocol::capabilities_e::ASM_API_SECURITY_SAMPLE_RATE; + } + return supported; } void init() override {} @@ -38,6 +50,8 @@ class asm_features_listener : public listener_base { void parse_asm(const rapidjson::Document &serialized_doc); void parse_api_security(const rapidjson::Document &serialized_doc); std::shared_ptr service_config_; + bool dynamic_enablement_; + bool api_security_enabled_; }; } // namespace dds::remote_config diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index 8b11e202f0..4f3e64004f 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -57,18 +57,18 @@ remote_config::config get_disabled_config(bool as_string = true) TEST(RemoteConfigAsmFeaturesListener, ByDefaultListenerIsNotSet) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ListenerGetActiveWhenConfigSaysSoOnUpdateAsString) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); try { listener.on_update(get_enabled_config()); @@ -76,29 +76,29 @@ TEST(RemoteConfigAsmFeaturesListener, std::cout << error.what() << std::endl; } - EXPECT_EQ(enable_asm_status::ENABLED, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, AsmParserIsCaseInsensitive) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); listener.on_update(get_config_with_status("\"TrUe\"")); - EXPECT_EQ(enable_asm_status::ENABLED, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ListenerGetDeactivedWhenConfigSaysSoOnUpdateAsString) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); try { listener.on_update(get_disabled_config()); @@ -106,15 +106,15 @@ TEST(RemoteConfigAsmFeaturesListener, std::cout << error.what() << std::endl; } - EXPECT_EQ(enable_asm_status::DISABLED, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::DISABLED, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ListenerGetActiveWhenConfigSaysSoOnUpdateAsBoolean) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); try { listener.on_update(get_enabled_config(false)); @@ -122,15 +122,15 @@ TEST(RemoteConfigAsmFeaturesListener, std::cout << error.what() << std::endl; } - EXPECT_EQ(enable_asm_status::ENABLED, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ListenerGetDeactivedWhenConfigSaysSoOnUpdateAsBoolean) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); try { listener.on_update(get_disabled_config(false)); @@ -138,15 +138,15 @@ TEST(RemoteConfigAsmFeaturesListener, std::cout << error.what() << std::endl; } - EXPECT_EQ(enable_asm_status::DISABLED, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::DISABLED, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenContentOfConfigAreNotValidBase64) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); std::string invalid_content = "&&&"; std::string error_message = ""; std::string expected_error_message = "Invalid config contents"; @@ -159,8 +159,8 @@ TEST(RemoteConfigAsmFeaturesListener, error_message = error.what(); } - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(0, expected_error_message.length(), expected_error_message)); } @@ -170,8 +170,8 @@ TEST(RemoteConfigAsmFeaturesListener, { std::string error_message = ""; std::string expected_error_message = "Invalid config contents"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); std::string invalid_content = "invalidJsonContent"; remote_config::config config = get_config(invalid_content); @@ -181,8 +181,8 @@ TEST(RemoteConfigAsmFeaturesListener, error_message = error.what(); } - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(0, expected_error_message.length(), expected_error_message)); } @@ -192,8 +192,8 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmKeyMissing) std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: asm key missing or invalid"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); remote_config::config asm_key_missing = get_config("{}"); try { @@ -202,8 +202,8 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmKeyMissing) error_message = error.what(); } - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } @@ -212,8 +212,8 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmIsNotValid) std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: asm key missing or invalid"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); remote_config::config invalid_asm_key = get_config("{ \"asm\": 123}"); try { @@ -222,8 +222,8 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmIsNotValid) error_message = error.what(); } - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } @@ -233,8 +233,8 @@ TEST( std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: enabled key missing"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); remote_config::config enabled_key_missing = get_config( "{ \"asm\": {}, \"api_security\": { \"request_sample_rate\": 0.1}}"); @@ -244,8 +244,8 @@ TEST( error_message = error.what(); } - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } @@ -255,8 +255,8 @@ TEST(RemoteConfigAsmFeaturesListener, std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: enabled key invalid"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); remote_config::config enabled_key_invalid = get_config("{ \"asm\": { \"enabled\": 123}, \"api_security\": { " "\"request_sample_rate\": 0.1}}"); @@ -267,25 +267,25 @@ TEST(RemoteConfigAsmFeaturesListener, error_message = error.what(); } - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } TEST(RemoteConfigAsmFeaturesListener, WhenListenerGetsUnapplyItGetsNotSet) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); listener.on_update(get_enabled_config(false)); - EXPECT_EQ(enable_asm_status::ENABLED, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); remote_config::config some_key; listener.on_unapply(some_key); - EXPECT_EQ(enable_asm_status::NOT_SET, - remote_config_service->get_asm_enabled_status()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoApiSecurityPresent) @@ -294,8 +294,8 @@ TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoApiSecurityPresent) std::string expected_error_message = "Invalid config json encoded contents: api_security key missing or " "invalid"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, false, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}}"); @@ -314,8 +314,8 @@ TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenApiSecurityHasWrongType) std::string expected_error_message = "Invalid config json encoded contents: api_security key missing or " "invalid"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": 1234}"); @@ -333,8 +333,8 @@ TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoRequestSampleRatePresent) std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: request_sample_rate key missing"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": {}}"); @@ -354,8 +354,8 @@ TEST(RemoteConfigAsmFeaturesListener, std::string expected_error_message = "Invalid config json encoded contents: request_sample_rate is not " "double"; - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": { " "\"request_sample_rate\": true}}"); @@ -371,8 +371,8 @@ TEST(RemoteConfigAsmFeaturesListener, TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, true); { // It parses floats auto sample_rate = 0.12; @@ -381,8 +381,7 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ( - sample_rate, remote_config_service->get_request_sample_rate()); + EXPECT_EQ(sample_rate, config_service->get_request_sample_rate()); } { // It parses integers @@ -392,15 +391,14 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ( - sample_rate, remote_config_service->get_request_sample_rate()); + EXPECT_EQ(sample_rate, config_service->get_request_sample_rate()); } } TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateLimits) { - auto remote_config_service = std::make_shared(); - remote_config::asm_features_listener listener(remote_config_service); + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, true); { // Over 1, sets default 0.1 try { @@ -408,7 +406,7 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateLimits) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(0.1, remote_config_service->get_request_sample_rate()); + EXPECT_EQ(0.1, config_service->get_request_sample_rate()); } { // Below 0, sets default 0.1 @@ -417,7 +415,109 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateLimits) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(0.1, remote_config_service->get_request_sample_rate()); + EXPECT_EQ(0.1, config_service->get_request_sample_rate()); + } +} + +TEST(RemoteConfigAsmFeaturesListener, DynamicEnablementIsDisabled) +{ + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, false, true); + + { // Asm value is ignored on parse + try { + listener.on_update( + get_config_with_status_and_sample_rate("true", 0.2)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ(0.2, config_service->get_request_sample_rate()); + EXPECT_EQ(enable_asm_status::NOT_SET, + config_service->get_asm_enabled_status()); + } + + { // Asm can be missing + try { + auto missing_asm = get_config( + "{\"api_security\": { \"request_sample_rate\": 0.3}}", true); + listener.on_update(missing_asm); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ(0.3, config_service->get_request_sample_rate()); + } +} + +TEST(RemoteConfigAsmFeaturesListener, ApiSecurityIsDisabled) +{ + auto config_service = std::make_shared(); + remote_config::asm_features_listener listener(config_service, true, false); + auto default_sample_rate = 0.1; + + { // Api security value is ignored on parse + try { + listener.on_update( + get_config_with_status_and_sample_rate("true", 0.2)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ( + default_sample_rate, config_service->get_request_sample_rate()); + EXPECT_EQ(enable_asm_status::ENABLED, + config_service->get_asm_enabled_status()); + } + + { // Api security can be missing + try { + auto missing_api_security = + get_config("{ \"asm\": { \"enabled\": true}}"); + listener.on_update(missing_api_security); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; + } + EXPECT_EQ( + default_sample_rate, config_service->get_request_sample_rate()); + EXPECT_EQ(enable_asm_status::ENABLED, + config_service->get_asm_enabled_status()); + } +} + +TEST(RemoteConfigAsmFeaturesListener, ProductsAreDynamic) +{ + auto config_service = std::make_shared(); + + { // All disabled + remote_config::asm_features_listener listener( + config_service, false, false); + EXPECT_EQ(0, listener.get_supported_products().size()); + } + + { // Asm disabled + remote_config::asm_features_listener listener( + config_service, true, false); + EXPECT_EQ(1, listener.get_supported_products().size()); + EXPECT_EQ(dds::remote_config::protocol::capabilities_e::ASM_ACTIVATION, + listener.get_supported_products()["ASM_FEATURES"]); + } + + { // Api security disabled + remote_config::asm_features_listener listener( + config_service, false, true); + EXPECT_EQ(1, listener.get_supported_products().size()); + EXPECT_EQ(dds::remote_config::protocol::capabilities_e:: + ASM_API_SECURITY_SAMPLE_RATE, + listener.get_supported_products()["ASM_API_SECURITY_SAMPLE_RATE"]); + } + + { // All enabled + remote_config::asm_features_listener listener( + config_service, true, true); + EXPECT_EQ(2, listener.get_supported_products().size()); + EXPECT_EQ(dds::remote_config::protocol::capabilities_e::ASM_ACTIVATION, + listener.get_supported_products()["ASM_FEATURES"]); + EXPECT_EQ(dds::remote_config::protocol::capabilities_e:: + ASM_API_SECURITY_SAMPLE_RATE, + listener.get_supported_products()["ASM_API_SECURITY_SAMPLE_RATE"]); } } From 21368f54945725c40328bff0e84079cbfb0fa1b1 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Fri, 22 Dec 2023 12:38:03 +0100 Subject: [PATCH 05/14] Add schema extraction rc logic --- .../listeners/asm_features_listener.cpp | 14 +++++-- .../listeners/asm_features_listener.hpp | 17 ++++---- .../listeners/asm_features_listener_test.cpp | 41 ++++++------------- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp index b848bf7317..80ba695c3c 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp @@ -10,7 +10,7 @@ #include "utils.hpp" #include -void dds::remote_config::asm_features_listener::parse_api_security( +double dds::remote_config::asm_features_listener::parse_api_security( const rapidjson::Document &serialized_doc) { auto api_security_itr = json_helper::get_field_of_type( @@ -35,8 +35,7 @@ void dds::remote_config::asm_features_listener::parse_api_security( "request_sample_rate is not double"); } - service_config_->set_request_sample_rate( - request_sample_rate_itr->value.GetDouble()); + return request_sample_rate_itr->value.GetDouble(); } void dds::remote_config::asm_features_listener::parse_asm( @@ -83,7 +82,14 @@ void dds::remote_config::asm_features_listener::on_update(const config &config) if (dynamic_enablement_) { parse_asm(serialized_doc); } + if (api_security_enabled_) { - parse_api_security(serialized_doc); + double sample_rate = 0; + if (!(service_config_->get_asm_enabled_status() == + enable_asm_status::DISABLED && + dynamic_enablement_)) { + sample_rate = parse_api_security(serialized_doc); + } + service_config_->set_request_sample_rate(sample_rate); } } diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp index 7fdcde93ac..b7a0f5ada2 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp @@ -29,26 +29,29 @@ class asm_features_listener : public listener_base { [[nodiscard]] std::unordered_map get_supported_products() override { - std::unordered_map - supported; + protocol::capabilities_e capabilities = protocol::capabilities_e::NONE; if (dynamic_enablement_) { - supported["ASM_FEATURES"] = - protocol::capabilities_e::ASM_ACTIVATION; + capabilities = protocol::capabilities_e::ASM_ACTIVATION; } if (api_security_enabled_) { - supported["ASM_API_SECURITY_SAMPLE_RATE"] = + capabilities |= protocol::capabilities_e::ASM_API_SECURITY_SAMPLE_RATE; } - return supported; + + if (capabilities != protocol::capabilities_e::NONE) { + return {{asm_features, capabilities}}; + } + return {}; } void init() override {} void commit() override {} protected: + static constexpr std::string_view asm_features = "ASM_FEATURES"; void parse_asm(const rapidjson::Document &serialized_doc); - void parse_api_security(const rapidjson::Document &serialized_doc); + double parse_api_security(const rapidjson::Document &serialized_doc); std::shared_ptr service_config_; bool dynamic_enablement_; bool api_security_enabled_; diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index 4f3e64004f..4c684f524b 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -295,7 +295,7 @@ TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoApiSecurityPresent) "Invalid config json encoded contents: api_security key missing or " "invalid"; auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, false, true); + remote_config::asm_features_listener listener(config_service, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}}"); @@ -424,28 +424,14 @@ TEST(RemoteConfigAsmFeaturesListener, DynamicEnablementIsDisabled) auto config_service = std::make_shared(); remote_config::asm_features_listener listener(config_service, false, true); - { // Asm value is ignored on parse - try { - listener.on_update( - get_config_with_status_and_sample_rate("true", 0.2)); - } catch (remote_config::error_applying_config &error) { - std::cout << error.what() << std::endl; - } - EXPECT_EQ(0.2, config_service->get_request_sample_rate()); - EXPECT_EQ(enable_asm_status::NOT_SET, - config_service->get_asm_enabled_status()); - } - - { // Asm can be missing - try { - auto missing_asm = get_config( - "{\"api_security\": { \"request_sample_rate\": 0.3}}", true); - listener.on_update(missing_asm); - } catch (remote_config::error_applying_config &error) { - std::cout << error.what() << std::endl; - } - EXPECT_EQ(0.3, config_service->get_request_sample_rate()); + try { + listener.on_update(get_config_with_status_and_sample_rate("true", 0.2)); + } catch (remote_config::error_applying_config &error) { + std::cout << error.what() << std::endl; } + EXPECT_EQ(0.2, config_service->get_request_sample_rate()); + EXPECT_EQ( + enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); } TEST(RemoteConfigAsmFeaturesListener, ApiSecurityIsDisabled) @@ -506,18 +492,17 @@ TEST(RemoteConfigAsmFeaturesListener, ProductsAreDynamic) EXPECT_EQ(1, listener.get_supported_products().size()); EXPECT_EQ(dds::remote_config::protocol::capabilities_e:: ASM_API_SECURITY_SAMPLE_RATE, - listener.get_supported_products()["ASM_API_SECURITY_SAMPLE_RATE"]); + listener.get_supported_products()["ASM_FEATURES"]); } { // All enabled remote_config::asm_features_listener listener( config_service, true, true); - EXPECT_EQ(2, listener.get_supported_products().size()); - EXPECT_EQ(dds::remote_config::protocol::capabilities_e::ASM_ACTIVATION, + EXPECT_EQ(1, listener.get_supported_products().size()); + EXPECT_EQ(dds::remote_config::protocol::capabilities_e::ASM_ACTIVATION | + dds::remote_config::protocol::capabilities_e:: + ASM_API_SECURITY_SAMPLE_RATE, listener.get_supported_products()["ASM_FEATURES"]); - EXPECT_EQ(dds::remote_config::protocol::capabilities_e:: - ASM_API_SECURITY_SAMPLE_RATE, - listener.get_supported_products()["ASM_API_SECURITY_SAMPLE_RATE"]); } } From 76827d94cbcac1befb573cb9c52996da1438b462 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Tue, 2 Jan 2024 12:58:12 +0100 Subject: [PATCH 06/14] Amend lint errors --- .../remote_config/listeners/asm_features_listener.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp index 80ba695c3c..eb433cc0a1 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp @@ -85,9 +85,9 @@ void dds::remote_config::asm_features_listener::on_update(const config &config) if (api_security_enabled_) { double sample_rate = 0; - if (!(service_config_->get_asm_enabled_status() == - enable_asm_status::DISABLED && - dynamic_enablement_)) { + if (service_config_->get_asm_enabled_status() != + enable_asm_status::DISABLED || + !dynamic_enablement_) { sample_rate = parse_api_security(serialized_doc); } service_config_->set_request_sample_rate(sample_rate); From 6bf3fd045eed654255670ffcc2bf487024cfd31a Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Thu, 4 Jan 2024 10:06:29 +0100 Subject: [PATCH 07/14] Lint --- .../helper/remote_config/listeners/asm_features_listener.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp index b7a0f5ada2..c6876ae9e9 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp @@ -51,7 +51,7 @@ class asm_features_listener : public listener_base { protected: static constexpr std::string_view asm_features = "ASM_FEATURES"; void parse_asm(const rapidjson::Document &serialized_doc); - double parse_api_security(const rapidjson::Document &serialized_doc); + static double parse_api_security(const rapidjson::Document &serialized_doc); std::shared_ptr service_config_; bool dynamic_enablement_; bool api_security_enabled_; From 97b6d497ff32ef8ce9c47b2ad4bf20b336719667 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Thu, 4 Jan 2024 16:45:59 +0100 Subject: [PATCH 08/14] Refactor setting request sample rate --- appsec/src/helper/client.cpp | 2 - appsec/src/helper/sampler.hpp | 28 ++- appsec/src/helper/service.cpp | 2 +- appsec/src/helper/service_config.hpp | 3 - .../listeners/asm_features_listener_test.cpp | 24 --- appsec/tests/helper/sampler_test.cpp | 169 +++++++++++++----- 6 files changed, 153 insertions(+), 75 deletions(-) diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index e949b132fe..d7f859e790 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -441,8 +441,6 @@ bool client::handle_command(network::request_shutdown::request &command) auto sampler = service_->get_schema_sampler(); std::optional scope; if (sampler) { - sampler->set_sampler_rate( - service_->get_service_config()->get_request_sample_rate()); scope = sampler->get(); if (scope.has_value()) { parameter context_processor = parameter::map(); diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index 909f7ba573..34aa6c92ff 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -13,13 +13,16 @@ #include #include +#include "service_config.hpp" + namespace dds { static const double min_rate = 0.0001; class sampler { public: - sampler(double sample_rate) : sample_rate_(sample_rate) + sampler(std::shared_ptr service_config) + : service_config_(std::move(service_config)) { - set_sampler_rate(sample_rate); + set_sampler_rate(service_config_->get_request_sample_rate()); } class scope { public: @@ -56,10 +59,20 @@ class sampler { std::optional get() { + if (service_config_->get_asm_enabled_status() == + enable_asm_status::DISABLED) { + return std::nullopt; + } + const std::lock_guard lock_guard(mtx_); std::optional result = std::nullopt; + if (sample_rate_ != + valid_sample_rate(service_config_->get_request_sample_rate())) { + set_sampler_rate(service_config_->get_request_sample_rate()); + } + if (!concurrent_ && floor(request_ * sample_rate_) != floor((request_ + 1) * sample_rate_)) { result = {scope{concurrent_}}; @@ -74,7 +87,7 @@ class sampler { return result; } - void set_sampler_rate(double sampler_rate) + static double valid_sample_rate(double sampler_rate) { // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) if (sampler_rate <= 0) { @@ -86,6 +99,14 @@ class sampler { } // NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + return sampler_rate; + } + + void set_sampler_rate(double sampler_rate) + { + + sampler_rate = valid_sample_rate(sampler_rate); + if (sampler_rate == sample_rate_) { return; } @@ -99,5 +120,6 @@ class sampler { double sample_rate_{0}; std::atomic concurrent_{false}; std::mutex mtx_; + std::shared_ptr service_config_; }; } // namespace dds diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index 0b8c3f97a3..d597d9806e 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -30,8 +30,8 @@ service::service(std::shared_ptr engine, sample_rate = 0; } - schema_sampler_ = std::make_shared(sample_rate); service_config_->set_request_sample_rate(sample_rate); + schema_sampler_ = std::make_shared(service_config_); } service::ptr service::from_settings(service_identifier &&id, diff --git a/appsec/src/helper/service_config.hpp b/appsec/src/helper/service_config.hpp index cf70961155..f0cf00c8fe 100644 --- a/appsec/src/helper/service_config.hpp +++ b/appsec/src/helper/service_config.hpp @@ -22,9 +22,6 @@ struct service_config { double get_request_sample_rate() { return request_sample_rate; } void set_request_sample_rate(double sample_rate) { - if (sample_rate < 0 || sample_rate > 1) { - return; - } request_sample_rate = sample_rate; } diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index 4c684f524b..cc154627a2 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -395,30 +395,6 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) } } -TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateLimits) -{ - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, true); - - { // Over 1, sets default 0.1 - try { - listener.on_update(get_config_with_sample_rate(2)); - } catch (remote_config::error_applying_config &error) { - std::cout << error.what() << std::endl; - } - EXPECT_EQ(0.1, config_service->get_request_sample_rate()); - } - - { // Below 0, sets default 0.1 - try { - listener.on_update(get_config_with_sample_rate(-2)); - } catch (remote_config::error_applying_config &error) { - std::cout << error.what() << std::endl; - } - EXPECT_EQ(0.1, config_service->get_request_sample_rate()); - } -} - TEST(RemoteConfigAsmFeaturesListener, DynamicEnablementIsDisabled) { auto config_service = std::make_shared(); diff --git a/appsec/tests/helper/sampler_test.cpp b/appsec/tests/helper/sampler_test.cpp index b0c101b98d..51f008baa1 100644 --- a/appsec/tests/helper/sampler_test.cpp +++ b/appsec/tests/helper/sampler_test.cpp @@ -4,6 +4,7 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "common.hpp" +#include "service_config.hpp" #include #include @@ -13,13 +14,11 @@ namespace mock { class sampler : public dds::sampler { public: - sampler(double sample_rate) : dds::sampler(sample_rate) {} + sampler(std::shared_ptr service_config) + : dds::sampler(service_config) + {} void set_request(unsigned int i) { request_ = i; } auto get_request() { return request_; } - void set_sampler_rate(double sampler_rate) - { - dds::sampler::set_sampler_rate(sampler_rate); - } }; } // namespace mock @@ -38,7 +37,10 @@ void count_picked(dds::sampler &sampler, int iterations) TEST(SamplerTest, ItPicksAllWhenRateIs1) { - sampler s(1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(1); + sampler s(service_config); picked = 0; count_picked(s, 100); @@ -47,7 +49,10 @@ TEST(SamplerTest, ItPicksAllWhenRateIs1) TEST(SamplerTest, ItPicksNoneWhenRateIs0) { - sampler s(0); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0); + sampler s(service_config); picked = 0; count_picked(s, 100); @@ -56,7 +61,10 @@ TEST(SamplerTest, ItPicksNoneWhenRateIs0) TEST(SamplerTest, ItPicksHalfWhenPortionGiven) { - sampler s(0.5); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.5); + sampler s(service_config); picked = 0; count_picked(s, 100); @@ -65,7 +73,10 @@ TEST(SamplerTest, ItPicksHalfWhenPortionGiven) TEST(SamplerTest, ItResetTokensAfter100Calls) { - sampler s(1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(1); + sampler s(service_config); picked = 0; count_picked(s, 100); @@ -77,70 +88,100 @@ TEST(SamplerTest, ItResetTokensAfter100Calls) TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { { - sampler s(0.1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.1); + sampler s(service_config); picked = 0; count_picked(s, 10); EXPECT_EQ(1, picked); } { - sampler s(0.5); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.5); + sampler s(service_config); picked = 0; count_picked(s, 10); EXPECT_EQ(5, picked); } { - sampler s(0.01); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.01); + sampler s(service_config); picked = 0; count_picked(s, 100); EXPECT_EQ(1, picked); } { - sampler s(0.02); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.02); + sampler s(service_config); picked = 0; count_picked(s, 100); EXPECT_EQ(2, picked); } { - sampler s(0.001); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.001); + sampler s(service_config); picked = 0; count_picked(s, 1000); EXPECT_EQ(1, picked); } { - sampler s(0.003); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.003); + sampler s(service_config); picked = 0; count_picked(s, 1000); EXPECT_EQ(3, picked); } { - sampler s(0.0001); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.0001); + sampler s(service_config); picked = 0; count_picked(s, 10000); EXPECT_EQ(1, picked); } { - sampler s(0.0007); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.0007); + sampler s(service_config); picked = 0; count_picked(s, 10000); EXPECT_EQ(7, picked); } { - sampler s(0.123); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.123); + sampler s(service_config); picked = 0; count_picked(s, 1000); EXPECT_EQ(123, picked); } { - sampler s(0.6); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.6); + sampler s(service_config); picked = 0; count_picked(s, 10); @@ -151,21 +192,30 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) TEST(SamplerTest, TestInvalidSampleRatesDefaultToTenPercent) { { - sampler s(2); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(2); + sampler s(service_config); picked = 0; count_picked(s, 10); EXPECT_EQ(10, picked); } { - sampler s(-1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(-1); + sampler s(service_config); picked = 0; count_picked(s, 10); EXPECT_EQ(0, picked); } { // Below limit goes to default 10 percent - sampler s(0.000001); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.000001); + sampler s(service_config); picked = 0; count_picked(s, 1000000); @@ -176,21 +226,30 @@ TEST(SamplerTest, TestInvalidSampleRatesDefaultToTenPercent) TEST(SamplerTest, TestLimits) { { - sampler s(0); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0); + sampler s(service_config); picked = 0; count_picked(s, 10); EXPECT_EQ(0, picked); } { - sampler s(1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(1); + sampler s(service_config); picked = 0; count_picked(s, 10); EXPECT_EQ(10, picked); } { - sampler s(0.0001); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.0001); + sampler s(service_config); picked = 0; count_picked(s, 10000); @@ -200,7 +259,10 @@ TEST(SamplerTest, TestLimits) TEST(SamplerTest, TestOverflow) { - mock::sampler s(0); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0); + mock::sampler s(service_config); s.set_request(UINT_MAX); s.get(); EXPECT_EQ(1, s.get_request()); @@ -209,42 +271,62 @@ TEST(SamplerTest, TestOverflow) TEST(SamplerTest, ModifySamplerRate) { { // New sampler rate reset requests - mock::sampler s(0.1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.1); + mock::sampler s(service_config); + s.get(); + EXPECT_EQ(2, s.get_request()); + service_config->set_request_sample_rate(0.2); s.get(); EXPECT_EQ(2, s.get_request()); - s.set_sampler_rate(0.2); - EXPECT_EQ(1, s.get_request()); } { // Setting same sampler rate does do anything - mock::sampler s(0.1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.1); + mock::sampler s(service_config); s.get(); EXPECT_EQ(2, s.get_request()); - s.set_sampler_rate(0.1); - EXPECT_EQ(2, s.get_request()); + service_config->set_request_sample_rate(0.1); + s.get(); + EXPECT_EQ(3, s.get_request()); } { // Over Zero: If given rate is invalid and gets defaulted to a value which // is same as before, it does not change anything - mock::sampler s(3); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(3); + mock::sampler s(service_config); s.get(); EXPECT_EQ(2, s.get_request()); - s.set_sampler_rate(4); - EXPECT_EQ(2, s.get_request()); + service_config->set_request_sample_rate(4); + s.get(); + EXPECT_EQ(3, s.get_request()); } { // Below zero: If given rate is invalid and gets defaulted to a value // which is same as before, it does not change anything - mock::sampler s(-3); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(-3); + mock::sampler s(service_config); s.get(); EXPECT_EQ(2, s.get_request()); - s.set_sampler_rate(-4); - EXPECT_EQ(2, s.get_request()); + service_config->set_request_sample_rate(-4); + s.get(); + EXPECT_EQ(3, s.get_request()); } { // Below min: If given rate is invalid and gets defaulted to a value which // is same as before, it does not change anything - mock::sampler s(0.000001); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(0.000001); + mock::sampler s(service_config); s.get(); EXPECT_EQ(2, s.get_request()); - s.set_sampler_rate(0.000002); - EXPECT_EQ(2, s.get_request()); + service_config->set_request_sample_rate(0.000002); + s.get(); + EXPECT_EQ(3, s.get_request()); } } @@ -260,7 +342,10 @@ TEST(ScopeTest, TestConcurrent) TEST(ScopeTest, TestItDoesNotPickTokenUntilScopeReleased) { - sampler sampler(1); + auto service_config = std::make_shared(); + service_config->enable_asm(); + service_config->set_request_sample_rate(1); + sampler sampler(service_config); auto is_pick = sampler.get(); EXPECT_TRUE(is_pick != std::nullopt); is_pick = sampler.get(); From ede31c9b7c083123c82e5278118eea425371dd17 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Fri, 5 Jan 2024 17:09:25 +0100 Subject: [PATCH 09/14] Adapt api security parsing --- .../listeners/asm_features_listener.cpp | 46 ++-- .../listeners/asm_features_listener.hpp | 2 +- .../listeners/asm_features_listener_test.cpp | 207 +++++++----------- 3 files changed, 102 insertions(+), 153 deletions(-) diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp index eb433cc0a1..60340a3faa 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.cpp @@ -10,21 +10,24 @@ #include "utils.hpp" #include -double dds::remote_config::asm_features_listener::parse_api_security( +void dds::remote_config::asm_features_listener::parse_api_security( const rapidjson::Document &serialized_doc) { - auto api_security_itr = json_helper::get_field_of_type( - serialized_doc, "api_security", rapidjson::kObjectType); + rapidjson::Value::ConstMemberIterator const api_security_itr = + serialized_doc.FindMember("api_security"); - if (!api_security_itr) { + if (api_security_itr == serialized_doc.MemberEnd()) { + return; + } + + if (rapidjson::kObjectType != api_security_itr->value.GetType()) { throw error_applying_config("Invalid config json encoded contents: " - "api_security key missing or invalid"); + "api_security key invalid"); } auto request_sample_rate_itr = - api_security_itr.value()->value.FindMember("request_sample_rate"); - if (request_sample_rate_itr == - api_security_itr.value()->value.MemberEnd()) { + api_security_itr->value.FindMember("request_sample_rate"); + if (request_sample_rate_itr == api_security_itr->value.MemberEnd()) { throw error_applying_config("Invalid config json encoded contents: " "request_sample_rate key missing"); } @@ -35,22 +38,27 @@ double dds::remote_config::asm_features_listener::parse_api_security( "request_sample_rate is not double"); } - return request_sample_rate_itr->value.GetDouble(); + service_config_->set_request_sample_rate( + request_sample_rate_itr->value.GetDouble()); } void dds::remote_config::asm_features_listener::parse_asm( const rapidjson::Document &serialized_doc) { - auto asm_itr = json_helper::get_field_of_type( - serialized_doc, "asm", rapidjson::kObjectType); + rapidjson::Value::ConstMemberIterator const asm_itr = + serialized_doc.FindMember("asm"); - if (!asm_itr) { + if (asm_itr == serialized_doc.MemberEnd()) { + return; + } + + if (rapidjson::kObjectType != asm_itr->value.GetType()) { throw error_applying_config("Invalid config json encoded contents: " - "asm key missing or invalid"); + "asm key invalid"); } - auto enabled_itr = asm_itr.value()->value.FindMember("enabled"); - if (enabled_itr == asm_itr.value()->value.MemberEnd()) { + auto enabled_itr = asm_itr->value.FindMember("enabled"); + if (enabled_itr == asm_itr->value.MemberEnd()) { throw error_applying_config( "Invalid config json encoded contents: enabled key missing"); } @@ -84,12 +92,6 @@ void dds::remote_config::asm_features_listener::on_update(const config &config) } if (api_security_enabled_) { - double sample_rate = 0; - if (service_config_->get_asm_enabled_status() != - enable_asm_status::DISABLED || - !dynamic_enablement_) { - sample_rate = parse_api_security(serialized_doc); - } - service_config_->set_request_sample_rate(sample_rate); + parse_api_security(serialized_doc); } } diff --git a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp index c6876ae9e9..7137457e32 100644 --- a/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/asm_features_listener.hpp @@ -51,7 +51,7 @@ class asm_features_listener : public listener_base { protected: static constexpr std::string_view asm_features = "ASM_FEATURES"; void parse_asm(const rapidjson::Document &serialized_doc); - static double parse_api_security(const rapidjson::Document &serialized_doc); + void parse_api_security(const rapidjson::Document &serialized_doc); std::shared_ptr service_config_; bool dynamic_enablement_; bool api_security_enabled_; diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index cc154627a2..f5e5e21ad9 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -55,20 +55,25 @@ remote_config::config get_disabled_config(bool as_string = true) return get_config_with_status(quotes + "false" + quotes); } -TEST(RemoteConfigAsmFeaturesListener, ByDefaultListenerIsNotSet) +class RemoteConfigAsmFeaturesListenerTest : public ::testing::Test { +public: + std::shared_ptr service_config; + + void SetUp() { service_config = std::make_shared(); } +}; + +TEST_F(RemoteConfigAsmFeaturesListenerTest, ByDefaultListenerIsNotSet) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerGetActiveWhenConfigSaysSoOnUpdateAsString) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); try { listener.on_update(get_enabled_config()); @@ -77,28 +82,26 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); + enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, AsmParserIsCaseInsensitive) +TEST_F(RemoteConfigAsmFeaturesListenerTest, AsmParserIsCaseInsensitive) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); listener.on_update(get_config_with_status("\"TrUe\"")); EXPECT_EQ( - enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); + enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerGetDeactivedWhenConfigSaysSoOnUpdateAsString) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); try { listener.on_update(get_disabled_config()); @@ -107,14 +110,13 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::DISABLED, config_service->get_asm_enabled_status()); + enable_asm_status::DISABLED, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerGetActiveWhenConfigSaysSoOnUpdateAsBoolean) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); try { listener.on_update(get_enabled_config(false)); @@ -123,14 +125,13 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); + enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerGetDeactivedWhenConfigSaysSoOnUpdateAsBoolean) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); try { listener.on_update(get_disabled_config(false)); @@ -139,14 +140,13 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::DISABLED, config_service->get_asm_enabled_status()); + enable_asm_status::DISABLED, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerThrowsAnErrorWhenContentOfConfigAreNotValidBase64) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); std::string invalid_content = "&&&"; std::string error_message = ""; std::string expected_error_message = "Invalid config contents"; @@ -160,18 +160,17 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(0, expected_error_message.length(), expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerThrowsAnErrorWhenContentIsNotValidJson) { std::string error_message = ""; std::string expected_error_message = "Invalid config contents"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); std::string invalid_content = "invalidJsonContent"; remote_config::config config = get_config(invalid_content); @@ -182,38 +181,18 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(0, expected_error_message.length(), expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmKeyMissing) -{ - std::string error_message = ""; - std::string expected_error_message = - "Invalid config json encoded contents: asm key missing or invalid"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); - remote_config::config asm_key_missing = get_config("{}"); - - try { - listener.on_update(asm_key_missing); - } catch (remote_config::error_applying_config &error) { - error_message = error.what(); - } - - EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); - EXPECT_EQ(0, error_message.compare(expected_error_message)); -} - -TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmIsNotValid) +TEST_F( + RemoteConfigAsmFeaturesListenerTest, ListenerThrowsAnErrorWhenAsmIsNotValid) { std::string error_message = ""; std::string expected_error_message = - "Invalid config json encoded contents: asm key missing or invalid"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + "Invalid config json encoded contents: asm key invalid"; + remote_config::asm_features_listener listener(service_config, true, false); remote_config::config invalid_asm_key = get_config("{ \"asm\": 123}"); try { @@ -223,18 +202,17 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmIsNotValid) } EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } -TEST( - RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenEnabledKeyMissing) +TEST_F(RemoteConfigAsmFeaturesListenerTest, + ListenerThrowsAnErrorWhenEnabledKeyMissing) { std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: enabled key missing"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); remote_config::config enabled_key_missing = get_config( "{ \"asm\": {}, \"api_security\": { \"request_sample_rate\": 0.1}}"); @@ -245,18 +223,17 @@ TEST( } EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ListenerThrowsAnErrorWhenEnabledKeyIsInvalid) { std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: enabled key invalid"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); remote_config::config enabled_key_invalid = get_config("{ \"asm\": { \"enabled\": 123}, \"api_security\": { " "\"request_sample_rate\": 0.1}}"); @@ -268,54 +245,32 @@ TEST(RemoteConfigAsmFeaturesListener, } EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); EXPECT_EQ(0, error_message.compare(expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, WhenListenerGetsUnapplyItGetsNotSet) +TEST_F(RemoteConfigAsmFeaturesListenerTest, WhenListenerGetsUnapplyItGetsNotSet) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); + remote_config::asm_features_listener listener(service_config, true, false); listener.on_update(get_enabled_config(false)); EXPECT_EQ( - enable_asm_status::ENABLED, config_service->get_asm_enabled_status()); + enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); remote_config::config some_key; listener.on_unapply(some_key); EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoApiSecurityPresent) +TEST_F( + RemoteConfigAsmFeaturesListenerTest, ThrowsErrorWhenApiSecurityHasWrongType) { std::string error_message = ""; std::string expected_error_message = - "Invalid config json encoded contents: api_security key missing or " - "invalid"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, true); - remote_config::config payload = - get_config("{ \"asm\": { \"enabled\": true}}"); - - try { - listener.on_update(payload); - } catch (remote_config::error_applying_config &error) { - error_message = error.what(); - } - - EXPECT_EQ(0, error_message.compare(expected_error_message)); -} - -TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenApiSecurityHasWrongType) -{ - std::string error_message = ""; - std::string expected_error_message = - "Invalid config json encoded contents: api_security key missing or " - "invalid"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, true); + "Invalid config json encoded contents: api_security key invalid"; + remote_config::asm_features_listener listener(service_config, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": 1234}"); @@ -328,13 +283,13 @@ TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenApiSecurityHasWrongType) EXPECT_EQ(0, error_message.compare(expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoRequestSampleRatePresent) +TEST_F(RemoteConfigAsmFeaturesListenerTest, + ThrowsErrorWhenNoRequestSampleRatePresent) { std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: request_sample_rate key missing"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, true); + remote_config::asm_features_listener listener(service_config, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": {}}"); @@ -347,15 +302,14 @@ TEST(RemoteConfigAsmFeaturesListener, ThrowsErrorWhenNoRequestSampleRatePresent) EXPECT_EQ(0, error_message.compare(expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, +TEST_F(RemoteConfigAsmFeaturesListenerTest, ThrowsErrorWhenRequestSampleRateHasWrongType) { std::string error_message = ""; std::string expected_error_message = "Invalid config json encoded contents: request_sample_rate is not " "double"; - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, true); + remote_config::asm_features_listener listener(service_config, true, true); remote_config::config payload = get_config("{ \"asm\": { \"enabled\": true}, \"api_security\": { " "\"request_sample_rate\": true}}"); @@ -369,10 +323,9 @@ TEST(RemoteConfigAsmFeaturesListener, EXPECT_EQ(0, error_message.compare(expected_error_message)); } -TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) +TEST_F(RemoteConfigAsmFeaturesListenerTest, RequestSampleRateIsParsed) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, true); + remote_config::asm_features_listener listener(service_config, true, true); { // It parses floats auto sample_rate = 0.12; @@ -381,7 +334,7 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(sample_rate, config_service->get_request_sample_rate()); + EXPECT_EQ(sample_rate, service_config->get_request_sample_rate()); } { // It parses integers @@ -391,42 +344,38 @@ TEST(RemoteConfigAsmFeaturesListener, RequestSampleRateIsParsed) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(sample_rate, config_service->get_request_sample_rate()); + EXPECT_EQ(sample_rate, service_config->get_request_sample_rate()); } } -TEST(RemoteConfigAsmFeaturesListener, DynamicEnablementIsDisabled) +TEST_F(RemoteConfigAsmFeaturesListenerTest, DynamicEnablementIsDisabled) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, false, true); + remote_config::asm_features_listener listener(service_config, false, true); try { listener.on_update(get_config_with_status_and_sample_rate("true", 0.2)); } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(0.2, config_service->get_request_sample_rate()); + EXPECT_EQ(0.2, service_config->get_request_sample_rate()); EXPECT_EQ( - enable_asm_status::NOT_SET, config_service->get_asm_enabled_status()); + enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); } -TEST(RemoteConfigAsmFeaturesListener, ApiSecurityIsDisabled) +TEST_F(RemoteConfigAsmFeaturesListenerTest, ApiSecurityIsDisabled) { - auto config_service = std::make_shared(); - remote_config::asm_features_listener listener(config_service, true, false); - auto default_sample_rate = 0.1; + remote_config::asm_features_listener listener(service_config, true, false); - { // Api security value is ignored on parse + { // Api security value is stored regardless try { listener.on_update( get_config_with_status_and_sample_rate("true", 0.2)); } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ( - default_sample_rate, config_service->get_request_sample_rate()); + EXPECT_EQ(0.2, service_config->get_request_sample_rate()); EXPECT_EQ(enable_asm_status::ENABLED, - config_service->get_asm_enabled_status()); + service_config->get_asm_enabled_status()); } { // Api security can be missing @@ -438,25 +387,23 @@ TEST(RemoteConfigAsmFeaturesListener, ApiSecurityIsDisabled) std::cout << error.what() << std::endl; } EXPECT_EQ( - default_sample_rate, config_service->get_request_sample_rate()); + 0.2, service_config->get_request_sample_rate()); // same as before EXPECT_EQ(enable_asm_status::ENABLED, - config_service->get_asm_enabled_status()); + service_config->get_asm_enabled_status()); } } -TEST(RemoteConfigAsmFeaturesListener, ProductsAreDynamic) +TEST_F(RemoteConfigAsmFeaturesListenerTest, ProductsAreDynamic) { - auto config_service = std::make_shared(); - { // All disabled remote_config::asm_features_listener listener( - config_service, false, false); + service_config, false, false); EXPECT_EQ(0, listener.get_supported_products().size()); } { // Asm disabled remote_config::asm_features_listener listener( - config_service, true, false); + service_config, true, false); EXPECT_EQ(1, listener.get_supported_products().size()); EXPECT_EQ(dds::remote_config::protocol::capabilities_e::ASM_ACTIVATION, listener.get_supported_products()["ASM_FEATURES"]); @@ -464,7 +411,7 @@ TEST(RemoteConfigAsmFeaturesListener, ProductsAreDynamic) { // Api security disabled remote_config::asm_features_listener listener( - config_service, false, true); + service_config, false, true); EXPECT_EQ(1, listener.get_supported_products().size()); EXPECT_EQ(dds::remote_config::protocol::capabilities_e:: ASM_API_SECURITY_SAMPLE_RATE, @@ -473,7 +420,7 @@ TEST(RemoteConfigAsmFeaturesListener, ProductsAreDynamic) { // All enabled remote_config::asm_features_listener listener( - config_service, true, true); + service_config, true, true); EXPECT_EQ(1, listener.get_supported_products().size()); EXPECT_EQ(dds::remote_config::protocol::capabilities_e::ASM_ACTIVATION | dds::remote_config::protocol::capabilities_e:: From 20cac6545ea71946c327e57614d3e7a9338ffca4 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Mon, 8 Jan 2024 12:22:55 +0100 Subject: [PATCH 10/14] Amend failing test --- appsec/src/helper/sampler.hpp | 1 - .../listeners/asm_features_listener_test.cpp | 10 ++++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index 34aa6c92ff..f87f452d07 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -104,7 +104,6 @@ class sampler { void set_sampler_rate(double sampler_rate) { - sampler_rate = valid_sample_rate(sampler_rate); if (sampler_rate == sample_rate_) { diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index f5e5e21ad9..e75e177436 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -364,16 +364,18 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, DynamicEnablementIsDisabled) TEST_F(RemoteConfigAsmFeaturesListenerTest, ApiSecurityIsDisabled) { + auto some_rate = 0.123; + service_config->set_request_sample_rate(some_rate); remote_config::asm_features_listener listener(service_config, true, false); - { // Api security value is stored regardless + { // Api security is not parsed if not enabled try { listener.on_update( get_config_with_status_and_sample_rate("true", 0.2)); } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(0.2, service_config->get_request_sample_rate()); + EXPECT_EQ(some_rate, service_config->get_request_sample_rate()); EXPECT_EQ(enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } @@ -386,8 +388,8 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, ApiSecurityIsDisabled) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ( - 0.2, service_config->get_request_sample_rate()); // same as before + EXPECT_EQ(some_rate, + service_config->get_request_sample_rate()); // same as before EXPECT_EQ(enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } From ec6cbba441cb6e4cd50cdcb0b0f1637917b6e495 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Wed, 17 Apr 2024 09:42:28 +0200 Subject: [PATCH 11/14] Refactor sampler --- appsec/src/helper/sampler.hpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index f87f452d07..93077104b0 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -59,11 +59,6 @@ class sampler { std::optional get() { - if (service_config_->get_asm_enabled_status() == - enable_asm_status::DISABLED) { - return std::nullopt; - } - const std::lock_guard lock_guard(mtx_); std::optional result = std::nullopt; @@ -87,6 +82,7 @@ class sampler { return result; } +protected: static double valid_sample_rate(double sampler_rate) { // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) @@ -113,8 +109,6 @@ class sampler { request_ = 1; sample_rate_ = sampler_rate; } - -protected: unsigned request_{1}; double sample_rate_{0}; std::atomic concurrent_{false}; From 97901bc45fb7e0ced8c1e6ea70ec0d08aa513eb3 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Wed, 17 Apr 2024 12:57:04 +0200 Subject: [PATCH 12/14] Refactor sampler --- appsec/src/helper/sampler.hpp | 10 +++------- appsec/tests/helper/sampler_test.cpp | 24 ++++++++++++------------ 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index 93077104b0..3fb58c7d34 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -73,11 +73,7 @@ class sampler { result = {scope{concurrent_}}; } - if (request_ < std::numeric_limits::max()) { - request_++; - } else { - request_ = 1; - } + request_++; return result; } @@ -106,10 +102,10 @@ class sampler { return; } - request_ = 1; + request_ = 0; sample_rate_ = sampler_rate; } - unsigned request_{1}; + unsigned request_{0}; double sample_rate_{0}; std::atomic concurrent_{false}; std::mutex mtx_; diff --git a/appsec/tests/helper/sampler_test.cpp b/appsec/tests/helper/sampler_test.cpp index 51f008baa1..5623087261 100644 --- a/appsec/tests/helper/sampler_test.cpp +++ b/appsec/tests/helper/sampler_test.cpp @@ -18,7 +18,7 @@ class sampler : public dds::sampler { : dds::sampler(service_config) {} void set_request(unsigned int i) { request_ = i; } - auto get_request() { return request_; } + unsigned get_request() { return request_; } }; } // namespace mock @@ -265,7 +265,7 @@ TEST(SamplerTest, TestOverflow) mock::sampler s(service_config); s.set_request(UINT_MAX); s.get(); - EXPECT_EQ(1, s.get_request()); + EXPECT_EQ(0, s.get_request()); } TEST(SamplerTest, ModifySamplerRate) @@ -276,10 +276,10 @@ TEST(SamplerTest, ModifySamplerRate) service_config->set_request_sample_rate(0.1); mock::sampler s(service_config); s.get(); - EXPECT_EQ(2, s.get_request()); + EXPECT_EQ(1, s.get_request()); service_config->set_request_sample_rate(0.2); s.get(); - EXPECT_EQ(2, s.get_request()); + EXPECT_EQ(1, s.get_request()); } { // Setting same sampler rate does do anything auto service_config = std::make_shared(); @@ -287,10 +287,10 @@ TEST(SamplerTest, ModifySamplerRate) service_config->set_request_sample_rate(0.1); mock::sampler s(service_config); s.get(); - EXPECT_EQ(2, s.get_request()); + EXPECT_EQ(1, s.get_request()); service_config->set_request_sample_rate(0.1); s.get(); - EXPECT_EQ(3, s.get_request()); + EXPECT_EQ(2, s.get_request()); } { // Over Zero: If given rate is invalid and gets defaulted to a value which // is same as before, it does not change anything @@ -299,10 +299,10 @@ TEST(SamplerTest, ModifySamplerRate) service_config->set_request_sample_rate(3); mock::sampler s(service_config); s.get(); - EXPECT_EQ(2, s.get_request()); + EXPECT_EQ(1, s.get_request()); service_config->set_request_sample_rate(4); s.get(); - EXPECT_EQ(3, s.get_request()); + EXPECT_EQ(2, s.get_request()); } { // Below zero: If given rate is invalid and gets defaulted to a value // which is same as before, it does not change anything @@ -311,10 +311,10 @@ TEST(SamplerTest, ModifySamplerRate) service_config->set_request_sample_rate(-3); mock::sampler s(service_config); s.get(); - EXPECT_EQ(2, s.get_request()); + EXPECT_EQ(1, s.get_request()); service_config->set_request_sample_rate(-4); s.get(); - EXPECT_EQ(3, s.get_request()); + EXPECT_EQ(2, s.get_request()); } { // Below min: If given rate is invalid and gets defaulted to a value which // is same as before, it does not change anything @@ -323,10 +323,10 @@ TEST(SamplerTest, ModifySamplerRate) service_config->set_request_sample_rate(0.000001); mock::sampler s(service_config); s.get(); - EXPECT_EQ(2, s.get_request()); + EXPECT_EQ(1, s.get_request()); service_config->set_request_sample_rate(0.000002); s.get(); - EXPECT_EQ(3, s.get_request()); + EXPECT_EQ(2, s.get_request()); } } From 84a0fc868b406a1a8559b43f39b0e7eee7023311 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Mon, 22 Apr 2024 13:06:12 +0200 Subject: [PATCH 13/14] Refactor sampler --- appsec/src/helper/client.cpp | 4 +- appsec/src/helper/sampler.hpp | 89 ++------- appsec/src/helper/service.cpp | 5 +- appsec/src/helper/service.hpp | 5 + appsec/src/helper/service_config.hpp | 9 +- .../listeners/asm_features_listener_test.cpp | 18 +- appsec/tests/helper/sampler_test.cpp | 171 +++--------------- appsec/tests/helper/service_test.cpp | 18 +- 8 files changed, 73 insertions(+), 246 deletions(-) diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index d7f859e790..eff230748c 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -439,10 +439,8 @@ bool client::handle_command(network::request_shutdown::request &command) try { auto sampler = service_->get_schema_sampler(); - std::optional scope; if (sampler) { - scope = sampler->get(); - if (scope.has_value()) { + if (sampler->get()) { parameter context_processor = parameter::map(); context_processor.add( "extract-schema", parameter::as_boolean(true)); diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index 3fb58c7d34..a724e38a0c 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -19,69 +19,8 @@ namespace dds { static const double min_rate = 0.0001; class sampler { public: - sampler(std::shared_ptr service_config) - : service_config_(std::move(service_config)) + sampler(double sampler_rate) { - set_sampler_rate(service_config_->get_request_sample_rate()); - } - class scope { - public: - explicit scope(std::atomic &concurrent) : concurrent_(&concurrent) - { - concurrent_->store(true, std::memory_order_relaxed); - } - - scope(const scope &) = delete; - scope &operator=(const scope &) = delete; - scope(scope &&oth) noexcept - { - concurrent_ = oth.concurrent_; - oth.concurrent_ = nullptr; - } - scope &operator=(scope &&oth) - { - concurrent_ = oth.concurrent_; - oth.concurrent_ = nullptr; - - return *this; - } - - ~scope() - { - if (concurrent_ != nullptr) { - concurrent_->store(false, std::memory_order_relaxed); - } - } - - protected: - std::atomic *concurrent_; - }; - - std::optional get() - { - const std::lock_guard lock_guard(mtx_); - - std::optional result = std::nullopt; - - if (sample_rate_ != - valid_sample_rate(service_config_->get_request_sample_rate())) { - set_sampler_rate(service_config_->get_request_sample_rate()); - } - - if (!concurrent_ && floor(request_ * sample_rate_) != - floor((request_ + 1) * sample_rate_)) { - result = {scope{concurrent_}}; - } - - request_++; - - return result; - } - -protected: - static double valid_sample_rate(double sampler_rate) - { - // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) if (sampler_rate <= 0) { sampler_rate = 0; } else if (sampler_rate > 1) { @@ -89,26 +28,26 @@ class sampler { } else if (sampler_rate < min_rate) { sampler_rate = min_rate; } - // NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - return sampler_rate; + sample_rate_ = sampler_rate; } - void set_sampler_rate(double sampler_rate) + bool get() { - sampler_rate = valid_sample_rate(sampler_rate); - - if (sampler_rate == sample_rate_) { - return; + bool result = false; + if (floor(request_ * sample_rate_) != + floor((request_ + 1) * sample_rate_)) { + result = true; } - request_ = 0; - sample_rate_ = sampler_rate; + request_.fetch_add(1, std::memory_order_relaxed); + + return result; } - unsigned request_{0}; + + double get_sample_rate() { return sample_rate_; } + + std::atomic request_{0}; double sample_rate_{0}; - std::atomic concurrent_{false}; - std::mutex mtx_; - std::shared_ptr service_config_; }; } // namespace dds diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index d597d9806e..7ead85688b 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -30,8 +30,9 @@ service::service(std::shared_ptr engine, sample_rate = 0; } - service_config_->set_request_sample_rate(sample_rate); - schema_sampler_ = std::make_shared(service_config_); + service_config_->set_request_sampler_listener( + [this](double rate) { set_sampler_rate(rate); }); + schema_sampler_ = std::make_shared(sample_rate); } service::ptr service::from_settings(service_identifier &&id, diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index 72e3a8a5dd..260571ae80 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -75,6 +75,11 @@ class service { return schema_sampler_; } + void set_sampler_rate(double rate) + { + schema_sampler_ = std::make_shared(rate); + } + protected: std::shared_ptr engine_{}; std::shared_ptr service_config_{}; diff --git a/appsec/src/helper/service_config.hpp b/appsec/src/helper/service_config.hpp index f0cf00c8fe..642e815949 100644 --- a/appsec/src/helper/service_config.hpp +++ b/appsec/src/helper/service_config.hpp @@ -19,15 +19,18 @@ struct service_config { void disable_asm() { asm_enabled = enable_asm_status::DISABLED; } void unset_asm() { asm_enabled = enable_asm_status::NOT_SET; } enable_asm_status get_asm_enabled_status() { return asm_enabled; } - double get_request_sample_rate() { return request_sample_rate; } + void set_request_sampler_listener(std::function listener) + { + request_sample_rate_listener = listener; + } void set_request_sample_rate(double sample_rate) { - request_sample_rate = sample_rate; + request_sample_rate_listener(sample_rate); } protected: std::atomic asm_enabled = {enable_asm_status::NOT_SET}; - std::atomic request_sample_rate = 0.1; + std::function request_sample_rate_listener; }; } // namespace dds diff --git a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp index e75e177436..7766f56e4c 100644 --- a/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/asm_features_listener_test.cpp @@ -58,8 +58,14 @@ remote_config::config get_disabled_config(bool as_string = true) class RemoteConfigAsmFeaturesListenerTest : public ::testing::Test { public: std::shared_ptr service_config; + double rate_; - void SetUp() { service_config = std::make_shared(); } + void SetUp() + { + service_config = std::make_shared(); + service_config->set_request_sampler_listener( + [this](double rate) { rate_ = rate; }); + } }; TEST_F(RemoteConfigAsmFeaturesListenerTest, ByDefaultListenerIsNotSet) @@ -334,7 +340,7 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, RequestSampleRateIsParsed) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(sample_rate, service_config->get_request_sample_rate()); + EXPECT_EQ(sample_rate, rate_); } { // It parses integers @@ -344,7 +350,7 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, RequestSampleRateIsParsed) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(sample_rate, service_config->get_request_sample_rate()); + EXPECT_EQ(sample_rate, rate_); } } @@ -357,7 +363,7 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, DynamicEnablementIsDisabled) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(0.2, service_config->get_request_sample_rate()); + EXPECT_EQ(0.2, rate_); EXPECT_EQ( enable_asm_status::NOT_SET, service_config->get_asm_enabled_status()); } @@ -375,7 +381,7 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, ApiSecurityIsDisabled) } catch (remote_config::error_applying_config &error) { std::cout << error.what() << std::endl; } - EXPECT_EQ(some_rate, service_config->get_request_sample_rate()); + EXPECT_EQ(some_rate, rate_); EXPECT_EQ(enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } @@ -389,7 +395,7 @@ TEST_F(RemoteConfigAsmFeaturesListenerTest, ApiSecurityIsDisabled) std::cout << error.what() << std::endl; } EXPECT_EQ(some_rate, - service_config->get_request_sample_rate()); // same as before + rate_); // same as before EXPECT_EQ(enable_asm_status::ENABLED, service_config->get_asm_enabled_status()); } diff --git a/appsec/tests/helper/sampler_test.cpp b/appsec/tests/helper/sampler_test.cpp index 5623087261..02e6f9b613 100644 --- a/appsec/tests/helper/sampler_test.cpp +++ b/appsec/tests/helper/sampler_test.cpp @@ -14,9 +14,7 @@ namespace mock { class sampler : public dds::sampler { public: - sampler(std::shared_ptr service_config) - : dds::sampler(service_config) - {} + sampler(double sampler_rate) : dds::sampler(sampler_rate) {} void set_request(unsigned int i) { request_ = i; } unsigned get_request() { return request_; } }; @@ -28,8 +26,7 @@ std::atomic picked = 0; void count_picked(dds::sampler &sampler, int iterations) { for (int i = 0; i < iterations; i++) { - auto is_pick = sampler.get(); - if (is_pick != std::nullopt) { + if (sampler.get()) { picked++; } } @@ -39,8 +36,7 @@ TEST(SamplerTest, ItPicksAllWhenRateIs1) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(1); - sampler s(service_config); + sampler s(1); picked = 0; count_picked(s, 100); @@ -51,8 +47,7 @@ TEST(SamplerTest, ItPicksNoneWhenRateIs0) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0); - sampler s(service_config); + sampler s(0); picked = 0; count_picked(s, 100); @@ -63,8 +58,7 @@ TEST(SamplerTest, ItPicksHalfWhenPortionGiven) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.5); - sampler s(service_config); + sampler s(0.5); picked = 0; count_picked(s, 100); @@ -75,8 +69,7 @@ TEST(SamplerTest, ItResetTokensAfter100Calls) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(1); - sampler s(service_config); + sampler s(1); picked = 0; count_picked(s, 100); @@ -90,8 +83,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.1); - sampler s(service_config); + sampler s(0.1); picked = 0; count_picked(s, 10); @@ -100,8 +92,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.5); - sampler s(service_config); + sampler s(0.5); picked = 0; count_picked(s, 10); @@ -110,8 +101,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.01); - sampler s(service_config); + sampler s(0.01); picked = 0; count_picked(s, 100); @@ -120,8 +110,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.02); - sampler s(service_config); + sampler s(0.02); picked = 0; count_picked(s, 100); @@ -130,8 +119,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.001); - sampler s(service_config); + sampler s(0.001); picked = 0; count_picked(s, 1000); @@ -140,8 +128,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.003); - sampler s(service_config); + sampler s(0.003); picked = 0; count_picked(s, 1000); @@ -150,8 +137,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.0001); - sampler s(service_config); + sampler s(0.0001); picked = 0; count_picked(s, 10000); @@ -160,8 +146,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.0007); - sampler s(service_config); + sampler s(0.0007); picked = 0; count_picked(s, 10000); @@ -170,8 +155,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.123); - sampler s(service_config); + sampler s(0.123); picked = 0; count_picked(s, 1000); @@ -180,8 +164,7 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) { auto service_config = std::make_shared(); service_config->enable_asm(); - service_config->set_request_sample_rate(0.6); - sampler s(service_config); + sampler s(0.6); picked = 0; count_picked(s, 10); @@ -192,30 +175,21 @@ TEST(SamplerTest, ItWorksWithDifferentMagnitudes) TEST(SamplerTest, TestInvalidSampleRatesDefaultToTenPercent) { { - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(2); - sampler s(service_config); + sampler s(2); picked = 0; count_picked(s, 10); EXPECT_EQ(10, picked); } { - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(-1); - sampler s(service_config); + sampler s(-1); picked = 0; count_picked(s, 10); EXPECT_EQ(0, picked); } { // Below limit goes to default 10 percent - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0.000001); - sampler s(service_config); + sampler s(0.000001); picked = 0; count_picked(s, 1000000); @@ -226,30 +200,21 @@ TEST(SamplerTest, TestInvalidSampleRatesDefaultToTenPercent) TEST(SamplerTest, TestLimits) { { - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0); - sampler s(service_config); + sampler s(0); picked = 0; count_picked(s, 10); EXPECT_EQ(0, picked); } { - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(1); - sampler s(service_config); + sampler s(1); picked = 0; count_picked(s, 10); EXPECT_EQ(10, picked); } { - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0.0001); - sampler s(service_config); + sampler s(0.0001); picked = 0; count_picked(s, 10000); @@ -259,99 +224,9 @@ TEST(SamplerTest, TestLimits) TEST(SamplerTest, TestOverflow) { - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0); - mock::sampler s(service_config); + mock::sampler s(0); s.set_request(UINT_MAX); s.get(); EXPECT_EQ(0, s.get_request()); } - -TEST(SamplerTest, ModifySamplerRate) -{ - { // New sampler rate reset requests - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0.1); - mock::sampler s(service_config); - s.get(); - EXPECT_EQ(1, s.get_request()); - service_config->set_request_sample_rate(0.2); - s.get(); - EXPECT_EQ(1, s.get_request()); - } - { // Setting same sampler rate does do anything - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0.1); - mock::sampler s(service_config); - s.get(); - EXPECT_EQ(1, s.get_request()); - service_config->set_request_sample_rate(0.1); - s.get(); - EXPECT_EQ(2, s.get_request()); - } - { // Over Zero: If given rate is invalid and gets defaulted to a value which - // is same as before, it does not change anything - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(3); - mock::sampler s(service_config); - s.get(); - EXPECT_EQ(1, s.get_request()); - service_config->set_request_sample_rate(4); - s.get(); - EXPECT_EQ(2, s.get_request()); - } - { // Below zero: If given rate is invalid and gets defaulted to a value - // which is same as before, it does not change anything - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(-3); - mock::sampler s(service_config); - s.get(); - EXPECT_EQ(1, s.get_request()); - service_config->set_request_sample_rate(-4); - s.get(); - EXPECT_EQ(2, s.get_request()); - } - { // Below min: If given rate is invalid and gets defaulted to a value which - // is same as before, it does not change anything - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(0.000001); - mock::sampler s(service_config); - s.get(); - EXPECT_EQ(1, s.get_request()); - service_config->set_request_sample_rate(0.000002); - s.get(); - EXPECT_EQ(2, s.get_request()); - } -} - -TEST(ScopeTest, TestConcurrent) -{ - std::atomic concurrent = false; - { - auto s = sampler::scope(std::ref(concurrent)); - EXPECT_TRUE(concurrent); - } - EXPECT_FALSE(concurrent); -} - -TEST(ScopeTest, TestItDoesNotPickTokenUntilScopeReleased) -{ - auto service_config = std::make_shared(); - service_config->enable_asm(); - service_config->set_request_sample_rate(1); - sampler sampler(service_config); - auto is_pick = sampler.get(); - EXPECT_TRUE(is_pick != std::nullopt); - is_pick = sampler.get(); - EXPECT_FALSE(is_pick != std::nullopt); - is_pick.reset(); - is_pick = sampler.get(); - EXPECT_TRUE(is_pick != std::nullopt); -} } // namespace dds diff --git a/appsec/tests/helper/service_test.cpp b/appsec/tests/helper/service_test.cpp index afa7a950f2..f0c79784bf 100644 --- a/appsec/tests/helper/service_test.cpp +++ b/appsec/tests/helper/service_test.cpp @@ -58,8 +58,8 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) engine, service_config, nullptr, {true, all_requests_are_picked}); EXPECT_EQ( - all_requests_are_picked, service_config->get_request_sample_rate()); - EXPECT_TRUE(s.get_schema_sampler()->get().has_value()); + all_requests_are_picked, s.get_schema_sampler()->get_sample_rate()); + EXPECT_TRUE(s.get_schema_sampler()->get()); } { // Constructor. It does not pick based on rate @@ -68,8 +68,8 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) engine, service_config, nullptr, {true, no_request_is_picked}); EXPECT_EQ( - no_request_is_picked, service_config->get_request_sample_rate()); - EXPECT_FALSE(s.get_schema_sampler()->get().has_value()); + no_request_is_picked, s.get_schema_sampler()->get_sample_rate()); + EXPECT_FALSE(s.get_schema_sampler()->get()); } { // Constructor. It does not pick if disabled @@ -78,8 +78,8 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto s = service(engine, service_config, nullptr, {schema_extraction_disabled, all_requests_are_picked}); - EXPECT_EQ(0, service_config->get_request_sample_rate()); - EXPECT_FALSE(s.get_schema_sampler()->get().has_value()); + EXPECT_EQ(0, s.get_schema_sampler()->get_sample_rate()); + EXPECT_FALSE(s.get_schema_sampler()->get()); } { // Static constructor. It picks based on rate @@ -88,7 +88,7 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto service = service::from_settings( service_identifier(sid), engine_settings, {}, meta, metrics, false); - EXPECT_TRUE(service->get_schema_sampler()->get().has_value()); + EXPECT_TRUE(service->get_schema_sampler()->get()); } { // Static constructor. It does not pick based on rate @@ -97,7 +97,7 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto service = service::from_settings( service_identifier(sid), engine_settings, {}, meta, metrics, false); - EXPECT_FALSE(service->get_schema_sampler()->get().has_value()); + EXPECT_FALSE(service->get_schema_sampler()->get()); } { // Static constructor. It does not pick if disabled @@ -106,7 +106,7 @@ TEST(ServiceTest, ServicePickSchemaExtractionSamples) auto service = service::from_settings( service_identifier(sid), engine_settings, {}, meta, metrics, false); - EXPECT_FALSE(service->get_schema_sampler()->get().has_value()); + EXPECT_FALSE(service->get_schema_sampler()->get()); } } From ccf25146368f96e462453ecb55bbb5e136070d28 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Tue, 23 Apr 2024 10:49:02 +0200 Subject: [PATCH 14/14] Make sampler atomic --- appsec/src/helper/sampler.hpp | 11 ++--------- appsec/src/helper/service.hpp | 5 +++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/appsec/src/helper/sampler.hpp b/appsec/src/helper/sampler.hpp index a724e38a0c..f7c868341d 100644 --- a/appsec/src/helper/sampler.hpp +++ b/appsec/src/helper/sampler.hpp @@ -34,15 +34,8 @@ class sampler { bool get() { - bool result = false; - if (floor(request_ * sample_rate_) != - floor((request_ + 1) * sample_rate_)) { - result = true; - } - - request_.fetch_add(1, std::memory_order_relaxed); - - return result; + unsigned prev = request_.fetch_add(1, std::memory_order_relaxed); + return floor(prev * sample_rate_) != floor((prev + 1) * sample_rate_); } double get_sample_rate() { return sample_rate_; } diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index 260571ae80..7231d0aaf4 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -72,12 +72,13 @@ class service { [[nodiscard]] std::shared_ptr get_schema_sampler() { - return schema_sampler_; + return std::atomic_load(&schema_sampler_); } void set_sampler_rate(double rate) { - schema_sampler_ = std::make_shared(rate); + auto new_sampler = std::make_shared(rate); + std::atomic_store(&schema_sampler_, new_sampler); } protected: