Merge pull request #2443 from DataDog/glopes/roadrunner

Roadrunner appsec support

.circleci/continue_config.yml

@@ -1085,6 +1085,24 @@ jobs:
           name: Test
           command: make -C appsec/build -j $(nproc) xtest
 
+  test_appsec_integration:
+    parameters:
+      resource_class:
+        type: string
+        default: large
+      targets:
+        type: string
+    working_directory: ~/datadog
+    <<: *BARE_DOCKER_MACHINE
+    steps:
+      - <<: *STEP_CHECKOUT
+      - <<: *STEP_ATTACH_WORKSPACE
+      - run:
+          name: Integration tests
+          command: |
+            cd appsec/tests/integration && \
+            TERM=dumb ./gradlew loadCaches << parameters.targets >> --info -Pbuildscan --scan
+
   hunter_cache_ubuntu:
     parameters:
       resource_class:
@@ -4524,6 +4542,16 @@ workflows:
             switch_php_version:
               - debug-zts-asan
 
+    - test_appsec_integration:
+        matrix:
+          parameters:
+            resource_class:
+              - large
+            targets:
+              - test7.0-release test7.0-release-zts test7.1-release test7.1-release-zts test7.2-release test7.2-release-zts
+              - test7.3-release test7.3-release-zts test7.4-release test7.4-release-zts test8.0-release test8.0-release-zts
+              - test8.1-release test8.1-release-zts test8.2-release test8.2-release-zts test8.3-release test8.3-release-zts
+
     - coverage_appsec:
         requires: [ hunter_cache_ubuntu ]
         matrix:

appsec/CMakeLists.txt

@@ -1,13 +1,14 @@
 cmake_minimum_required(VERSION 3.14)
 
+
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/../cmake/Modules")
 
 option(HUNTER_STATUS_DEBUG "Print Hunter debug info" OFF)
 
 include("cmake/HunterGate.cmake")
 HunterGate(
-    URL "https://github.com/cpp-pm/hunter/archive/v0.23.314.tar.gz"
-    SHA1 "95c47c92f68edb091b5d6d18924baabe02a6962a")
+    URL "https://github.com/cpp-pm/hunter/archive/v0.25.3.tar.gz"
+    SHA1 "0dfbc2cb5c4cf7e83533733bdfd2125ff96680cb")
 
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/hunter-cache.id.in ${CMAKE_CURRENT_SOURCE_DIR}/hunter-cache.id)
 
@@ -30,6 +31,8 @@ option(DD_APPSEC_TESTING "Whether to enable testing" ON)
 
 add_subdirectory(third_party EXCLUDE_FROM_ALL)
 
+include("cmake/patchelf.cmake")
+
 if (DD_APPSEC_BUILD_EXTENSION)
     include("cmake/extension.cmake")
 endif()

appsec/cmake/extension.cmake

@@ -43,7 +43,7 @@ if(COMPILER_HAS_NO_GNU_UNIQUE)
 target_compile_options(extension PRIVATE $<$<COMPILE_LANGUAGE:CXX>:-fno-gnu-unique>)
 endif()
 target_compile_options(extension PRIVATE $<$<COMPILE_LANGUAGE:CXX>:-fno-rtti -fno-exceptions>)
-target_compile_options(extension PRIVATE -Wall -Wextra -Wno-unused-parameter)
+target_compile_options(extension PRIVATE -Wall -Wextra -Werror)
 # our thread local variables are only used by ourselves
 target_compile_options(extension PRIVATE -ftls-model=local-dynamic)
 
@@ -54,9 +54,10 @@ target_linker_flag_conditional(extension -Wl,--as-needed)
 target_linker_flag_conditional(extension "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/ddappsec.version")
 
 # Mac OS
-target_linker_flag_conditional(extension -flat_namespace -undefined suppress)
+target_linker_flag_conditional(extension -flat_namespace "-undefined suppress")
 target_linker_flag_conditional(extension -Wl,-exported_symbol -Wl,_get_module)
 
+patch_away_libc(extension)
 
 if(DD_APPSEC_TESTING)
     if(DD_APPSEC_ENABLE_COVERAGE)

