ddtrace cmake build. Adjust reported headers. Fix out-of-data test inddtrace

Author: cataphract

appsec/CMakeLists.txt

@@ -28,6 +28,7 @@ option(DD_APPSEC_BUILD_HELPER "Whether to builder the helper" ON)
 option(DD_APPSEC_BUILD_EXTENSION "Whether to builder the extension" ON)
 option(DD_APPSEC_ENABLE_COVERAGE "Whether to enable coverage calculation" OFF)
 option(DD_APPSEC_TESTING "Whether to enable testing" ON)
+option(DD_APPSEC_DDTRACE_ALT "Whether to build appsec with cmake" OFF)
 
 add_subdirectory(third_party EXCLUDE_FROM_ALL)
 

appsec/cmake/ddtrace.cmake

@@ -0,0 +1,120 @@
+include(ExternalProject)
+
+set(CARGO_BUILD_CMD "cargo build")
+set(CARGO_BUILD_ENV "") # Initialize to empty
+
+
+if(CMAKE_BUILD_TYPE STREQUAL "Release")
+    set(CARGO_BUILD_CMD "${CARGO_BUILD_CMD} --release")
+elseif(CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
+    set(CARGO_BUILD_CMD "${CARGO_BUILD_CMD} --release")
+    set(CARGO_BUILD_ENV "RUSTFLAGS='-C debuginfo=2'")
+endif()
+
+set(LIBDATADOG_DIR "${CMAKE_SOURCE_DIR}/../libdatadog")
+set(LIBDATADOG_STAMP_FILE "${CMAKE_BINARY_DIR}/libdatadog.stamp")
+add_custom_target(libdatadog_stamp
+    COMMAND ${CMAKE_COMMAND} -E touch ${LIBDATADOG_STAMP_FILE} #XXX: use a script to find modifications
+    BYPRODUCT ${LIBDATADOG_STAMP_FILE}
+)
+
+set(EXPORTS_FILE "${CMAKE_BINARY_DIR}/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
+)
+
+ExternalProject_Add(components_rs_proj
+    PREFIX ${CMAKE_BINARY_DIR}/components_rs
+    SOURCE_DIR ${CMAKE_SOURCE_DIR}/../components-rs
+    CONFIGURE_COMMAND ""
+    BUILD_COMMAND RUSTC_BOOTSTRAP=1 ${CARGO_BUILD_ENV} cargo build --target-dir=${CMAKE_BINARY_DIR}/components_rs
+    INSTALL_COMMAND ""
+    DEPENDS libdatadog_stamp
+    BUILD_IN_SOURCE TRUE
+)
+
+add_library(components_rs STATIC IMPORTED)
+if(CMAKE_BUILD_TYPE STREQUAL "Debug")
+    set(CARGO_BUILD_LOCATION ${CMAKE_BINARY_DIR}/components_rs/debug)
+else()
+    set(CARGO_BUILD_LOCATION ${CMAKE_BINARY_DIR}/components_rs/release)
+endif()
+set_property(TARGET components_rs PROPERTY IMPORTED_LOCATION ${CARGO_BUILD_LOCATION}/libddtrace_php.a)
+add_dependencies(components_rs components_rs_proj)
+
+
+execute_process(
+    COMMAND ${PhpConfig_EXECUTABLE} --vernum
+    RESULT_VARIABLE PhpConfig_VERNUM_RESULT
+    OUTPUT_VARIABLE PhpConfig_VERNUM
+    OUTPUT_STRIP_TRAILING_WHITESPACE COMMAND_ERROR_IS_FATAL ANY)
+
+file(GLOB_RECURSE FILES_DDTRACE
+    CONFIGURE_DEPENDS
+    "${CMAKE_SOURCE_DIR}/../ext/*.c"
+    "${CMAKE_SOURCE_DIR}/../ext/**/*.c"
+    "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/*.c"
+    "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/**/*.c"
+)
+
+list(APPEND FILES_DDTRACE
+    "${CMAKE_SOURCE_DIR}/../src/dogstatsd/client.c"
+    "${CMAKE_SOURCE_DIR}/../components/container_id/container_id.c"
+    "${CMAKE_SOURCE_DIR}/../components/log/log.c"
+    "${CMAKE_SOURCE_DIR}/../components/sapi/sapi.c"
+    "${CMAKE_SOURCE_DIR}/../components/string_view/string_view.c"
+)
+if (PhpConfig_VERNUM GREATER_EQUAL 80000)
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/handlers_curl_php7.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php7/interceptor.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php7/resolver.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/sandbox/php7/sandbox.c")
+else() # PHP 7
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/handlers_curl.c"
+        "${CMAKE_SOURCE_DIR}/../ext/hook/uhook_attributes.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/interceptor.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/jit_utils/jit_blacklist.c"
+        "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/sandbox/php8/sandbox.c")
+endif()
+if (PhpConfig_VERNUM LESS 80200)
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/weakrefs.c")
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver.c")
+else() # PHP 8.2+
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c")
+endif()
+if (PhpConfig_VERNUM LESS 80100)
+    list(REMOVE_ITEM FILES_DDTRACE "${CMAKE_SOURCE_DIR}/../ext/handlers_fiber.c")
+endif()
+
+find_package(CURL REQUIRED)
+
+add_library(ddtrace SHARED ${FILES_DDTRACE})
+set_target_properties(ddtrace PROPERTIES
+    C_VISIBILITY_PRESET hidden
+    OUTPUT_NAME ddtrace
+    DEBUG_POSTFIX ""
+    PREFIX "")
+target_compile_options(ddtrace PRIVATE -fms-extensions)
+target_link_options(ddtrace PRIVATE "-Wl,--version-script=${EXPORTS_FILE}")
+target_link_libraries(ddtrace PRIVATE PhpConfig components_rs ${CURL_LIBRARIES})
+if(CURL_DEFINITIONS)
+    target_compile_definitions(ddtrace PRIVATE ${CURL_DEFINITIONS})
+endif()
+target_compile_definitions(ddtrace PRIVATE ZEND_ENABLE_STATIC_TSRMLS_CACHE=1 COMPILE_DL_DDTRACE=1)
+target_include_directories(ddtrace PRIVATE
+    ${CURL_INCLUDE_DIRS}
+    ${CMAKE_SOURCE_DIR}/..
+    ${CMAKE_SOURCE_DIR}/../src/dogstatsd
+    ${CMAKE_SOURCE_DIR}/../zend_abstract_interface
+    ${CMAKE_SOURCE_DIR}/../ext
+    ${CMAKE_SOURCE_DIR}/../ext/vendor
+    ${CMAKE_SOURCE_DIR}/../ext/vendor/mt19937
+)
+add_dependencies(ddtrace ddtrace_exports)
+
+patch_away_libc(ddtrace)

