From 4e0779186ccdccc801c2156a401a94a3573245a3 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Wed, 2 Oct 2024 18:34:15 +0100 Subject: [PATCH] Telemetry in AppSec --- appsec/src/extension/commands/client_init.c | 7 + appsec/src/extension/commands_helpers.c | 202 ++++++++++--- appsec/src/extension/commands_helpers.h | 1 + appsec/src/extension/ddtrace.c | 54 ++++ appsec/src/extension/ddtrace.h | 25 ++ appsec/src/helper/client.cpp | 115 ++++++-- appsec/src/helper/client.hpp | 10 +- appsec/src/helper/engine.cpp | 30 +- appsec/src/helper/engine.hpp | 23 +- appsec/src/helper/engine_settings.hpp | 9 +- appsec/src/helper/metrics.cpp | 11 + appsec/src/helper/metrics.hpp | 59 ++++ appsec/src/helper/network/acceptor.cpp | 3 +- appsec/src/helper/network/acceptor.hpp | 6 +- appsec/src/helper/network/broker.hpp | 8 +- appsec/src/helper/network/proto.hpp | 12 +- appsec/src/helper/network/socket.hpp | 2 - .../helper/remote_config/client_handler.cpp | 43 ++- .../helper/remote_config/client_handler.hpp | 23 +- .../listeners/engine_listener.cpp | 14 +- .../listeners/engine_listener.hpp | 8 +- appsec/src/helper/runner.cpp | 8 +- appsec/src/helper/runner.hpp | 5 +- appsec/src/helper/service.cpp | 26 +- appsec/src/helper/service.hpp | 133 ++++++++- appsec/src/helper/service_manager.cpp | 7 +- appsec/src/helper/service_manager.hpp | 4 +- appsec/src/helper/subscriber/base.hpp | 11 +- appsec/src/helper/subscriber/waf.cpp | 262 +++++++++++++---- appsec/src/helper/subscriber/waf.hpp | 32 +- .../tests/extension/rshutdown_telemetry.phpt | 55 ++++ appsec/tests/fuzzer/helpers.hpp | 56 ++-- appsec/tests/fuzzer/main.cpp | 7 +- appsec/tests/fuzzer/network.hpp | 39 ++- appsec/tests/helper/broker_test.cpp | 6 +- appsec/tests/helper/client_test.cpp | 47 +-- appsec/tests/helper/common.hpp | 3 + appsec/tests/helper/engine_test.cpp | 160 ++++++---- .../listeners/asm_features_listener_test.cpp | 16 +- .../listeners/engine_listener_test.cpp | 163 ++++++---- appsec/tests/helper/remote_config/mocks.hpp | 6 +- appsec/tests/helper/service_manager_test.cpp | 30 +- appsec/tests/helper/service_test.cpp | 91 ++---- appsec/tests/helper/tel_subm_mock.hpp | 16 + appsec/tests/helper/waf_test.cpp | 167 ++++++----- .../appsec/php/docker/AppSecContainer.groovy | 57 +++- .../php/mock_agent/ConfigV07Handler.groovy | 12 +- .../src/test/bin/enable_extensions.sh | 4 +- .../php/integration/RemoteConfigTests.groovy | 55 +--- .../php/integration/TelemetryTests.groovy | 278 ++++++++++++++++++ components-rs/ddtrace.h | 1 + components-rs/telemetry.rs | 5 +- ddtrace.sym | 2 + ext/otel_config.c | 3 +- ext/telemetry.c | 48 ++- ext/telemetry.h | 10 +- libdatadog | 2 +- 57 files changed, 1826 insertions(+), 666 deletions(-) create mode 100644 appsec/src/helper/metrics.cpp create mode 100644 appsec/src/helper/metrics.hpp create mode 100644 appsec/tests/extension/rshutdown_telemetry.phpt create mode 100644 appsec/tests/helper/tel_subm_mock.hpp create mode 100644 appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy diff --git a/appsec/src/extension/commands/client_init.c b/appsec/src/extension/commands/client_init.c index a20513129d..2c4cd5e46b 100644 --- a/appsec/src/extension/commands/client_init.c +++ b/appsec/src/extension/commands/client_init.c @@ -174,6 +174,13 @@ static void _process_meta_and_metrics( mpack_node_t metrics = mpack_node_array_at(root, 4); dd_command_process_metrics(metrics, span); + + // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + if (mpack_node_array_length(root) >= 6) { + // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + mpack_node_t tel_metrics = mpack_node_array_at(root, 5); + dd_command_process_telemetry_metrics(tel_metrics); + } } static dd_result _check_helper_version(mpack_node_t root) diff --git a/appsec/src/extension/commands_helpers.c b/appsec/src/extension/commands_helpers.c index 1bedf8323e..620fb0f4d4 100644 --- a/appsec/src/extension/commands_helpers.c +++ b/appsec/src/extension/commands_helpers.c @@ -14,6 +14,8 @@ #include "request_abort.h" #include "tags.h" #include +#include +#include typedef struct _dd_omsg { zend_llist iovecs; @@ -437,7 +439,71 @@ static void _command_process_stack_trace_parameters(mpack_node_t root) } } -dd_result _command_process_actions(mpack_node_t root, struct req_info *ctx) +static dd_result _command_process_actions( + mpack_node_t root, struct req_info *ctx); + +/* + * array( + * 0: [<"ok" / "record" / "block" / "redirect">, + * [if block/redirect parameters: (map)]] + * 1: [if block/redirect/record: appsec span data (array of strings: json + * fragments)], + * 2: [force keep: bool] + * 3: [meta: map] + * 4: [metrics: map] + * 5: [telemetry metrics: map string -> + * array(array(value: double, tags: string)]) + * ) + */ +#define RESP_INDEX_ACTION_PARAMS 0 +#define RESP_INDEX_APPSEC_SPAN_DATA 1 +#define RESP_INDEX_FORCE_KEEP 2 +#define RESP_INDEX_SPAN_META 3 +#define RESP_INDEX_SPAN_METRICS 4 +#define RESP_INDEX_TELEMETRY_METRICS 5 + +dd_result dd_command_proc_resp_verd_span_data( + mpack_node_t root, void *unspecnull _ctx) +{ + struct req_info *ctx = _ctx; + assert(ctx != NULL); + + mpack_node_t actions = mpack_node_array_at(root, RESP_INDEX_ACTION_PARAMS); + dd_result res = _command_process_actions(actions, ctx); + + if (res == dd_should_block || res == dd_should_redirect || + res == dd_should_record) { + _set_appsec_span_data( + mpack_node_array_at(root, RESP_INDEX_APPSEC_SPAN_DATA)); + } + + mpack_node_t force_keep = mpack_node_array_at(root, RESP_INDEX_FORCE_KEEP); + if (mpack_node_type(force_keep) == mpack_type_bool && + mpack_node_bool(force_keep)) { + dd_tags_set_sampling_priority(); + } + + if (mpack_node_array_length(root) >= RESP_INDEX_SPAN_METRICS + 1 && + ctx->root_span) { + zend_object *span = ctx->root_span; + + mpack_node_t meta = mpack_node_array_at(root, RESP_INDEX_SPAN_META); + dd_command_process_meta(meta, span); + mpack_node_t metrics = + mpack_node_array_at(root, RESP_INDEX_SPAN_METRICS); + dd_command_process_metrics(metrics, span); + } + + if (mpack_node_array_length(root) >= RESP_INDEX_TELEMETRY_METRICS + 1) { + dd_command_process_telemetry_metrics( + mpack_node_array_at(root, RESP_INDEX_TELEMETRY_METRICS)); + } + + return res; +} + +static dd_result _command_process_actions( + mpack_node_t root, struct req_info *ctx) { size_t actions = mpack_node_array_length(root); dd_result res = dd_success; @@ -482,42 +548,6 @@ dd_result _command_process_actions(mpack_node_t root, struct req_info *ctx) return res; } -dd_result dd_command_proc_resp_verd_span_data( - mpack_node_t root, void *unspecnull _ctx) -{ - struct req_info *ctx = _ctx; - assert(ctx != NULL); - - mpack_node_t actions = mpack_node_array_at(root, 0); - - dd_result res = _command_process_actions(actions, ctx); - - if (res == dd_should_block || res == dd_should_redirect || - res == dd_should_record) { - _set_appsec_span_data(mpack_node_array_at(root, 1)); - } - - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - mpack_node_t force_keep = mpack_node_array_at(root, 2); - if (mpack_node_type(force_keep) == mpack_type_bool && - mpack_node_bool(force_keep)) { - dd_tags_set_sampling_priority(); - } - - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - if (mpack_node_array_length(root) >= 5 && ctx->root_span) { - zend_object *span = ctx->root_span; - - mpack_node_t meta = mpack_node_array_at(root, 3); - dd_command_process_meta(meta, span); - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - mpack_node_t metrics = mpack_node_array_at(root, 4); - dd_command_process_metrics(metrics, span); - } - - return res; -} - static void _add_appsec_span_data_frag(mpack_node_t node) { const char *data = mpack_node_data(node); @@ -648,6 +678,104 @@ bool dd_command_process_metrics(mpack_node_t root, zend_object *nonnull span) return true; } +static void _handle_telemetry_metric(const char *nonnull key_str, + size_t key_len, double value, const char *nonnull tags_str, + size_t tags_len); + +bool dd_command_process_telemetry_metrics(mpack_node_t metrics) +{ + if (mpack_node_type(metrics) != mpack_type_map) { + return false; + } + + if (!ddtrace_metric_register_buffer) { + mlog_g(dd_log_debug, "ddtrace_metric_register_buffer unavailable"); + return true; + } + + for (size_t i = 0; i < mpack_node_map_count(metrics); i++) { + mpack_node_t key = mpack_node_map_key_at(metrics, i); + + const char *key_str = mpack_node_str(key); + if (!key_str) { + continue; + } + + size_t key_len = mpack_node_strlen(key); + mpack_node_t arr_value = mpack_node_map_value_at(metrics, i); + + for (size_t j = 0; j < mpack_node_array_length(arr_value); j++) { + mpack_node_t value = mpack_node_array_at(arr_value, j); + mpack_node_t dval_node = mpack_node_array_at(value, 0); + double dval = mpack_node_double(dval_node); + + const char *tags_str = ""; + size_t tags_len = 0; + if (mpack_node_array_length(value) >= 2) { + mpack_node_t tags = mpack_node_array_at(value, 1); + tags_str = mpack_node_str(tags); + tags_len = mpack_node_strlen(tags); + } + if (mpack_node_error(metrics) != mpack_ok) { + break; + } + + _handle_telemetry_metric( + key_str, key_len, dval, tags_str, tags_len); + } + } + + return true; +} + +static void _init_zstr( + zend_string *_Atomic *nonnull zstr, const char *nonnull str, size_t len) +{ + zend_string *zstr_cur = atomic_load_explicit(zstr, memory_order_acquire); + if (zstr_cur != NULL) { + return; + } + zend_string *zstr_new = zend_string_init(str, len, 1); + if (atomic_compare_exchange_strong_explicit(zstr, &(zend_string *){NULL}, + zstr_new, memory_order_release, memory_order_relaxed)) { + return; + } + zend_string_release(zstr_new); +} + +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) +void _handle_telemetry_metric(const char *nonnull key_str, size_t key_len, + double value, const char *nonnull tags_str, size_t tags_len) +{ +#define HANDLE_METRIC(name, type) \ + do { \ + if (key_len == sizeof(name "") - 1 && \ + memcmp(key_str, name, key_len) == 0) { \ + static zend_string *_Atomic key_zstr; \ + _init_zstr(&key_zstr, name, sizeof(name) - 1); \ + zend_string *tags_zstr = zend_string_init(tags_str, tags_len, 1); \ + ddtrace_metric_register_buffer( \ + key_zstr, type, DDTRACE_METRIC_NAMESPACE_APPSEC); \ + ddtrace_metric_add_point(key_zstr, value, tags_zstr); \ + zend_string_release(tags_zstr); \ + mlog_g(dd_log_debug, \ + "Telemetry metric %.*s added with tags %.*s and value %f", \ + (int)key_len, key_str, (int)tags_len, tags_str, value); \ + return; \ + } \ + } while (0) + + HANDLE_METRIC("waf.requests", DDTRACE_METRIC_TYPE_COUNT); + HANDLE_METRIC("waf.updates", DDTRACE_METRIC_TYPE_COUNT); + HANDLE_METRIC("waf.init", DDTRACE_METRIC_TYPE_COUNT); + HANDLE_METRIC("waf.config_errors", DDTRACE_METRIC_TYPE_COUNT); + + HANDLE_METRIC("remote_config.first_pull", DDTRACE_METRIC_TYPE_GAUGE); + HANDLE_METRIC("remote_config.last_success", DDTRACE_METRIC_TYPE_GAUGE); + + mlog_g(dd_log_info, "Unknown telemetry metric %.*s", (int)key_len, key_str); +} + static void _dump_in_msg( dd_log_level_t lvl, const char *nonnull data, size_t data_len) { diff --git a/appsec/src/extension/commands_helpers.h b/appsec/src/extension/commands_helpers.h index 45321bb0aa..10cc9cbce3 100644 --- a/appsec/src/extension/commands_helpers.h +++ b/appsec/src/extension/commands_helpers.h @@ -38,6 +38,7 @@ dd_result dd_command_proc_resp_verd_span_data(mpack_node_t root, /* Common helpers */ void dd_command_process_meta(mpack_node_t root, zend_object *nonnull span); bool dd_command_process_metrics(mpack_node_t root, zend_object *nonnull span); +bool dd_command_process_telemetry_metrics(mpack_node_t root); dd_result dd_command_process_config_features( mpack_node_t root, ATTR_UNUSED void *nullable ctx); dd_result dd_command_process_config_features_unexpected( diff --git a/appsec/src/extension/ddtrace.c b/appsec/src/extension/ddtrace.c index 36fb0da7fd..6edd699aaa 100644 --- a/appsec/src/extension/ddtrace.c +++ b/appsec/src/extension/ddtrace.c @@ -18,6 +18,11 @@ #include "zend_object_handlers.h" #include "zend_types.h" +void (*nullable ddtrace_metric_register_buffer)( + zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns); +bool (*nullable ddtrace_metric_add_point)( + zend_string *nonnull name, double value, zend_string *nonnull tags); + static int (*_orig_ddtrace_shutdown)(SHUTDOWN_FUNC_ARGS); static int _mod_type; static int _mod_number; @@ -30,6 +35,7 @@ static zend_string *_meta_struct_propname; static THREAD_LOCAL_ON_ZTS bool _suppress_ddtrace_rshutdown; static uint8_t *_ddtrace_runtime_id = NULL; +static void _setup_testing_telemetry_functions(void); static zend_module_entry *_find_ddtrace_module(void); static int _ddtrace_rshutdown_testing(SHUTDOWN_FUNC_ARGS); static void _register_testing_objects(void); @@ -47,6 +53,11 @@ static zend_string *(*_ddtrace_ip_extraction_find)(zval *server); static const char *nullable (*_ddtrace_remote_config_get_path)(void); +static void _test_ddtrace_metric_register_buffer( + zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns); +static bool _test_ddtrace_metric_add_point( + zend_string *nonnull name, double value, zend_string *nonnull tags); + static void dd_trace_load_symbols(void) { bool testing = get_global_DD_APPSEC_TESTING(); @@ -108,6 +119,19 @@ static void dd_trace_load_symbols(void) "Failed to load ddtrace_remote_config_get_path: %s", dlerror()); } + ddtrace_metric_register_buffer = + dlsym(handle, "ddtrace_metric_register_buffer"); + if (ddtrace_metric_register_buffer == NULL && !testing) { + mlog(dd_log_error, "Failed to load ddtrace_metric_register_buffer: %s", + dlerror()); // NOLINT(concurrency-mt-unsafe) + } + + ddtrace_metric_add_point = dlsym(handle, "ddtrace_metric_add_point"); + if (ddtrace_metric_add_point == NULL && !testing) { + mlog(dd_log_error, "Failed to load ddtrace_metric_add_point: %s", + dlerror()); // NOLINT(concurrency-mt-unsafe) + } + dlclose(handle); } @@ -122,6 +146,7 @@ void dd_trace_startup() if (get_global_DD_APPSEC_TESTING()) { _register_testing_objects(); + _setup_testing_telemetry_functions(); } zend_module_entry *mod = _find_ddtrace_module(); @@ -144,6 +169,16 @@ void dd_trace_startup() } } +static void _setup_testing_telemetry_functions() +{ + if (ddtrace_metric_register_buffer == NULL) { + ddtrace_metric_register_buffer = _test_ddtrace_metric_register_buffer; + } + if (ddtrace_metric_add_point == NULL) { + ddtrace_metric_add_point = _test_ddtrace_metric_add_point; + } +} + static zend_module_entry *_find_ddtrace_module() { zend_string *ddtrace_name = @@ -515,3 +550,22 @@ static const zend_function_entry functions[] = { // clang-format on static void _register_testing_objects() { dd_phpobj_reg_funcs(functions); } + +static void _test_ddtrace_metric_register_buffer( + zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns) +{ + php_error_docref(NULL, E_NOTICE, + "Would call ddtrace_metric_register_buffer with name=%.*s " + "type=%d ns=%d", + (int)ZSTR_LEN(name), ZSTR_VAL(name), type, ns); +} +static bool _test_ddtrace_metric_add_point( + zend_string *nonnull name, double value, zend_string *nonnull tags) +{ + php_error_docref(NULL, E_NOTICE, + "Would call to ddtrace_metric_add_point with name=%.*s value=%f " + "tags=%.*s", + (int)ZSTR_LEN(name), ZSTR_VAL(name), value, (int)ZSTR_LEN(tags), + ZSTR_VAL(tags)); + return true; +} diff --git a/appsec/src/extension/ddtrace.h b/appsec/src/extension/ddtrace.h index 4a8a6ca624..62c7a6d382 100644 --- a/appsec/src/extension/ddtrace.h +++ b/appsec/src/extension/ddtrace.h @@ -81,3 +81,28 @@ bool dd_trace_user_req_add_listeners( zend_string *nullable dd_ip_extraction_find(zval *nonnull server); const char *nullable dd_trace_remote_config_get_path(void); + +typedef enum { + DDTRACE_METRIC_TYPE_GAUGE, + DDTRACE_METRIC_TYPE_COUNT, + DDTRACE_METRIC_TYPE_DISTRIBUTION, +} ddtrace_metric_type; + +typedef enum { + DDTRACE_METRIC_NAMESPACE_TRACERS, + DDTRACE_METRIC_NAMESPACE_PROFILERS, + DDTRACE_METRIC_NAMESPACE_RUM, + DDTRACE_METRIC_NAMESPACE_APPSEC, + DDTRACE_METRIC_NAMESPACE_IDE_PLUGINS, + DDTRACE_METRIC_NAMESPACE_LIVE_DEBUGGER, + DDTRACE_METRIC_NAMESPACE_IAST, + DDTRACE_METRIC_NAMESPACE_GENERAL, + DDTRACE_METRIC_NAMESPACE_TELEMETRY, + DDTRACE_METRIC_NAMESPACE_APM, + DDTRACE_METRIC_NAMESPACE_SIDECAR, +} ddtrace_metric_ns; + +extern void (*nullable ddtrace_metric_register_buffer)( + zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns); +extern bool (*nullable ddtrace_metric_add_point)(zend_string *nonnull name, + double value, zend_string *nonnull tags); diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index 17b9470176..b68bc8b6fe 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -16,8 +16,10 @@ #include "client.hpp" #include "compression.hpp" #include "exception.hpp" +#include "metrics.hpp" #include "network/broker.hpp" #include "network/proto.hpp" +#include "service.hpp" #include "std_logging.hpp" using namespace std::chrono_literals; @@ -26,6 +28,11 @@ namespace dds { namespace { +void collect_metrics(network::request_shutdown::response &response, + service &service, std::optional &context); +void collect_metrics(network::client_init::response &response, service &service, + std::optional &context); + template // NOLINTNEXTLINE(google-runtime-references) bool maybe_exec_cmd_M(client &client, network::request &msg) @@ -155,18 +162,14 @@ bool client::handle_command(const network::client_init::request &command) auto &&eng_settings = command.engine_settings; DD_STDLOG(DD_STDLOG_STARTUP); - std::map meta; - std::map metrics; - std::vector errors; bool has_errors = false; client_enabled_conf = command.enabled_configuration; try { - service_ = - service_manager_->create_service(eng_settings, command.rc_settings, - meta, metrics, !client_enabled_conf.has_value()); + service_ = service_manager_->create_service(eng_settings, + command.rc_settings, !client_enabled_conf.has_value()); // save engine settings so we can recreate the service if rc path // changes @@ -190,8 +193,11 @@ bool client::handle_command(const network::client_init::request &command) auto response = std::make_shared(); response->status = has_errors ? "fail" : "ok"; response->errors = std::move(errors); - response->meta = std::move(meta); - response->metrics = std::move(metrics); + + if (service_) { + // may be null in testing + collect_metrics(*response, *service_, context_); + } try { if (!broker_->send(response)) { @@ -345,13 +351,7 @@ bool client::handle_command(network::config_sync::request &command) SPDLOG_DEBUG( "received command config_sync with path {}", command.rem_cfg_path); - std::map meta; - std::map metrics; - update_remote_config_path(command.rem_cfg_path, meta, metrics); - - // TODO: meta/metrics not transmitted fwd - // wait for new interface; see - // https://github.com/DataDog/dd-trace-php/pull/2735 + update_remote_config_path(command.rem_cfg_path); if (compute_client_status()) { auto response_cf = @@ -433,15 +433,12 @@ bool client::handle_command(network::request_shutdown::request &command) return false; } - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - context_->get_meta_and_metrics(response->meta, response->metrics); + collect_metrics(*response, *service_, context_); return send_message(response); } -void client::update_remote_config_path(std::string_view path, - std::map &meta, - std::map &metrics) +void client::update_remote_config_path(std::string_view path) { if (service_->is_remote_config_shmem_path(path) || !engine_settings_.has_value()) { @@ -453,8 +450,8 @@ void client::update_remote_config_path(std::string_view path, rc_settings.enabled = true; rc_settings.shmem_path = path; - service_ = service_manager_->create_service( - *engine_settings_, rc_settings, meta, metrics, true); + service_ = + service_manager_->create_service(*engine_settings_, rc_settings, true); } bool client::run_client_init() @@ -494,4 +491,78 @@ void client::run(worker::queue_consumer &q) SPDLOG_DEBUG("Finished handling client"); } +namespace { + +struct RequestMetricsSubmitter : public metrics::TelemetrySubmitter { + RequestMetricsSubmitter() = default; + ~RequestMetricsSubmitter() override = default; + RequestMetricsSubmitter(const RequestMetricsSubmitter &) = delete; + RequestMetricsSubmitter &operator=( + const RequestMetricsSubmitter &) = delete; + RequestMetricsSubmitter(RequestMetricsSubmitter &&) = delete; + RequestMetricsSubmitter &operator=(RequestMetricsSubmitter &&) = delete; + + void submit_metric( + std::string_view name, double value, std::string tags) override + { + SPDLOG_TRACE("submit_metric [req]: name={}, value={}, tags={}", name, + value, tags); + tel_metrics[name].emplace_back(value, tags); + }; + void submit_legacy_metric(std::string_view name, double value) override + { + SPDLOG_TRACE( + "submit_legacy_metric [req]: name={}, value={}", name, value); + metrics[name] = value; + }; + void submit_legacy_meta(std::string_view name, std::string value) override + { + SPDLOG_TRACE( + "submit_legacy_meta [req]: name={}, value={}", name, value); + meta[std::string{name}] = value; + }; + void submit_legacy_meta_copy_key( + std::string name, std::string value) override + { + SPDLOG_TRACE("submit_legacy_meta_copy_key [req]: name={}, value={}", + name, value); + meta[name] = value; + } + + std::map meta; + std::map metrics; + std::map>> + tel_metrics; +}; + +template +void collect_metrics_impl(Response &response, service &service, + std::optional &context) +{ + RequestMetricsSubmitter msubmitter{}; + if (context) { + context->get_metrics(msubmitter); + } + service.drain_metrics( + [&msubmitter](std::string_view name, double value, std::string tags) { + msubmitter.submit_metric(name, value, std::move(tags)); + }); + msubmitter.metrics.merge(service.drain_legacy_metrics()); + msubmitter.meta.merge(service.drain_legacy_meta()); + response.tel_metrics = std::move(msubmitter.tel_metrics); + response.meta = std::move(msubmitter.meta); + response.metrics = std::move(msubmitter.metrics); +} +void collect_metrics(network::request_shutdown::response &response, + service &service, std::optional &context) +{ + collect_metrics_impl(response, service, context); +} +void collect_metrics(network::client_init::response &response, service &service, + std::optional &context) +{ + collect_metrics_impl(response, service, context); +} +} // namespace + } // namespace dds diff --git a/appsec/src/helper/client.hpp b/appsec/src/helper/client.hpp index aa5f8a15b7..29046e49c4 100644 --- a/appsec/src/helper/client.hpp +++ b/appsec/src/helper/client.hpp @@ -22,13 +22,13 @@ class client { public: // Below this limit the encoding+compression might result on a longer string client(std::shared_ptr service_manager, - network::base_broker::ptr &&broker) + std::unique_ptr &&broker) : service_manager_(std::move(service_manager)), broker_(std::move(broker)) {} client(std::shared_ptr service_manager, - network::base_socket::ptr &&socket) + std::unique_ptr &&socket) : service_manager_(std::move(service_manager)), broker_(std::make_unique(std::move(socket))) {} @@ -61,9 +61,7 @@ class client { void run(worker::queue_consumer &q); bool compute_client_status(); - void update_remote_config_path(std::string_view path, - std::map &meta, - std::map &metrics); + void update_remote_config_path(std::string_view path); protected: template @@ -73,7 +71,7 @@ class client { bool send_message(const std::shared_ptr &message); bool initialised{false}; uint32_t version{}; - network::base_broker::ptr broker_; + std::unique_ptr broker_; std::shared_ptr service_manager_; std::optional engine_settings_; std::shared_ptr service_ = {nullptr}; diff --git a/appsec/src/helper/engine.cpp b/appsec/src/helper/engine.cpp index e9b6fcabfb..4d684416d8 100644 --- a/appsec/src/helper/engine.cpp +++ b/appsec/src/helper/engine.cpp @@ -4,6 +4,7 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include +#include #include #include @@ -11,6 +12,7 @@ #include "engine_settings.hpp" #include "exception.hpp" #include "json_helper.hpp" +#include "metrics.hpp" #include "parameter_view.hpp" #include "std_logging.hpp" #include "subscriber/waf.hpp" @@ -22,16 +24,17 @@ void engine::subscribe(std::unique_ptr sub) common_->subscribers.emplace_back(std::move(sub)); } -void engine::update(engine_ruleset &ruleset, - std::map &meta, - std::map &metrics) +void engine::update( + engine_ruleset &ruleset, metrics::TelemetrySubmitter &submit_metric) { std::vector> new_subscribers; - new_subscribers.reserve(common_->subscribers.size()); + auto old_common = + std::atomic_load_explicit(&common_, std::memory_order_acquire); + new_subscribers.reserve(old_common->subscribers.size()); dds::parameter param = json_to_parameter(ruleset.get_document()); - for (auto &sub : common_->subscribers) { + for (auto &sub : old_common->subscribers) { try { - new_subscribers.emplace_back(sub->update(param, meta, metrics)); + new_subscribers.emplace_back(sub->update(param, submit_metric)); } catch (const std::exception &e) { SPDLOG_WARN("Failed to update subscriber {}: {}", sub->get_name(), e.what()); @@ -65,7 +68,9 @@ std::optional engine::context::publish(parameter &¶m) event event_; - for (auto &sub : common_->subscribers) { + auto common = + std::atomic_load_explicit(&common_, std::memory_order_acquire); + for (auto &sub : common->subscribers) { auto it = listeners_.find(sub.get()); if (it == listeners_.end()) { auto listener = sub->get_listener(); @@ -109,19 +114,16 @@ std::optional engine::context::publish(parameter &¶m) return res; } -void engine::context::get_meta_and_metrics( - std::map &meta, - std::map &metrics) +void engine::context::get_metrics(metrics::TelemetrySubmitter &msubmitter) { for (const auto &[subscriber, listener] : listeners_) { - listener->get_meta_and_metrics(meta, metrics); + listener->submit_metrics(msubmitter); } } std::unique_ptr engine::from_settings( const dds::engine_settings &eng_settings, - std::map &meta, - std::map &metrics) + metrics::TelemetrySubmitter &msubmitter) { auto &&rules_path = eng_settings.rules_file_or_default(); auto ruleset = engine_ruleset::from_path(rules_path); @@ -132,7 +134,7 @@ std::unique_ptr engine::from_settings( SPDLOG_DEBUG("Will load WAF rules from {}", rules_path); // may throw std::exception auto waf = - waf::instance::from_settings(eng_settings, ruleset, meta, metrics); + waf::instance::from_settings(eng_settings, ruleset, msubmitter); engine_ptr->subscribe(std::move(waf)); } catch (...) { DD_STDLOG(DD_STDLOG_WAF_INIT_FAILED, rules_path); diff --git a/appsec/src/helper/engine.hpp b/appsec/src/helper/engine.hpp index a42797e8db..e4e904de2f 100644 --- a/appsec/src/helper/engine.hpp +++ b/appsec/src/helper/engine.hpp @@ -9,6 +9,7 @@ #include "config.hpp" #include "engine_ruleset.hpp" #include "engine_settings.hpp" +#include "metrics.hpp" #include "parameter.hpp" #include "rate_limit.hpp" #include "subscriber/base.hpp" @@ -67,8 +68,7 @@ class engine { std::optional publish(parameter &¶m); // NOLINTNEXTLINE(google-runtime-references) - void get_meta_and_metrics(std::map &meta, - std::map &metrics); + void get_metrics(metrics::TelemetrySubmitter &msubmitter); protected: std::shared_ptr common_; @@ -87,8 +87,7 @@ class engine { static std::unique_ptr from_settings( const dds::engine_settings &eng_settings, - std::map &meta, - std::map &metrics); + metrics::TelemetrySubmitter &msubmitter); static auto create( uint32_t trace_rate_limit = engine_settings::default_trace_rate_limit) @@ -96,22 +95,26 @@ class engine { return std::unique_ptr(new engine(trace_rate_limit)); } - context get_context() { return context{*this}; } - // Not thread-safe, should only be called after construction void subscribe(std::unique_ptr sub); - virtual void update(engine_ruleset &ruleset, - std::map &meta, - std::map &metrics); + context get_context() { return context{*this}; } + + // Should not be called concurrently but safely publishes changes to common_ + // the rc client has a lock that ensures this + virtual void update( + engine_ruleset &ruleset, metrics::TelemetrySubmitter &submit_metric); protected: explicit engine(uint32_t trace_rate_limit) : limiter_(trace_rate_limit), common_(new shared_state{{}}) {} - // in practice: the current ddwaf_handle, atomically swapped in update + // in practice: the current ddwaf_handle, swapped in update + // should use only atomic operations (pre-c++20 + // std::atomic) std::shared_ptr common_; + std::shared_ptr msubmitter_; rate_limiter limiter_; }; diff --git a/appsec/src/helper/engine_settings.hpp b/appsec/src/helper/engine_settings.hpp index b61fc52b41..2ded9713e9 100644 --- a/appsec/src/helper/engine_settings.hpp +++ b/appsec/src/helper/engine_settings.hpp @@ -39,16 +39,9 @@ struct engine_settings { std::string obfuscator_value_regex; schema_extraction_settings schema_extraction; - engine_settings() = default; - engine_settings(const engine_settings &) = default; - engine_settings(engine_settings &&) = default; - engine_settings &operator=(const engine_settings &) = default; - engine_settings &operator=(engine_settings &&) = default; - virtual ~engine_settings() = default; - static const std::string &default_rules_file(); - [[nodiscard]] virtual const std::string &rules_file_or_default() const + [[nodiscard]] const std::string &rules_file_or_default() const { if (rules_file.empty()) { return default_rules_file(); diff --git a/appsec/src/helper/metrics.cpp b/appsec/src/helper/metrics.cpp new file mode 100644 index 0000000000..5fbd863862 --- /dev/null +++ b/appsec/src/helper/metrics.cpp @@ -0,0 +1,11 @@ +// Unless explicitly stated otherwise all files in this repository are +// dual-licensed under the Apache-2.0 License or BSD-3-Clause License. +// +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc. + +#include "metrics.hpp" + +namespace dds::metrics { +TelemetrySubmitter::~TelemetrySubmitter() = default; +} // namespace dds::metrics diff --git a/appsec/src/helper/metrics.hpp b/appsec/src/helper/metrics.hpp new file mode 100644 index 0000000000..f90820de32 --- /dev/null +++ b/appsec/src/helper/metrics.hpp @@ -0,0 +1,59 @@ +// Unless explicitly stated otherwise all files in this repository are +// dual-licensed under the Apache-2.0 License or BSD-3-Clause License. +// +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc. + +#pragma once + +#include +#include +#include + +namespace dds::metrics { + +struct TelemetrySubmitter { + TelemetrySubmitter() = default; + TelemetrySubmitter(const TelemetrySubmitter &) = delete; + TelemetrySubmitter &operator=(const TelemetrySubmitter &) = delete; + TelemetrySubmitter(TelemetrySubmitter &&) = delete; + TelemetrySubmitter &operator=(TelemetrySubmitter &&) = delete; + + virtual ~TelemetrySubmitter() = 0; + virtual void submit_metric(std::string_view, double, std::string) = 0; + virtual void submit_legacy_metric(std::string_view, double) = 0; + virtual void submit_legacy_meta(std::string_view, std::string) = 0; + virtual void submit_legacy_meta_copy_key(std::string, std::string) = 0; +}; + +constexpr std::string_view waf_init = "waf.init"; +constexpr std::string_view waf_updates = "waf.updates"; +constexpr std::string_view waf_requests = "waf.requests"; +constexpr std::string_view waf_config_errors = "waf.config_errors"; + +// not implemented: +constexpr std::string_view waf_input_truncated = "waf.input_truncated"; +constexpr std::string_view waf_truncated_value_size = + "waf.truncated_value_size"; +constexpr std::string_view waf_duration_tel = "waf.duration"; +constexpr std::string_view waf_duration_ext = "waf.duration_ext"; + +constexpr std::string_view rc_first_pull = "remote_config.first_pull"; +constexpr std::string_view rc_last_success = "remote_config.last_success"; + +// not implemented (difficult to count requests on the helper) +constexpr std::string_view rc_requests_before_running = + "remote_config.requests_before_running"; + +// legacy +constexpr std::string_view event_rules_loaded = "_dd.appsec.event_rules.loaded"; +constexpr std::string_view event_rules_failed = + "_dd.appsec.event_rules.error_count"; +constexpr std::string_view event_rules_errors = "_dd.appsec.event_rules.errors"; +constexpr std::string_view event_rules_version = + "_dd.appsec.event_rules.version"; + +constexpr std::string_view waf_version = "_dd.appsec.waf.version"; +constexpr std::string_view waf_duration = "_dd.appsec.waf.duration"; + +} // namespace dds::metrics diff --git a/appsec/src/helper/network/acceptor.cpp b/appsec/src/helper/network/acceptor.cpp index 2c2a567d57..c5f2e57db4 100644 --- a/appsec/src/helper/network/acceptor.cpp +++ b/appsec/src/helper/network/acceptor.cpp @@ -5,6 +5,7 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "acceptor.hpp" #include "../exception.hpp" +#include "socket.hpp" #include #include #include @@ -75,7 +76,7 @@ void acceptor::set_accept_timeout(std::chrono::seconds timeout) } } -socket::ptr acceptor::accept() +std::unique_ptr acceptor::accept() { struct sockaddr_un addr {}; socklen_t len = sizeof(addr); diff --git a/appsec/src/helper/network/acceptor.hpp b/appsec/src/helper/network/acceptor.hpp index 7ed5e36fa3..e7f4cd44c7 100644 --- a/appsec/src/helper/network/acceptor.hpp +++ b/appsec/src/helper/network/acceptor.hpp @@ -13,8 +13,6 @@ namespace dds::network { class base_acceptor { public: - using ptr = std::unique_ptr; - base_acceptor() = default; base_acceptor(const base_acceptor &) = delete; base_acceptor &operator=(const base_acceptor &) = delete; @@ -23,7 +21,7 @@ class base_acceptor { virtual ~base_acceptor() = default; virtual void set_accept_timeout(std::chrono::seconds timeout) = 0; - [[nodiscard]] virtual base_socket::ptr accept() = 0; + [[nodiscard]] virtual std::unique_ptr accept() = 0; }; namespace local { @@ -55,7 +53,7 @@ class acceptor : public base_acceptor { } void set_accept_timeout(std::chrono::seconds timeout) override; - [[nodiscard]] base_socket::ptr accept() override; + [[nodiscard]] std::unique_ptr accept() override; private: int sock_{-1}; diff --git a/appsec/src/helper/network/broker.hpp b/appsec/src/helper/network/broker.hpp index 712836530b..cd04096a7f 100644 --- a/appsec/src/helper/network/broker.hpp +++ b/appsec/src/helper/network/broker.hpp @@ -13,8 +13,6 @@ namespace dds::network { class base_broker { public: - using ptr = std::unique_ptr; - base_broker() = default; base_broker(const base_broker &) = delete; base_broker &operator=(const base_broker &) = delete; @@ -43,7 +41,9 @@ class broker : public base_broker { // other limits static constexpr std::size_t max_msg_body_size = 65536; - explicit broker(base_socket::ptr &&socket) : socket_(std::move(socket)) {} + explicit broker(std::unique_ptr &&socket) + : socket_(std::move(socket)) + {} broker(const broker &) = delete; broker &operator=(const broker &) = delete; broker(broker &&) = default; @@ -59,7 +59,7 @@ class broker : public base_broker { const std::shared_ptr &message) const override; protected: - base_socket::ptr socket_; + std::unique_ptr socket_; }; } // namespace dds::network diff --git a/appsec/src/helper/network/proto.hpp b/appsec/src/helper/network/proto.hpp index 3c629b4363..b202623177 100644 --- a/appsec/src/helper/network/proto.hpp +++ b/appsec/src/helper/network/proto.hpp @@ -89,6 +89,9 @@ template struct base_response_generic : public base_response { } }; +using telemetry_metrics = + std::unordered_map>; + struct client_init { static constexpr const char *name = "client_init"; struct request : base_request { @@ -127,8 +130,10 @@ struct client_init { std::map meta; std::map metrics; + std::map>> + tel_metrics; - MSGPACK_DEFINE(status, version, errors, meta, metrics); + MSGPACK_DEFINE(status, version, errors, meta, metrics, tel_metrics); }; }; @@ -286,8 +291,11 @@ struct request_shutdown { std::map meta; std::map metrics; + std::map>> + tel_metrics; - MSGPACK_DEFINE(actions, triggers, force_keep, meta, metrics); + MSGPACK_DEFINE( + actions, triggers, force_keep, meta, metrics, tel_metrics); }; }; diff --git a/appsec/src/helper/network/socket.hpp b/appsec/src/helper/network/socket.hpp index a2fce7350d..e25d267357 100644 --- a/appsec/src/helper/network/socket.hpp +++ b/appsec/src/helper/network/socket.hpp @@ -15,8 +15,6 @@ namespace dds::network { class base_socket { public: - using ptr = std::unique_ptr; - base_socket() = default; base_socket(const base_socket &) = delete; base_socket &operator=(const base_socket &) = delete; diff --git a/appsec/src/helper/remote_config/client_handler.cpp b/appsec/src/helper/remote_config/client_handler.cpp index d61f999255..bec29b3bef 100644 --- a/appsec/src/helper/remote_config/client_handler.cpp +++ b/appsec/src/helper/remote_config/client_handler.cpp @@ -5,25 +5,33 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "client_handler.hpp" +#include "../metrics.hpp" #include "listeners/asm_features_listener.hpp" #include "listeners/engine_listener.hpp" #include "listeners/listener.hpp" +#include +#include +#include namespace dds::remote_config { static constexpr std::chrono::milliseconds default_max_interval = 5min; client_handler::client_handler(std::unique_ptr &&rc_client, - std::shared_ptr service_config) + std::shared_ptr service_config, + std::shared_ptr msubmitter) : rc_client_{std::move(rc_client)}, - service_config_{std::move(service_config)} + service_config_{std::move(service_config)}, + msubmitter_{std::move(msubmitter)} {} std::unique_ptr client_handler::from_settings( const dds::engine_settings &eng_settings, std::shared_ptr service_config, const remote_config::settings &rc_settings, - const std::shared_ptr &engine_ptr, bool dynamic_enablement) + const std::shared_ptr &engine_ptr, + std::shared_ptr msubmitter, + bool dynamic_enablement) { if (!rc_settings.enabled) { return {}; @@ -42,7 +50,7 @@ std::unique_ptr client_handler::from_settings( if (eng_settings.rules_file.empty()) { listeners.emplace_back(std::make_shared( - engine_ptr, eng_settings.rules_file_or_default())); + engine_ptr, msubmitter, eng_settings.rules_file_or_default())); } if (listeners.empty()) { @@ -57,13 +65,36 @@ std::unique_ptr client_handler::from_settings( remote_config::client::from_settings(rc_settings, std::move(listeners)); return std::make_unique( - std::move(rc_client), std::move(service_config)); + std::move(rc_client), std::move(service_config), std::move(msubmitter)); } void client_handler::poll() { + std::lock_guard lock{mutex_}; + try { - rc_client_->poll(); + if (last_success_ != empty_time) { + auto now = std::chrono::steady_clock::now(); + auto elapsed = + std::chrono::duration_cast( + now - last_success_); + msubmitter_->submit_metric(metrics::rc_last_success, + static_cast(elapsed.count()), {}); + } + + const bool result = rc_client_->poll(); + + auto now = std::chrono::steady_clock::now(); + last_success_ = now; + + if (result && creation_time_ != empty_time) { + auto elapsed = + std::chrono::duration_cast( + now - creation_time_); + msubmitter_->submit_metric(metrics::rc_first_pull, + static_cast(elapsed.count()), {}); + creation_time_ = empty_time; + } } catch (const std::exception &e) { SPDLOG_WARN("Error polling remote config: {}", e.what()); } diff --git a/appsec/src/helper/remote_config/client_handler.hpp b/appsec/src/helper/remote_config/client_handler.hpp index 2a88649b00..b85cfc6285 100644 --- a/appsec/src/helper/remote_config/client_handler.hpp +++ b/appsec/src/helper/remote_config/client_handler.hpp @@ -9,7 +9,9 @@ #include "../service_config.hpp" #include "client.hpp" #include "settings.hpp" +#include #include +#include namespace dds::remote_config { @@ -18,7 +20,8 @@ using namespace std::chrono_literals; class client_handler { public: client_handler(std::unique_ptr &&rc_client, - std::shared_ptr service_config); + std::shared_ptr service_config, + std::shared_ptr msubmitter); ~client_handler() = default; client_handler(const client_handler &) = delete; @@ -31,13 +34,29 @@ class client_handler { const dds::engine_settings &eng_settings, std::shared_ptr service_config, const remote_config::settings &rc_settings, - const std::shared_ptr &engine_ptr, bool dynamic_enablement); + const std::shared_ptr &engine_ptr, + std::shared_ptr msubmitter, + bool dynamic_enablement); void poll(); + bool has_applied_rc() + { + std::lock_guard lock{mutex_}; + return creation_time_ == empty_time; + } + protected: + static constexpr auto empty_time = std::chrono::steady_clock::time_point{}; + std::shared_ptr service_config_; std::unique_ptr rc_client_; + std::shared_ptr msubmitter_; + + std::mutex mutex_{}; + std::chrono::steady_clock::time_point creation_time_{ + std::chrono::steady_clock::now()}; // def value after first poll() done + std::chrono::steady_clock::time_point last_success_{}; }; } // namespace dds::remote_config diff --git a/appsec/src/helper/remote_config/listeners/engine_listener.cpp b/appsec/src/helper/remote_config/listeners/engine_listener.cpp index 29d92f1c6e..c6510f2665 100644 --- a/appsec/src/helper/remote_config/listeners/engine_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/engine_listener.cpp @@ -15,12 +15,14 @@ #include #include #include +#include namespace dds::remote_config { -engine_listener::engine_listener( - std::shared_ptr engine, const std::string &rules_file) - : engine_(std::move(engine)) +engine_listener::engine_listener(std::shared_ptr engine, + std::shared_ptr msubmitter, + const std::string &rules_file) + : engine_{std::move(engine)}, msubmitter_{std::move(msubmitter)} { aggregators_.emplace( known_products::ASM, std::make_unique()); @@ -82,12 +84,8 @@ void engine_listener::commit() } } - // TODO find a way to provide this information to the service - std::map meta; - std::map metrics; - engine_ruleset ruleset = dds::engine_ruleset(std::move(ruleset_)); - engine_->update(ruleset, meta, metrics); + engine_->update(ruleset, *msubmitter_); } } // namespace dds::remote_config diff --git a/appsec/src/helper/remote_config/listeners/engine_listener.hpp b/appsec/src/helper/remote_config/listeners/engine_listener.hpp index 6f961b6c2c..3ca0fcfa8b 100644 --- a/appsec/src/helper/remote_config/listeners/engine_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/engine_listener.hpp @@ -20,12 +20,13 @@ namespace dds::remote_config { //// ENGINE PROXY LISTENER class engine_listener : public listener_base { public: - explicit engine_listener( - std::shared_ptr engine, const std::string &rules_file = {}); + explicit engine_listener(std::shared_ptr engine, + std::shared_ptr msubmitter, + const std::string &rules_file = {}); engine_listener(const engine_listener &) = delete; engine_listener(engine_listener &&) = default; engine_listener &operator=(const engine_listener &) = delete; - engine_listener &operator=(engine_listener &&) = default; + engine_listener &operator=(engine_listener &&) = delete; ~engine_listener() override = default; @@ -46,6 +47,7 @@ class engine_listener : public listener_base { std::shared_ptr engine_; rapidjson::Document ruleset_; std::unordered_set to_commit_; + std::shared_ptr msubmitter_; }; } // namespace dds::remote_config diff --git a/appsec/src/helper/runner.cpp b/appsec/src/helper/runner.cpp index dee62304fd..2cee4e6969 100644 --- a/appsec/src/helper/runner.cpp +++ b/appsec/src/helper/runner.cpp @@ -32,7 +32,8 @@ void (*ddog_remote_config_path_free)(char *); namespace dds { namespace { -network::base_acceptor::ptr acceptor_from_config(const config::config &cfg) +std::unique_ptr acceptor_from_config( + const config::config &cfg) { std::string_view const sock_path{cfg.socket_file_path()}; if (sock_path.size() >= 4 && sock_path.substr(0, 3) == "fd:") { @@ -89,7 +90,8 @@ runner::runner(const config::config &cfg, std::atomic &interrupted) {} runner::runner(const config::config &cfg, - network::base_acceptor::ptr &&acceptor, std::atomic &interrupted) + std::unique_ptr &&acceptor, + std::atomic &interrupted) : cfg_(cfg), service_manager_{std::make_shared()}, acceptor_(std::move(acceptor)), interrupted_{interrupted} { @@ -157,7 +159,7 @@ void runner::run() while (!interrupted()) { unblock_sigusr1(); - network::base_socket::ptr socket = acceptor_->accept(); + std::unique_ptr socket = acceptor_->accept(); block_sigusr1(); if (!socket) { diff --git a/appsec/src/helper/runner.hpp b/appsec/src/helper/runner.hpp index 2c495b3910..3dce035197 100644 --- a/appsec/src/helper/runner.hpp +++ b/appsec/src/helper/runner.hpp @@ -20,7 +20,8 @@ namespace dds { class runner : public std::enable_shared_from_this { public: runner(const config::config &cfg, std::atomic &interrupted); - runner(const config::config &cfg, network::base_acceptor::ptr &&acceptor, + runner(const config::config &cfg, + std::unique_ptr &&acceptor, std::atomic &interrupted); runner(const runner &) = delete; runner &operator=(const runner &) = delete; @@ -47,7 +48,7 @@ class runner : public std::enable_shared_from_this { worker::pool worker_pool_; // Server variables - network::base_acceptor::ptr acceptor_; + std::unique_ptr acceptor_; std::atomic &interrupted_; // NOLINT }; diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index bedb5e1e2d..835b52e6dc 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -5,16 +5,18 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "service.hpp" +#include "metrics.hpp" namespace dds { service::service(std::shared_ptr engine, std::shared_ptr service_config, std::unique_ptr &&client_handler, - std::string rc_path, + std::shared_ptr msubmitter, std::string rc_path, const schema_extraction_settings &schema_extraction_settings) : engine_{std::move(engine)}, service_config_{std::move(service_config)}, - client_handler_{std::move(client_handler)}, rc_path_{std::move(rc_path)} + client_handler_{std::move(client_handler)}, + msubmitter_{std::move(msubmitter)}, rc_path_{std::move(rc_path)} { // The engine should always be valid if (!engine_) { @@ -36,21 +38,21 @@ service::service(std::shared_ptr engine, std::shared_ptr service::from_settings( const dds::engine_settings &eng_settings, - const remote_config::settings &rc_settings, - std::map &meta, - std::map &metrics, bool dynamic_enablement) + const remote_config::settings &rc_settings, bool dynamic_enablement) { + std::shared_ptr msubmitter = std::make_shared(); + const std::shared_ptr engine_ptr = - engine::from_settings(eng_settings, meta, metrics); + engine::from_settings(eng_settings, *msubmitter); auto service_config = std::make_shared(); - auto client_handler = - remote_config::client_handler::from_settings(eng_settings, - service_config, rc_settings, engine_ptr, dynamic_enablement); + auto client_handler = remote_config::client_handler::from_settings( + eng_settings, service_config, rc_settings, engine_ptr, msubmitter, + dynamic_enablement); - return std::make_shared(engine_ptr, std::move(service_config), - std::move(client_handler), rc_settings.shmem_path, - eng_settings.schema_extraction); + return create_shared(engine_ptr, std::move(service_config), + std::move(client_handler), std::move(msubmitter), + rc_settings.shmem_path, eng_settings.schema_extraction); } } // namespace dds diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index 233926367d..71b5d28c3a 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -7,12 +7,15 @@ #include "engine.hpp" #include "exception.hpp" +#include "metrics.hpp" #include "remote_config/client_handler.hpp" #include "sampler.hpp" #include "service_config.hpp" #include "std_logging.hpp" #include "utils.hpp" +#include #include +#include #include #include @@ -21,13 +24,109 @@ namespace dds { using namespace std::chrono_literals; class service { -public: +protected: + class MetricsImpl : public metrics::TelemetrySubmitter { + struct tel_metric { + tel_metric(std::string_view name, double value, std::string tags) + : name{name}, value{value}, tags{std::move(tags)} + {} + std::string_view name; + double value; + std::string tags; + }; + + public: + MetricsImpl() = default; + MetricsImpl(const MetricsImpl &) = delete; + MetricsImpl &operator=(const MetricsImpl &) = delete; + MetricsImpl(MetricsImpl &&) = delete; + MetricsImpl &operator=(MetricsImpl &&) = delete; + + ~MetricsImpl() override = default; + + void submit_metric(std::string_view metric_name, double value, + std::string tags) override + { + SPDLOG_TRACE("submit_metric: {} {} {}", metric_name, value, tags); + const std::lock_guard lock{pending_metrics_mutex_}; + pending_metrics_.emplace_back(metric_name, value, std::move(tags)); + } + + void submit_legacy_metric(std::string_view name, double value) override + { + SPDLOG_TRACE("submit_legacy_metric: {} {}", name, value); + const std::lock_guard lock{legacy_metrics_mutex_}; + legacy_metrics_[name] = value; + } + void submit_legacy_meta( + std::string_view name, std::string value) override + { + SPDLOG_TRACE("submit_legacy_meta: {} {}", name, value); + const std::lock_guard lock{meta_mutex_}; + meta_[std::string{name}] = std::move(value); + } + + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + void submit_legacy_meta_copy_key( + std::string name, std::string value) override + { + const std::lock_guard lock{meta_mutex_}; + meta_[std::move(name)] = std::move(value); + } + + template void drain_metrics(Func &&func) + { + std::vector metrics; + { + const std::lock_guard lock(pending_metrics_mutex_); + metrics.swap(pending_metrics_); + } + for (auto &metric : metrics) { + std::invoke(std::forward(func), metric.name, metric.value, + std::move(metric.tags)); + } + } + + std::map drain_legacy_metrics() + { + const std::lock_guard lock{legacy_metrics_mutex_}; + return std::move(legacy_metrics_); + } + + std::map drain_legacy_meta() + { + const std::lock_guard lock{meta_mutex_}; + return std::move(meta_); + } + + private: + std::vector pending_metrics_; + std::mutex pending_metrics_mutex_; + std::map legacy_metrics_; + std::mutex legacy_metrics_mutex_; + std::map meta_; + std::mutex meta_mutex_; + }; + + static std::shared_ptr create_shared_metrics() + { + return std::make_shared(); + } + service(std::shared_ptr engine, std::shared_ptr service_config, std::unique_ptr &&client_handler, - std::string rc_path, + std::shared_ptr msubmitter, std::string rc_path, const schema_extraction_settings &schema_extraction_settings = {}); + template + static std::shared_ptr create_shared(Args &&...args) + { + return std::shared_ptr( + new service(std::forward(args)...)); + } + +public: service(const service &) = delete; service &operator=(const service &) = delete; @@ -38,9 +137,7 @@ class service { static std::shared_ptr from_settings( const dds::engine_settings &eng_settings, - const remote_config::settings &rc_settings, - std::map &meta, - std::map &metrics, bool dynamic_enablement); + const remote_config::settings &rc_settings, bool dynamic_enablement); [[nodiscard]] std::shared_ptr get_engine() const { @@ -66,12 +163,38 @@ class service { void notify_of_rc_updates() { client_handler_->poll(); } + template void drain_metrics(Func &&func) + { + msubmitter_->drain_metrics(std::forward(func)); + } + + [[nodiscard]] std::map drain_legacy_metrics() + { + return msubmitter_->drain_legacy_metrics(); + } + + [[nodiscard]] std::map drain_legacy_meta() + { + return msubmitter_->drain_legacy_meta(); + } + + // to be called just before the submitting data to the engine for the first + // time in the request + void before_first_publish() const + { + if (client_handler_ && !client_handler_->has_applied_rc()) { + msubmitter_->submit_metric( + "remote_config.requests_before_running"sv, 1, ""); + } + } + protected: std::shared_ptr engine_{}; std::shared_ptr service_config_{}; std::unique_ptr client_handler_{}; std::shared_ptr schema_sampler_; std::string rc_path_; + std::shared_ptr msubmitter_; }; } // namespace dds diff --git a/appsec/src/helper/service_manager.cpp b/appsec/src/helper/service_manager.cpp index d4c058265d..f06925997f 100644 --- a/appsec/src/helper/service_manager.cpp +++ b/appsec/src/helper/service_manager.cpp @@ -9,8 +9,7 @@ namespace dds { std::shared_ptr service_manager::create_service( const engine_settings &settings, const remote_config::settings &rc_settings, - std::map &meta, - std::map &metrics, bool dynamic_enablement) + bool dynamic_enablement) { const cache_key key{settings, rc_settings}; @@ -29,8 +28,8 @@ std::shared_ptr service_manager::create_service( SPDLOG_DEBUG("Creating a service for settings={} rc_settings={}", settings, rc_settings); - auto service_ptr = service::from_settings( - settings, rc_settings, meta, metrics, dynamic_enablement); + auto service_ptr = + service::from_settings(settings, rc_settings, dynamic_enablement); cache_.emplace(key, std::move(service_ptr)); last_service_ = service_ptr; diff --git a/appsec/src/helper/service_manager.hpp b/appsec/src/helper/service_manager.hpp index d4cb7a2186..0e238b7ab2 100644 --- a/appsec/src/helper/service_manager.hpp +++ b/appsec/src/helper/service_manager.hpp @@ -31,9 +31,7 @@ class service_manager { virtual std::shared_ptr create_service( const engine_settings &settings, - const remote_config::settings &rc_settings, - std::map &meta, - std::map &metrics, bool dynamic_enablement); + const remote_config::settings &rc_settings, bool dynamic_enablement); void notify_of_rc_updates(std::string_view shmem_path); diff --git a/appsec/src/helper/subscriber/base.hpp b/appsec/src/helper/subscriber/base.hpp index addfc6752c..6c539cde52 100644 --- a/appsec/src/helper/subscriber/base.hpp +++ b/appsec/src/helper/subscriber/base.hpp @@ -7,6 +7,7 @@ #include "../action.hpp" #include "../engine_settings.hpp" +#include "../metrics.hpp" #include "../parameter.hpp" #include "../parameter_view.hpp" #include @@ -30,9 +31,8 @@ class subscriber { virtual void call(parameter_view &data, event &event) = 0; // NOLINTNEXTLINE(google-runtime-references) - virtual void get_meta_and_metrics( - std::map &meta, - std::map &metrics) = 0; + virtual void submit_metrics( + metrics::TelemetrySubmitter &msubmitter) = 0; }; subscriber() = default; @@ -46,9 +46,8 @@ class subscriber { virtual std::string_view get_name() = 0; virtual std::unordered_set get_subscriptions() = 0; virtual std::unique_ptr get_listener() = 0; - virtual std::unique_ptr update(parameter &rule, - std::map &meta, - std::map &metrics) = 0; + virtual std::unique_ptr update( + parameter &rule, metrics::TelemetrySubmitter &submit_metric) = 0; }; } // namespace dds diff --git a/appsec/src/helper/subscriber/waf.cpp b/appsec/src/helper/subscriber/waf.cpp index 4a5062c006..8dd56a0a15 100644 --- a/appsec/src/helper/subscriber/waf.cpp +++ b/appsec/src/helper/subscriber/waf.cpp @@ -17,8 +17,8 @@ #include "../compression.hpp" #include "../json_helper.hpp" +#include "../metrics.hpp" #include "../std_logging.hpp" -#include "../tags.hpp" #include "base.hpp" #include "waf.hpp" #include @@ -126,9 +126,111 @@ void log_cb(DDWAF_LOG_LEVEL level, const char *function, const char *file, std::string_view(message, message_len)); } -void extract_tags_and_metrics(parameter_view diagnostics, std::string &version, - std::map &meta, - std::map &metrics) +std::string waf_update_init_report_tags( + bool success, std::optional rules_version) +{ + std::string tags{"success"sv}; + if (success) { + tags += ":true"sv; + } else { + tags += ":false"sv; + } + if (rules_version) { + tags += ",event_rules_version:"sv; + tags += rules_version.value(); + } + tags += ",waf_version:"sv; + tags += ddwaf_get_version(); + return tags; +} + +void waf_init_report(metrics::TelemetrySubmitter &msubmitter, bool success, + std::optional rules_version) +{ + msubmitter.submit_metric(metrics::waf_init, 1.0, + waf_update_init_report_tags(success, std::move(rules_version))); +} + +void waf_update_report(metrics::TelemetrySubmitter &msubmitter, bool success, + std::optional rules_version) +{ + msubmitter.submit_metric(metrics::waf_updates, 1.0, + waf_update_init_report_tags(success, std::move(rules_version))); +} + +void load_result_report( + parameter_view diagnostics, metrics::TelemetrySubmitter &msubmitter) +{ + const auto info = static_cast(diagnostics); + + std::string_view ruleset_version{"unknown"}; + auto ruleset_version_it = info.find("ruleset_version"); + if (ruleset_version_it != info.end() && + ruleset_version_it->second.is_string()) { + ruleset_version = + static_cast(ruleset_version_it->second); + } + + static constexpr std::array keys{ + "custom_rules", + "exclusions", + "rules", + "rules_data", + "rules_override", + }; + + for (auto k : keys) { + auto it = info.find(k); + if (it == info.end()) { + continue; + } + + const parameter_view &value = it->second; + if (!value.is_map()) { + continue; + } + + // map has either "error" key or "loaded", "failed", "errors" keys + const parameter_view::map map = static_cast(value); + + auto tags_prefix = [&]() { + std::string tags{"waf_version:"}; + tags += ddwaf_get_version(); + tags += ",event_rules_version:"; + tags += ruleset_version; + tags += ",config_key:"; + tags += k; + return tags; + }; + if (map.contains("error")) { + std::string tags{tags_prefix()}; + tags += ",scope:top-level"; + + msubmitter.submit_metric( + metrics::waf_config_errors, 1.0, std::move(tags)); + } else if (map.contains("errors")) { + const auto errors_map = map.at("errors"); + if (errors_map.size() == 0) { + continue; + } + std::uint64_t error_count = 0; + for (const parameter_view &err : errors_map) { + // key is error message, value is array of ids + assert(err.type() == parameter_type::array); + error_count += err.size(); + } + + std::string tags{tags_prefix()}; + tags += ",scope:item"; + + msubmitter.submit_metric(metrics::waf_config_errors, + static_cast(error_count), std::move(tags)); + } + } +} + +void load_result_report_legacy(parameter_view diagnostics, std::string &version, + metrics::TelemetrySubmitter &msubmitter) { try { const parameter_view diagnostics_view{diagnostics}; @@ -139,24 +241,25 @@ void extract_tags_and_metrics(parameter_view diagnostics, std::string &version, auto rules = static_cast(rules_it->second); auto it = rules.find("loaded"); if (it != rules.end()) { - metrics[tag::event_rules_loaded] = - static_cast(it->second.size()); + msubmitter.submit_legacy_metric(metrics::event_rules_loaded, + static_cast(it->second.size())); } it = rules.find("failed"); if (it != rules.end()) { - metrics[tag::event_rules_failed] = - static_cast(it->second.size()); + msubmitter.submit_legacy_metric(metrics::event_rules_failed, + static_cast(it->second.size())); } it = rules.find("errors"); if (it != rules.end()) { - meta[std::string(tag::event_rules_errors)] = - parameter_to_json(it->second); + msubmitter.submit_legacy_meta( + metrics::event_rules_errors, parameter_to_json(it->second)); } } - meta[std::string(tag::waf_version)] = ddwaf_get_version(); + msubmitter.submit_legacy_meta( + metrics::waf_version, ddwaf_get_version()); auto version_it = info.find("ruleset_version"); if (version_it != info.end()) { @@ -175,9 +278,15 @@ void initialise_logging(spdlog::level::level_enum level) } instance::listener::listener(ddwaf_context ctx, - std::chrono::microseconds waf_timeout, std::string_view ruleset_version) - : handle_{ctx}, waf_timeout_{waf_timeout}, ruleset_version_(ruleset_version) -{} + std::chrono::microseconds waf_timeout, std::string ruleset_version) + : handle_{ctx}, waf_timeout_{waf_timeout}, + ruleset_version_{std::move(ruleset_version)} +{ + base_tags_ += "event_rules_version:"sv; + base_tags_ += ruleset_version_; + base_tags_ += ",waf_version:"sv; + base_tags_ += ddwaf_get_version(); +} instance::listener::listener(instance::listener &&other) noexcept : handle_{other.handle_}, waf_timeout_{other.waf_timeout_} @@ -222,7 +331,6 @@ void instance::listener::call(dds::parameter_view &data, event &event) SPDLOG_DEBUG("Waf response: code {} - actions {} - derivatives {}", code, parameter_to_json(parameter_view{res.actions}), parameter_to_json(parameter_view{res.derivatives})); - } else { run_waf(); } @@ -233,6 +341,18 @@ void instance::listener::call(dds::parameter_view &data, event &event) // NOLINTNEXTLINE total_runtime_ += res.total_runtime / 1000.0; + if (res.timeout) { + waf_hit_timeout_ = true; + } + const parameter_view actions{res.actions}; + for (const auto &action : actions) { + const std::string_view action_type = action.key(); + if (action_type == "block_request" || + action_type == "redirect_request") { + request_blocked_ = true; + break; + } + } const parameter_view schemas{res.derivatives}; for (const auto &schema : schemas) { @@ -241,15 +361,20 @@ void instance::listener::call(dds::parameter_view &data, event &event) switch (code) { case DDWAF_MATCH: + rule_triggered_ = true; return format_waf_result(res, event); case DDWAF_ERR_INTERNAL: + waf_run_error_ = true; throw internal_error(); case DDWAF_ERR_INVALID_OBJECT: + waf_run_error_ = true; throw invalid_object(); case DDWAF_ERR_INVALID_ARGUMENT: + waf_run_error_ = true; throw invalid_argument(); case DDWAF_OK: if (res.timeout) { + waf_hit_timeout_ = true; throw timeout_error(); } break; @@ -258,12 +383,27 @@ void instance::listener::call(dds::parameter_view &data, event &event) } } -void instance::listener::get_meta_and_metrics( - std::map &meta, - std::map &metrics) +void instance::listener::submit_metrics(metrics::TelemetrySubmitter &msubmitter) { - meta[std::string(tag::event_rules_version)] = ruleset_version_; - metrics[tag::waf_duration] = total_runtime_; + std::string tags = base_tags_; + if (rule_triggered_) { + tags += ",rule_triggered:true"sv; + } + if (request_blocked_) { + tags += ",request_blocked:true"sv; + } + if (waf_hit_timeout_) { + tags += ",waf_timeout:true"sv; + } + if (waf_run_error_) { + tags += ",waf_run_error:true"sv; + } + msubmitter.submit_metric(metrics::waf_requests, 1.0, std::move(tags)); + + // legacy + msubmitter.submit_legacy_meta( + metrics::event_rules_version, std::string{ruleset_version_}); + msubmitter.submit_legacy_metric(metrics::waf_duration, total_runtime_); for (const auto &[key, value] : schemas_) { std::string schema = value; @@ -275,15 +415,15 @@ void instance::listener::get_meta_and_metrics( } if (schema.length() <= max_schema_size) { - meta.emplace(key, std::move(schema)); + msubmitter.submit_legacy_meta_copy_key(key, std::move(schema)); } } } -instance::instance(parameter &rule, std::map &meta, - std::map &metrics, std::uint64_t waf_timeout_us, - std::string_view key_regex, std::string_view value_regex) - : waf_timeout_{waf_timeout_us} +instance::instance(parameter &rule, metrics::TelemetrySubmitter &msubmit, + std::uint64_t waf_timeout_us, std::string_view key_regex, + std::string_view value_regex) + : waf_timeout_{waf_timeout_us}, msubmitter_{msubmit} { const ddwaf_config config{ {0, 0, 0}, {key_regex.data(), value_regex.data()}, nullptr}; @@ -291,9 +431,12 @@ instance::instance(parameter &rule, std::map &meta, ddwaf_object diagnostics; handle_ = ddwaf_init(rule, &config, &diagnostics); - extract_tags_and_metrics( - parameter_view{diagnostics}, ruleset_version_, meta, metrics); - meta[std::string(tag::waf_version)] = ddwaf_get_version(); + load_result_report(parameter_view{diagnostics}, msubmit); + load_result_report_legacy( + parameter_view{diagnostics}, ruleset_version_, msubmit); + waf_init_report(msubmit, handle_ != nullptr, + ruleset_version_.empty() ? std::nullopt + : std::make_optional(ruleset_version_)); ddwaf_object_free(&diagnostics); @@ -308,8 +451,21 @@ instance::instance(parameter &rule, std::map &meta, for (uint32_t i = 0; i < size; i++) { addresses_.emplace(addrs[i]); } } +instance::instance(ddwaf_handle handle, metrics::TelemetrySubmitter &msubmitter, + std::chrono::microseconds timeout, std::string version) + : handle_{handle}, msubmitter_{msubmitter}, waf_timeout_{timeout}, + ruleset_version_{std::move(version)} +{ + uint32_t size; + const auto *addrs = ddwaf_known_addresses(handle_, &size); + + addresses_.clear(); + for (uint32_t i = 0; i < size; i++) { addresses_.emplace(addrs[i]); } +} + instance::instance(instance &&other) noexcept - : handle_(other.handle_), waf_timeout_(other.waf_timeout_), + : handle_(other.handle_), msubmitter_(other.msubmitter_), + waf_timeout_(other.waf_timeout_), ruleset_version_(std::move(other.ruleset_version_)), addresses_(std::move(other.addresses_)) { @@ -344,63 +500,51 @@ std::unique_ptr instance::get_listener() ddwaf_context_init(handle_), waf_timeout_, ruleset_version_); } -instance::instance( - ddwaf_handle handle, std::chrono::microseconds timeout, std::string version) - : handle_(handle), waf_timeout_(timeout), - ruleset_version_(std::move(version)) -{ - uint32_t size; - const auto *addrs = ddwaf_known_addresses(handle_, &size); - - addresses_.clear(); - for (uint32_t i = 0; i < size; i++) { addresses_.emplace(addrs[i]); } -} - -std::unique_ptr instance::update(parameter &rule, - std::map &meta, - std::map &metrics) +std::unique_ptr instance::update( + parameter &rule, metrics::TelemetrySubmitter &msubmitter) { ddwaf_object diagnostics; auto *new_handle = ddwaf_update(handle_, rule, &diagnostics); std::string version; - extract_tags_and_metrics( - parameter_view{diagnostics}, version, meta, metrics); - meta[std::string(tag::waf_version)] = ddwaf_get_version(); - if (version.empty()) { - version = ruleset_version_; - } + { + load_result_report(parameter_view{diagnostics}, msubmitter); + load_result_report_legacy( + parameter_view{diagnostics}, version, msubmitter); + if (version.empty()) { + version = ruleset_version_; + } + ddwaf_object_free(&diagnostics); - ddwaf_object_free(&diagnostics); + waf_update_report(msubmitter, new_handle != nullptr, version); + } if (new_handle == nullptr) { throw invalid_object(); } - return std::unique_ptr( - new instance(new_handle, waf_timeout_, std::move(version))); + return std::unique_ptr(new instance( + new_handle, msubmitter_, waf_timeout_, std::move(version))); } std::unique_ptr instance::from_settings( const engine_settings &settings, const engine_ruleset &ruleset, - std::map &meta, - std::map &metrics) + metrics::TelemetrySubmitter &msubmitter) { dds::parameter param = json_to_parameter(ruleset.get_document()); - return std::make_unique(param, meta, metrics, + return std::make_unique(param, msubmitter, settings.waf_timeout_us, settings.obfuscator_key_regex, settings.obfuscator_value_regex); } std::unique_ptr instance::from_string(std::string_view rule, - std::map &meta, - std::map &metrics, std::uint64_t waf_timeout_us, + metrics::TelemetrySubmitter &msubmitter, std::uint64_t waf_timeout_us, std::string_view key_regex, std::string_view value_regex) { engine_ruleset const ruleset{rule}; dds::parameter param = json_to_parameter(ruleset.get_document()); return std::make_unique( - param, meta, metrics, waf_timeout_us, key_regex, value_regex); + param, msubmitter, waf_timeout_us, key_regex, value_regex); } } // namespace dds::waf diff --git a/appsec/src/helper/subscriber/waf.hpp b/appsec/src/helper/subscriber/waf.hpp index 513855ddf6..86558a7a2b 100644 --- a/appsec/src/helper/subscriber/waf.hpp +++ b/appsec/src/helper/subscriber/waf.hpp @@ -8,6 +8,7 @@ #include "../engine.hpp" #include "../engine_ruleset.hpp" #include "../exception.hpp" +#include "../metrics.hpp" #include "../parameter.hpp" #include #include @@ -28,7 +29,7 @@ class instance : public dds::subscriber { class listener : public dds::subscriber::listener { public: listener(ddwaf_context ctx, std::chrono::microseconds waf_timeout, - std::string_view ruleset_version = std::string_view()); + std::string ruleset_version = {}); listener(const listener &) = delete; listener &operator=(const listener &) = delete; listener(listener &&) noexcept; @@ -38,20 +39,23 @@ class instance : public dds::subscriber { void call(dds::parameter_view &data, event &event) override; // NOLINTNEXTLINE(google-runtime-references) - void get_meta_and_metrics(std::map &meta, - std::map &metrics) override; + void submit_metrics(metrics::TelemetrySubmitter &msubmitter) override; protected: ddwaf_context handle_{}; std::chrono::microseconds waf_timeout_; double total_runtime_{0.0}; - std::string_view ruleset_version_; + std::string ruleset_version_; std::map schemas_; + std::string base_tags_; + bool rule_triggered_{}; + bool request_blocked_{}; + bool waf_hit_timeout_{}; + bool waf_run_error_{}; }; // NOLINTNEXTLINE(google-runtime-references) - instance(dds::parameter &rule, std::map &meta, - std::map &metrics, + instance(dds::parameter &rule, metrics::TelemetrySubmitter &msubmit, std::uint64_t waf_timeout_us, std::string_view key_regex = std::string_view(), std::string_view value_regex = std::string_view()); @@ -70,31 +74,29 @@ class instance : public dds::subscriber { std::unique_ptr get_listener() override; - std::unique_ptr update(parameter &rule, - std::map &meta, - std::map &metrics) override; + std::unique_ptr update( + parameter &rule, metrics::TelemetrySubmitter &msubmitter) override; static std::unique_ptr from_settings( const engine_settings &settings, const engine_ruleset &ruleset, - std::map &meta, - std::map &metrics); + metrics::TelemetrySubmitter &msubmitter); // testing only static std::unique_ptr from_string(std::string_view rule, - std::map &meta, - std::map &metrics, + metrics::TelemetrySubmitter &msubmitter, std::uint64_t waf_timeout_us = default_waf_timeout_us, std::string_view key_regex = std::string_view(), std::string_view value_regex = std::string_view()); protected: - instance(ddwaf_handle handle, std::chrono::microseconds timeout, - std::string version); + instance(ddwaf_handle handle, metrics::TelemetrySubmitter &msubmitter, + std::chrono::microseconds timeout, std::string version); ddwaf_handle handle_{nullptr}; std::chrono::microseconds waf_timeout_; std::string ruleset_version_; std::unordered_set addresses_; + metrics::TelemetrySubmitter &msubmitter_; // NOLINT }; parameter parse_file(std::string_view filename); diff --git a/appsec/tests/extension/rshutdown_telemetry.phpt b/appsec/tests/extension/rshutdown_telemetry.phpt new file mode 100644 index 0000000000..02f1e72006 --- /dev/null +++ b/appsec/tests/extension/rshutdown_telemetry.phpt @@ -0,0 +1,55 @@ +--TEST-- +request_shutdown relays telemetry metrics from the daemon +--INI-- +expose_php=0 +datadog.appsec.enabled=1 +datadog.appsec.log_file=/tmp/php_appsec_test.log +datadog.appsec.log_level=debug +--GET-- +a=b +--FILE-- + [[2.0, "foo=bar"], [1.0, "a=b"]]]])) +]); + +var_dump(rinit()); +$helper->get_commands(); // ignore + +var_dump(rshutdown()); +$c = $helper->get_commands(); +print_r($c[0]); + +?> +--EXPECTF-- +bool(true) +bool(true) +Array +( + [0] => request_shutdown + [1] => Array + ( + [0] => Array + ( + [server.response.status] => 200 + [server.response.headers.no_cookies] => Array + ( + [content-type] => Array + ( + [0] => %s + ) + + ) + + ) + + ) + +) diff --git a/appsec/tests/fuzzer/helpers.hpp b/appsec/tests/fuzzer/helpers.hpp index ba38b3648a..9d8b1026f2 100644 --- a/appsec/tests/fuzzer/helpers.hpp +++ b/appsec/tests/fuzzer/helpers.hpp @@ -1,10 +1,12 @@ // Unless explicitly stated otherwise all files in this repository are // dual-licensed under the Apache-2.0 License or BSD-3-Clause License. // -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2021 Datadog, Inc. +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #pragma once +#include +#include #include #include #include @@ -14,14 +16,17 @@ namespace dds::fuzzer { class reader { public: reader() = default; - reader(uint8_t *bytes_, size_t size_): - start(bytes_), cursor(bytes_), end(start + size_) {} + reader(uint8_t *bytes_, size_t size_) + : start(bytes_), cursor(bytes_), end(start + size_) + {} size_t remaining_bytes() { return end - cursor; } - - std::size_t read_bytes(uint8_t *buffer, size_t size) { - if (cursor >= end) { return 0; } + std::size_t read_bytes(uint8_t *buffer, size_t size) + { + if (cursor >= end) { + return 0; + } size_t available = std::min(static_cast(end - cursor), size); if (buffer != nullptr) { memcpy(buffer, cursor, available); @@ -30,15 +35,18 @@ class reader { return available; } - template - bool read(T &value) { - if (sizeof(T) > remaining_bytes()) { return false; } + template bool read(T &value) + { + if (sizeof(T) > remaining_bytes()) { + return false; + } memcpy(&value, cursor, sizeof(T)); cursor += sizeof(T); return true; } - const uint8_t* get_cursor() { return cursor; } + const uint8_t *get_cursor() { return cursor; } + protected: const uint8_t *start{nullptr}; const uint8_t *cursor{nullptr}; @@ -47,37 +55,41 @@ class reader { class writer { public: - writer(size_t size): - start(new uint8_t[size]()), cursor(start), end(start + size) {} + explicit writer(size_t size) + : start(new uint8_t[size]()), cursor(start), end(start + size) + {} ~writer() { delete[] start; } size_t remaining_bytes() { return end - cursor; } size_t written_bytes() { return cursor - start; } - void copy_to(uint8_t *buffer, size_t max_len) { - memcpy(buffer, start, std::min(static_cast(end-start), max_len)); + void copy_to(uint8_t *buffer, size_t max_len) + { + memcpy( + buffer, start, std::min(static_cast(end - start), max_len)); } - std::pair write(const uint8_t *buffer, size_t len) { + std::pair write(const uint8_t *buffer, size_t len) + { if (cursor + len > end) { len = end - cursor; return std::make_pair(end, 0); } memcpy(cursor, buffer, len); - auto current = cursor; + auto *current = cursor; cursor += len; return std::make_pair(current, len); } - template - std::pair write(T *value) { + template std::pair write(T *value) + { return write(reinterpret_cast(value), sizeof(T)); } - uint8_t* get_cursor() { return cursor; } - uint8_t* get_start() { return start; } + uint8_t *get_cursor() { return cursor; } + uint8_t *get_start() { return start; } protected: uint8_t *start{nullptr}; @@ -85,4 +97,4 @@ class writer { uint8_t *end{nullptr}; }; -} +} // namespace dds::fuzzer diff --git a/appsec/tests/fuzzer/main.cpp b/appsec/tests/fuzzer/main.cpp index f6add1dfea..7e054ec5b0 100644 --- a/appsec/tests/fuzzer/main.cpp +++ b/appsec/tests/fuzzer/main.cpp @@ -1,11 +1,12 @@ // Unless explicitly stated otherwise all files in this repository are // dual-licensed under the Apache-2.0 License or BSD-3-Clause License. // -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2021 Datadog, Inc. +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "mutators.hpp" #include "network.hpp" +#include "network/acceptor.hpp" #include #include #include @@ -172,7 +173,7 @@ int main(int argc, char **argv) auto acceptor_ptr = std::make_unique(); acceptor = acceptor_ptr.get(); - std::atomic interrupted; + std::atomic interrupted{}; dds::runner runner{config, std::move(acceptor_ptr), interrupted}; std::thread runner_thread([&runner] { runner.run(); }); diff --git a/appsec/tests/fuzzer/network.hpp b/appsec/tests/fuzzer/network.hpp index 09b6267ab3..2881100df1 100644 --- a/appsec/tests/fuzzer/network.hpp +++ b/appsec/tests/fuzzer/network.hpp @@ -1,25 +1,26 @@ // Unless explicitly stated otherwise all files in this repository are // dual-licensed under the Apache-2.0 License or BSD-3-Clause License. // -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2021 Datadog, Inc. +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #pragma once +#include "helpers.hpp" #include #include #include #include #include #include -#include #include -#include "helpers.hpp" +#include namespace dds::fuzzer { class raw_socket : public network::base_socket { public: - raw_socket(const uint8_t *bytes, size_t size) : start(new uint8_t[size]) { + raw_socket(const uint8_t *bytes, size_t size) : start(new uint8_t[size]) + { memcpy(start, bytes, size); r = reader(start, size); } @@ -28,7 +29,7 @@ class raw_socket : public network::base_socket { std::size_t recv(char *buffer, std::size_t size) override { - return r.read_bytes(reinterpret_cast(buffer), size); + return r.read_bytes(reinterpret_cast(buffer), size); } std::size_t send(const char * /*buffer*/, std::size_t len) override @@ -43,6 +44,7 @@ class raw_socket : public network::base_socket { void set_send_timeout(std::chrono::milliseconds timeout) override {} void set_recv_timeout(std::chrono::milliseconds timeout) override {} + protected: uint8_t *start{nullptr}; reader r; @@ -52,38 +54,47 @@ class acceptor : public network::base_acceptor { public: ~acceptor() override = default; - void push_socket(network::base_socket::ptr &&new_socket) + void push_socket(std::unique_ptr &&new_socket) { std::unique_lock lk(mtx); socket = std::move(new_socket); lk.unlock(); - cv.notify_one(); + cv.notify_one(); } void set_accept_timeout(std::chrono::seconds timeout) override {} - [[nodiscard]] network::base_socket::ptr accept() override + [[nodiscard]] std::unique_ptr accept() override { while (true) { std::unique_lock lk(mtx); cv.wait(lk); - if (exit_flag) { break; } - if (!socket) { continue; } + if (exit_flag) { + break; + } + if (!socket) { + continue; + } return std::move(socket); } return {}; } - void exit() { exit_flag = true; cv.notify_all(); } + void exit() + { + exit_flag = true; + cv.notify_all(); + } + protected: std::atomic exit_flag{false}; // A queue would result in oom unless we somehow rate-limit - network::base_socket::ptr socket; + std::unique_ptr socket; std::mutex mtx; std::condition_variable cv; }; -} +} // namespace dds::fuzzer diff --git a/appsec/tests/helper/broker_test.cpp b/appsec/tests/helper/broker_test.cpp index a1085af5cb..528753cba5 100644 --- a/appsec/tests/helper/broker_test.cpp +++ b/appsec/tests/helper/broker_test.cpp @@ -80,7 +80,7 @@ TEST(BrokerTest, SendClientInit) packer.pack_array(1); // Array of messages packer.pack_array(2); // First message pack_str(packer, "client_init"); // Type - packer.pack_array(5); + packer.pack_array(6); pack_str(packer, "ok"); pack_str(packer, dds::php_ddappsec_version); packer.pack_array(2); @@ -88,6 +88,7 @@ TEST(BrokerTest, SendClientInit) pack_str(packer, "two"); packer.pack_map(0); packer.pack_map(0); + packer.pack_map(0); const auto &expected_data = ss.str(); network::header_t h; @@ -168,7 +169,7 @@ TEST(BrokerTest, SendRequestShutdown) packer.pack_array(1); // Array of messages packer.pack_array(2); // First message pack_str(packer, "request_shutdown"); // Type - packer.pack_array(5); + packer.pack_array(6); packer.pack_array(1); packer.pack_array(2); pack_str(packer, "block"); @@ -183,6 +184,7 @@ TEST(BrokerTest, SendRequestShutdown) packer.pack_true(); // Force keep packer.pack_map(0); packer.pack_map(0); + packer.pack_map(0); const auto &expected_data = ss.str(); network::header_t h; diff --git a/appsec/tests/helper/client_test.cpp b/appsec/tests/helper/client_test.cpp index 50e8c2c117..a5bcdcea23 100644 --- a/appsec/tests/helper/client_test.cpp +++ b/appsec/tests/helper/client_test.cpp @@ -4,14 +4,16 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "common.hpp" +#include "ddwaf.h" #include #include #include #include +#include +#include #include #include #include -#include namespace dds { @@ -32,8 +34,6 @@ class service_manager : public dds::service_manager { MOCK_METHOD(std::shared_ptr, create_service, (const dds::engine_settings &settings, const dds::remote_config::settings &rc_settings, - (std::map & meta), - (std::map & metrics), bool dynamic_enablement), (override)); }; @@ -42,7 +42,8 @@ class service : public dds::service { public: service(std::shared_ptr engine, std::shared_ptr service_config) - : dds::service(engine, service_config, {}, "/rc_path") + : dds::service{engine, service_config, {}, + dds::service::create_shared_metrics(), "/rc_path"} {} }; @@ -154,16 +155,16 @@ TEST(ClientTest, ClientInit) EXPECT_STREQ(msg_res->status.c_str(), "ok"); EXPECT_EQ(msg_res->meta.size(), 2); + EXPECT_STREQ(msg_res->meta[std::string(metrics::waf_version)].c_str(), + ddwaf_get_version()); EXPECT_STREQ( - msg_res->meta[std::string(tag::waf_version)].c_str(), "1.18.0"); - EXPECT_STREQ( - msg_res->meta[std::string(tag::event_rules_errors)].c_str(), "{}"); + msg_res->meta[std::string(metrics::event_rules_errors)].c_str(), "{}"); EXPECT_EQ(msg_res->metrics.size(), 2); // For small enough integers this comparison should work, otherwise replace // with EXPECT_NEAR. - EXPECT_EQ(msg_res->metrics[tag::event_rules_loaded], 4.0); - EXPECT_EQ(msg_res->metrics[tag::event_rules_failed], 0.0); + EXPECT_EQ(msg_res->metrics[metrics::event_rules_loaded], 4.0); + EXPECT_EQ(msg_res->metrics[metrics::event_rules_failed], 0.0); } TEST(ClientTest, ClientInitRegisterRuntimeId) @@ -193,7 +194,7 @@ TEST(ClientTest, ClientInitRegisterRuntimeId) send(testing::An &>())) .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); - EXPECT_CALL(*smanager, create_service(_, _, _, _, true)) + EXPECT_CALL(*smanager, create_service(_, _, true)) .Times(1) .WillOnce(Return(service)); @@ -227,7 +228,7 @@ TEST(ClientTest, ClientInitGeneratesRuntimeId) send(testing::An &>())) .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); - EXPECT_CALL(*smanager, create_service(_, _, _, _, true)) + EXPECT_CALL(*smanager, create_service(_, _, true)) .Times(1) .WillOnce(Return(service)); @@ -263,11 +264,11 @@ TEST(ClientTest, ClientInitInvalidRules) EXPECT_STREQ(msg_res->status.c_str(), "ok"); EXPECT_EQ(msg_res->meta.size(), 2); - EXPECT_STREQ( - msg_res->meta[std::string(tag::waf_version)].c_str(), "1.18.0"); + EXPECT_STREQ(msg_res->meta[std::string(metrics::waf_version)].c_str(), + ddwaf_get_version()); rapidjson::Document doc; - doc.Parse(msg_res->meta[std::string(tag::event_rules_errors)]); + doc.Parse(msg_res->meta[std::string(metrics::event_rules_errors)]); EXPECT_FALSE(doc.HasParseError()); EXPECT_TRUE(doc.IsObject()); EXPECT_TRUE(doc.HasMember("missing key 'type'")); @@ -277,8 +278,8 @@ TEST(ClientTest, ClientInitInvalidRules) EXPECT_EQ(msg_res->metrics.size(), 2); // For small enough integers this comparison should work, otherwise replace // with EXPECT_NEAR. - EXPECT_EQ(msg_res->metrics[tag::event_rules_loaded], 1.0); - EXPECT_EQ(msg_res->metrics[tag::event_rules_failed], 4.0); + EXPECT_EQ(msg_res->metrics[metrics::event_rules_loaded], 1.0); + EXPECT_EQ(msg_res->metrics[metrics::event_rules_failed], 4.0); } TEST(ClientTest, ClientInitResponseFail) @@ -803,10 +804,10 @@ TEST(ClientTest, RequestShutdown) EXPECT_EQ(msg_res->triggers.size(), 1); EXPECT_EQ(msg_res->metrics.size(), 1); - EXPECT_GT(msg_res->metrics[tag::waf_duration], 0.0); + EXPECT_GT(msg_res->metrics[metrics::waf_duration], 0.0); EXPECT_EQ(msg_res->meta.size(), 1); EXPECT_STREQ( - msg_res->meta[std::string(tag::event_rules_version)].c_str(), + msg_res->meta[std::string(metrics::event_rules_version)].c_str(), "1.2.3"); } } @@ -847,10 +848,10 @@ TEST(ClientTest, RequestShutdownBlock) EXPECT_EQ(msg_res->triggers.size(), 1); EXPECT_EQ(msg_res->metrics.size(), 1); - EXPECT_GT(msg_res->metrics[tag::waf_duration], 0.0); + EXPECT_GT(msg_res->metrics[metrics::waf_duration], 0.0); EXPECT_EQ(msg_res->meta.size(), 1); EXPECT_STREQ( - msg_res->meta[std::string(tag::event_rules_version)].c_str(), + msg_res->meta[std::string(metrics::event_rules_version)].c_str(), "1.2.3"); } } @@ -1830,7 +1831,7 @@ TEST(ClientTest, ServiceIsCreatedDependingOnEnabledConfigurationValue) testing::An &>())) .WillRepeatedly(Return(true)); - EXPECT_CALL(*smanager, create_service(_, _, _, _, true)) + EXPECT_CALL(*smanager, create_service(_, _, true)) .Times(1) .WillOnce(Return(service)); client c(smanager, std::unique_ptr(broker)); @@ -1846,7 +1847,7 @@ TEST(ClientTest, ServiceIsCreatedDependingOnEnabledConfigurationValue) send( testing::An &>())) .WillRepeatedly(Return(true)); - EXPECT_CALL(*smanager, create_service(_, _, _, _, false)) + EXPECT_CALL(*smanager, create_service(_, _, false)) .Times(1) .WillOnce(Return(service)); client c(smanager, std::unique_ptr(broker)); @@ -1862,7 +1863,7 @@ TEST(ClientTest, ServiceIsCreatedDependingOnEnabledConfigurationValue) send( testing::An &>())) .WillRepeatedly(Return(true)); - EXPECT_CALL(*smanager, create_service(_, _, _, _, false)) + EXPECT_CALL(*smanager, create_service(_, _, false)) .Times(1) .WillOnce(Return(service)); client c(smanager, std::unique_ptr(broker)); diff --git a/appsec/tests/helper/common.hpp b/appsec/tests/helper/common.hpp index 0a27bc7269..08bf486ea5 100644 --- a/appsec/tests/helper/common.hpp +++ b/appsec/tests/helper/common.hpp @@ -21,10 +21,13 @@ using ::testing::ByRef; using ::testing::DoAll; using ::testing::ElementsAre; using ::testing::Invoke; +using ::testing::Mock; +using ::testing::NiceMock; using ::testing::Return; using ::testing::SaveArg; using ::testing::SetArgPointee; using ::testing::SetArgReferee; +using ::testing::StrictMock; using ::testing::Throw; using ::testing::WithArg; diff --git a/appsec/tests/helper/engine_test.cpp b/appsec/tests/helper/engine_test.cpp index 1778e17337..5b308e1cb2 100644 --- a/appsec/tests/helper/engine_test.cpp +++ b/appsec/tests/helper/engine_test.cpp @@ -3,9 +3,13 @@ // // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. + #include "common.hpp" +#include "ddwaf.h" #include "json_helper.hpp" +#include "metrics.hpp" #include +#include #include #include #include @@ -21,9 +25,7 @@ namespace mock { class listener : public dds::subscriber::listener { public: MOCK_METHOD2(call, void(dds::parameter_view &, dds::event &)); - MOCK_METHOD2( - get_meta_and_metrics, void(std::map &, - std::map &)); + MOCK_METHOD1(submit_metrics, void(metrics::TelemetrySubmitter &)); }; class subscriber : public dds::subscriber { @@ -31,9 +33,20 @@ class subscriber : public dds::subscriber { MOCK_METHOD0(get_name, std::string_view()); MOCK_METHOD0(get_listener, std::unique_ptr()); MOCK_METHOD0(get_subscriptions, std::unordered_set()); - MOCK_METHOD3(update, std::unique_ptr(dds::parameter &, - std::map &meta, - std::map &metrics)); + MOCK_METHOD2(update, std::unique_ptr( + dds::parameter &, metrics::TelemetrySubmitter &)); +}; + +class tel_submitter : public metrics::TelemetrySubmitter { +public: + MOCK_METHOD(void, submit_metric, (std::string_view, double, std::string), + (override)); + MOCK_METHOD( + void, submit_legacy_metric, (std::string_view, double), (override)); + MOCK_METHOD( + void, submit_legacy_meta, (std::string_view, std::string), (override)); + MOCK_METHOD(void, submit_legacy_meta_copy_key, (std::string, std::string), + (override)); }; } // namespace mock @@ -340,15 +353,22 @@ TEST(EngineTest, InvalidActionsAreDiscarded) TEST(EngineTest, WafSubscriptorBasic) { - std::map meta; - std::map metrics; - auto e{engine::create()}; - - auto waf_uniq_ptr = waf::instance::from_string(waf_rule, meta, metrics); - auto *waf_ptr = waf_uniq_ptr.get(); - - e->subscribe(std::move(waf_uniq_ptr)); + auto msubmitter = mock::tel_submitter{}; + + EXPECT_CALL(msubmitter, + submit_legacy_metric("_dd.appsec.event_rules.loaded"sv, 1.0)); + EXPECT_CALL(msubmitter, + submit_legacy_metric("_dd.appsec.event_rules.error_count"sv, 0.0)); + EXPECT_CALL( + msubmitter, submit_legacy_meta( + "_dd.appsec.event_rules.errors"sv, std::string{"{}"})); + EXPECT_CALL(msubmitter, submit_legacy_meta("_dd.appsec.waf.version"sv, _)); + EXPECT_CALL(msubmitter, submit_metric("waf.init"sv, 1, _)); + + auto waf_uptr = waf::instance::from_string(waf_rule, msubmitter); + auto *waf_ptr = waf_uptr.get(); + e->subscribe(static_cast(waf_uptr)); EXPECT_STREQ(waf_ptr->get_name().data(), "waf"); @@ -359,6 +379,7 @@ TEST(EngineTest, WafSubscriptorBasic) p.add("arg2", parameter::string("string 3"sv)); auto res = ctx.publish(std::move(p)); + Mock::VerifyAndClearExpectations(&msubmitter); EXPECT_TRUE(res); EXPECT_EQ(res->actions[0].type, dds::action_type::record); EXPECT_EQ(res->events.size(), 1); @@ -372,11 +393,10 @@ TEST(EngineTest, WafSubscriptorBasic) TEST(EngineTest, WafSubscriptorInvalidParam) { - std::map meta; - std::map metrics; + auto mock_msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule, mock_msubmitter)); auto ctx = e->get_context(); @@ -387,11 +407,10 @@ TEST(EngineTest, WafSubscriptorInvalidParam) TEST(EngineTest, WafSubscriptorTimeout) { - std::map meta; - std::map metrics; + auto mock_msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule, meta, metrics, 0)); + e->subscribe(waf::instance::from_string(waf_rule, mock_msubmitter, 0)); auto ctx = e->get_context(); @@ -405,6 +424,7 @@ TEST(EngineTest, WafSubscriptorTimeout) TEST(EngineTest, MockSubscriptorsUpdateRuleData) { + auto mock_submitter = mock::tel_submitter{}; auto e{engine::create()}; auto ignorer = []() { @@ -419,7 +439,7 @@ TEST(EngineTest, MockSubscriptorsUpdateRuleData) })); auto sub1 = std::make_unique(); - EXPECT_CALL(*sub1, update(_, _, _)).WillOnce(Invoke([&]() { + EXPECT_CALL(*sub1, update(_, _)).WillOnce(Invoke([&]() { return std::move(new_sub1); })); EXPECT_CALL(*sub1, get_name()).WillRepeatedly(Return("")); @@ -430,7 +450,7 @@ TEST(EngineTest, MockSubscriptorsUpdateRuleData) })); auto sub2 = std::make_unique(); - EXPECT_CALL(*sub2, update(_, _, _)).WillOnce(Invoke([&]() { + EXPECT_CALL(*sub2, update(_, _)).WillOnce(Invoke([&]() { return std::move(new_sub2); })); EXPECT_CALL(*sub2, get_name()).WillRepeatedly(Return("")); @@ -438,12 +458,9 @@ TEST(EngineTest, MockSubscriptorsUpdateRuleData) e->subscribe(std::move(sub1)); e->subscribe(std::move(sub2)); - std::map meta; - std::map metrics; - engine_ruleset ruleset( R"({"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"); - e->update(ruleset, meta, metrics); + e->update(ruleset, mock_submitter); // Ensure after the update we still have the same number of subscribers auto ctx = e->get_context(); @@ -457,6 +474,7 @@ TEST(EngineTest, MockSubscriptorsUpdateRuleData) TEST(EngineTest, MockSubscriptorsInvalidRuleData) { + auto msubmitter = mock::tel_submitter{}; auto e{engine::create()}; auto ignorer = []() { @@ -466,14 +484,14 @@ TEST(EngineTest, MockSubscriptorsInvalidRuleData) }; auto sub1 = std::make_unique(); - EXPECT_CALL(*sub1, update(_, _, _)).WillRepeatedly(Throw(std::exception())); + EXPECT_CALL(*sub1, update(_, _)).WillRepeatedly(Throw(std::exception())); EXPECT_CALL(*sub1, get_name()).WillRepeatedly(Return("")); EXPECT_CALL(*sub1, get_listener()).WillOnce(Invoke([&]() { return ignorer(); })); auto sub2 = std::make_unique(); - EXPECT_CALL(*sub2, update(_, _, _)).WillRepeatedly(Throw(std::exception())); + EXPECT_CALL(*sub2, update(_, _)).WillRepeatedly(Throw(std::exception())); EXPECT_CALL(*sub2, get_name()).WillRepeatedly(Return("")); EXPECT_CALL(*sub2, get_listener()).WillOnce(Invoke([&]() { return ignorer(); @@ -487,7 +505,7 @@ TEST(EngineTest, MockSubscriptorsInvalidRuleData) engine_ruleset ruleset(R"({})"); // All subscribers should be called regardless of failures - e->update(ruleset, meta, metrics); + e->update(ruleset, msubmitter); // Ensure after the update we still have the same number of subscribers auto ctx = e->get_context(); @@ -501,11 +519,10 @@ TEST(EngineTest, MockSubscriptorsInvalidRuleData) TEST(EngineTest, WafSubscriptorUpdateRuleData) { - std::map meta; - std::map metrics; + auto msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule_with_data, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule_with_data, msubmitter)); { auto ctx = e->get_context(); @@ -518,10 +535,18 @@ TEST(EngineTest, WafSubscriptorUpdateRuleData) } { + EXPECT_CALL( + msubmitter, submit_legacy_meta("_dd.appsec.waf.version"sv, _)); + EXPECT_CALL(msubmitter, + submit_metric("waf.updates"sv, 1, + std::string{"success:true,event_rules_version:,waf_version:"} + + ddwaf_get_version())); engine_ruleset rule_data( R"({"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"); - e->update(rule_data, meta, metrics); + e->update(rule_data, msubmitter); + + Mock::VerifyAndClear(&msubmitter); } { @@ -537,9 +562,18 @@ TEST(EngineTest, WafSubscriptorUpdateRuleData) } { + EXPECT_CALL( + msubmitter, submit_legacy_meta("_dd.appsec.waf.version"sv, _)); + EXPECT_CALL(msubmitter, + submit_metric("waf.updates"sv, 1, + std::string{"success:true,event_rules_version:,waf_version:"} + + ddwaf_get_version())); + engine_ruleset rule_data( R"({"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.2","expiration":"9999999999"}]}]})"); - e->update(rule_data, meta, metrics); + e->update(rule_data, msubmitter); + + Mock::VerifyAndClearExpectations(&msubmitter); } { @@ -555,11 +589,10 @@ TEST(EngineTest, WafSubscriptorUpdateRuleData) TEST(EngineTest, WafSubscriptorInvalidRuleData) { - std::map meta; - std::map metrics; + auto submitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule_with_data, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule_with_data, submitter)); { auto ctx = e->get_context(); @@ -572,9 +605,16 @@ TEST(EngineTest, WafSubscriptorInvalidRuleData) } { + EXPECT_CALL(submitter, + submit_metric("waf.updates"sv, 1, + std::string{"success:false,event_rules_version:,waf_version:"} + + ddwaf_get_version())); + engine_ruleset rule_data( R"({"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]})"); - e->update(rule_data, meta, metrics); + e->update(rule_data, submitter); + + Mock::VerifyAndClearExpectations(&submitter); } { @@ -590,11 +630,10 @@ TEST(EngineTest, WafSubscriptorInvalidRuleData) TEST(EngineTest, WafSubscriptorUpdateRules) { - std::map meta; - std::map metrics; + auto submitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule_with_data, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule_with_data, submitter)); { auto ctx = e->get_context(); @@ -609,7 +648,7 @@ TEST(EngineTest, WafSubscriptorUpdateRules) { engine_ruleset update( R"({"version": "2.2", "rules": [{"id": "some id", "name": "some name", "tags": {"type": "lfi", "category": "attack_attempt"}, "conditions": [{"parameters": {"inputs": [{"address": "server.request.query"} ], "list": ["/some-url"] }, "operator": "phrase_match"} ], "on_match": ["block"] } ] })"); - e->update(update, meta, metrics); + e->update(update, submitter); } { @@ -627,11 +666,10 @@ TEST(EngineTest, WafSubscriptorUpdateRules) TEST(EngineTest, WafSubscriptorUpdateRuleOverride) { - std::map meta; - std::map metrics; + auto msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule, msubmitter)); { auto ctx = e->get_context(); @@ -648,7 +686,7 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverride) engine_ruleset update( R"({"rules_override": [{"rules_target":[{"rule_id":"1"}], "enabled": "false"}]})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -664,7 +702,7 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverride) { engine_ruleset update(R"({"rules_override": []})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -681,11 +719,10 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverride) TEST(EngineTest, WafSubscriptorUpdateRuleOverrideAndActions) { - std::map meta; - std::map metrics; + auto msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule, msubmitter)); { auto ctx = e->get_context(); @@ -705,7 +742,7 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverrideAndActions) "on_match": ["redirect"]}], "actions": [{"id": "redirect", "type": "redirect_request", "parameters": {"status_code": "303", "location": "localhost"}}]})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -724,7 +761,7 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverrideAndActions) engine_ruleset update( R"({"rules_override": [{"rules_target":[{"rule_id":"1"}], "on_match": ["redirect"]}], "actions": []})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -742,11 +779,10 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverrideAndActions) TEST(EngineTest, WafSubscriptorExclusions) { - std::map meta; - std::map metrics; + auto msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule, msubmitter)); { auto ctx = e->get_context(); @@ -764,7 +800,7 @@ TEST(EngineTest, WafSubscriptorExclusions) engine_ruleset update( R"({"exclusions": [{"id": "1", "rules_target":[{"rule_id":"1"}]}]})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -780,7 +816,7 @@ TEST(EngineTest, WafSubscriptorExclusions) { engine_ruleset update(R"({"exclusions": []})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -797,11 +833,9 @@ TEST(EngineTest, WafSubscriptorExclusions) TEST(EngineTest, WafSubscriptorCustomRules) { - std::map meta; - std::map metrics; - + auto msubmitter = NiceMock{}; auto e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule, msubmitter)); { auto ctx = e->get_context(); @@ -828,7 +862,7 @@ TEST(EngineTest, WafSubscriptorCustomRules) { engine_ruleset update( R"({"custom_rules":[{"id":"1","name":"custom_rule1","tags":{"type":"custom","category":"custom"},"conditions":[{"operator":"match_regex","parameters":{"inputs":[{"address":"arg3","key_path":[]}],"regex":"^custom.*"}}],"on_match":["block"]}]})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -857,7 +891,7 @@ TEST(EngineTest, WafSubscriptorCustomRules) { engine_ruleset update(R"({"custom_rules": []})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { 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 35c6e249ba..5b8c10c60d 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 @@ -13,11 +13,11 @@ namespace dds { -namespace mock = remote_config::mock; +namespace rcmock = remote_config::mock; remote_config::config get_config_with_status(std::string status) { - return mock::get_config( + return rcmock::get_config( "ASM_FEATURES", "{\"asm\":{\"enabled\":" + status + "}}"); } @@ -129,7 +129,7 @@ TEST(RemoteConfigAsmFeaturesListener, std::string error_message = ""; std::string expected_error_message = "Invalid config contents"; remote_config::config non_base_64_content_config = - mock::get_config("ASM_FEATURES", invalid_content); + rcmock::get_config("ASM_FEATURES", invalid_content); try { listener.on_update(non_base_64_content_config); @@ -152,7 +152,7 @@ TEST(RemoteConfigAsmFeaturesListener, remote_config::asm_features_listener listener(remote_config_service); std::string invalid_content = "invalidJsonContent"; remote_config::config config = - mock::get_config("ASM_FEATURES", invalid_content); + rcmock::get_config("ASM_FEATURES", invalid_content); try { listener.on_update(config); @@ -174,7 +174,7 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmKeyMissing) auto remote_config_service = std::make_shared(); remote_config::asm_features_listener listener(remote_config_service); remote_config::config asm_key_missing = - mock::get_config("ASM_FEATURES", "{}"); + rcmock::get_config("ASM_FEATURES", "{}"); try { listener.on_update(asm_key_missing); @@ -195,7 +195,7 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmIsNotValid) auto remote_config_service = std::make_shared(); remote_config::asm_features_listener listener(remote_config_service); remote_config::config invalid_asm_key = - mock::get_config("ASM_FEATURES", "{ \"asm\": 123}"); + rcmock::get_config("ASM_FEATURES", "{ \"asm\": 123}"); try { listener.on_update(invalid_asm_key); @@ -217,7 +217,7 @@ TEST( auto remote_config_service = std::make_shared(); remote_config::asm_features_listener listener(remote_config_service); remote_config::config enabled_key_missing = - mock::get_config("ASM_FEATURES", "{ \"asm\": {}}"); + rcmock::get_config("ASM_FEATURES", "{ \"asm\": {}}"); try { listener.on_update(enabled_key_missing); @@ -239,7 +239,7 @@ TEST(RemoteConfigAsmFeaturesListener, auto remote_config_service = std::make_shared(); remote_config::asm_features_listener listener(remote_config_service); remote_config::config enabled_key_invalid = - mock::get_config("ASM_FEATURES", "{ \"asm\": { \"enabled\": 123}}"); + rcmock::get_config("ASM_FEATURES", "{ \"asm\": { \"enabled\": 123}}"); try { listener.on_update(enabled_key_invalid); diff --git a/appsec/tests/helper/remote_config/listeners/engine_listener_test.cpp b/appsec/tests/helper/remote_config/listeners/engine_listener_test.cpp index 1e3a3d4bb1..12cc59f6ec 100644 --- a/appsec/tests/helper/remote_config/listeners/engine_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/engine_listener_test.cpp @@ -9,6 +9,7 @@ #include "base64.h" #include "engine.hpp" #include "json_helper.hpp" +#include "metrics.hpp" #include "remote_config/exception.hpp" #include "remote_config/listeners/engine_listener.hpp" #include "remote_config/product.hpp" @@ -21,6 +22,7 @@ const std::string waf_rule = namespace dds::remote_config { +using dds::mock::tel_submitter; using mock::get_config; namespace { @@ -40,22 +42,26 @@ TEST(RemoteConfigEngineListener, NoUpdates) rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)).Times(0); + EXPECT_CALL(*engine, update(_, _)).Times(0); - remote_config::engine_listener listener(engine); + auto msubmitter = + std::shared_ptr(new tel_submitter()); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.commit(); } TEST(RemoteConfigEngineListener, UnknownConfig) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)).Times(0); + EXPECT_CALL(*engine, update(_, _)).Times(0); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); EXPECT_THROW(listener.on_update(get_config("UNKNOWN", waf_rule)), error_applying_config); @@ -64,15 +70,17 @@ TEST(RemoteConfigEngineListener, UnknownConfig) TEST(RemoteConfigEngineListener, RuleUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); listener.commit(); @@ -94,15 +102,18 @@ TEST(RemoteConfigEngineListener, RuleUpdate) TEST(RemoteConfigEngineListener, RuleUpdateFallback) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); - remote_config::engine_listener listener(engine, create_sample_rules_ok()); + remote_config::engine_listener listener( + engine, msubmitter, create_sample_rules_ok()); listener.init(); listener.on_unapply(get_config("ASM_DD", waf_rule)); listener.commit(); @@ -124,18 +135,20 @@ TEST(RemoteConfigEngineListener, RuleUpdateFallback) TEST(RemoteConfigEngineListener, RulesOverrideUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); const std::string update = R"({"rules_override": [{"rules_target": [{"rule_id": "1"}], "enabled":"false"}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM", update)); listener.commit(); @@ -165,18 +178,20 @@ TEST(RemoteConfigEngineListener, RulesOverrideUpdate) TEST(RemoteConfigEngineListener, RulesAndRulesOverrideUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); const std::string update = R"({"rules_override": [{"rules_target": [{"rule_id": "1"}], "enabled":"false"}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); listener.on_update(get_config("ASM", update)); @@ -214,18 +229,20 @@ TEST(RemoteConfigEngineListener, RulesAndRulesOverrideUpdate) TEST(RemoteConfigEngineListener, ExclusionsUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); const std::string update = R"({"exclusions":[{"id":1,"rules_target":[{"rule_id":1}]}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM", update)); listener.commit(); @@ -256,18 +273,20 @@ TEST(RemoteConfigEngineListener, ExclusionsUpdate) TEST(RemoteConfigEngineListener, RulesAndExclusionsUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); const std::string update = R"({"exclusions":[{"id":1,"rules_target":[{"rule_id":1}]}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); listener.on_update(get_config("ASM", update)); @@ -305,11 +324,13 @@ TEST(RemoteConfigEngineListener, RulesAndExclusionsUpdate) TEST(RemoteConfigEngineListener, ActionsUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); @@ -317,7 +338,7 @@ TEST(RemoteConfigEngineListener, ActionsUpdate) R"({"actions": [{"id": "redirect", "type": "redirect_request", "parameters": {"status_code": "303", "location": "localhost"}}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM", update)); listener.commit(); @@ -348,11 +369,13 @@ TEST(RemoteConfigEngineListener, ActionsUpdate) TEST(RemoteConfigEngineListener, RulesAndActionsUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); @@ -360,7 +383,7 @@ TEST(RemoteConfigEngineListener, RulesAndActionsUpdate) R"({"actions": [{"id": "redirect", "type": "redirect_request", "parameters": {"status_code": "303", "location": "localhost"}}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); listener.on_update(get_config("ASM", update)); @@ -398,11 +421,13 @@ TEST(RemoteConfigEngineListener, RulesAndActionsUpdate) TEST(RemoteConfigEngineListener, CustomRulesUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); @@ -412,7 +437,7 @@ TEST(RemoteConfigEngineListener, CustomRulesUpdate) {"inputs":[{"address":"arg3","key_path":[]}],"regex":"^custom.*"}}], "on_match":["block"]}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM", update)); listener.commit(); @@ -443,11 +468,13 @@ TEST(RemoteConfigEngineListener, CustomRulesUpdate) TEST(RemoteConfigEngineListener, RulesAndCustomRulesUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); @@ -457,7 +484,7 @@ TEST(RemoteConfigEngineListener, RulesAndCustomRulesUpdate) {"inputs":[{"address":"arg3","key_path":[]}],"regex":"^custom.*"}}], "on_match":["block"]}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); listener.on_update(get_config("ASM", update)); @@ -495,18 +522,20 @@ TEST(RemoteConfigEngineListener, RulesAndCustomRulesUpdate) TEST(RemoteConfigEngineListener, RulesDataUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); const std::string update = R"({"rules_data":[{"id":"blocked_ips","type":"ip_with_expiration","data":[{"value":"1.2.3.4","expiration":0}]}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DATA", update)); listener.commit(); @@ -529,18 +558,21 @@ TEST(RemoteConfigEngineListener, RulesDataUpdate) TEST(RemoteConfigEngineListener, RulesAndRuleDataUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); const std::string update = R"({"rules_data":[{"id":"blocked_ips","type":"ip_with_expiration","data":[{"value":"1.2.3.4","expiration":0}]}]})"; - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); + listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); listener.on_update(get_config("ASM_DATA", update)); @@ -570,15 +602,17 @@ TEST(RemoteConfigEngineListener, RulesAndRuleDataUpdate) TEST(RemoteConfigEngineListener, FullUpdate) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(1) .WillOnce(DoAll(SaveDocument(&doc))); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); { @@ -625,15 +659,18 @@ TEST(RemoteConfigEngineListener, FullUpdate) TEST(RemoteConfigEngineListener, MultipleInitCommitUpdates) { + auto msubmitter = + std::shared_ptr(new tel_submitter()); auto engine = mock::engine::create(); rapidjson::Document doc; - EXPECT_CALL(*engine, update(_, _, _)) + EXPECT_CALL(*engine, update(_, _)) .Times(3) .WillRepeatedly(DoAll(SaveDocument(&doc))); - remote_config::engine_listener listener(engine, create_sample_rules_ok()); + remote_config::engine_listener listener( + engine, msubmitter, create_sample_rules_ok()); listener.init(); listener.on_update(get_config("ASM_DD", waf_rule)); @@ -776,10 +813,12 @@ TEST(RemoteConfigEngineListener, EngineRuleUpdate) {"inputs": [{"address": "server.request.query"} ], "list": ["/other/url"] }, "operator": "phrase_match"} ], "on_match": ["block"] } ] })"; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::map meta; std::map metrics; std::shared_ptr e{engine::create()}; - e->subscribe(waf::instance::from_string(rules, meta, metrics)); + e->subscribe(waf::instance::from_string(rules, *msubmitter)); { auto ctx = e->get_context(); @@ -797,7 +836,7 @@ TEST(RemoteConfigEngineListener, EngineRuleUpdate) {"inputs": [{"address": "server.request.query"} ], "list": ["/anotherUrl"] }, "operator": "phrase_match"} ], "on_match": ["block"]}]})"; - remote_config::engine_listener listener(e); + remote_config::engine_listener listener(e, msubmitter); listener.init(); listener.on_update(get_config("ASM_DD", new_rules)); listener.commit(); @@ -823,10 +862,13 @@ TEST(RemoteConfigEngineListener, EngineRuleUpdateFallback) {"inputs": [{"address": "server.request.query"} ], "list": ["/a/url"] }, "operator": "phrase_match"} ], "on_match": ["block"] } ] })"; + auto msubmitter = std::shared_ptr( + new NiceMock()); + std::map meta; std::map metrics; std::shared_ptr e{engine::create()}; - e->subscribe(waf::instance::from_string(rules, meta, metrics)); + e->subscribe(waf::instance::from_string(rules, *msubmitter)); { auto ctx = e->get_context(); @@ -840,7 +882,8 @@ TEST(RemoteConfigEngineListener, EngineRuleUpdateFallback) EXPECT_EQ(res->events.size(), 1); } - remote_config::engine_listener listener(e, create_sample_rules_ok()); + remote_config::engine_listener listener( + e, msubmitter, create_sample_rules_ok()); listener.init(); listener.on_unapply(get_config("ASM_DD", "")); listener.commit(); @@ -858,13 +901,13 @@ TEST(RemoteConfigEngineListener, EngineRuleUpdateFallback) TEST(RemoteConfigEngineListener, EngineRuleOverrideUpdateDisableRule) { - std::map meta; - std::map metrics; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::shared_ptr engine{dds::engine::create()}; - engine->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + engine->subscribe(waf::instance::from_string(waf_rule, *msubmitter)); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); { @@ -905,13 +948,13 @@ TEST(RemoteConfigEngineListener, EngineRuleOverrideUpdateDisableRule) TEST(RemoteConfigEngineListener, RuleOverrideUpdateSetOnMatch) { - std::map meta; - std::map metrics; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::shared_ptr engine{dds::engine::create()}; - engine->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + engine->subscribe(waf::instance::from_string(waf_rule, *msubmitter)); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); @@ -956,13 +999,13 @@ TEST(RemoteConfigEngineListener, RuleOverrideUpdateSetOnMatch) TEST(RemoteConfigEngineListener, EngineRuleOverrideAndActionsUpdate) { - std::map meta; - std::map metrics; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::shared_ptr engine{dds::engine::create()}; - engine->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + engine->subscribe(waf::instance::from_string(waf_rule, *msubmitter)); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); @@ -1009,13 +1052,13 @@ TEST(RemoteConfigEngineListener, EngineRuleOverrideAndActionsUpdate) TEST(RemoteConfigEngineListener, EngineExclusionsUpdatePasslistRule) { - std::map meta; - std::map metrics; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::shared_ptr engine{dds::engine::create()}; - engine->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + engine->subscribe(waf::instance::from_string(waf_rule, *msubmitter)); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); @@ -1057,13 +1100,13 @@ TEST(RemoteConfigEngineListener, EngineExclusionsUpdatePasslistRule) TEST(RemoteConfigEngineListener, EngineCustomRulesUpdate) { - std::map meta; - std::map metrics; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::shared_ptr engine{dds::engine::create()}; - engine->subscribe(waf::instance::from_string(waf_rule, meta, metrics)); + engine->subscribe(waf::instance::from_string(waf_rule, *msubmitter)); - remote_config::engine_listener listener(engine); + remote_config::engine_listener listener(engine, msubmitter); listener.init(); @@ -1168,12 +1211,12 @@ TEST(RemoteConfigEngineListener, EngineRuleDataUpdate) [{"parameters":{"inputs":[{"address":"http.client_ip"}],"data":"blocked_ips"}, "operator":"ip_match"}],"transformers":[],"on_match":["block"]}]})"; - std::map meta; - std::map metrics; + auto msubmitter = std::shared_ptr( + new NiceMock()); std::shared_ptr e{engine::create()}; - e->subscribe(waf::instance::from_string(waf_rule_with_data, meta, metrics)); + e->subscribe(waf::instance::from_string(waf_rule_with_data, *msubmitter)); - remote_config::engine_listener listener(e); + remote_config::engine_listener listener(e, msubmitter); listener.init(); { diff --git a/appsec/tests/helper/remote_config/mocks.hpp b/appsec/tests/helper/remote_config/mocks.hpp index f34f1d7701..e2cf102336 100644 --- a/appsec/tests/helper/remote_config/mocks.hpp +++ b/appsec/tests/helper/remote_config/mocks.hpp @@ -7,8 +7,10 @@ #pragma once #include "../common.hpp" +#include "../tel_subm_mock.hpp" #include "base64.h" #include "engine.hpp" +#include "metrics.hpp" #include "remote_config/client.hpp" #include "remote_config/config.hpp" @@ -21,9 +23,7 @@ class engine : public dds::engine { action_map &&actions = {}) : dds::engine(trace_rate_limit) {} - MOCK_METHOD(void, update, - (engine_ruleset &, (std::map &), - (std::map &)), + MOCK_METHOD(void, update, (engine_ruleset &, metrics::TelemetrySubmitter &), (override)); static auto create() { return std::shared_ptr(new engine()); } diff --git a/appsec/tests/helper/service_manager_test.cpp b/appsec/tests/helper/service_manager_test.cpp index bd1995dc85..d984997f2c 100644 --- a/appsec/tests/helper/service_manager_test.cpp +++ b/appsec/tests/helper/service_manager_test.cpp @@ -6,8 +6,8 @@ #include "common.hpp" #include "engine_settings.hpp" #include +#include #include -#include namespace algo = boost::algorithm; @@ -25,23 +25,19 @@ struct service_manager_exp : public service_manager { TEST(ServiceManagerTest, LoadRulesOK) { - std::map meta; - std::map metrics; - service_manager_exp manager; auto fn = create_sample_rules_ok(); dds::engine_settings engine_settings; engine_settings.rules_file = fn; engine_settings.waf_timeout_us = 42; - auto service = - manager.create_service(engine_settings, {}, meta, metrics, {}); + auto service = manager.create_service(engine_settings, {}, {}); auto *service_rp = service.get(); + auto metrics = service->drain_legacy_metrics(); EXPECT_EQ(manager.get_cache().size(), 1); - EXPECT_EQ(metrics[tag::event_rules_loaded], 4); + EXPECT_EQ(metrics[metrics::event_rules_loaded], 4); // loading again should take from the cache - auto service2 = - manager.create_service(engine_settings, {}, meta, metrics, {}); + auto service2 = manager.create_service(engine_settings, {}, {}); EXPECT_EQ(manager.get_cache().size(), 1); EXPECT_EQ(service, service2); @@ -58,15 +54,13 @@ TEST(ServiceManagerTest, LoadRulesOK) ASSERT_FALSE(weak_ptr.expired()); // loading another service should cleanup the cache - auto service3 = - manager.create_service(engine_settings, {true}, meta, metrics, {}); + auto service3 = manager.create_service(engine_settings, {true}, {}); ASSERT_NE(service3.get(), service_rp); ASSERT_TRUE(weak_ptr.expired()); EXPECT_EQ(manager.get_cache().size(), 1); // another service identifier should result in another service - auto service4 = - manager.create_service(engine_settings, {}, meta, metrics, {}); + auto service4 = manager.create_service(engine_settings, {}, {}); EXPECT_EQ(manager.get_cache().size(), 2); } @@ -81,7 +75,7 @@ TEST(ServiceManagerTest, LoadRulesFileNotFound) dds::engine_settings engine_settings; engine_settings.rules_file = "/file/that/does/not/exist"; engine_settings.waf_timeout_us = 42; - manager.create_service(engine_settings, {}, meta, metrics, {}); + manager.create_service(engine_settings, {}, {}); }, std::runtime_error); } @@ -97,7 +91,7 @@ TEST(ServiceManagerTest, BadRulesFile) dds::engine_settings engine_settings; engine_settings.rules_file = "/dev/null"; engine_settings.waf_timeout_us = 42; - manager.create_service(engine_settings, {}, meta, metrics, {}); + manager.create_service(engine_settings, {}, {}); }, dds::parsing_error); } @@ -112,10 +106,8 @@ TEST(ServiceManagerTest, UniqueServices) dds::engine_settings engine_settings; engine_settings.rules_file = fn; - auto service1 = - manager.create_service(engine_settings, {}, meta, metrics, {}); - auto service2 = - manager.create_service(engine_settings, {}, meta, metrics, {}); + auto service1 = manager.create_service(engine_settings, {}, {}); + auto service2 = manager.create_service(engine_settings, {}, {}); EXPECT_EQ(service1.get(), service2.get()); } diff --git a/appsec/tests/helper/service_test.cpp b/appsec/tests/helper/service_test.cpp index 815fa0d03a..c2d88525cb 100644 --- a/appsec/tests/helper/service_test.cpp +++ b/appsec/tests/helper/service_test.cpp @@ -49,93 +49,38 @@ __attribute__((constructor)) void resolve_symbols() namespace dds { -TEST(ServiceTest, NullEngine) -{ - std::shared_ptr engine{}; - remote_config::settings rc_settings{true, ""}; - auto client = remote_config::client::from_settings(rc_settings, {}); - - auto service_config = std::make_shared(); - auto client_handler = std::make_unique( - std::move(client), service_config); - - EXPECT_THROW( - auto s = service(engine, service_config, std::move(client_handler), ""), - std::runtime_error); -} - -TEST(ServiceTest, NullServiceHandler) -{ - std::shared_ptr engine{engine::create()}; - auto service_config = std::make_shared(); - - // A null service handler doesn't make a difference as remote config is - // optional - service svc{engine, service_config, nullptr, ""}; - EXPECT_EQ(engine.get(), svc.get_engine().get()); -} - TEST(ServiceTest, ServicePickSchemaExtractionSamples) { std::shared_ptr engine{engine::create()}; - auto client = remote_config::client::from_settings({true}, {}); + auto create_engine_settings = [](bool enabled, double sample_rate) { + engine_settings engine_settings = {}; + engine_settings.rules_file = create_sample_rules_ok(); + engine_settings.schema_extraction.enabled = enabled; + engine_settings.schema_extraction.sample_rate = sample_rate; + return engine_settings; + }; auto service_config = std::make_shared(); - engine_settings engine_settings = {}; - engine_settings.rules_file = create_sample_rules_ok(); - std::map meta; - std::map metrics; - - { // Constructor. It picks based on rate - double all_requests_are_picked = 1.0; - auto s = service(engine, service_config, nullptr, "", - {true, all_requests_are_picked}); - - EXPECT_TRUE(s.get_schema_sampler()->picked()); - } - - { // Constructor. It does not pick based on rate - double no_request_is_picked = 0.0; - auto s = service( - engine, service_config, nullptr, "", {true, no_request_is_picked}); - - EXPECT_FALSE(s.get_schema_sampler()->picked()); - } - - { // Constructor. It does not pick if disabled - double all_requests_are_picked = 1.0; - bool schema_extraction_disabled = false; - auto s = service(engine, service_config, nullptr, "", - {schema_extraction_disabled, all_requests_are_picked}); - - EXPECT_FALSE(s.get_schema_sampler()->picked()); - } - { // Static constructor. It picks based on rate - engine_settings.schema_extraction.enabled = true; - engine_settings.schema_extraction.sample_rate = 1.0; - auto service = - service::from_settings(engine_settings, {}, meta, metrics, false); + { // All requests are picked + auto s = service::from_settings( + create_engine_settings(true, 1.0), {}, false); - EXPECT_TRUE(service->get_schema_sampler()->picked()); + EXPECT_TRUE(s->get_schema_sampler()->picked()); } - { // Static constructor. It does not pick based on rate - engine_settings.schema_extraction.enabled = true; - engine_settings.schema_extraction.sample_rate = 0.0; - auto service = - service::from_settings(engine_settings, {}, meta, metrics, false); + { // Static constructor. It does not pick based on rate + auto s = service::from_settings( + create_engine_settings(true, 0.0), {}, false); - EXPECT_FALSE(service->get_schema_sampler()->picked()); + EXPECT_FALSE(s->get_schema_sampler()->picked()); } { // Static constructor. It does not pick if disabled - engine_settings.schema_extraction.enabled = false; - engine_settings.schema_extraction.sample_rate = 1.0; - auto service = - service::from_settings(engine_settings, {}, meta, metrics, false); + auto s = service::from_settings( + create_engine_settings(false, 1.0), {}, false); - EXPECT_FALSE(service->get_schema_sampler()->picked()); + EXPECT_FALSE(s->get_schema_sampler()->picked()); } } diff --git a/appsec/tests/helper/tel_subm_mock.hpp b/appsec/tests/helper/tel_subm_mock.hpp new file mode 100644 index 0000000000..00c023d90f --- /dev/null +++ b/appsec/tests/helper/tel_subm_mock.hpp @@ -0,0 +1,16 @@ +#include +#include + +namespace dds::mock { +class tel_submitter : public dds::metrics::TelemetrySubmitter { +public: + MOCK_METHOD(void, submit_metric, (std::string_view, double, std::string), + (override)); + MOCK_METHOD( + void, submit_legacy_metric, (std::string_view, double), (override)); + MOCK_METHOD( + void, submit_legacy_meta, (std::string_view, std::string), (override)); + MOCK_METHOD(void, submit_legacy_meta_copy_key, (std::string, std::string), + (override)); +}; +} // namespace dds::mock diff --git a/appsec/tests/helper/waf_test.cpp b/appsec/tests/helper/waf_test.cpp index 0c3d533ce5..7984bfd2d8 100644 --- a/appsec/tests/helper/waf_test.cpp +++ b/appsec/tests/helper/waf_test.cpp @@ -3,14 +3,17 @@ // // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. + #include "common.hpp" +#include "ddwaf.h" #include "engine_settings.hpp" #include "json_helper.hpp" +#include "metrics.hpp" +#include "tel_subm_mock.hpp" #include #include #include #include -#include #include const std::string waf_rule = @@ -41,37 +44,44 @@ TEST(WafTest, InitWithInvalidRules) engine_settings cs; cs.rules_file = create_sample_rules_invalid(); auto ruleset = engine_ruleset::from_path(cs.rules_file); - std::map meta; - std::map metrics; + mock::tel_submitter submitm{}; + + EXPECT_CALL(submitm, submit_legacy_meta(metrics::waf_version, + std::string{ddwaf_get_version()})); + std::string rules_errors; + EXPECT_CALL(submitm, submit_legacy_meta(metrics::event_rules_errors, _)) + .WillOnce(SaveArg<1>(&rules_errors)); + + EXPECT_CALL( + submitm, submit_legacy_metric(metrics::event_rules_loaded, 1.0)); + EXPECT_CALL( + submitm, submit_legacy_metric(metrics::event_rules_failed, 4.0)); + + EXPECT_CALL(submitm, submit_metric("waf.init"sv, 1, _)); + EXPECT_CALL(submitm, submit_metric("waf.config_errors", 4., + std::string{"waf_version:"} + ddwaf_get_version() + + ",event_rules_version:1.2.3," + "config_key:rules,scope:item")); std::shared_ptr wi{ - waf::instance::from_settings(cs, ruleset, meta, metrics)}; + waf::instance::from_settings(cs, ruleset, submitm)}; - EXPECT_EQ(meta.size(), 2); - EXPECT_STREQ(meta[std::string(tag::waf_version)].c_str(), "1.18.0"); + Mock::VerifyAndClearExpectations(&submitm); rapidjson::Document doc; - doc.Parse(meta[std::string(tag::event_rules_errors)]); + doc.Parse(rules_errors); EXPECT_FALSE(doc.HasParseError()); EXPECT_TRUE(doc.IsObject()); EXPECT_TRUE(doc.HasMember("missing key 'type'")); EXPECT_TRUE(doc.HasMember("unknown matcher: squash")); EXPECT_TRUE(doc.HasMember("missing key 'inputs'")); - - EXPECT_EQ(metrics.size(), 2); - // For small enough integers this comparison should work, otherwise replace - // with EXPECT_NEAR. - EXPECT_EQ(metrics[tag::event_rules_loaded], 1.0); - EXPECT_EQ(metrics[tag::event_rules_failed], 4.0); } TEST(WafTest, RunWithInvalidParam) { - std::map meta; - std::map metrics; - + NiceMock submitm{}; std::shared_ptr wi{ - waf::instance::from_string(waf_rule, meta, metrics)}; + waf::instance::from_string(waf_rule, submitm)}; auto ctx = wi->get_listener(); parameter_view pv; dds::event e; @@ -80,11 +90,10 @@ TEST(WafTest, RunWithInvalidParam) TEST(WafTest, RunWithTimeout) { - std::map meta; - std::map metrics; + NiceMock submitm{}; std::shared_ptr wi( - waf::instance::from_string(waf_rule, meta, metrics, 0)); + waf::instance::from_string(waf_rule, submitm, 0)); auto ctx = wi->get_listener(); auto p = parameter::map(); @@ -98,11 +107,9 @@ TEST(WafTest, RunWithTimeout) TEST(WafTest, ValidRunGood) { - std::map meta; - std::map metrics; - + NiceMock submitm{}; std::shared_ptr wi{ - waf::instance::from_string(waf_rule, meta, metrics)}; + waf::instance::from_string(waf_rule, submitm)}; auto ctx = wi->get_listener(); auto p = parameter::map(); @@ -112,18 +119,25 @@ TEST(WafTest, ValidRunGood) dds::event e; ctx->call(pv, e); - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_STREQ(meta[std::string(tag::event_rules_version)].c_str(), "1.2.3"); - EXPECT_GT(metrics[tag::waf_duration], 0.0); + EXPECT_CALL(submitm, + submit_legacy_meta(metrics::event_rules_version, std::string{"1.2.3"})); + double duration; + EXPECT_CALL(submitm, submit_legacy_metric(metrics::waf_duration, _)) + .WillOnce(SaveArg<1>(&duration)); + EXPECT_CALL( + submitm, submit_metric("waf.requests"sv, 1, + std::string{"event_rules_version:1.2.3,waf_version:"} + + ddwaf_get_version())); + ctx->submit_metrics(submitm); + EXPECT_GT(duration, 0.0); + Mock::VerifyAndClearExpectations(&submitm); } TEST(WafTest, ValidRunMonitor) { - std::map meta; - std::map metrics; - + NiceMock submitm{}; std::shared_ptr wi{ - waf::instance::from_string(waf_rule, meta, metrics)}; + waf::instance::from_string(waf_rule, submitm)}; auto ctx = wi->get_listener(); auto p = parameter::map(); @@ -142,19 +156,27 @@ TEST(WafTest, ValidRunMonitor) } EXPECT_TRUE(e.actions.empty()); - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_STREQ(meta[std::string(tag::event_rules_version)].c_str(), "1.2.3"); - EXPECT_GT(metrics[tag::waf_duration], 0.0); + + EXPECT_CALL(submitm, + submit_legacy_meta(metrics::event_rules_version, std::string{"1.2.3"})); + EXPECT_CALL(submitm, submit_legacy_metric(metrics::waf_duration, _)); + EXPECT_CALL( + submitm, submit_metric("waf.requests"sv, 1, + std::string{"event_rules_version:1.2.3,waf_version:"} + + ddwaf_get_version() + ",rule_triggered:true")); + EXPECT_CALL( + submitm, submit_legacy_meta_copy_key( + std::string{"_dd.appsec.s.arg2"}, std::string{"[8]"})); + ctx->submit_metrics(submitm); + Mock::VerifyAndClearExpectations(&submitm); } TEST(WafTest, ValidRunMonitorObfuscated) { - std::map meta; - std::map metrics; + NiceMock submitm{}; - std::shared_ptr wi{ - waf::instance::from_string(waf_rule, meta, metrics, - waf::instance::default_waf_timeout_us, "password"sv, "string 3"sv)}; + std::shared_ptr wi{waf::instance::from_string(waf_rule, submitm, + waf::instance::default_waf_timeout_us, "password"sv, "string 3"sv)}; auto ctx = wi->get_listener(); auto p = parameter::map(), sub_p = parameter::map(); @@ -176,18 +198,11 @@ TEST(WafTest, ValidRunMonitorObfuscated) ""); EXPECT_STREQ(doc["rule_matches"][1]["parameters"][0]["value"].GetString(), ""); - - EXPECT_TRUE(e.actions.empty()); - - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_STREQ(meta[std::string(tag::event_rules_version)].c_str(), "1.2.3"); - EXPECT_GT(metrics[tag::waf_duration], 0.0); } TEST(WafTest, ValidRunMonitorObfuscatedFromSettings) { - std::map meta; - std::map metrics; + NiceMock submitm{}; engine_settings cs; cs.rules_file = create_sample_rules_ok(); @@ -195,7 +210,7 @@ TEST(WafTest, ValidRunMonitorObfuscatedFromSettings) auto ruleset = engine_ruleset::from_path(cs.rules_file); std::shared_ptr wi{ - waf::instance::from_settings(cs, ruleset, meta, metrics)}; + waf::instance::from_settings(cs, ruleset, submitm)}; auto ctx = wi->get_listener(); @@ -217,19 +232,14 @@ TEST(WafTest, ValidRunMonitorObfuscatedFromSettings) EXPECT_STREQ(doc["rule_matches"][0]["parameters"][0]["value"].GetString(), ""); - - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_STREQ(meta[std::string(tag::event_rules_version)].c_str(), "1.2.3"); - EXPECT_GT(metrics[tag::waf_duration], 0.0); } TEST(WafTest, UpdateRuleData) { - std::map meta; - std::map metrics; + NiceMock submitm{}; std::shared_ptr wi{ - waf::instance::from_string(waf_rule_with_data, meta, metrics)}; + waf::instance::from_string(waf_rule_with_data, submitm)}; ASSERT_TRUE(wi); auto addresses = wi->get_subscriptions(); @@ -250,7 +260,7 @@ TEST(WafTest, UpdateRuleData) auto param = json_to_parameter( R"({"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"); - wi = wi->update(param, meta, metrics); + wi = wi->update(param, submitm); ASSERT_TRUE(wi); addresses = wi->get_subscriptions(); @@ -280,15 +290,15 @@ TEST(WafTest, UpdateRuleData) EXPECT_EQ(e.actions.size(), 1); EXPECT_EQ(e.actions.begin()->type, dds::action_type::block); } + + Mock::VerifyAndClearExpectations(&submitm); } TEST(WafTest, UpdateInvalid) { - std::map meta; - std::map metrics; - + NiceMock submitm{}; std::shared_ptr wi{ - waf::instance::from_string(waf_rule_with_data, meta, metrics)}; + waf::instance::from_string(waf_rule_with_data, submitm)}; ASSERT_TRUE(wi); { @@ -304,16 +314,19 @@ TEST(WafTest, UpdateInvalid) auto param = json_to_parameter(R"({})"); - ASSERT_THROW(wi->update(param, meta, metrics), invalid_object); + EXPECT_CALL(submitm, + submit_metric("waf.updates"sv, 1, + std::string{"success:false,event_rules_version:,waf_version:"} + + ddwaf_get_version())); + ASSERT_THROW(wi->update(param, submitm), invalid_object); } TEST(WafTest, SchemasAreAdded) { - std::map meta; - std::map metrics; + NiceMock submitm{}; std::shared_ptr wi{ - waf::instance::from_string(waf_rule, meta, metrics)}; + waf::instance::from_string(waf_rule, submitm)}; auto ctx = wi->get_listener(); auto p = parameter::map(), sub_p = parameter::map(); @@ -331,15 +344,23 @@ TEST(WafTest, SchemasAreAdded) EXPECT_FALSE(doc.HasParseError()); EXPECT_TRUE(doc.IsObject()); - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_FALSE(meta.empty()); - EXPECT_STREQ(meta["_dd.appsec.s.arg2"].c_str(), "[8]"); + EXPECT_CALL( + submitm, submit_metric("waf.requests"sv, 1, + std::string{"event_rules_version:1.2.3,waf_version:"} + + ddwaf_get_version() + ",rule_triggered:true")); + EXPECT_CALL( + submitm, submit_legacy_meta("_dd.appsec.event_rules.version", "1.2.3")); + EXPECT_CALL(submitm, submit_legacy_metric("_dd.appsec.waf.duration"sv, _)); + EXPECT_CALL( + submitm, submit_legacy_meta_copy_key( + std::string{"_dd.appsec.s.arg2"}, std::string{"[8]"})); + ctx->submit_metrics(submitm); + Mock::VerifyAndClearExpectations(&submitm); } TEST(WafTest, ActionsAreSentAndParsed) { - std::map meta; - std::map metrics; + NiceMock submitm{}; auto p = parameter::map(); p.add("http.client_ip", parameter::string("192.168.1.1"sv)); @@ -350,7 +371,7 @@ TEST(WafTest, ActionsAreSentAndParsed) R"({"version":"2.1","rules":[{"id":"blk-001-001","name":"BlockIPAddresses","tags":{"type":"block_ip","category":"security_response"},"conditions":[{"parameters":{"inputs":[{"address":"http.client_ip"}],"data":"blocked_ips"},"operator":"ip_match"}],"transformers":[],"on_match":["custom"]}],"actions":[{"id":"custom","type":"block_request","parameters":{"status_code":123,"grpc_status_code":321,"type":"json","custom_param":"foo"}}],"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"; std::shared_ptr wi{ - waf::instance::from_string(rules_with_actions, meta, metrics)}; + waf::instance::from_string(rules_with_actions, submitm)}; ASSERT_TRUE(wi); auto addresses = wi->get_subscriptions(); @@ -389,7 +410,7 @@ TEST(WafTest, ActionsAreSentAndParsed) R"({"version":"2.1","rules":[{"id":"blk-001-001","name":"BlockIPAddresses","tags":{"type":"block_ip","category":"security_response"},"conditions":[{"parameters":{"inputs":[{"address":"http.client_ip"}],"data":"blocked_ips"},"operator":"ip_match"}],"transformers":[],"on_match":["custom"]}],"actions":[{"id":"custom","type":"block_request","parameters":{}}],"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"; std::shared_ptr wi{ - waf::instance::from_string(rules_with_actions, meta, metrics)}; + waf::instance::from_string(rules_with_actions, submitm)}; ASSERT_TRUE(wi); auto addresses = wi->get_subscriptions(); @@ -428,7 +449,7 @@ TEST(WafTest, ActionsAreSentAndParsed) R"({"version":"2.1","rules":[{"id":"blk-001-001","name":"BlockIPAddresses","tags":{"type":"block_ip","category":"security_response"},"conditions":[{"parameters":{"inputs":[{"address":"http.client_ip"}],"data":"blocked_ips"},"operator":"ip_match"}],"transformers":[],"on_match":["custom"]}],"actions":[{"id":"custom","type":"custom_type","parameters":{"some":"parameter"}}],"rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"; std::shared_ptr wi{ - waf::instance::from_string(rules_with_actions, meta, metrics)}; + waf::instance::from_string(rules_with_actions, submitm)}; ASSERT_TRUE(wi); auto addresses = wi->get_subscriptions(); @@ -462,7 +483,7 @@ TEST(WafTest, ActionsAreSentAndParsed) R"({"version":"2.1","rules":[{"id":"blk-001-001","name":"BlockIPAddresses","tags":{"type":"block_ip","category":"security_response"},"conditions":[{"parameters":{"inputs":[{"address":"http.client_ip"}],"data":"blocked_ips"},"operator":"ip_match"}],"transformers":[],"on_match":["block"]}], "rules_data":[{"id":"blocked_ips","type":"data_with_expiration","data":[{"value":"192.168.1.1","expiration":"9999999999"}]}]})"; std::shared_ptr wi{ - waf::instance::from_string(rules_with_actions, meta, metrics)}; + waf::instance::from_string(rules_with_actions, submitm)}; ASSERT_TRUE(wi); auto addresses = wi->get_subscriptions(); diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy index 605f8450f5..d007a7a645 100644 --- a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy +++ b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy @@ -4,7 +4,6 @@ import com.datadog.appsec.php.mock_agent.MockDatadogAgent import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigRequest import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigResponse import com.datadog.appsec.php.mock_agent.rem_cfg.Target -import com.datadog.appsec.php.model.Span import com.datadog.appsec.php.model.Trace import com.github.dockerjava.api.command.CreateContainerCmd import com.github.dockerjava.api.command.ExecCreateCmdResponse @@ -12,6 +11,7 @@ import com.github.dockerjava.api.exception.NotFoundException import com.github.dockerjava.api.model.Bind import com.github.dockerjava.api.model.Volume import com.google.common.util.concurrent.SettableFuture +import groovy.json.JsonOutput import groovy.transform.CompileStatic import groovy.transform.TypeCheckingMode import groovy.transform.stc.ClosureParams @@ -30,9 +30,12 @@ import org.testcontainers.containers.output.Slf4jLogConsumer import java.net.http.HttpClient import java.net.http.HttpRequest import java.net.http.HttpResponse +import java.nio.charset.StandardCharsets import java.time.Duration +import java.time.Instant import java.util.concurrent.Future import java.util.function.Consumer +import java.util.function.Supplier @CompileStatic @Slf4j @@ -129,6 +132,58 @@ class AppSecContainer> extends GenericContain mockDatadogAgent.waitForRCVersion(target, version, timeoutInMs) } + Supplier applyRemoteConfig(Target target, Map files) { + Map encodedFiles = files + .findAll { it.value != null } + .collectEntries { + [ + it.key, + JsonOutput.toJson(it.value).getBytes(StandardCharsets.UTF_8) + ] + } + long newVersion = Instant.now().epochSecond + def rcr = new RemoteConfigResponse() + rcr.clientConfigs = files.keySet() as List + rcr.targetFiles = encodedFiles.collect { + new RemoteConfigResponse.TargetFile( + path: it.key, + raw: new String( + Base64.encoder.encode(it.value), + StandardCharsets.ISO_8859_1) + ) + } + rcr.targets = new RemoteConfigResponse.Targets( + signatures: [], + targetsSigned: new RemoteConfigResponse.Targets.TargetsSigned( + type: 'root', + custom: new RemoteConfigResponse.Targets.TargetsSigned.TargetsCustom( + opaqueBackendState: 'ABCDEF' + ), + specVersion: '1.0.0', + expires: Instant.parse('2030-01-01T00:00:00Z'), + version: newVersion, + targets: encodedFiles.collectEntries { + [ + it.key, + new RemoteConfigResponse.Targets.ConfigTarget( + hashes: [sha256: RemoteConfigResponse.sha256(it.value).toString(16).padLeft(64, '0')], + length: it.value.size(), + custom: new RemoteConfigResponse.Targets.ConfigTarget.ConfigTargetCustom( + version: newVersion + ) + ) + ] + } + ), + ) + + setNextRCResponse(target, rcr) + + long start = System.currentTimeMillis() + return { -> waitForRCVersion(target, newVersion, + 5_000 - (Math.max(0, System.currentTimeMillis() - start))) } + } + void close() { copyLogs() super.close() diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/mock_agent/ConfigV07Handler.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/mock_agent/ConfigV07Handler.groovy index e4bca9a83a..3a547440dd 100644 --- a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/mock_agent/ConfigV07Handler.groovy +++ b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/mock_agent/ConfigV07Handler.groovy @@ -1,7 +1,7 @@ package com.datadog.appsec.php.mock_agent -import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigResponse import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigRequest +import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigResponse import com.datadog.appsec.php.mock_agent.rem_cfg.Target import com.google.common.collect.Lists import groovy.transform.CompileStatic @@ -47,6 +47,16 @@ class ConfigV07Handler implements Handler { } RemoteConfigRequest waitForVersion(Target target, long version, long timeoutInMs) { + if (timeoutInMs <= 5) { + synchronized (capturedRequests) { + RemoteConfigRequest request = capturedRequests[target] + if (request != null && request.client.clientState.targetsVersion == version) { + return request + } + } + throw new AssertionError("No request with version $version is stord at this point" as Object) + } + log.debug("waitForVersion start") long deadline = System.currentTimeMillis() + timeoutInMs synchronized (capturedRequests) { diff --git a/appsec/tests/integration/src/test/bin/enable_extensions.sh b/appsec/tests/integration/src/test/bin/enable_extensions.sh index 3d91dadc5f..98603bada9 100755 --- a/appsec/tests/integration/src/test/bin/enable_extensions.sh +++ b/appsec/tests/integration/src/test/bin/enable_extensions.sh @@ -24,7 +24,7 @@ if [[ -f /appsec/ddappsec.so && -d /project ]]; then echo datadog.appsec.helper_path=/appsec/libddappsec-helper.so echo datadog.appsec.helper_log_file=/tmp/logs/helper.log echo datadog.appsec.helper_log_level=info -# echo datadog.appsec.helper_log_level=debug +# echo datadog.appsec.helper_log_level=trace echo datadog.appsec.rules=/etc/recommended.json echo datadog.appsec.log_file=/tmp/logs/appsec.log echo datadog.appsec.log_level=debug @@ -33,6 +33,8 @@ fi if [[ -n $XDEBUG ]]; then echo "Enabling Xdebug" >&2 + touch /tmp/logs/xdebug.log + chown www-data:www-data /tmp/logs/xdebug.log { echo zend_extension = xdebug.so echo xdebug.mode = debug diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RemoteConfigTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RemoteConfigTests.groovy index b06027dca1..8dc31f04b0 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RemoteConfigTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RemoteConfigTests.groovy @@ -4,9 +4,7 @@ import com.datadog.appsec.php.docker.AppSecContainer import com.datadog.appsec.php.docker.FailOnUnmatchedTraces import com.datadog.appsec.php.mock_agent.rem_cfg.Capability import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigRequest -import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigResponse import com.datadog.appsec.php.mock_agent.rem_cfg.Target -import groovy.json.JsonOutput import groovy.util.logging.Slf4j import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test @@ -16,8 +14,6 @@ import org.testcontainers.junit.jupiter.Testcontainers import java.net.http.HttpRequest import java.net.http.HttpResponse -import java.nio.charset.StandardCharsets -import java.time.Instant import static com.datadog.appsec.php.integration.TestParams.getPhpVersion import static com.datadog.appsec.php.integration.TestParams.getVariant @@ -50,7 +46,7 @@ class RemoteConfigTests { 'bash', '-c', '''sed -e '/appsec.enabled/d' -e '/appsec.rules=/d' /etc/php/php.ini > /etc/php/php-rc.ini; kill -9 `pgrep php-fpm`; - export _DD_DEBUG_SIDECAR_RC_POLL_INTERVAL_MILLIS=1000 DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS=1 DD_ENV=; + export DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS=1 DD_ENV=; php-fpm -y /etc/php-fpm.conf -c /etc/php/php-rc.ini''') assert res.exitCode == 0 } @@ -349,55 +345,10 @@ class RemoteConfigTests { } private RemoteConfigRequest applyRemoteConfig(Target target, Map files) { - Map encodedFiles = files - .findAll { it.value != null } - .collectEntries { - [ - it.key, - JsonOutput.toJson(it.value).getBytes(StandardCharsets.UTF_8) - ] - } - long newVersion = Instant.now().epochSecond - def rcr = new RemoteConfigResponse() - rcr.clientConfigs = files.keySet() as List - rcr.targetFiles = encodedFiles.collect { - new RemoteConfigResponse.TargetFile( - path: it.key, - raw: new String( - Base64.encoder.encode(it.value), - StandardCharsets.ISO_8859_1) - ) - } - rcr.targets = new RemoteConfigResponse.Targets( - signatures: [], - targetsSigned: new RemoteConfigResponse.Targets.TargetsSigned( - type: 'root', - custom: new RemoteConfigResponse.Targets.TargetsSigned.TargetsCustom( - opaqueBackendState: 'ABCDEF' - ), - specVersion:'1.0.0', - expires: Instant.parse('2030-01-01T00:00:00Z'), - version: newVersion, - targets: encodedFiles.collectEntries { - [ - it.key, - new RemoteConfigResponse.Targets.ConfigTarget( - hashes: [sha256: RemoteConfigResponse.sha256(it.value).toString(16).padLeft(64, '0')], - length: it.value.size(), - custom: new RemoteConfigResponse.Targets.ConfigTarget.ConfigTargetCustom( - version: newVersion - ) - ) - ] - } - ), - ) - - CONTAINER.setNextRCResponse(target, rcr) - CONTAINER.waitForRCVersion(target, newVersion, 5_000) + CONTAINER.applyRemoteConfig(target, files).get() } - RemoteConfigRequest dropRemoteConfig(Target target) { + private RemoteConfigRequest dropRemoteConfig(Target target) { applyRemoteConfig(target, [:]) } diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy new file mode 100644 index 0000000000..93c215b18b --- /dev/null +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy @@ -0,0 +1,278 @@ +package com.datadog.appsec.php.integration + +import com.datadog.appsec.php.TelemetryHelpers +import com.datadog.appsec.php.docker.AppSecContainer +import com.datadog.appsec.php.docker.FailOnUnmatchedTraces +import com.datadog.appsec.php.mock_agent.rem_cfg.RemoteConfigRequest +import com.datadog.appsec.php.mock_agent.rem_cfg.Target +import com.datadog.appsec.php.model.Trace +import groovy.util.logging.Slf4j +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.MethodOrderer +import org.junit.jupiter.api.Order +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestMethodOrder +import org.junit.jupiter.api.condition.DisabledIf +import org.testcontainers.junit.jupiter.Container +import org.testcontainers.junit.jupiter.Testcontainers + +import java.net.http.HttpRequest +import java.net.http.HttpResponse +import java.util.function.Supplier + +import static com.datadog.appsec.php.integration.TestParams.getPhpVersion +import static com.datadog.appsec.php.integration.TestParams.getVariant +import static java.net.http.HttpResponse.BodyHandlers.ofString + +@Testcontainers +@Slf4j +@TestMethodOrder(MethodOrderer.OrderAnnotation) +@DisabledIf('isDisabled') +class TelemetryTests { + static boolean disabled = variant.contains('zts') || phpVersion != '8.3' + + private static final Target RC_TARGET = new Target('appsec_int_tests', 'integration', '') + + @Container + @FailOnUnmatchedTraces + public static final AppSecContainer CONTAINER = + new AppSecContainer( + workVolume: this.name, + baseTag: 'apache2-fpm-php', + phpVersion: phpVersion, + phpVariant: variant, + www: 'base', + ) + + @BeforeAll + static void beforeAll() { + org.testcontainers.containers.Container.ExecResult res = CONTAINER.execInContainer( + 'bash', '-c', + '''sed -e '/appsec.enabled/d' -e '/appsec.rules=/d' /etc/php/php.ini > /etc/php/php-rc.ini; + kill -9 `pgrep php-fpm`; + export DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS=1; + php-fpm -y /etc/php-fpm.conf -c /etc/php/php-rc.ini''') + assert res.exitCode == 0 + } + + /** + * This test takes a long time (around 10-12 seconds) because the metric + * interval is hardcoded to 10 seconds in the metrics.rs. + */ + @Test + @Order(1) + void 'telemetry data is received'() { + Supplier requestSup = CONTAINER.applyRemoteConfig(RC_TARGET, [ + 'datadog/2/ASM_FEATURES/asm_features_activation/config': [ + asm: [enabled: true] + ] + ]) + + // first request to start helper + // Generally won't be covered by appsec because it doesn't receive RC data in time + // for the response to config_sync + Trace trace = CONTAINER.traceFromRequest('/hello.php') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + assert trace.traceId != null + + RemoteConfigRequest rcReq = requestSup.get() + assert rcReq != null, 'No RC request received' + + // request covered by Appsec; no attack + trace = CONTAINER.traceFromRequest('/hello.php') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + // now do an attack + HttpRequest req = CONTAINER.buildReq('/hello.php') + .header('User-Agent', 'Arachni/v1').GET().build() + trace = CONTAINER.traceFromRequest(req, ofString()) { HttpResponse resp -> + assert resp.body().size() > 0 + } + assert trace.traceId != null + + TelemetryHelpers.Metric wafInit + TelemetryHelpers.Metric wafReq1 + TelemetryHelpers.Metric wafReq2 + TelemetryHelpers.Metric rcFirstPull + TelemetryHelpers.Metric rcLastSuccess + + waitForMetrics(30) { List messages -> + def allSeries = messages.collectMany { it.series } + wafInit = allSeries.find { it.name == 'waf.init' } + wafReq1 = allSeries.find { it.name == 'waf.requests' && it.tags.size() == 2 } + wafReq2 = allSeries.find { it.name == 'waf.requests' && it.tags.size() == 3 } + rcFirstPull = allSeries.find { it.name == 'remote_config.first_pull' } + rcLastSuccess = allSeries.find { it.name == 'remote_config.last_success' } + + wafInit && wafReq1 && wafReq2 && rcFirstPull && rcLastSuccess + } + + assert wafInit != null + assert wafInit.namespace == 'appsec' + assert wafInit.points[0][1] == 1.0 + assert 'success:true' in wafInit.tags + assert wafInit.tags.size() == 3 + assert wafInit.type == 'count' + assert wafInit.interval == 10 + + assert wafReq1 != null + assert wafReq1.namespace == 'appsec' + assert wafReq1.points[0][1] >= 1.0 + assert wafReq1.tags.find { it.startsWith('event_rules_version:') } != null + assert wafReq1.tags.find { it.startsWith('waf_version:') } != null + assert wafReq1.type == 'count' + + assert wafReq2 != null + assert 'rule_triggered:true' in wafReq2.tags + assert wafReq2.points[0][1] == 1.0 + + assert rcFirstPull != null + assert rcFirstPull.namespace == 'appsec' + assert rcFirstPull.points[0][1] > 0 + assert rcFirstPull.type == 'gauge' + + assert rcLastSuccess != null + assert rcLastSuccess.namespace == 'appsec' + assert rcLastSuccess.points[0][1] > 0 + assert rcLastSuccess.type == 'gauge' + + // no error for data, it seems + } + + @Test + @Order(2) + void 'telemetry data for failed ddwaf updates'() { + def requestSup = CONTAINER.applyRemoteConfig(RC_TARGET, [ + 'datadog/2/ASM_FEATURES/asm_features_activation/config': [ + asm: [enabled: true] + ], + 'datadog/2/ASM_DATA/bad_data/config': [ + rules_data: [ + [ + id: 'bad_blocked_ips', + type: 'bad_type', + data: [ + [foo: 'bar'] + ] + ] + ] + + ], + 'datadog/2/ASM/custom_user_cfg_2/config': [ + actions: [[ + id: 'blocked_ips', + type: 'bad_block_id', + parameters: [:] + ]], + custom_rules: [[ + id: 'custom_rule_id', + name: 'Bad custom rule id', + tags: [], + conditions: [[ + parameters: [:], + operator: 'bad_operator' + ]], + transformers: ['values_only'], + on_match: ['block_custom'] + + ]], + exclusions: [[ id: 'bad_exclusion' ]], + rules_override: [[ foo: 'bar' ], [ bar: 'foo' ]], + ], + 'datadog/2/ASM_DD/full_cfg/config': + [ + version: '2.1', + metadata: [rules_version: '1.1.1'], + rules: [[ + id: 'blk-001-001', + name: 'Block IP Addresses', + tags: [ + type: 'block_ip', + category: 'attack_attempt' + ], + conditions: [[ + parameters: [ + inputs: [[ address: 'http.client_ip' ]], + data: 'blocked_ips', + ], + operator: 'ip_match' + ]], + transformers: [], + on_match: ['block'] + ], + [ + id: 'bad rule' + ] + ] + ] + ]) + + def messages = waitForMetrics(30) { List messages -> + def allSeries = messages + .collectMany { it.series } + .findAll { + it.name == 'waf.config_errors' + } + + allSeries.size() >= 3 + } + + assert requestSup.get() != null + + def series = messages + .collectMany { it.series } + .findAll { + it.name == 'waf.config_errors' + } + + series.each { + assert 'event_rules_version:1.1.1' in it.tags + assert 'scope:item' in it.tags + } + + def rulesOverride = series.find { + it.namespace == 'appsec' && 'config_key:rules_override' in it.tags + } + def exclusions = series.find { + it.namespace == 'appsec' && 'config_key:exclusions' in it.tags + } + def customRules = series.find { + it.namespace == 'appsec' && 'config_key:custom_rules' in it.tags + } + def rules = series.find { + it.namespace == 'appsec' && 'config_key:rules' in it.tags + } + + assert rulesOverride.points[0][1] == 2.0d + assert exclusions.points[0][1] == 1.0d + assert customRules.points[0][1] == 1.0d + assert rules.points[0][1] == 1.0d + + // TODO: no error for ASM_DATA. Bad data never reaches ddwaf_update + } + + private List waitForMetrics(int timeoutSec, Closure cl) { + List messages = [] + def deadline = System.currentTimeSeconds() + timeoutSec + def lastHttpReq = System.currentTimeSeconds() - 6 + while (System.currentTimeSeconds() < deadline) { + if (System.currentTimeSeconds() - lastHttpReq > 5) { + lastHttpReq = System.currentTimeSeconds() + // used to flush global (not request-bound) telemetry metrics + def request = CONTAINER.buildReq('/hello.php').GET().build() + def trace = CONTAINER.traceFromRequest(request, ofString()) { HttpResponse resp -> + assert resp.body().size() > 0 + } + } + def telData = CONTAINER.drainTelemetry(500) + messages.addAll( + TelemetryHelpers.filterMessages(telData, TelemetryHelpers.GenerateMetrics)) + if (cl.call(messages)) { + break + } + } + messages + } +} diff --git a/components-rs/ddtrace.h b/components-rs/ddtrace.h index 8bef6a8387..72e36566fd 100644 --- a/components-rs/ddtrace.h +++ b/components-rs/ddtrace.h @@ -259,6 +259,7 @@ ddog_MaybeError ddog_sidecar_telemetry_buffer_flush(struct ddog_SidecarTransport void ddog_sidecar_telemetry_register_metric_buffer(struct ddog_SidecarActionsBuffer *buffer, ddog_CharSlice metric_name, + enum ddog_MetricType metric_type, enum ddog_MetricNamespace namespace_); void ddog_sidecar_telemetry_add_span_metric_point_buffer(struct ddog_SidecarActionsBuffer *buffer, diff --git a/components-rs/telemetry.rs b/components-rs/telemetry.rs index b62e825cdb..53d82e9e76 100644 --- a/components-rs/telemetry.rs +++ b/components-rs/telemetry.rs @@ -4,7 +4,7 @@ use ddcommon_ffi::slice::AsBytes; use ddcommon_ffi::{CharSlice, MaybeError, self as ffi}; use ddcommon::tag::parse_tags; use ddtelemetry::data; -use ddtelemetry::data::metrics::MetricNamespace; +use ddtelemetry::data::metrics::{MetricNamespace, MetricType}; use ddtelemetry::data::{Dependency, Integration}; use ddtelemetry::metrics::MetricContext; use ddtelemetry::worker::TelemetryActions; @@ -138,13 +138,14 @@ pub extern "C" fn ddog_sidecar_telemetry_buffer_flush( pub unsafe extern "C" fn ddog_sidecar_telemetry_register_metric_buffer( buffer: &mut SidecarActionsBuffer, metric_name: CharSlice, + metric_type: MetricType, namespace: MetricNamespace, ) { buffer.buffer.push(SidecarAction::RegisterTelemetryMetric(MetricContext { name: metric_name.to_utf8_lossy().into_owned(), namespace, - metric_type: data::metrics::MetricType::Count, + metric_type, tags: Vec::default(), common: true, })); diff --git a/ddtrace.sym b/ddtrace.sym index d20803d5d3..f0493dd9d5 100644 --- a/ddtrace.sym +++ b/ddtrace.sym @@ -7,6 +7,8 @@ ddtrace_user_req_add_listeners ddtrace_ip_extraction_find ddtrace_set_all_thread_vm_interrupt ddtrace_remote_config_get_path +ddtrace_metric_register_buffer +ddtrace_metric_add_point ddog_remote_config_reader_for_path ddog_remote_config_read ddog_remote_config_reader_drop diff --git a/ext/otel_config.c b/ext/otel_config.c index 5a7638b4c0..f61ef22974 100644 --- a/ext/otel_config.c +++ b/ext/otel_config.c @@ -11,7 +11,8 @@ ZEND_EXTERN_MODULE_GLOBALS(ddtrace); static void report_otel_cfg_telemetry_invalid(const char *otel_cfg, const char *dd_cfg, bool pre_rinit) { if (!pre_rinit && ddtrace_sidecar && get_DD_INSTRUMENTATION_TELEMETRY_ENABLED()) { ddog_SidecarActionsBuffer *buffer = ddtrace_telemetry_buffer(); - ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("tracers.otel.env.invalid"), DDOG_METRIC_NAMESPACE_TRACERS); + ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("tracers.otel.env.invalid"), DDOG_METRIC_TYPE_COUNT, + DDOG_METRIC_NAMESPACE_TRACERS); ddog_CharSlice tags; tags.len = asprintf((char **)&tags.ptr, "config_opentelemetry:%s,config_datadog:%s", otel_cfg, dd_cfg); ddog_sidecar_telemetry_add_span_metric_point_buffer(buffer, DDOG_CHARSLICE_C("tracers.otel.env.invalid"), 1, tags); diff --git a/ext/telemetry.c b/ext/telemetry.c index 333b6a81fa..75f5be5b09 100644 --- a/ext/telemetry.c +++ b/ext/telemetry.c @@ -12,6 +12,8 @@ ZEND_EXTERN_MODULE_GLOBALS(ddtrace); zend_long dd_composer_hook_id; ddog_QueueId dd_bgs_queued_id; +static void dd_commit_metrics(void); + ddog_SidecarActionsBuffer *ddtrace_telemetry_buffer(void) { if (DDTRACE_G(telemetry_buffer)) { return DDTRACE_G(telemetry_buffer); @@ -49,9 +51,12 @@ void ddtrace_telemetry_register_services(ddog_SidecarTransport *sidecar) { } ddog_SidecarActionsBuffer *buffer = ddog_sidecar_telemetry_buffer_alloc(); - ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("trace_api.requests"), DDOG_METRIC_NAMESPACE_TRACERS); - ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("trace_api.responses"), DDOG_METRIC_NAMESPACE_TRACERS); - ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("trace_api.errors"), DDOG_METRIC_NAMESPACE_TRACERS); + ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("trace_api.requests"), DDOG_METRIC_TYPE_COUNT, + DDOG_METRIC_NAMESPACE_TRACERS); + ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("trace_api.responses"), DDOG_METRIC_TYPE_COUNT, + DDOG_METRIC_NAMESPACE_TRACERS); + ddog_sidecar_telemetry_register_metric_buffer(buffer, DDOG_CHARSLICE_C("trace_api.errors"), DDOG_METRIC_TYPE_COUNT, + DDOG_METRIC_NAMESPACE_TRACERS); // FIXME: it seems we must call "enqueue_actions" (even with an empty list of actions) for things to work properly ddtrace_ffi_try("Failed flushing background sender telemetry buffer", @@ -115,7 +120,7 @@ void ddtrace_telemetry_finalize(void) { // Telemetry metrics ddog_CharSlice metric_name = DDOG_CHARSLICE_C("spans_created"); - ddog_sidecar_telemetry_register_metric_buffer(buffer, metric_name, DDOG_METRIC_NAMESPACE_TRACERS); + ddog_sidecar_telemetry_register_metric_buffer(buffer, metric_name, DDOG_METRIC_TYPE_COUNT, DDOG_METRIC_NAMESPACE_TRACERS); zend_string *integration_name; zval *metric_value; ZEND_HASH_FOREACH_STR_KEY_VAL(&DDTRACE_G(telemetry_spans_created_per_integration), integration_name, metric_value) { @@ -125,7 +130,7 @@ void ddtrace_telemetry_finalize(void) { } ZEND_HASH_FOREACH_END(); metric_name = DDOG_CHARSLICE_C("logs_created"); - ddog_sidecar_telemetry_register_metric_buffer(buffer, metric_name, DDOG_METRIC_NAMESPACE_GENERAL); + ddog_sidecar_telemetry_register_metric_buffer(buffer, metric_name, DDOG_METRIC_TYPE_COUNT, DDOG_METRIC_NAMESPACE_GENERAL); static struct { ddog_CharSlice level; ddog_CharSlice tags; @@ -143,6 +148,8 @@ void ddtrace_telemetry_finalize(void) { } } + dd_commit_metrics(); + ddtrace_ffi_try("Failed flushing telemetry buffer", ddog_sidecar_telemetry_buffer_flush(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), buffer)); @@ -256,3 +263,34 @@ void ddtrace_telemetry_send_trace_api_metrics(trace_api_metrics metrics) { ddtrace_ffi_try("Failed flushing background sender metrics", ddog_sidecar_telemetry_buffer_flush(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &dd_bgs_queued_id, buffer)); } + +ZEND_TLS ddog_SidecarActionsBuffer *metrics_buffer; + +DDTRACE_PUBLIC void ddtrace_metric_register_buffer(zend_string *name, ddog_MetricType type, ddog_MetricNamespace ns) +{ + if (!metrics_buffer) { + metrics_buffer = ddog_sidecar_telemetry_buffer_alloc(); + } + + ddog_CharSlice metric_name = dd_zend_string_to_CharSlice(name); + ddog_sidecar_telemetry_register_metric_buffer(metrics_buffer, metric_name, type, ns); +} + +DDTRACE_PUBLIC bool ddtrace_metric_add_point(zend_string *name, double value, zend_string *tags) { + if (!metrics_buffer) { + metrics_buffer = ddog_sidecar_telemetry_buffer_alloc(); + } + ddog_CharSlice metric_name = dd_zend_string_to_CharSlice(name); + ddog_CharSlice tags_slice = dd_zend_string_to_CharSlice(tags); + ddog_sidecar_telemetry_add_span_metric_point_buffer(metrics_buffer, metric_name, value, tags_slice); + return true; +} + +static void dd_commit_metrics() { + if (!metrics_buffer) { + return; + } + ddog_sidecar_telemetry_buffer_flush( + &ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), metrics_buffer); + metrics_buffer = NULL; +} diff --git a/ext/telemetry.h b/ext/telemetry.h index 8d204fe9b8..78c97d12dd 100644 --- a/ext/telemetry.h +++ b/ext/telemetry.h @@ -2,7 +2,11 @@ #define DDTRACE_TELEMETRY_H #include -#include "ddtrace.h" +#include + +#include "components-rs/common.h" +#include "ddtrace_export.h" +#include "span.h" typedef struct _trace_api_metrics { int requests; @@ -27,4 +31,8 @@ void ddtrace_telemetry_register_services(ddog_SidecarTransport *sidecar); void ddtrace_telemetry_inc_spans_created(ddtrace_span_data *span); void ddtrace_telemetry_send_trace_api_metrics(trace_api_metrics metrics); +// public API +DDTRACE_PUBLIC void ddtrace_metric_register_buffer(zend_string *name, ddog_MetricType type, ddog_MetricNamespace ns); +DDTRACE_PUBLIC bool ddtrace_metric_add_point(zend_string *name, double value, zend_string *tags); + #endif // DDTRACE_TELEMETRY_H diff --git a/libdatadog b/libdatadog index f363618c74..9c345412c4 160000 --- a/libdatadog +++ b/libdatadog @@ -1 +1 @@ -Subproject commit f363618c74f039e77fece732e68b9f9faddd9113 +Subproject commit 9c345412c40179e8990fd24d2c236eff68a29062