appsec/cmake/patchelf.cmake

@@ -0,0 +1,17 @@
+function(patch_away_libc target)
+    if (NOT ${DD_APPSEC_ENABLE_PATCHELF_LIBC})
+        return()
+    endif()
+
+    if (CMAKE_SYSTEM_NAME STREQUAL Darwin)
+        return()
+    endif()
+
+    find_program(PATCHELF patchelf)
+    if (PATCHELF STREQUAL "PATCHELF-NOTFOUND")
+        message(WARNING "Patchelf not found. Can't build glibc + musl binaries")
+    else()
+        add_custom_command(TARGET ${target} POST_BUILD
+            COMMAND patchelf --remove-needed libc.so $<TARGET_FILE:${target}> ${SYMBOL_FILE})
+    endif()
+endfunction()

appsec/cmake/run-tests-wrapper.sh

@@ -9,6 +9,7 @@ export DD_TRACE_ENABLED=true
 export DD_TRACE_GENERATE_ROOT_SPAN=true
 export DD_TRACE_CLI_ENABLED=true
 export DD_TRACE_AGENT_PORT=18126
+export PHPRC=
 
 CMAKE_BINARY_DIR="$1"
 MOCK_HELPER_BINARY="$2"
@@ -44,7 +45,7 @@ function link_extensions {
     if [[ -L $ddtrace && $(readlink "$ddtrace") != $TRACER_EXT_FILE ]]; then
       rm -v "$ddtrace"
     fi
-    if [[ ! -f $ddtrace ]]; then
+    if [[ ! -L $ddtrace ]]; then
       ln -s -v "$TRACER_EXT_FILE" $ddtrace
     fi
   fi

appsec/cmake/run_tests.cmake

@@ -1,7 +1,8 @@
-set(DD_APPSEC_TRACER_EXT_FILE ${CMAKE_SOURCE_DIR}/../tmp/build_extension/modules/ddtrace.so)
+get_filename_component(DD_APPSEC_TRACER_EXT_FILE "${CMAKE_SOURCE_DIR}/../tmp/build_extension/modules/ddtrace.so" REALPATH)
 
 add_custom_target(ddtrace
-    COMMAND make
+    COMMAND ${CMAKE_COMMAND} -E env "PATH=${PhpConfig_ROOT_DIR}/bin:$ENV{PATH}" PHPRC=
+            make "${DD_APPSEC_TRACER_EXT_FILE}"
     BYPRODUCTS ${DD_APPSEC_TRACER_EXT_FILE}
     WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/../)
 

appsec/run-tests-internal.php

@@ -2842,10 +2842,10 @@ function run_test($php, $file, array $env)
     gdb --args {$cmd}
     ;;
 "valgrind")
-    USE_ZEND_ALLOC=0 valgrind $2 ${cmd}
+    USE_ZEND_ALLOC=0 valgrind $2 {$cmd}
     ;;
 "rr")
-    rr record $2 ${cmd}
+    rr record $2 {$cmd}
     ;;
 *)
     {$cmd}
@@ -3773,7 +3773,7 @@ public function __construct(array $environment, $tool = 'memcheck')
     public function wrapCommand($cmd, $memcheck_filename, $check_all)
     {
         $supp_file = INIT_DIR . "/valgrind.supp";
-        $vcmd = "valgrind -q --tool={$this->tool} --trace-children=no --child-silent-after-fork=yes --leak-check=full " .
+        $vcmd = "valgrind --gen-suppressions=all -q --tool={$this->tool} --trace-children=no --child-silent-after-fork=yes --leak-check=full " .
             "--num-callers=16 --run-libc-freeres=no";
         if (file_exists($supp_file)) {
             $vcmd .= " --suppressions='$supp_file'";

appsec/src/extension/commands/client_init.c

@@ -23,7 +23,8 @@ 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);
+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 = {
@@ -78,24 +79,22 @@ static void _pack_agent_details(mpack_writer_t *nonnull w)
     }
 }
 
