Merge pull request #2773 from DataDog/glopes/helper-to-sidecar

Move appsec helper to sidecar

.circleci/continue_config.yml

@@ -3708,23 +3708,19 @@ jobs:
       - git_checkout
       - append_build_id
       - setup_docker:
-          docker_image: "datadog/libddwaf:toolchain"
+          docker_image: "public.ecr.aws/b1o7r7e0/nginx_musl_toolchain"
       - run: mkdir -p appsec_$(uname -m)
-      - run:
-          name: Create clang symlinks
-          command: |
-            ln -s /usr/bin/clang++-16 /usr/bin/clang++
-            ln -s /usr/bin/clang-16 /usr/bin/clang
-            ln -s /usr/bin/clang-cpp-16 /usr/bin/clang-cpp
       - run:
           name: Build
           command: |
             git config --global --add safe.directory $(pwd)/appsec/third_party/libddwaf
             mkdir -p appsec/build ; cd appsec/build
-            cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDD_APPSEC_BUILD_EXTENSION=OFF -DCMAKE_TOOLCHAIN_FILE=$(pwd)/../cmake/Toolchain.$(uname -m).cmake
+            cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDD_APPSEC_BUILD_EXTENSION=OFF \
+              -DDD_APPSEC_ENABLE_PATCHELF_LIBC=ON \
+              -DCMAKE_TOOLCHAIN_FILE=/sysroot/$(arch)-none-linux-musl/Toolchain.cmake
             make -j $(nproc)
-            objcopy --compress-debug-sections ddappsec-helper
-            cp -v ddappsec-helper ../../appsec_$(uname -m)/ddappsec-helper
+            objcopy --compress-debug-sections libddappsec-helper.so
+            cp -v libddappsec-helper.so ../../appsec_$(uname -m)/libddappsec-helper.so
       - run:
           name: Test
           command: |

Makefile

