Move helper to sidecar remote config (#2864)

* Use sidecar Remote Config impl in AppSec helper

* wip

* Fixes after review

* More review changes

* Helper: check shmem against process euid, rather than uid

* Remove stray test

* Suppress appsec on certain tracer tests

* Fix Appsec integration tests

---------

Co-authored-by: Anil Mahtani <929854+Anilm3@users.noreply.github.com>

.circleci/continue_config.yml

@@ -1412,8 +1412,13 @@ jobs:
             export DEBIAN_FRONTEND=noninteractive
             apt update
             apt install -y wget sudo git g++ gcc gcovr cmake make curl libcurl4-gnutls-dev clang clang-tidy clang-format git php-dev php8.2-xml php-cgi
-            curl https://sh.rustup.rs -sSf | sh -s -- --profile minimal -y --default-toolchain "1.76.0"
-            echo 'export PATH=${PATH}:$HOME/.cargo/bin' >> $BASH_ENV
+      - run:
+          name: Install rust
+          command: |
+            curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > /tmp/rustup.sh
+            chmod +x /tmp/rustup.sh
+            /tmp/rustup.sh -y --default-toolchain 1.76
+            sudo ln -s $HOME/.cargo/bin/* /usr/bin/
       - run: git config --global --add safe.directory /home/circleci/datadog/appsec/third_party/libddwaf
       - run:
           name: CMake
@@ -1424,7 +1429,7 @@ jobs:
       - run:
           name: Test
           command: |
-            make -C appsec/build -j $(nproc) xtest ddappsec_helper_test
+            PATH=$PATH:$HOME/.cargo/bin make -C appsec/build -j $(nproc) xtest ddappsec_helper_test
             ./appsec/build/tests/helper/ddappsec_helper_test
       - run:
           name: Generate XML coverage
@@ -3857,7 +3862,7 @@ jobs:
               php datadog-setup.php --file "${installable_bundle}" --php-bin php --enable-profiling
               # run phpize just to get run-tests.php
               phpize
-              php run-tests.php -p $(which php) --show-diff -g "FAIL,XFAIL,BORK,WARN,LEAK,XLEAK,SKIP" tests/ext/profiling
+              php run-tests.php -p $(which php) -d datadog.remote_config_enabled=false --show-diff -g "FAIL,XFAIL,BORK,WARN,LEAK,XLEAK,SKIP" tests/ext/profiling
 
   "cbindgen up-to-date":
     working_directory: ~/datadog

appsec/cmake/helper.cmake

@@ -20,7 +20,7 @@ set_target_properties(helper_objects PROPERTIES
     CXX_STANDARD 20
     CXX_STANDARD_REQUIRED YES
     POSITION_INDEPENDENT_CODE 1)
-target_include_directories(helper_objects PUBLIC ${HELPER_INCLUDE_DIR})
+target_include_directories(helper_objects INTERFACE ${HELPER_INCLUDE_DIR})
 target_compile_definitions(helper_objects PUBLIC SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE)
 target_compile_options(helper_objects PRIVATE -ftls-model=global-dynamic)
 target_link_libraries(helper_objects PUBLIC libddwaf_objects pthread spdlog cpp-base64 msgpack_c RapidJSON::rapidjson Boost::system zlibstatic)
@@ -35,6 +35,8 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
     # Bind symbols lookup of symbols defined in the library to the library itself
     # also avoids relocation problems with libc++.a on linux/aarch64
     target_link_options(ddappsec-helper PRIVATE -Wl,-Bsymbolic)
+elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
+    target_link_options(ddappsec-helper PRIVATE -undefined dynamic_lookup)
 endif()
 set_target_properties(ddappsec-helper PROPERTIES
     CXX_VISIBILITY_PRESET hidden

appsec/src/extension/backtrace.c

@@ -48,7 +48,8 @@ php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-easil
     if (file) {
         // In order to be able to test full path encoded everywhere lets set
         // only the file name without path
-        char *file_name = memrchr(Z_STRVAL_P(file), '/', Z_STRLEN_P(file));
+        const char *file_name =
+            zend_memrchr(Z_STRVAL_P(file), '/', Z_STRLEN_P(file));
         if (file_name) {
             zend_string *new_file = zend_string_init(file_name + 1,
                 Z_STRLEN_P(file) - (file_name + 1 - Z_STRVAL_P(file)), 0);

appsec/src/extension/commands/client_init.c

@@ -17,68 +17,20 @@
 #include "../version.h"
 #include "client_init.h"
 
-static const unsigned int DEFAULT_AGENT_PORT = 8126;
-static const char *DEFAULT_AGENT_HOST = "127.0.0.1";
-static const unsigned int MAX_TCP_PORT_ALLOWED = UINT16_MAX;
-
 static dd_result _pack_command(mpack_writer_t *nonnull w, void *nullable ctx);
 static dd_result _process_response(mpack_node_t root, void *nullable ctx);
 static void _process_meta_and_metrics(
     mpack_node_t root, struct req_info *nonnull ctx);
-static void _pack_agent_details(mpack_writer_t *nonnull w);
 
 static const dd_command_spec _spec = {
     .name = "client_init",
     .name_len = sizeof("client_init") - 1,
-    .num_args = 7,
+    .num_args = 6,
     .outgoing_cb = _pack_command,
     .incoming_cb = _process_response,
     .config_features_cb = dd_command_process_config_features_unexpected,
 };
 
-static void _pack_agent_details(mpack_writer_t *nonnull w)
-{
-    zend_string *agent_host = get_global_DD_AGENT_HOST();
-    zend_string *agent_url = get_global_DD_TRACE_AGENT_URL();
-    unsigned int port = get_global_DD_TRACE_AGENT_PORT();
-    char *host = NULL;
-    php_url *parsed_url = NULL;
-
-    if (agent_host && ZSTR_LEN(agent_host) > 0) {
-        host = ZSTR_VAL(agent_host);
-    } else if (agent_url && ZSTR_LEN(agent_url) > 0) {
-        parsed_url = php_url_parse(ZSTR_VAL(agent_url));
-        if (parsed_url) {
-#if PHP_VERSION_ID < 70300
-            if (parsed_url->host && strlen(parsed_url->host) > 0) {
-                host = parsed_url->host;
-            }
-#else
-            if (parsed_url->host && ZSTR_LEN(parsed_url->host) > 0) {
-                host = ZSTR_VAL(parsed_url->host);
-            }
-#endif
-            port = parsed_url->port;
-        }
-    }
-
-    if (!host) {
-        host = (char *)DEFAULT_AGENT_HOST;
-    }
-    if (port <= 0 || port > MAX_TCP_PORT_ALLOWED) {
-        port = DEFAULT_AGENT_PORT;
-    }
-
-    dd_mpack_write_lstr(w, "host");
-    dd_mpack_write_nullable_cstr(w, host);
-    dd_mpack_write_lstr(w, "port");
-    mpack_write_uint(w, port);
-
-    if (parsed_url) {
-        php_url_free(parsed_url);
-    }
-}
-
 dd_result dd_client_init(dd_conn *nonnull conn, struct req_info *nonnull ctx)
 {
     return dd_command_exec_cred(conn, &_spec, ctx);
@@ -97,39 +49,6 @@ static dd_result _pack_command(
         mpack_write_bool(w, DDAPPSEC_G(active));
     }
 
-    // Service details
-    // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
-    mpack_start_map(w, 6);
-
-    dd_mpack_write_lstr(w, "service");
-    dd_mpack_write_nullable_cstr(w, ZSTR_VAL(get_DD_SERVICE()));
-
-    dd_mpack_write_lstr(w, "extra_services");
-    zval extra_services;
-    ZVAL_ARR(&extra_services, get_global_DD_EXTRA_SERVICES());
-    dd_mpack_write_zval(w, &extra_services);
-
-    dd_mpack_write_lstr(w, "env");
-    dd_mpack_write_nullable_cstr(w, ZSTR_VAL(get_DD_ENV()));
-
-    dd_mpack_write_lstr(w, "tracer_version");
-    dd_mpack_write_nullable_cstr(w, dd_trace_version());
-
-    dd_mpack_write_lstr(w, "app_version");
-    dd_mpack_write_nullable_cstr(w, ZSTR_VAL(get_DD_VERSION()));
-
-    // We send this empty for now. The helper will check for empty and if so it
-    // will generate it
-    dd_mpack_write_lstr(w, "runtime_id");
-    zend_string *runtime_id = dd_trace_get_formatted_runtime_id(false);
-    if (runtime_id == NULL) {
-        dd_mpack_write_nullable_cstr(w, "");
-    } else {
-        dd_mpack_write_nullable_zstr(w, runtime_id);
-        zend_string_free(runtime_id);
-    }
-    mpack_finish_map(w);
-
     // Engine settings
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
     mpack_start_map(w, 6);
@@ -180,15 +99,13 @@ static dd_result _pack_command(
 
     // Remote config settings
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
-    mpack_start_map(w, 4);
+    mpack_start_map(w, 2);
 
     dd_mpack_write_lstr(w, "enabled");
     mpack_write_bool(w, get_DD_REMOTE_CONFIG_ENABLED());
 
-    _pack_agent_details(w);
-
-    dd_mpack_write_lstr(w, "poll_interval");
-    mpack_write_u32(w, get_DD_REMOTE_CONFIG_POLL_INTERVAL());
+    dd_mpack_write_lstr(w, "shmem_path");
+    dd_mpack_write_nullable_cstr(w, dd_trace_remote_config_get_path());
 
     mpack_finish_map(w);
 

appsec/src/extension/commands/config_sync.c

@@ -8,32 +8,36 @@
 #include <php.h>
 
 #include "../commands_helpers.h"
+#include "../ddtrace.h"
+#include "../msgpack_helpers.h"
+#include "config_sync.h"
 #include <mpack.h>
 
-static dd_result _request_pack(
-    mpack_writer_t *nonnull w, void *nullable ATTR_UNUSED ctx);
+static dd_result _request_pack(mpack_writer_t *nonnull w, void *nonnull ctx);
 dd_result dd_command_process_config_sync(
     mpack_node_t root, ATTR_UNUSED void *unspecnull ctx);
 
 static const dd_command_spec _spec = {
     .name = "config_sync",
     .name_len = sizeof("config_sync") - 1,
-    .num_args = 0, // a single map
+    .num_args = 1,
     .outgoing_cb = _request_pack,
     .incoming_cb = dd_command_process_config_sync,
     .config_features_cb = dd_command_process_config_features,
 };
 
-dd_result dd_config_sync(dd_conn *nonnull conn)
+dd_result dd_config_sync(
+    dd_conn *nonnull conn, const struct config_sync_data *nonnull data)
 {
-    return dd_command_exec(conn, &_spec, NULL);
+    return dd_command_exec(conn, &_spec, (void *)data);
 }
 
-static dd_result _request_pack(
-    mpack_writer_t *nonnull w, void *nullable ATTR_UNUSED ctx)
+static dd_result _request_pack(mpack_writer_t *nonnull w, void *nonnull ctx_)
 {
-    UNUSED(ctx);
-    UNUSED(w);
+    const struct config_sync_data *nonnull data =
+        (struct config_sync_data *)ctx_;
+
+    dd_mpack_write_nullable_cstr(w, data->rem_cfg_path);
 
     return dd_success;
 }

appsec/src/extension/commands/config_sync.h

@@ -7,4 +7,9 @@
 
 #include "../network.h"
 
-dd_result dd_config_sync(dd_conn *nonnull conn);
+struct config_sync_data {
+    char *nullable rem_cfg_path;
+};
+
+dd_result dd_config_sync(
+    dd_conn *nonnull conn, const struct config_sync_data *nonnull data);

appsec/src/extension/ddappsec.c

@@ -45,6 +45,7 @@
 #include "user_tracking.h"
 
 #include <json/json.h>
+#include <zend_string.h>
 
 #if ZTS
 static atomic_int _thread_count;
@@ -100,7 +101,7 @@ static zend_extension ddappsec_extension_entry = {
     PHP_DDAPPSEC_EXTNAME,
     PHP_DDAPPSEC_VERSION,
     "Datadog",
-    "https://github.com/DataDog/dd-appsec-php",
+    "https://github.com/DataDog/dd-trace-php",
     "Copyright Datadog",
     ddappsec_startup,
     NULL,
@@ -253,6 +254,21 @@ void dd_appsec_rinit_once()
     pthread_once(&_rinit_once_control, _rinit_once);
 }
 
+static void _warn_on_empty_service_or_env()
+{
+    if (!get_global_DD_APPSEC_TESTING() && get_DD_REMOTE_CONFIG_ENABLED() &&
+        DDAPPSEC_G(enabled) != APPSEC_FULLY_DISABLED &&
+        (zend_string_equals_literal(get_DD_ENV(), "") ||
+            zend_string_equals_literal(get_DD_SERVICE(), ""))) {
+        mlog(dd_log_warning,
+            "AppSec is not disabled and Datadog service or env is empty. "
+            "Please set DD_SERVICE and DD_ENV rather than setting the "
+            "corresponding properties on the root span. Otherwise, remote "
+            "configuration for AppSec will use service=unnamed-php-service and "
+            "env=none");
+    }
+}
+
 // NOLINTNEXTLINE
 static PHP_RINIT_FUNCTION(ddappsec)
 {
@@ -265,6 +281,7 @@ static PHP_RINIT_FUNCTION(ddappsec)
     dd_appsec_rinit_once();
     zai_config_rinit();
     _check_enabled();
+    _warn_on_empty_service_or_env();
 
     if (DDAPPSEC_G(enabled) == APPSEC_FULLY_DISABLED) {
         return SUCCESS;
@@ -378,6 +395,23 @@ static void _check_enabled()
     };
 }
 
+__attribute__((visibility("default"))) void dd_appsec_rc_conf(
+    bool *nonnull appsec_features, bool *nonnull appsec_conf) // NOLINT
+{
+    bool prev_enabled = DDAPPSEC_G(enabled);
+    bool prev_active = DDAPPSEC_G(active);
+    bool prev_to_be_configured = DDAPPSEC_G(to_be_configured);
+    _check_enabled();
+
+    *appsec_features = DDAPPSEC_G(enabled) == APPSEC_ENABLED_VIA_REMCFG;
+    // only enable ASM / ASM_DD / ASM_DATA if no rules file is specified
+    *appsec_conf = get_global_DD_APPSEC_RULES()->len == 0;
+
+    DDAPPSEC_G(enabled) = prev_enabled;
+    DDAPPSEC_G(active) = prev_active;
+    DDAPPSEC_G(to_be_configured) = prev_to_be_configured;
+}
+
 static PHP_FUNCTION(datadog_appsec_is_enabled)
 {
     if (zend_parse_parameters_none() == FAILURE) {

appsec/src/extension/ddappsec.h

@@ -56,6 +56,9 @@ extern __thread void *unspecnull ATTR_TLS_LOCAL_DYNAMIC TSRMLS_CACHE;
 void dd_appsec_rinit_once(void);
 int dd_appsec_rshutdown(bool ignore_verdict);
 
+__attribute__((visibility("default"))) void dd_appsec_rc_conf(
+    bool *nonnull appsec_features, bool *nonnull appsec_conf); // NOLINT
+
 // Add a NO_CACHE version.
 // Use tsrm_get_ls_cache() instead of thread-local _tsrmls_ls_cache
 #ifdef ZTS

appsec/src/extension/ddappsec.version

@@ -2,5 +2,6 @@
 	global:
 		get_module;
 		dd_appsec_maybe_enable_helper;
+		dd_appsec_rc_conf;
 	local: *;
 };

appsec/src/extension/ddtrace.c

@@ -45,6 +45,8 @@ static bool (*nullable _ddtrace_user_req_add_listeners)(
 
 static zend_string *(*_ddtrace_ip_extraction_find)(zval *server);
 
+static const char *nullable (*_ddtrace_remote_config_get_path)(void);
+
 static void dd_trace_load_symbols(void)
 {
     bool testing = get_global_DD_APPSEC_TESTING();
@@ -98,6 +100,14 @@ static void dd_trace_load_symbols(void)
             dlerror()); // NOLINT(concurrency-mt-unsafe)
     }
 
+    _ddtrace_remote_config_get_path =
+        dlsym(handle, "ddtrace_remote_config_get_path");
+    if (_ddtrace_remote_config_get_path == NULL && !testing) {
+        mlog(dd_log_error,
+            // NOLINTNEXTLINE(concurrency-mt-unsafe)
+            "Failed to load ddtrace_remote_config_get_path: %s", dlerror());
+    }
+
     dlclose(handle);
 }
 
@@ -358,6 +368,16 @@ zend_string *nullable dd_ip_extraction_find(zval *nonnull server)
     return _ddtrace_ip_extraction_find(server);
 }
 
+const char *nullable dd_trace_remote_config_get_path()
+{
+    if (!_ddtrace_remote_config_get_path) {
+        return NULL;
+    }
+    __auto_type path = _ddtrace_remote_config_get_path();
+    mlog(dd_log_trace, "Remote config path: %s", path ? path : "(unset)");
+    return path;
+}
+
 static PHP_FUNCTION(datadog_appsec_testing_ddtrace_rshutdown)
 {
     if (zend_parse_parameters_none() == FAILURE) {

appsec/src/extension/ddtrace.h

@@ -79,3 +79,5 @@ bool dd_trace_user_req_add_listeners(
     ddtrace_user_req_listeners *nonnull listeners);
 
 zend_string *nullable dd_ip_extraction_find(zval *nonnull server);
+
+const char *nullable dd_trace_remote_config_get_path(void);

appsec/src/extension/helper_process.c

@@ -129,9 +129,8 @@ dd_conn *nullable dd_helper_mgr_cur_conn(void)
     return NULL;
 }
 
-bool dd_on_runtime_path_update(
-    // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
-    zval *nullable old_val, zval *nonnull new_val,
+// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
+bool dd_on_runtime_path_update(zval *nullable old_val, zval *nonnull new_val,
     zend_string *nullable new_str)
 {
     UNUSED(old_val);