appsec/cmake/run_tests.cmake

@@ -1,10 +1,14 @@
-get_filename_component(DD_APPSEC_TRACER_EXT_FILE "${CMAKE_SOURCE_DIR}/../tmp/build_extension/modules/ddtrace.so" REALPATH)
-
-add_custom_target(ddtrace
-    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}/../)
+if(DD_APPSEC_DDTRACE_ALT)
+    include(cmake/ddtrace.cmake)
+    set(DD_APPSEC_TRACER_EXT_FILE $<TARGET_FILE:ddtrace>)
+else()
+    get_filename_component(DD_APPSEC_TRACER_EXT_FILE "${CMAKE_SOURCE_DIR}/../tmp/build_extension/modules/ddtrace.so" REALPATH)
+    add_custom_target(ddtrace
+        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}/../)
+endif()
 
 add_custom_target(xtest-prepare
     COMMAND mkdir -p /tmp/appsec-ext-test)

appsec/src/extension/ddtrace.c

@@ -42,6 +42,8 @@ static void (*nullable _ddtrace_set_priority_sampling_on_span_zobj)(
 static bool (*nullable _ddtrace_user_req_add_listeners)(
     ddtrace_user_req_listeners *listeners);
 
+static zend_string *(*_ddtrace_ip_extraction_find)(zval *server);
+
 static void dd_trace_load_symbols(void)
 {
     bool testing = get_global_DD_APPSEC_TESTING();
@@ -89,6 +91,12 @@ static void dd_trace_load_symbols(void)
             dlerror()); // NOLINT(concurrency-mt-unsafe)
     }
 
+    _ddtrace_ip_extraction_find = dlsym(handle, "ddtrace_ip_extraction_find");
+    if (_ddtrace_ip_extraction_find == NULL && !testing) {
+        mlog(dd_log_error, "Failed to load ddtrace_ip_extraction_find: %s",
+            dlerror()); // NOLINT(concurrency-mt-unsafe)
+    }
+
     dlclose(handle);
 }
 
@@ -334,6 +342,14 @@ zend_object *nullable dd_trace_get_active_root_span()
     return _ddtrace_get_root_span();
 }
 