@@ -46,10 +46,10 @@ M4_FILES = $(shell find m4 -name '*.m4*' | awk '{ printf "$(BUILD_DIR)/%s\n", $$
 XDEBUG_SO_FILE = $(shell find $(shell php-config --extension-dir) -type f -name "xdebug*.so" -exec basename {} \; | tail -n 1)
 
 # Make 'sed -i' portable
-ifeq ($(shell uname),Darwin)
-	SED_I = sed -i ''
-else
+ifeq ($(shell { sed --version 2>&1 || echo ''; } | grep GNU > /dev/null && echo GNU || true),GNU)
 	SED_I = sed -i
+else
+	SED_I = sed -i ''
 endif
 
 all: $(BUILD_DIR)/configure $(SO_FILE)

appsec/README.md

@@ -74,15 +74,15 @@ cd build
 cmake ..
 make -j
 ```
-This will produce the extension, `ddappsec.so` and the helper process `ddappsec-helper`.
+This will produce the extension, `ddappsec.so` and the helper library `libddappsec-helper.so`.
 
 Alternatively, to build the extension but not the helper, you can disable the helper build on the cmake step:
 ```
-cmake .. -DDD_APPSEC_BUILD_HELPER=OFF 
+cmake .. -DDD_APPSEC_BUILD_HELPER=OFF
 ```
 Similarly, to build the helper but not the extension:
 ```
-cmake .. DDD_APPSEC_BUILD_EXTENSION=OFF 
+cmake .. DDD_APPSEC_BUILD_EXTENSION=OFF
 ```
 
 #### Testing the extension

appsec/cmake/ddtrace.cmake

@@ -18,13 +18,39 @@ add_custom_target(libdatadog_stamp
     BYPRODUCT ${LIBDATADOG_STAMP_FILE}
 )
 
-set(EXPORTS_FILE "${CMAKE_BINARY_DIR}/exports.version")
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
+set(EXPORTS_FILE "${CMAKE_BINARY_DIR}/ddtrace_exports.version")
 add_custom_target(ddtrace_exports
     COMMAND bash -c "{ echo -e '{\\nglobal:'; sed 's/$/;/' '${CMAKE_SOURCE_DIR}'/../ddtrace.sym; echo -e 'local:\\n*;\\n};'; } > '${EXPORTS_FILE}'"
     BYPRODUCT ${EXPORTS_FILE}
     DEPENDS ${CMAKE_SOURCE_DIR}/../ddtrace.sym
     VERBATIM
 )
+elseif(APPLE)
+set(EXPORTS_FILE "${CMAKE_BINARY_DIR}/ddtrace_exports.sym")
+add_custom_target(ddtrace_exports
+    COMMAND bash -c "sed 's/^/_/' '${CMAKE_SOURCE_DIR}'/../ddtrace.sym > '${EXPORTS_FILE}'"
+    BYPRODUCT ${EXPORTS_FILE}
+    DEPENDS ${CMAKE_SOURCE_DIR}/../ddtrace.sym
+    VERBATIM
+)
+endif()
+
+file(READ "${CMAKE_SOURCE_DIR}/../VERSION" VERSION_CONTENTS)
+string(STRIP "${VERSION_CONTENTS}" PHP_DDTRACE_VERSION)
+file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/gen_ddtrace/ext")
+set(VERSION_H_PATH "${CMAKE_BINARY_DIR}/gen_ddtrace/ext/version.h")
+
+add_custom_command(
+    OUTPUT "${VERSION_H_PATH}"
+    COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --switch= --green "Updating version.h"
+    COMMAND ${CMAKE_COMMAND} -E remove -f "${VERSION_H_PATH}"
+    COMMAND ${CMAKE_COMMAND} -E touch "${VERSION_H_PATH}"
+    COMMAND printf "\\#ifndef PHP_DDTRACE_VERSION\\\\n\\#define PHP_DDTRACE_VERSION \"%s\"\\\\n\\#endif" "'\"${PHP_DDTRACE_VERSION}\"'" >> "${VERSION_H_PATH}"
+    DEPENDS "${CMAKE_SOURCE_DIR}/../VERSION"
+    COMMENT "Generating version.h"
+)
+add_custom_target(update_version_h ALL DEPENDS "${VERSION_H_PATH}")
 
 ExternalProject_Add(components_rs_proj
     PREFIX ${CMAKE_BINARY_DIR}/components_rs
@@ -99,8 +125,15 @@ set_target_properties(ddtrace PROPERTIES
     OUTPUT_NAME ddtrace
     DEBUG_POSTFIX ""
     PREFIX "")
-target_compile_options(ddtrace PRIVATE -fms-extensions)
-target_link_options(ddtrace PRIVATE "-Wl,--version-script=${EXPORTS_FILE}")
+target_compile_options(ddtrace PRIVATE -fms-extensions -Wno-microsoft-anon-tag)
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
+    target_compile_definitions(ddtrace PRIVATE _GNU_SOURCE)
+    target_link_options(ddtrace PRIVATE "-Wl,--version-script=${EXPORTS_FILE}")
+elseif(APPLE)
+    target_link_options(ddtrace PRIVATE "-exported_symbols_list" "${EXPORTS_FILE}")
+else()
+    message(FATAL_ERROR "Only Linux and Apple supported")
+endif()
 target_link_libraries(ddtrace PRIVATE PhpConfig components_rs ${CURL_LIBRARIES})
 if(CURL_DEFINITIONS)
     target_compile_definitions(ddtrace PRIVATE ${CURL_DEFINITIONS})
@@ -114,7 +147,8 @@ target_include_directories(ddtrace PRIVATE
     ${CMAKE_SOURCE_DIR}/../ext
     ${CMAKE_SOURCE_DIR}/../ext/vendor
     ${CMAKE_SOURCE_DIR}/../ext/vendor/mt19937
+    ${CMAKE_BINARY_DIR}/gen_ddtrace
 )
-add_dependencies(ddtrace ddtrace_exports)
+add_dependencies(ddtrace ddtrace_exports update_version_h)
 
 patch_away_libc(ddtrace)

appsec/cmake/extension.cmake

@@ -24,6 +24,7 @@ set_target_properties(extension PROPERTIES
 target_compile_definitions(extension PRIVATE TESTING=1 ZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -D_GNU_SOURCE)
 
 target_link_libraries(extension PRIVATE mpack PhpConfig zai)
+target_include_directories(extension PRIVATE ..)
 
 macro(target_linker_flag_conditional target) # flags as argv
     try_compile(LINKER_HAS_FLAG "${CMAKE_CURRENT_BINARY_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/cmake/check.c"

appsec/cmake/helper.cmake

@@ -10,33 +10,56 @@ configure_file(src/helper/version.hpp.in ${CMAKE_CURRENT_SOURCE_DIR}/src/helper/
 set(HELPER_SOURCE_DIR src/helper)
 set(HELPER_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src/helper)
 
-file(GLOB_RECURSE HELPER_SOURCE ${HELPER_SOURCE_DIR}/*.cpp)
+file(GLOB_RECURSE HELPER_SOURCE CONFIGURE_DEPENDS
+    ${HELPER_SOURCE_DIR}/*.cpp ${HELPER_SOURCE_DIR}/*.c)
 list(FILTER HELPER_SOURCE EXCLUDE REGEX "^.*main\.cpp$")
 
 add_library(helper_objects OBJECT ${HELPER_SOURCE})
 set_target_properties(helper_objects PROPERTIES
+    CXX_VISIBILITY_PRESET hidden
+    CXX_STANDARD 20
+    CXX_STANDARD_REQUIRED YES
     POSITION_INDEPENDENT_CODE 1)
 target_include_directories(helper_objects PUBLIC ${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)
 
-add_executable(ddappsec-helper src/helper/main.cpp
-	$<TARGET_OBJECTS:helper_objects>
-	$<TARGET_OBJECTS:libddwaf_objects>)
+add_library(ddappsec-helper SHARED
+    src/helper/main.cpp
+    $<TARGET_OBJECTS:helper_objects>
+    $<TARGET_OBJECTS:libddwaf_objects>)
 target_link_libraries(ddappsec-helper helper_objects) # for its PUBLIC deps
+if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+    target_compile_options(ddappsec-helper PRIVATE -ftls-model=global-dynamic)
+    # 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)
+endif()
+set_target_properties(ddappsec-helper PROPERTIES
+    CXX_VISIBILITY_PRESET hidden
+    CXX_STANDARD 20
+    CXX_STANDARD_REQUIRED YES
+    POSITION_INDEPENDENT_CODE 1
+    DEBUG_POSTFIX ""
+    SUFFIX .so
+)
+
+patch_away_libc(ddappsec-helper)
 
 try_compile(STDLIBXX_FS_NO_LIB_NEEDED ${CMAKE_CURRENT_BINARY_DIR}
     SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/check_fslib.cpp
-    CXX_STANDARD 17
+    CXX_STANDARD 20
     CXX_STANDARD_REQUIRED TRUE)
 try_compile(STDLIBXX_FS_NEEDS_STDCXXFS ${CMAKE_CURRENT_BINARY_DIR}
+
     SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/check_fslib.cpp
-    CXX_STANDARD 17
+    CXX_STANDARD 20
     CXX_STANDARD_REQUIRED TRUE
     LINK_LIBRARIES stdc++fs)
 try_compile(STDLIBXX_FS_NEEDS_CXXFS ${CMAKE_CURRENT_BINARY_DIR}
     SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/check_fslib.cpp
-    CXX_STANDARD 17
+    CXX_STANDARD 20
     CXX_STANDARD_REQUIRED TRUE
     LINK_LIBRARIES c++fs)
 if(NOT STDLIBXX_FS_NO_LIB_NEEDED)

appsec/cmake/patchelf.cmake

@@ -1,5 +1,5 @@
 function(patch_away_libc target)
-    if (NOT ${DD_APPSEC_ENABLE_PATCHELF_LIBC})
+    if(NOT ${DD_APPSEC_ENABLE_PATCHELF_LIBC})
         return()
     endif()
 
@@ -8,10 +8,15 @@ function(patch_away_libc target)
     endif()
 
     find_program(PATCHELF patchelf)
-    if (PATCHELF STREQUAL "PATCHELF-NOTFOUND")
+    find_program(READELF readelf)
+    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})
+        if(READELF STREQUAL "READELF-NOTFOUND")
+            message(WARNING "readelf not found. Can't build glibc + musl binaries")
+        else()
+            add_custom_command(TARGET ${target} POST_BUILD
+                COMMAND ${CMAKE_SOURCE_DIR}/cmake/strip_libc.sh "${PATCHELF}" "${READELF}" $<TARGET_FILE:${target}>)
+       endif()
     endif()
 endfunction()

appsec/cmake/strip_libc.sh

@@ -0,0 +1,17 @@
+#!/bin/sh
+
+set -e
+
+main() {
+  local patchelf=$1
+  local readelf=$2
+  local target=$3
+
+  "$patchelf" $(
+    "$readelf" -d "$target" 2>/dev/null | grep libc\\. | grep NEEDED | \
+      awk -F'[][]' '{print "--remove-needed " $2;}' | xargs
+     ) \
+     "$target"
+}
+
+main "$@"

appsec/src/extension/commands_helpers.c

@@ -175,7 +175,7 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn, bool check_cred,
             return dd_error;
         }
         if (res != dd_success && res != dd_should_block &&
-            res != dd_should_redirect) {
+            res != dd_should_redirect && res != dd_should_record) {
             mlog(dd_log_warning, "Processing for command %.*s failed: %s",
                 NAME_L, dd_result_to_string(res));
             return res;

appsec/src/extension/configuration.h

@@ -45,11 +45,11 @@ extern bool runtime_config_first_init;
     CONFIG(CUSTOM(INT), DD_APPSEC_LOG_LEVEL, "warn", .parser = dd_parse_log_level)                                                    \
     SYSCFG(STRING, DD_APPSEC_LOG_FILE, "php_error_reporting")                                                                         \
     SYSCFG(BOOL, DD_APPSEC_HELPER_LAUNCH, "true")                                                                                     \
-    CONFIG(STRING, DD_APPSEC_HELPER_PATH, DD_BASE("bin/ddappsec-helper"))                                                             \
+    CONFIG(STRING, DD_APPSEC_HELPER_PATH, DD_BASE("bin/libddappsec-helper.so"))                                                             \
     CONFIG(STRING, DD_APPSEC_HELPER_RUNTIME_PATH, "/tmp", .ini_change = dd_on_runtime_path_update)                                    \
     SYSCFG(STRING, DD_APPSEC_HELPER_LOG_FILE, "/dev/null")                                                                            \
+    SYSCFG(STRING, DD_APPSEC_HELPER_LOG_LEVEL, "info")                                                                            \
     CONFIG(CUSTOM(SET), DD_EXTRA_SERVICES, "", .parser = _parse_list)                                                                 \
-    CONFIG(STRING, DD_APPSEC_HELPER_EXTRA_ARGS, "")                                                                                   \
     CONFIG(STRING, DD_SERVICE, "")                                                                                                    \
     CONFIG(STRING, DD_ENV, "")                                                                                                        \
     CONFIG(STRING, DD_VERSION, "")                                                                                                    \

appsec/src/extension/ddappsec.c

@@ -241,10 +241,14 @@ static PHP_MSHUTDOWN_FUNCTION(ddappsec)
     return SUCCESS;
 }
 
-static pthread_once_t _rinit_once_control = PTHREAD_ONCE_INIT;
-
 static void _rinit_once() { dd_config_first_rinit(); }
 
+void dd_appsec_rinit_once()
+{
+    static pthread_once_t _rinit_once_control = PTHREAD_ONCE_INIT;
+    pthread_once(&_rinit_once_control, _rinit_once);
+}
+
 // NOLINTNEXTLINE
 static PHP_RINIT_FUNCTION(ddappsec)
 {
@@ -254,7 +258,7 @@ static PHP_RINIT_FUNCTION(ddappsec)
     // Safety precaution
     DDAPPSEC_G(during_request_shutdown) = false;
 
-    pthread_once(&_rinit_once_control, _rinit_once);
+    dd_appsec_rinit_once();
     zai_config_rinit();
     _check_enabled();
 

appsec/src/extension/ddappsec.h

@@ -53,6 +53,7 @@ extern __thread void *unspecnull ATTR_TLS_LOCAL_DYNAMIC TSRMLS_CACHE;
 #    define DDAPPSEC_G(v) (ddappsec_globals.v)
 #endif
 
+void dd_appsec_rinit_once(void);
 int dd_appsec_rshutdown(bool ignore_verdict);
 
 // Add a NO_CACHE version.