From b30d87e9567628634dd250a313b3481ecaaa2c45 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Wed, 2 Oct 2024 18:34:15 +0100 Subject: [PATCH] Telemetry in AppSec --- Cargo.lock | 1 + appsec/src/extension/commands/client_init.c | 7 + appsec/src/extension/commands_helpers.c | 201 +++++++++--- appsec/src/extension/commands_helpers.h | 1 + appsec/src/extension/ddtrace.c | 54 ++++ appsec/src/extension/ddtrace.h | 25 ++ appsec/src/extension/request_abort.c | 1 - appsec/src/helper/.clang-tidy | 13 + 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.hpp | 113 +++++++ appsec/src/helper/network/acceptor.cpp | 4 +- appsec/src/helper/network/acceptor.hpp | 6 +- appsec/src/helper/network/broker.cpp | 1 - appsec/src/helper/network/broker.hpp | 8 +- appsec/src/helper/network/proto.hpp | 14 +- appsec/src/helper/network/socket.hpp | 2 - appsec/src/helper/remote_config/client.cpp | 5 +- .../helper/remote_config/client_handler.cpp | 20 +- .../helper/remote_config/client_handler.hpp | 22 +- .../listeners/engine_listener.cpp | 17 +- .../listeners/engine_listener.hpp | 12 +- appsec/src/helper/runner.cpp | 9 +- appsec/src/helper/runner.hpp | 5 +- appsec/src/helper/service.cpp | 26 +- appsec/src/helper/service.hpp | 134 +++++++- 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 | 260 +++++++++++---- 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 | 48 +-- appsec/tests/helper/common.hpp | 3 + appsec/tests/helper/engine_test.cpp | 167 ++++++---- .../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 | 218 +++++++------ .../src/docker/apache2-mod/Dockerfile | 4 + .../appsec/php/TelemetryHelpers.groovy | 74 ++++- .../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/Symfony62Tests.groovy | 11 +- .../php/integration/TelemetryTests.groovy | 303 ++++++++++++++++++ .../www/base/public/custom_integrations.php | 24 ++ components-rs/common.h | 16 +- components-rs/crashtracker.h | 10 + 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 +- 67 files changed, 2046 insertions(+), 718 deletions(-) create mode 100644 appsec/src/helper/.clang-tidy 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 create mode 100644 appsec/tests/integration/src/test/www/base/public/custom_integrations.php diff --git a/Cargo.lock b/Cargo.lock index 8dca58a0e6..14963fd9b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1984,6 +1984,7 @@ dependencies = [ "datadog-trace-protobuf", "ddcommon 0.0.1", "http 0.2.11", + "hyper 0.14.28", "serde", "tracing", ] diff --git a/appsec/src/extension/commands/client_init.c b/appsec/src/extension/commands/client_init.c index 20cbef6502..6b6e7350c8 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..c63f5d97dc 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,103 @@ 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 == LSTRLEN(name) && memcmp(key_str, name, key_len) == 0) { \ + static zend_string *_Atomic key_zstr; \ + _init_zstr(&key_zstr, name, LSTRLEN(name)); \ + 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 459fa0ac1d..3ebe26c140 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 deff440f7e..f2993af4fd 100644 --- a/appsec/src/extension/ddtrace.c +++ b/appsec/src/extension/ddtrace.c @@ -19,6 +19,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; @@ -31,6 +36,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); @@ -48,6 +54,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(); @@ -109,6 +120,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); } @@ -123,6 +147,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(); @@ -145,6 +170,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 = @@ -516,3 +551,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 7615854df8..79d6f430b9 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/extension/request_abort.c b/appsec/src/extension/request_abort.c index af1a876891..3ad3f17be3 100644 --- a/appsec/src/extension/request_abort.c +++ b/appsec/src/extension/request_abort.c @@ -12,7 +12,6 @@ #include "compatibility.h" #include "configuration.h" #include "ddappsec.h" -#include "dddefs.h" #include "ddtrace.h" #include "logging.h" #include "php_compat.h" diff --git a/appsec/src/helper/.clang-tidy b/appsec/src/helper/.clang-tidy new file mode 100644 index 0000000000..de8680d420 --- /dev/null +++ b/appsec/src/helper/.clang-tidy @@ -0,0 +1,13 @@ +Checks: 'readability-identifier-naming' + +CheckOptions: + - key: readability-identifier-naming.StructCase + value: lower_case + - key: readability-identifier-naming.StructPrefix + value: '' + - key: readability-identifier-naming.ClassCase + value: lower_case + - key: readability-identifier-naming.ClassPrefix + value: '' + +InheritParentConfig: true diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index 4e98218bb6..c7a62090e8 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -13,8 +13,10 @@ #include "action.hpp" #include "client.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; @@ -23,6 +25,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) @@ -152,18 +159,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 @@ -187,8 +190,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)) { @@ -342,13 +348,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 = @@ -430,15 +430,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()) { @@ -457,8 +454,8 @@ void client::update_remote_config_path(std::string_view path, 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() @@ -498,4 +495,78 @@ void client::run(worker::queue_consumer &q) SPDLOG_DEBUG("Finished handling client"); } +namespace { + +struct request_metrics_submitter : public metrics::telemetry_submitter { + request_metrics_submitter() = default; + ~request_metrics_submitter() override = default; + request_metrics_submitter(const request_metrics_submitter &) = delete; + request_metrics_submitter &operator=( + const request_metrics_submitter &) = delete; + request_metrics_submitter(request_metrics_submitter &&) = delete; + request_metrics_submitter &operator=(request_metrics_submitter &&) = delete; + + void submit_metric(std::string_view name, double value, + metrics::telemetry_tags tags) override + { + std::string tags_s = tags.consume(); + SPDLOG_TRACE("submit_metric [req]: name={}, value={}, tags={}", name, + value, tags_s); + tel_metrics[name].emplace_back(value, std::move(tags_s)); + }; + void submit_span_metric(std::string_view name, double value) override + { + SPDLOG_TRACE( + "submit_span_metric [req]: name={}, value={}", name, value); + metrics[name] = value; + }; + void submit_span_meta(std::string_view name, std::string value) override + { + SPDLOG_TRACE("submit_span_meta [req]: name={}, value={}", name, value); + meta[std::string{name}] = value; + }; + void submit_span_meta_copy_key(std::string name, std::string value) override + { + SPDLOG_TRACE( + "submit_span_meta_copy_key [req]: name={}, value={}", name, value); + meta[name] = value; + } + + std::map meta; + std::map metrics; + std::unordered_map>> + tel_metrics; +}; + +template +void collect_metrics_impl(Response &response, service &service, + std::optional &context) +{ + request_metrics_submitter msubmitter{}; + if (context) { + context->get_metrics(msubmitter); + } + service.drain_metrics( + [&msubmitter](std::string_view name, double value, auto 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..01057df313 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::telemetry_submitter &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::telemetry_submitter &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::telemetry_submitter &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..dd41b4c70c 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::telemetry_submitter &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::telemetry_submitter &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::telemetry_submitter &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 c22df8e8d0..f3914341d1 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.hpp b/appsec/src/helper/metrics.hpp new file mode 100644 index 0000000000..31585edeb0 --- /dev/null +++ b/appsec/src/helper/metrics.hpp @@ -0,0 +1,113 @@ +// 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 { + +class telemetry_tags { +public: + telemetry_tags &add(std::string_view key, std::string_view value) + { + data_.reserve(data_.size() + key.size() + value.size() + 2); + if (!data_.empty()) { + data_ += ','; + } + data_ += key; + data_ += ':'; + data_ += value; + return *this; + } + std::string consume() { return std::move(data_); } + + // the rest of the methods are for testing + static telemetry_tags from_string(std::string str) + { + telemetry_tags tags; + tags.data_ = std::move(str); + return tags; + } + + bool operator==(const telemetry_tags &other) const + { + return data_ == other.data_; + } + + friend std::ostream &operator<<( + std::ostream &os, const telemetry_tags &tags) + { + os << tags.data_; + return os; + } + +private: + std::string data_; + + friend struct fmt::formatter; +}; + +struct telemetry_submitter { + telemetry_submitter() = default; + telemetry_submitter(const telemetry_submitter &) = delete; + telemetry_submitter &operator=(const telemetry_submitter &) = delete; + telemetry_submitter(telemetry_submitter &&) = delete; + telemetry_submitter &operator=(telemetry_submitter &&) = delete; + + virtual ~telemetry_submitter() = 0; + // first arguments of type string_view should have static storage + virtual void submit_metric(std::string_view, double, telemetry_tags) = 0; + virtual void submit_span_metric(std::string_view, double) = 0; + virtual void submit_span_meta(std::string_view, std::string) = 0; + void submit_span_meta(std::string, std::string) = delete; + virtual void submit_span_meta_copy_key(std::string, std::string) = 0; + void submit_span_meta_copy_key(std::string_view, std::string) = delete; +}; +inline telemetry_submitter::~telemetry_submitter() = default; + +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"; + +// 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 + +template <> +struct fmt::formatter + : fmt::formatter { + + auto format( + const dds::metrics::telemetry_tags tags, format_context &ctx) const + { + return formatter::format( + std::string_view{tags.data_}, ctx); + } +}; diff --git a/appsec/src/helper/network/acceptor.cpp b/appsec/src/helper/network/acceptor.cpp index 2c2a567d57..2cb714c617 100644 --- a/appsec/src/helper/network/acceptor.cpp +++ b/appsec/src/helper/network/acceptor.cpp @@ -4,7 +4,7 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "acceptor.hpp" -#include "../exception.hpp" +#include "socket.hpp" #include #include #include @@ -75,7 +75,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.cpp b/appsec/src/helper/network/broker.cpp index 48405c44b1..94154576c7 100644 --- a/appsec/src/helper/network/broker.cpp +++ b/appsec/src/helper/network/broker.cpp @@ -7,7 +7,6 @@ #include "../exception.hpp" #include "proto.hpp" #include -#include #include #include #include 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..e6205392dc 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,11 @@ struct client_init { std::map meta; std::map metrics; + std::unordered_map>> + tel_metrics; - MSGPACK_DEFINE(status, version, errors, meta, metrics); + MSGPACK_DEFINE(status, version, errors, meta, metrics, tel_metrics); }; }; @@ -286,8 +292,12 @@ struct request_shutdown { std::map meta; std::map metrics; + std::unordered_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.cpp b/appsec/src/helper/remote_config/client.cpp index 5b851b4bb3..df22dcf001 100644 --- a/appsec/src/helper/remote_config/client.cpp +++ b/appsec/src/helper/remote_config/client.cpp @@ -4,10 +4,7 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "client.hpp" -#include "exception.hpp" #include "product.hpp" -#include -#include #include #include #include @@ -18,7 +15,7 @@ extern "C" { } namespace { -struct ddog_CharSlice { +struct ddog_CharSlice { // NOLINT(readability-identifier-naming) const char *ptr; uintptr_t len; }; diff --git a/appsec/src/helper/remote_config/client_handler.cpp b/appsec/src/helper/remote_config/client_handler.cpp index d61f999255..8a6652e06e 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,11 +65,13 @@ 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() { + const std::lock_guard lock{mutex_}; + try { rc_client_->poll(); } catch (const std::exception &e) { diff --git a/appsec/src/helper/remote_config/client_handler.hpp b/appsec/src/helper/remote_config/client_handler.hpp index 2a88649b00..e265b364f9 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,28 @@ 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 }; } // 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..2a6f099290 100644 --- a/appsec/src/helper/remote_config/listeners/engine_listener.cpp +++ b/appsec/src/helper/remote_config/listeners/engine_listener.cpp @@ -4,23 +4,22 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "engine_listener.hpp" -#include "../../json_helper.hpp" #include "../exception.hpp" #include "../product.hpp" #include "config_aggregators/asm_aggregator.hpp" #include "config_aggregators/asm_data_aggregator.hpp" #include "config_aggregators/asm_dd_aggregator.hpp" -#include #include #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 +81,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..d58828bf35 100644 --- a/appsec/src/helper/remote_config/listeners/engine_listener.hpp +++ b/appsec/src/helper/remote_config/listeners/engine_listener.hpp @@ -5,27 +5,24 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #pragma once -#include "../../config.hpp" #include "../../engine.hpp" -#include "../../parameter.hpp" #include "../product.hpp" #include "config_aggregators/config_aggregator.hpp" #include "listener.hpp" -#include #include -#include 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 +43,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 8567db2a11..efd8e116b8 100644 --- a/appsec/src/helper/runner.cpp +++ b/appsec/src/helper/runner.cpp @@ -6,7 +6,6 @@ #include "runner.hpp" #include "client.hpp" -#include "subscriber/waf.hpp" #include #include #include @@ -32,7 +31,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 +89,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} { @@ -161,7 +162,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 b11b217a9f..6919c28eba 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; @@ -49,7 +50,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..21f554a5c2 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..198ad6c752 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,110 @@ namespace dds { using namespace std::chrono_literals; class service { -public: +protected: + class metrics_impl : public metrics::telemetry_submitter { + struct tel_metric { + tel_metric(std::string_view name, double value, + metrics::telemetry_tags tags) + : name{name}, value{value}, tags{std::move(tags)} + {} + std::string_view name; + double value; + metrics::telemetry_tags tags; + }; + + public: + metrics_impl() = default; + metrics_impl(const metrics_impl &) = delete; + metrics_impl &operator=(const metrics_impl &) = delete; + metrics_impl(metrics_impl &&) = delete; + metrics_impl &operator=(metrics_impl &&) = delete; + + ~metrics_impl() override = default; + + void submit_metric(std::string_view metric_name, double value, + metrics::telemetry_tags 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_span_metric(std::string_view name, double value) override + { + SPDLOG_TRACE("submit_span_metric: {} {}", name, value); + const std::lock_guard lock{legacy_metrics_mutex_}; + legacy_metrics_[name] = value; + } + void submit_span_meta(std::string_view name, std::string value) override + { + SPDLOG_TRACE("submit_span_meta: {} {}", name, value); + const std::lock_guard lock{meta_mutex_}; + meta_[std::string{name}] = std::move(value); + } + + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + void submit_span_meta_copy_key( + std::string name, std::string value) override + { + SPDLOG_TRACE("submit_span_meta_copy_key: {} {}", name, value); + 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 +138,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 +164,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..b3e7999332 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::telemetry_submitter &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::telemetry_submitter &submit_metric) = 0; }; } // namespace dds diff --git a/appsec/src/helper/subscriber/waf.cpp b/appsec/src/helper/subscriber/waf.cpp index 01401fcda5..de70ebdd96 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 @@ -124,9 +124,107 @@ 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) +metrics::telemetry_tags waf_update_init_report_tags( + bool success, std::optional rules_version) +{ + metrics::telemetry_tags tags; + if (success) { + tags.add("success", "true"); + } else { + tags.add("success", "false"); + } + if (rules_version) { + tags.add("event_rules_version", rules_version.value()); + } + tags.add("waf_version", ddwaf_get_version()); + return tags; +} + +void waf_init_report(metrics::telemetry_submitter &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::telemetry_submitter &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::telemetry_submitter &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_common = [&]() { + metrics::telemetry_tags tags; + tags.add("waf_version", ddwaf_get_version()) + .add("event_rules_version", ruleset_version) + .add("config_key", k); + return tags; + }; + if (map.contains("error")) { + metrics::telemetry_tags tags{tags_common()}; + tags.add("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(); + } + + metrics::telemetry_tags tags{tags_common()}; + tags.add("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::telemetry_submitter &msubmitter) { try { const parameter_view diagnostics_view{diagnostics}; @@ -137,24 +235,24 @@ 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_span_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_span_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_span_meta( + metrics::event_rules_errors, parameter_to_json(it->second)); } } - meta[std::string(tag::waf_version)] = ddwaf_get_version(); + msubmitter.submit_span_meta(metrics::waf_version, ddwaf_get_version()); auto version_it = info.find("ruleset_version"); if (version_it != info.end()) { @@ -173,9 +271,13 @@ 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_.add("event_rules_version", ruleset_version_); + base_tags_.add("waf_version", ddwaf_get_version()); +} instance::listener::listener(instance::listener &&other) noexcept : handle_{other.handle_}, waf_timeout_{other.waf_timeout_} @@ -221,7 +323,6 @@ void instance::listener::call(dds::parameter_view &data, event &event) fmt::underlying(code), parameter_to_json(parameter_view{res.actions}), parameter_to_json(parameter_view{res.derivatives})); - } else { run_waf(); } @@ -232,6 +333,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 derivatives{res.derivatives}; for (const auto &derivative : derivatives) { @@ -245,15 +358,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; @@ -262,18 +380,33 @@ 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::telemetry_submitter &msubmitter) { - meta[std::string(tag::event_rules_version)] = ruleset_version_; - metrics[tag::waf_duration] = total_runtime_; + metrics::telemetry_tags tags = base_tags_; + if (rule_triggered_) { + tags.add("rule_triggered", "true"); + } + if (request_blocked_) { + tags.add("request_blocked", "true"); + } + if (waf_hit_timeout_) { + tags.add("waf_timeout", "true"); + } + if (waf_run_error_) { + tags.add("waf_run_error", "true"); + } + msubmitter.submit_metric(metrics::waf_requests, 1.0, std::move(tags)); + + // span tags/metrics + msubmitter.submit_span_meta( + metrics::event_rules_version, std::string{ruleset_version_}); + msubmitter.submit_span_metric(metrics::waf_duration, total_runtime_); for (const auto &[key, value] : derivatives_) { std::string derivative = value; if (value.length() > max_plain_schema_allowed && key.starts_with("_dd.appsec.s.")) { - auto encoded = compress(derivative); if (encoded) { derivative = base64_encode(encoded.value(), false); @@ -281,15 +414,17 @@ void instance::listener::get_meta_and_metrics( } if (derivative.length() <= max_schema_size) { - meta.emplace(key, std::move(derivative)); + msubmitter.submit_span_meta_copy_key(key, std::move(derivative)); + } else { + SPDLOG_WARN("Schema for key {} is too large to submit", key); } } } -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::telemetry_submitter &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}; @@ -297,9 +432,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); @@ -314,8 +452,22 @@ 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::telemetry_submitter &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_)) { @@ -350,63 +502,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::telemetry_submitter &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::telemetry_submitter &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::telemetry_submitter &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 2b6f9c9e68..b4ed99d00b 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::telemetry_submitter &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 derivatives_; + metrics::telemetry_tags 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::telemetry_submitter &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::telemetry_submitter &msubmitter) override; static std::unique_ptr from_settings( const engine_settings &settings, const engine_ruleset &ruleset, - std::map &meta, - std::map &metrics); + metrics::telemetry_submitter &msubmitter); // testing only static std::unique_ptr from_string(std::string_view rule, - std::map &meta, - std::map &metrics, + metrics::telemetry_submitter &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::telemetry_submitter &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::telemetry_submitter &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 ffcdc539c0..b521c393a7 100644 --- a/appsec/tests/helper/client_test.cpp +++ b/appsec/tests/helper/client_test.cpp @@ -5,17 +5,18 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "common.hpp" #include "parameter.hpp" -#include #include #include #include #include +#include #include #include +#include +#include #include #include #include -#include #include namespace dds { @@ -37,8 +38,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)); }; @@ -47,7 +46,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"} {} }; @@ -156,16 +156,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.20.1"); - 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) @@ -195,7 +195,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)); @@ -229,7 +229,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)); @@ -265,11 +265,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.20.1"); + 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'")); @@ -279,8 +279,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) @@ -817,10 +817,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(), 3); 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"); } } @@ -861,10 +861,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"); } } @@ -2069,7 +2069,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)); @@ -2085,7 +2085,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)); @@ -2101,7 +2101,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 6fbde24eab..c4c760a7bb 100644 --- a/appsec/tests/helper/common.hpp +++ b/appsec/tests/helper/common.hpp @@ -22,10 +22,13 @@ using ::testing::DoAll; using ::testing::ElementsAre; using ::testing::Invoke; using ::testing::MatchesRegex; +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 06a8b0c576..7518d2af03 100644 --- a/appsec/tests/helper/engine_test.cpp +++ b/appsec/tests/helper/engine_test.cpp @@ -3,8 +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 @@ -20,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::telemetry_submitter &)); }; class subscriber : public dds::subscriber { @@ -30,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::telemetry_submitter &)); +}; + +class tel_submitter : public metrics::telemetry_submitter { +public: + MOCK_METHOD(void, submit_metric, + (std::string_view, double, metrics::telemetry_tags), (override)); + MOCK_METHOD( + void, submit_span_metric, (std::string_view, double), (override)); + MOCK_METHOD( + void, submit_span_meta, (std::string_view, std::string), (override)); + MOCK_METHOD(void, submit_span_meta_copy_key, (std::string, std::string), + (override)); }; } // namespace mock @@ -339,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{}; + + // the number of loaded rules is 2 due to rules added unconditionally by us + EXPECT_CALL( + msubmitter, submit_span_metric("_dd.appsec.event_rules.loaded"sv, 2.0)); + EXPECT_CALL(msubmitter, + submit_span_metric("_dd.appsec.event_rules.error_count"sv, 0.0)); + EXPECT_CALL(msubmitter, + submit_span_meta("_dd.appsec.event_rules.errors"sv, std::string{"{}"})); + EXPECT_CALL(msubmitter, submit_span_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"); @@ -358,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); @@ -371,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(); @@ -386,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(); @@ -404,6 +424,7 @@ TEST(EngineTest, WafSubscriptorTimeout) TEST(EngineTest, MockSubscriptorsUpdateRuleData) { + auto mock_submitter = mock::tel_submitter{}; auto e{engine::create()}; auto ignorer = []() { @@ -418,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("")); @@ -429,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("")); @@ -437,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(); @@ -456,6 +474,7 @@ TEST(EngineTest, MockSubscriptorsUpdateRuleData) TEST(EngineTest, MockSubscriptorsInvalidRuleData) { + auto msubmitter = mock::tel_submitter{}; auto e{engine::create()}; auto ignorer = []() { @@ -465,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(); @@ -486,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(); @@ -500,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(); @@ -517,10 +535,20 @@ TEST(EngineTest, WafSubscriptorUpdateRuleData) } { + EXPECT_CALL( + msubmitter, submit_span_meta("_dd.appsec.waf.version"sv, _)); + EXPECT_CALL(msubmitter, + submit_metric("waf.updates"sv, 1, + metrics::telemetry_tags::from_string( + 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); } { @@ -536,9 +564,20 @@ TEST(EngineTest, WafSubscriptorUpdateRuleData) } { + EXPECT_CALL( + msubmitter, submit_span_meta("_dd.appsec.waf.version"sv, _)); + EXPECT_CALL(msubmitter, + submit_metric("waf.updates"sv, 1, + metrics::telemetry_tags::from_string( + 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); } { @@ -554,11 +593,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(); @@ -571,9 +609,18 @@ TEST(EngineTest, WafSubscriptorInvalidRuleData) } { + EXPECT_CALL(submitter, + submit_metric("waf.updates"sv, 1, + metrics::telemetry_tags::from_string( + 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); } { @@ -589,11 +636,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(); @@ -608,7 +654,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); } { @@ -626,11 +672,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(); @@ -647,7 +692,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); } { @@ -663,7 +708,7 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverride) { engine_ruleset update(R"({"rules_override": []})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -680,11 +725,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(); @@ -704,7 +748,7 @@ TEST(EngineTest, WafSubscriptorUpdateRuleOverrideAndActions) "on_match": ["redirect"]}], "actions": [{"id": "redirect", "type": "redirect_request", "parameters": {"status_code": "303", "location": "https://localhost"}}]})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -723,7 +767,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); } { @@ -741,11 +785,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(); @@ -763,7 +806,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); } { @@ -779,7 +822,7 @@ TEST(EngineTest, WafSubscriptorExclusions) { engine_ruleset update(R"({"exclusions": []})"); - e->update(update, meta, metrics); + e->update(update, msubmitter); } { @@ -796,11 +839,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(); @@ -827,7 +868,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); } { @@ -856,7 +897,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 48281376ed..2a61080ff9 100644 --- a/appsec/tests/helper/remote_config/listeners/engine_listener_test.cpp +++ b/appsec/tests/helper/remote_config/listeners/engine_listener_test.cpp @@ -8,6 +8,7 @@ #include "../mocks.hpp" #include "engine.hpp" #include "json_helper.hpp" +#include "metrics.hpp" #include "remote_config/exception.hpp" #include "remote_config/listeners/engine_listener.hpp" #include "subscriber/waf.hpp" @@ -19,6 +20,7 @@ const std::string waf_rule = namespace dds::remote_config { +using dds::mock::tel_submitter; using mock::get_config; namespace { @@ -38,22 +40,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); @@ -62,15 +68,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(); @@ -92,15 +100,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(); @@ -122,18 +133,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(); @@ -163,18 +176,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)); @@ -212,18 +227,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(); @@ -254,18 +271,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)); @@ -303,11 +322,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))); @@ -315,7 +336,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(); @@ -346,11 +367,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))); @@ -358,7 +381,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)); @@ -396,11 +419,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))); @@ -410,7 +435,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(); @@ -441,11 +466,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))); @@ -455,7 +482,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)); @@ -493,18 +520,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(); @@ -527,18 +556,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)); @@ -568,15 +600,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)); { @@ -623,15 +657,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)); @@ -774,10 +811,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(); @@ -795,7 +834,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(); @@ -821,10 +860,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(); @@ -838,7 +880,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(); @@ -856,13 +899,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(); { @@ -903,13 +946,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(); @@ -954,13 +997,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(); @@ -1007,13 +1050,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(); @@ -1055,13 +1098,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(); @@ -1166,12 +1209,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..32cbd2d52f 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" @@ -22,9 +24,7 @@ class engine : public dds::engine { : dds::engine(trace_rate_limit) {} MOCK_METHOD(void, update, - (engine_ruleset &, (std::map &), - (std::map &)), - (override)); + (engine_ruleset &, metrics::telemetry_submitter &), (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..712a56bc99 --- /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::telemetry_submitter { +public: + MOCK_METHOD(void, submit_metric, + (std::string_view, double, dds::metrics::telemetry_tags), (override)); + MOCK_METHOD( + void, submit_span_metric, (std::string_view, double), (override)); + MOCK_METHOD( + void, submit_span_meta, (std::string_view, std::string), (override)); + MOCK_METHOD(void, submit_span_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 8be992ecf0..48a31ef90e 100644 --- a/appsec/tests/helper/waf_test.cpp +++ b/appsec/tests/helper/waf_test.cpp @@ -3,16 +3,18 @@ // // 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 -#include #include const std::string waf_rule = @@ -43,37 +45,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_span_meta(metrics::waf_version, + std::string{ddwaf_get_version()})); + std::string rules_errors; + EXPECT_CALL(submitm, submit_span_meta(metrics::event_rules_errors, _)) + .WillOnce(SaveArg<1>(&rules_errors)); + + EXPECT_CALL(submitm, submit_span_metric(metrics::event_rules_loaded, 1.0)); + EXPECT_CALL(submitm, submit_span_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., + metrics::telemetry_tags::from_string( + 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.20.1"); + 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; @@ -82,11 +91,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(); @@ -100,11 +108,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(); @@ -114,18 +120,26 @@ 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_span_meta(metrics::event_rules_version, std::string{"1.2.3"})); + double duration; + EXPECT_CALL(submitm, submit_span_metric(metrics::waf_duration, _)) + .WillOnce(SaveArg<1>(&duration)); + EXPECT_CALL( + submitm, submit_metric("waf.requests"sv, 1, + metrics::telemetry_tags::from_string( + 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(); @@ -144,19 +158,28 @@ 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_span_meta(metrics::event_rules_version, std::string{"1.2.3"})); + EXPECT_CALL(submitm, submit_span_metric(metrics::waf_duration, _)); + EXPECT_CALL( + submitm, submit_metric("waf.requests"sv, 1, + metrics::telemetry_tags::from_string( + std::string{"event_rules_version:1.2.3,waf_version:"} + + ddwaf_get_version() + ",rule_triggered:true"))); + EXPECT_CALL( + submitm, submit_span_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(); @@ -178,18 +201,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(); @@ -197,7 +213,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(); @@ -219,19 +235,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(); @@ -252,7 +263,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(); @@ -282,15 +293,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); { @@ -306,16 +317,20 @@ 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, + metrics::telemetry_tags::from_string( + 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(); @@ -333,22 +348,31 @@ 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, + metrics::telemetry_tags::from_string( + std::string{"event_rules_version:1.2.3,waf_version:"} + + ddwaf_get_version() + ",rule_triggered:true"))); + EXPECT_CALL( + submitm, submit_span_meta("_dd.appsec.event_rules.version", "1.2.3")); + EXPECT_CALL(submitm, submit_span_metric("_dd.appsec.waf.duration"sv, _)); + EXPECT_CALL( + submitm, submit_span_meta_copy_key( + std::string{"_dd.appsec.s.arg2"}, std::string{"[8]"})); + ctx->submit_metrics(submitm); + Mock::VerifyAndClearExpectations(&submitm); } TEST(WafTest, FingerprintAreNotAdded) { - std::map meta; - std::map metrics; + NiceMock submitm{}; engine_settings settings; settings.rules_file = create_sample_rules_ok(); auto ruleset = engine_ruleset::from_path(settings.rules_file); std::shared_ptr wi{ - waf::instance::from_settings(settings, ruleset, meta, metrics)}; + waf::instance::from_settings(settings, ruleset, submitm)}; auto ctx = wi->get_listener(); auto p = parameter::map(); @@ -357,25 +381,23 @@ TEST(WafTest, FingerprintAreNotAdded) dds::event e; ctx->call(pv, e); - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_FALSE(meta.empty()); - EXPECT_STREQ(meta["_dd.appsec.fp.http.endpoint"].c_str(), ""); - EXPECT_STREQ(meta["_dd.appsec.fp.http.network"].c_str(), ""); - EXPECT_STREQ(meta["_dd.appsec.fp.http.header"].c_str(), ""); - EXPECT_STREQ(meta["_dd.appsec.fp.fp.session"].c_str(), ""); + EXPECT_CALL(submitm, + submit_span_meta_copy_key(MatchesRegex("_dd\\.appsec\\.fp\\..+"), _)) + .Times(0); + ctx->submit_metrics(submitm); + Mock::VerifyAndClearExpectations(&submitm); } TEST(WafTest, FingerprintAreAdded) { - std::map meta; - std::map metrics; + NiceMock submitm{}; engine_settings settings; settings.rules_file = create_sample_rules_ok(); auto ruleset = engine_ruleset::from_path(settings.rules_file); std::shared_ptr wi{ - waf::instance::from_settings(settings, ruleset, meta, metrics)}; + waf::instance::from_settings(settings, ruleset, submitm)}; auto ctx = wi->get_listener(); auto p = parameter::map(); @@ -402,25 +424,23 @@ TEST(WafTest, FingerprintAreAdded) dds::event e; ctx->call(pv, e); - ctx->get_meta_and_metrics(meta, metrics); - EXPECT_FALSE(meta.empty()); - EXPECT_THAT(meta["_dd.appsec.fp.http.endpoint"].c_str(), - MatchesRegex("http-get(-[A-Za-z0-9]*){3}")); - - EXPECT_THAT(meta["_dd.appsec.fp.http.network"].c_str(), - MatchesRegex("net-[0-9]*-[a-zA-Z0-9]*")); - - EXPECT_THAT(meta["_dd.appsec.fp.http.header"].c_str(), - MatchesRegex("hdr(-[0-9]*-[a-zA-Z0-9]*){2}")); - - EXPECT_THAT(meta["_dd.appsec.fp.session"].c_str(), - MatchesRegex("ssn(-[a-zA-Z0-9]*){4}")); + EXPECT_CALL( + submitm, submit_span_meta_copy_key("_dd.appsec.fp.http.endpoint", + testing::MatchesRegex("http-get(-[A-Za-z0-9]*){3}"))); + EXPECT_CALL(submitm, submit_span_meta_copy_key("_dd.appsec.fp.http.network", + testing::MatchesRegex("net-[0-9]*-[a-zA-Z0-9]*"))); + EXPECT_CALL( + submitm, submit_span_meta_copy_key("_dd.appsec.fp.http.header", + testing::MatchesRegex("hdr(-[0-9]*-[a-zA-Z0-9]*){2}"))); + EXPECT_CALL(submitm, submit_span_meta_copy_key("_dd.appsec.fp.session", + testing::MatchesRegex("ssn(-[a-zA-Z0-9]*){4}"))); + 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)); @@ -431,7 +451,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(); @@ -470,7 +490,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(); @@ -509,7 +529,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(); @@ -543,7 +563,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/docker/apache2-mod/Dockerfile b/appsec/tests/integration/src/docker/apache2-mod/Dockerfile index c15985d1fd..5905fafdea 100644 --- a/appsec/tests/integration/src/docker/apache2-mod/Dockerfile +++ b/appsec/tests/integration/src/docker/apache2-mod/Dockerfile @@ -26,6 +26,10 @@ RUN if echo $VARIANT | grep -q zts; \ else sed -i "s/%MPM/prefork/" /etc/apache2/mods-available/php.load; \ fi RUN if ! { echo $VARIANT | grep -q zts; }; then a2dismod mpm_event; a2enmod mpm_prefork; fi + +RUN sed -i 's/MaxRequestWorkers.*/MaxRequestWorkers 1/' /etc/apache2/mods-available/mpm_prefork.conf +RUN sed -i 's/MaxRequestWorkers.*/MaxRequestWorkers 1/' /etc/apache2/mods-available/mpm_worker.conf + RUN a2enmod php RUN chmod a+rx /root diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy index 493cc4b6ce..241c113010 100644 --- a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy +++ b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/TelemetryHelpers.groovy @@ -1,15 +1,18 @@ package com.datadog.appsec.php +/** + * @link https://github.com/DataDog/instrumentation-telemetry-api-docs/blob/main/GeneratedDocumentation/ApiDocs/v2/producing-telemetry.md + */ class TelemetryHelpers { static List filterMessages(List telemetryData, Class type) { - telemetryData.findAll { it['request_type'] == type.name } + + (telemetryData.findAll { it['request_type'] in type.names } + telemetryData.findAll { it['request_type'] == 'message-batch'} - *.payload*.findAll { it['request_type'] == 'generate-metrics' }.flatten() + *.payload*.findAll { it['request_type'] in type.names }.flatten()) .collect { type.newInstance([it['payload']] as Object[]) } } static class GenerateMetrics { - static name = 'generate-metrics' + static names = ['generate-metrics'] List series GenerateMetrics(Map m) { @@ -36,4 +39,69 @@ class TelemetryHelpers { interval = m.interval } } + + static class WithConfiguration { + static names = ['app-started', 'app-client-configuration-change'] + List configuration + + WithConfiguration(Map m) { + configuration = m.configuration?.collect { new ConfigurationEntry(it) } + } + } + + static class WithDependencies { + static names = ['app-started', 'app-dependencies-loaded'] + List dependencies + + WithDependencies(Map m) { + dependencies = m.dependencies?.collect { new DependencyEntry(it) } + } + } + + static class WithIntegrations { + static names = ['app-started', 'app-integrations-change'] + List integrations + + WithIntegrations(Map m) { + integrations = m.integrations?.collect { new IntegrationEntry(it) } + } + } + + static class ConfigurationEntry { + String name + String value + String origin + + ConfigurationEntry(Map m) { + name = m.name + value = m.value + origin = m.origin + } + } + + static class DependencyEntry { + String name + String version + + DependencyEntry(Map m) { + name = m.name + version = m.version + } + } + + static class IntegrationEntry { + String name + Boolean enabled + String version + Boolean compatible + Boolean autoEnabled + + IntegrationEntry(Map m) { + name = m.name + enabled = m.enabled + version = m.version + compatible = m.compatible + autoEnabled = m.autoEnabled + } + } } 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 0ca7be22f3..ed8036751d 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 } @@ -350,55 +346,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/Symfony62Tests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony62Tests.groovy index dd205114bb..5ae5e1be81 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony62Tests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony62Tests.groovy @@ -5,8 +5,10 @@ import com.datadog.appsec.php.docker.FailOnUnmatchedTraces import com.datadog.appsec.php.docker.InspectContainerHelper import com.datadog.appsec.php.model.Span import com.datadog.appsec.php.model.Trace -import groovy.util.logging.Slf4j +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.EnabledIf import org.testcontainers.junit.jupiter.Container import org.testcontainers.junit.jupiter.Testcontainers @@ -20,6 +22,7 @@ import static java.net.http.HttpResponse.BodyHandlers.ofString @Testcontainers @EnabledIf('isExpectedVersion') +@TestMethodOrder(MethodOrderer.OrderAnnotation) class Symfony62Tests { static boolean expectedVersion = phpVersion.contains('8.1') && !variant.contains('zts') @@ -42,6 +45,12 @@ class Symfony62Tests { www: 'symfony62', ) + @Test + @Order(1) + void 'reported telemetry integrations are not repeated'() { + + } + @Test void 'login success automated event'() { //The user ciuser@example.com is already on the DB 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..6745b97230 --- /dev/null +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy @@ -0,0 +1,303 @@ +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 + + 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 } + + wafInit && wafReq1 && wafReq2 + } + + 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 + + // 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 + } + + @Test + @Order(3) + void 'telemetry reflects the loading of a new integration'() { + def trace = CONTAINER.traceFromRequest('/custom_integrations.php?integrations[]=redis') { + HttpResponse resp -> assert resp.statusCode() == 200 + } + + List allIntegrations = [] + boolean foundRedis = false + waitForIntegrations(30) { List messages -> + allIntegrations.addAll(messages.collectMany { it.integrations }) + foundRedis = allIntegrations.find { it.name == 'phpredis' && it.enabled == Boolean.TRUE } != null + } + assert foundRedis + + trace = CONTAINER.traceFromRequest('/custom_integrations.php?integrations[]=exec&integrations[]=redis') { + HttpResponse resp -> assert resp.statusCode() == 200 + } + allIntegrations = [] + foundRedis = false + boolean foundExec = false + waitForIntegrations(15) { List messages -> + allIntegrations.addAll(messages.collectMany { it.integrations }) + foundRedis = allIntegrations.find { it.name == 'phpredis' && it.enabled == Boolean.TRUE } != null + foundExec = allIntegrations.find { it.name == 'exec' && it.enabled == Boolean.TRUE } != null + } + + assert !foundRedis + assert foundExec + } + + private static List waitForMetrics(int timeoutSec, Closure cl) { + waitForTelemetryData(timeoutSec, cl, TelemetryHelpers.GenerateMetrics) + } + + private static List waitForIntegrations(int timeoutSec, Closure cl) { + waitForTelemetryData(timeoutSec, cl, TelemetryHelpers.WithIntegrations) + } + + private static List waitForTelemetryData(int timeoutSec, Closure cl, Class cls) { + 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, cls)) + if (cl.call(messages)) { + break + } + } + messages + } +} diff --git a/appsec/tests/integration/src/test/www/base/public/custom_integrations.php b/appsec/tests/integration/src/test/www/base/public/custom_integrations.php new file mode 100644 index 0000000000..1925342a63 --- /dev/null +++ b/appsec/tests/integration/src/test/www/base/public/custom_integrations.php @@ -0,0 +1,24 @@ + -#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 ce9ae2824a..b7a056ca56 160000 --- a/libdatadog +++ b/libdatadog @@ -1 +1 @@ -Subproject commit ce9ae2824a2ec8fc041490da26a3b54efdfe691c +Subproject commit b7a056ca566d65964606014fcd496da5db4a4273