+zend_string *nullable dd_ip_extraction_find(zval *nonnull server)
+{
+    if (!_ddtrace_ip_extraction_find) {
+        return NULL;
+    }
+    return _ddtrace_ip_extraction_find(server);
+}
+
 static PHP_FUNCTION(datadog_appsec_testing_ddtrace_rshutdown)
 {
     if (zend_parse_parameters_none() == FAILURE) {

appsec/src/extension/ddtrace.h

@@ -76,3 +76,5 @@ struct _ddtrace_user_req_listeners {
 };
 bool dd_trace_user_req_add_listeners(
     ddtrace_user_req_listeners *nonnull listeners);
+
+zend_string *nullable dd_ip_extraction_find(zval *nonnull server);

appsec/src/extension/ip_extraction.c

@@ -6,42 +6,13 @@
 
 #include "ip_extraction.h"
 #include "configuration.h"
+#include "ddtrace.h"
 #include "logging.h"
 #include "php_objects.h"
 
-static zend_string *(*_ddtrace_ip_extraction_find)(zval *server);
 static void _register_testing_objects(void);
 
-void dd_ip_extraction_startup()
-{
-    _register_testing_objects();
-
-    bool testing = get_global_DD_APPSEC_TESTING();
-    void *handle = dlopen(NULL, RTLD_NOW | RTLD_GLOBAL);
-    if (handle == NULL) {
-        if (!testing) {
-            // NOLINTNEXTLINE(concurrency-mt-unsafe)
-            mlog(dd_log_error, "Failed load process symbols: %s", dlerror());
-        }
-        return;
-    }
-
-    _ddtrace_ip_extraction_find = dlsym(handle, "ddtrace_ip_extraction_find");
-    if (_ddtrace_ip_extraction_find == NULL && !testing) {
-        mlog(dd_log_error, "Failed to load ddtrace_ip_extraction_find: %s",
-            dlerror()); // NOLINT(concurrency-mt-unsafe)
-    }
-
-    dlclose(handle);
-}
-
-zend_string *nullable dd_ip_extraction_find(zval *nonnull server)
-{
-    if (!_ddtrace_ip_extraction_find) {
-        return NULL;
-    }
-    return _ddtrace_ip_extraction_find(server);
-}
+void dd_ip_extraction_startup() { _register_testing_objects(); }
 
 static PHP_FUNCTION(datadog_appsec_testing_extract_ip_addr)
 {

appsec/src/extension/ip_extraction.h

@@ -6,19 +6,4 @@
 
 #pragma once
 
-#ifndef _GNU_SOURCE
-#    define _GNU_SOURCE
-#endif
-
-#include "attributes.h"
-#include "zai_string/string.h"
-#include <php.h>
-
 void dd_ip_extraction_startup(void);
-
-// Since the headers looked at can in principle be forged, it's very much
-// recommended that a datadog.appsec.ipheader is set to a header that the server
-// guarantees cannot be forged
-zend_string *nullable dd_ip_extraction_find(zval *nonnull server);
-bool dd_parse_client_ip_header_config(
-    zai_str value, zval *nonnull decoded_value, bool persistent);

appsec/src/extension/tags.c

@@ -42,7 +42,6 @@
 #define DD_TAG_HTTP_RH_CONTENT_LANGUAGE "http.response.headers.content-language"
 #define DD_TAG_HTTP_CLIENT_IP "http.client_ip"
 #define DD_TAG_USER_ID "usr.id"
-#define DD_MULTIPLE_BASIC_HEADERS "_dd.multiple-ip-headers"
 #define DD_METRIC_ENABLED "_dd.appsec.enabled"
 #define DD_APPSEC_EVENTS_PREFIX "appsec.events."
 #define DD_SIGNUP_EVENT DD_APPSEC_EVENTS_PREFIX "users.signup"
@@ -87,7 +86,6 @@ static zend_string *_dd_tag_rh_content_type;     // response
 static zend_string *_dd_tag_rh_content_encoding; // response
 static zend_string *_dd_tag_rh_content_language; // response
 static zend_string *_dd_tag_user_id;
-static zend_string *_dd_multiple_ip_headers;
 static zend_string *_dd_metric_enabled;
 static zend_string *_dd_signup_event;
 static zend_string *_dd_login_success_event;
@@ -171,8 +169,6 @@ void dd_tags_startup()
         zend_string_init_interned(LSTRARG(DD_TAG_HTTP_RH_CONTENT_LANGUAGE), 1);
     _dd_tag_user_id = zend_string_init_interned(LSTRARG(DD_TAG_USER_ID), 1);
 
-    _dd_multiple_ip_headers =
-        zend_string_init_interned(LSTRARG(DD_MULTIPLE_BASIC_HEADERS), 1);
     _dd_metric_enabled =
         zend_string_init_interned(LSTRARG(DD_METRIC_ENABLED), 1);
 
@@ -246,20 +242,20 @@ static void _init_relevant_headers()
         &_relevant_basic_headers, str "", sizeof(str) - 1, &nullzv);           \
     ADD_RELEVANT_HEADER(str)
 
-    ADD_RELEVANT_BASIC_HEADER("x-forwarded-for");
-    ADD_RELEVANT_BASIC_HEADER("x-client-ip");
-    ADD_RELEVANT_BASIC_HEADER("x-real-ip");
-    ADD_RELEVANT_BASIC_HEADER("x-forwarded");
-    ADD_RELEVANT_BASIC_HEADER("x-cluster-client-ip");
-    ADD_RELEVANT_BASIC_HEADER("forwarded-for");
-    ADD_RELEVANT_BASIC_HEADER("forwarded");
-    ADD_RELEVANT_BASIC_HEADER("via");
-    ADD_RELEVANT_BASIC_HEADER("true-client-ip");
-    ADD_RELEVANT_BASIC_HEADER("fastly-client-ip");
-    ADD_RELEVANT_BASIC_HEADER("cf-connecting-ip");
-    ADD_RELEVANT_BASIC_HEADER("cf-connecting-ipv6");
     ADD_RELEVANT_BASIC_HEADER("x-amzn-trace-id");
 
+    ADD_RELEVANT_HEADER("x-forwarded-for");
+    ADD_RELEVANT_HEADER("x-client-ip");
+    ADD_RELEVANT_HEADER("x-real-ip");
+    ADD_RELEVANT_HEADER("x-forwarded");
+    ADD_RELEVANT_HEADER("x-cluster-client-ip");
+    ADD_RELEVANT_HEADER("forwarded-for");
+    ADD_RELEVANT_HEADER("forwarded");
+    ADD_RELEVANT_HEADER("via");
+    ADD_RELEVANT_HEADER("true-client-ip");
+    ADD_RELEVANT_HEADER("fastly-client-ip");
+    ADD_RELEVANT_HEADER("cf-connecting-ip");
+    ADD_RELEVANT_HEADER("cf-connecting-ipv6");
     ADD_RELEVANT_HEADER("content-length");
     ADD_RELEVANT_HEADER("content-type");
     ADD_RELEVANT_HEADER("content-encoding");
@@ -649,8 +645,7 @@ static void _dd_http_network_client_ip(
 
 static void _dd_http_client_ip(zend_array *meta_ht)
 {
-    if (zend_hash_exists(meta_ht, _dd_tag_http_client_ip_zstr) ||
-        zend_hash_exists(meta_ht, _dd_multiple_ip_headers)) {
+    if (zend_hash_exists(meta_ht, _dd_tag_http_client_ip_zstr)) {
         return;
     }
     zend_string *client_ip = dd_req_lifecycle_get_client_ip();

appsec/tests/extension/ancillary_tags.phpt

@@ -101,14 +101,5 @@ Array
 Array
 (
     [http.client_ip] => 7.7.7.6
-    [http.request.headers.forwarded] => for="foo"
-    [http.request.headers.forwarded-for] => 7.7.7.10,10.0.0.1
-    [http.request.headers.true-client-ip] => 7.7.7.11
-    [http.request.headers.via] => HTTP/1.1 GWA
     [http.request.headers.x-amzn-trace-id] => amazontraceid
-    [http.request.headers.x-client-ip] => 7.7.7.7
-    [http.request.headers.x-cluster-client-ip] => 7.7.7.9
-    [http.request.headers.x-forwarded] => for="foo"
-    [http.request.headers.x-forwarded-for] => 7.7.7.6,10.0.0.1
-    [http.request.headers.x-real-ip] => 7.7.7.8
 )

appsec/tests/extension/ddtrace_disabled.phpt

@@ -11,5 +11,4 @@ DD_APPSEC_ENABLED=1
 var_dump(\datadog\appsec\is_enabled());
 ?>
 --EXPECTF--
-Warning: PHP Startup: [ddappsec] Failed to load ddtrace_ip_extraction_find: %s undefined symbol: ddtrace_ip_extraction_find in Unknown on line %d
-bool(false)
+bool(false)