-dd_result dd_client_init(dd_conn *nonnull conn)
+dd_result dd_client_init(dd_conn *nonnull conn, struct req_info *nonnull ctx)
 {
-    return dd_command_exec_cred(conn, &_spec, NULL);
+    return dd_command_exec_cred(conn, &_spec, ctx);
 }
 
 static dd_result _pack_command(
     mpack_writer_t *nonnull w, ATTR_UNUSED void *nullable ctx)
 {
-    // unsigned pid, string client_version, runtime_version, rules_file
     mpack_write(w, (uint32_t)getpid());
     dd_mpack_write_lstr(w, PHP_DDAPPSEC_VERSION);
     dd_mpack_write_lstr(w, PHP_VERSION);
 
-    enabled_configuration configuration = DDAPPSEC_G(enabled_by_configuration);
-    if (configuration == NOT_CONFIGURED) {
+    if (DDAPPSEC_G(enabled) == APPSEC_ENABLED_VIA_REMCFG) {
         mpack_write_nil(w);
     } else {
-        mpack_write_bool(w, configuration == ENABLED ? true : false);
+        mpack_write_bool(w, DDAPPSEC_G(active));
     }
 
     // Service details
@@ -206,7 +205,7 @@ static dd_result _process_response(
     mpack_node_t root, ATTR_UNUSED void *nullable ctx)
 {
     // Add any tags and metrics provided by the helper
-    _process_meta_and_metrics(root);
+    _process_meta_and_metrics(root, ctx);
 
     // check verdict
     mpack_node_t verdict = mpack_node_array_at(root, 0);
@@ -246,15 +245,23 @@ static dd_result _process_response(
     return dd_error;
 }
 
-static void _process_meta_and_metrics(mpack_node_t root)
+static void _process_meta_and_metrics(
+    mpack_node_t root, struct req_info *nonnull ctx)
 {
+    zend_object *span = ctx->root_span;
+    if (!span) {
+        mlog(
+            dd_log_debug, "Meta/metrics in client_init ignored (no root span)");
+        return;
+    }
+
     mpack_node_t meta = mpack_node_array_at(root, 3);
     if (mpack_node_map_count(meta) > 0) {
-        dd_command_process_meta(meta);
+        dd_command_process_meta(meta, span);
     }
 
     mpack_node_t metrics = mpack_node_array_at(root, 4);
-    dd_command_process_metrics(metrics);
+    dd_command_process_metrics(metrics, span);
 }
 
 static dd_result _check_helper_version(mpack_node_t root)

appsec/src/extension/commands/client_init.h

@@ -6,5 +6,6 @@
 #pragma once
 
 #include "../network.h"
+#include "../commands_ctx.h"
 
-dd_result dd_client_init(dd_conn *nonnull conn);
+dd_result dd_client_init(dd_conn *nonnull conn, struct req_info *nonnull ctx);

appsec/src/extension/commands/request_exec.c

@@ -12,8 +12,12 @@
 #include <zend_hash.h>
 #include <zend_types.h>
 
-static dd_result _pack_command(
-    mpack_writer_t *nonnull w, ATTR_UNUSED void *nullable ctx);
+struct ctx {
+    struct req_info req_info; // dd_command_proc_resp_verd_span_data expect it
+    zval *nonnull data;
+};
+
+static dd_result _pack_command(mpack_writer_t *nonnull w, void *nonnull ctx);
 
 static const dd_command_spec _spec = {
     .name = "request_exec",
@@ -32,14 +36,17 @@ dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data)
         return dd_error;
     }
 
-    return dd_command_exec(conn, &_spec, (void *)data);
+    struct ctx ctx = {.data = data};
+
+    return dd_command_exec(conn, &_spec, &ctx);
 }
 
-static dd_result _pack_command(
-    mpack_writer_t *nonnull w, ATTR_UNUSED void *nullable ctx)
+static dd_result _pack_command(mpack_writer_t *nonnull w, void *nonnull _ctx)
 {
-    zval *data = (zval *)ctx;
-    dd_mpack_write_zval(w, data);
+    assert(_ctx != NULL);
+    struct ctx *ctx = _ctx;
+
+    dd_mpack_write_zval(w, ctx->data);
 
     return dd_success;
 }