diff --git a/appsec/src/extension/commands/request_exec.c b/appsec/src/extension/commands/request_exec.c index 35c0be85ab..20e1f70159 100644 --- a/appsec/src/extension/commands/request_exec.c +++ b/appsec/src/extension/commands/request_exec.c @@ -30,8 +30,8 @@ static const dd_command_spec _spec = { .config_features_cb = dd_command_process_config_features_unexpected, }; -dd_result dd_request_exec( - dd_conn *nonnull conn, zval *nonnull data, zend_string *nullable rasp_rule) +dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data, + zend_string *nullable rasp_rule, struct block_params *nonnull block_params) { if (Z_TYPE_P(data) != IS_ARRAY) { mlog(dd_log_debug, "Invalid data provided to command request_exec, " @@ -41,7 +41,11 @@ dd_result dd_request_exec( struct ctx ctx = {.rasp_rule = rasp_rule, .data = data}; - return dd_command_exec_req_info(conn, &_spec, &ctx.req_info); + dd_result res = dd_command_exec_req_info(conn, &_spec, &ctx.req_info); + + memcpy(block_params, &ctx.req_info.block_params, sizeof *block_params); + + return res; } static dd_result _pack_command(mpack_writer_t *nonnull w, void *nonnull _ctx) diff --git a/appsec/src/extension/commands/request_exec.h b/appsec/src/extension/commands/request_exec.h index 261d6523e2..70bf538b1d 100644 --- a/appsec/src/extension/commands/request_exec.h +++ b/appsec/src/extension/commands/request_exec.h @@ -5,9 +5,11 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #pragma once -#include "../network.h" #include #include -dd_result dd_request_exec( - dd_conn *nonnull conn, zval *nonnull data, zend_string *nullable rasp_rule); +#include "../network.h" +#include "../request_abort.h" + +dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data, + zend_string *nullable rasp_rule, struct block_params *nonnull block_params); diff --git a/appsec/src/extension/commands_ctx.h b/appsec/src/extension/commands_ctx.h index fb3904451c..2aa1754797 100644 --- a/appsec/src/extension/commands_ctx.h +++ b/appsec/src/extension/commands_ctx.h @@ -2,9 +2,11 @@ #include #include "attributes.h" +#include "request_abort.h" struct req_info { const char *nullable command_name; // for logging zend_object *nullable root_span; zend_string *nullable client_ip; + struct block_params block_params; // out }; diff --git a/appsec/src/extension/commands_helpers.c b/appsec/src/extension/commands_helpers.c index ea3adca20c..a2e1b56ba4 100644 --- a/appsec/src/extension/commands_helpers.c +++ b/appsec/src/extension/commands_helpers.c @@ -56,6 +56,17 @@ static inline ATTR_WARN_UNUSED mpack_error_t _imsg_destroy( dd_imsg *nonnull imsg); static void _imsg_cleanup(dd_imsg *nullable *imsg); +static void _set_redirect_code_and_location( + struct block_params *nonnull block_params, + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + int code, zend_string *nullable location, + zend_string *nullable security_response_id); + +static void _set_block_code_and_type(struct block_params *nonnull block_params, + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + int code, dd_response_type type, + zend_string *nullable security_response_id); + static dd_result _dd_command_exec(dd_conn *nonnull conn, const dd_command_spec *nonnull spec, void *unspecnull ctx) { @@ -299,7 +310,8 @@ static void _imsg_cleanup(dd_imsg *nullable *imsg) static void _add_appsec_span_data_frag(mpack_node_t node); static void _set_appsec_span_data(mpack_node_t node); -static void _command_process_block_parameters(mpack_node_t root) +static void _command_process_block_parameters( + struct block_params *nonnull block_params, mpack_node_t root) { int status_code = DEFAULT_BLOCKING_RESPONSE_CODE; dd_response_type type = DEFAULT_RESPONSE_TYPE; @@ -365,10 +377,12 @@ static void _command_process_block_parameters(mpack_node_t root) "Blocking parameters: status_code=%d, type=%d, security_response_id=%s", status_code, type, security_response_id ? ZSTR_VAL(security_response_id) : "NULL"); - dd_set_block_code_and_type(status_code, type, security_response_id); + _set_block_code_and_type( + block_params, status_code, type, security_response_id); } -static void _command_process_redirect_parameters(mpack_node_t root) +static void _command_process_redirect_parameters( + struct block_params *nonnull block_params, mpack_node_t root) { int status_code = 0; zend_string *location = NULL; @@ -423,9 +437,46 @@ static void _command_process_redirect_parameters(mpack_node_t root) "security_response_id=%s", status_code, location ? ZSTR_VAL(location) : "NULL", security_response_id ? ZSTR_VAL(security_response_id) : "NULL"); - dd_set_redirect_code_and_location( - status_code, location, security_response_id); + + _set_redirect_code_and_location( + block_params, status_code, location, security_response_id); +} + +static void _set_block_code_and_type(struct block_params *nonnull block_params, + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + int code, dd_response_type type, zend_string *nullable security_response_id) +{ + dd_response_type _type = type; + // Account for lack of enum type safety + switch (type) { + case response_type_auto: + case response_type_html: + case response_type_json: + _type = type; + break; + default: + _type = response_type_auto; + break; + } + + block_params->security_response_id = security_response_id; + block_params->response_type = _type; + block_params->response_code = code; + block_params->redirection_location = NULL; } + +static void _set_redirect_code_and_location( + struct block_params *nonnull block_params, + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + int code, zend_string *nullable location, + zend_string *nullable security_response_id) +{ + block_params->security_response_id = security_response_id; + block_params->response_type = response_type_auto; + block_params->response_code = code; + block_params->redirection_location = location; +} + static void _command_process_stack_trace_parameters(mpack_node_t root) { size_t count = mpack_node_map_count(root); @@ -540,13 +591,14 @@ static dd_result _command_process_actions( if (dd_mpack_node_lstr_eq(verdict, "block") && res != dd_should_block && res != dd_should_redirect) { // Redirect take over block res = dd_should_block; - _command_process_block_parameters(mpack_node_array_at(action, 1)); + _command_process_block_parameters( + &ctx->block_params, mpack_node_array_at(action, 1)); dd_tags_add_blocked(); } else if (dd_mpack_node_lstr_eq(verdict, "redirect") && res != dd_should_redirect) { res = dd_should_redirect; _command_process_redirect_parameters( - mpack_node_array_at(action, 1)); + &ctx->block_params, mpack_node_array_at(action, 1)); dd_tags_add_blocked(); } else if (dd_mpack_node_lstr_eq(verdict, "record") && res == dd_success) { diff --git a/appsec/src/extension/ddappsec.c b/appsec/src/extension/ddappsec.c index b9a97a20aa..d964ede6e2 100644 --- a/appsec/src/extension/ddappsec.c +++ b/appsec/src/extension/ddappsec.c @@ -279,7 +279,6 @@ static PHP_RINIT_FUNCTION(ddappsec) DDAPPSEC_G(skip_rshutdown) = false; dd_msgpack_helpers_rinit(); dd_trace_rinit(); - dd_request_abort_rinit(); // Waf calls happen here. Not many rinits should go after this line. dd_req_lifecycle_rinit(false); @@ -288,7 +287,7 @@ static PHP_RINIT_FUNCTION(ddappsec) if (get_global_DD_APPSEC_TESTING_ABORT_RINIT()) { const char *pt = SG(request_info).path_translated; if (pt && !strstr(pt, "skip.php")) { - dd_request_abort_static_page(); + dd_request_abort_static_page(&(struct block_params){0}); } } } @@ -475,11 +474,13 @@ static PHP_FUNCTION(datadog_appsec_testing_request_exec) RETURN_FALSE; } - if (dd_request_exec(conn, data, false) != dd_success) { - RETURN_FALSE; + struct block_params block_params = {0}; + if (dd_request_exec(conn, data, false, &block_params) != dd_success) { + RETVAL_FALSE; + } else { + RETVAL_TRUE; } - - RETURN_TRUE; + dd_request_abort_destroy_block_params(&block_params); } static PHP_FUNCTION(datadog_appsec_push_addresses) @@ -517,7 +518,8 @@ static PHP_FUNCTION(datadog_appsec_push_addresses) return; } - dd_result res = dd_request_exec(conn, addresses, rasp_rule); + struct block_params block_params = {0}; + dd_result res = dd_request_exec(conn, addresses, rasp_rule, &block_params); if (rasp_rule && ZSTR_LEN(rasp_rule) > 0) { clock_gettime(CLOCK_MONOTONIC_RAW, &end); @@ -532,17 +534,8 @@ static PHP_FUNCTION(datadog_appsec_push_addresses) } } - if (dd_req_is_user_req()) { - if (res == dd_should_block || res == dd_should_redirect) { - dd_req_call_blocking_function(res); - } - } else { - if (res == dd_should_block) { - dd_request_abort_static_page(); - } else if (res == dd_should_redirect) { - dd_request_abort_redirect(); - } - } + dd_req_lifecycle_abort(REQUEST_STAGE_REQUEST_END, res, &block_params); + dd_request_abort_destroy_block_params(&block_params); } ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX( diff --git a/appsec/src/extension/request_abort.c b/appsec/src/extension/request_abort.c index 4742de9995..7a59809efb 100644 --- a/appsec/src/extension/request_abort.c +++ b/appsec/src/extension/request_abort.c @@ -18,6 +18,7 @@ #include "php_helpers.h" #include "php_objects.h" #include "request_abort.h" +#include "request_lifecycle.h" #include "string_helpers.h" #include "attributes.h" @@ -76,41 +77,6 @@ static zend_string *_content_length_zstr; static zend_string *_location_zstr; static zend_string *_content_type_html_zstr; static zend_string *_content_type_json_zstr; -struct _block_parameters { - zend_string *nullable security_response_id; - dd_response_type response_type; - int response_code; - zend_string *nullable redirection_location; -}; - -static THREAD_LOCAL_ON_ZTS struct _block_parameters *nullable _block_parameters; - -static void _block_parameters_free(void) -{ - if (_block_parameters) { - if (_block_parameters->security_response_id) { - zend_string_release(_block_parameters->security_response_id); - _block_parameters->security_response_id = NULL; - } - efree(_block_parameters); - _block_parameters = NULL; - } -} - -static void _block_parameters_set( - // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - zend_string *nullable security_response_id, dd_response_type response_type, - int response_code, zend_string *nullable redirection_location) -{ - if (_block_parameters) { - _block_parameters_free(); - } - _block_parameters = emalloc(sizeof(*_block_parameters)); - _block_parameters->security_response_id = security_response_id; - _block_parameters->response_type = response_type; - _block_parameters->response_code = response_code; - _block_parameters->redirection_location = redirection_location; -} #ifdef FRANKENPHP_SUPPORT static typeof(zend_compile_file) _orig_zend_compile_file; @@ -120,13 +86,19 @@ static THREAD_LOCAL_ON_ZTS char *_saved_prepend_file; static THREAD_LOCAL_ON_ZTS char *_saved_append_file; #endif -static bool _abort_prelude(void); -void _request_abort_static_page(int response_code, int type); +static bool _abort_prelude(struct block_params *nonnull block_params); +static void _request_abort_static_page( + struct block_params *nonnull block_params); ATTR_FORMAT(1, 2) static void _emit_error(const char *format, ...); static zend_string *nonnull _get_json_blocking_template(void); static zend_string *nonnull _get_html_blocking_template(void); +static inline bool _is_valid_redirect_code(int code) +{ + return code >= 300 && code < 400; // NOLINT +} + #ifdef FRANKENPHP_SUPPORT static zend_op_array *_req_init_block_zend_compile_file( zend_file_handle *file_handle, int type); @@ -229,50 +201,11 @@ static dd_response_type _get_response_type_from_accept_header( return response_type_json; } -void dd_set_block_code_and_type( - // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - int code, dd_response_type type, zend_string *nullable security_response_id) -{ - dd_response_type _type = type; - // Account for lack of enum type safety - switch (type) { - case response_type_auto: - case response_type_html: - case response_type_json: - _type = type; - break; - default: - _type = response_type_auto; - break; - } - - _block_parameters_set(security_response_id, _type, code, NULL); -} - -void dd_request_abort_rinit(void) +static void _replace_security_response_id( + struct block_params *nonnull block_params, + zend_string **nonnull target_ptr_ptr) { - _block_parameters_set(SECURITY_RESPONSE_ID_DEFAULT, DEFAULT_RESPONSE_TYPE, - DEFAULT_BLOCKING_RESPONSE_CODE, NULL); -} - -void dd_set_redirect_code_and_location( - // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - int code, zend_string *nullable location, - zend_string *nullable security_response_id) -{ - int response_code = DEFAULT_REDIRECTION_RESPONSE_CODE; - - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - if (code >= 300 && code < 400) { - response_code = code; - } - _block_parameters_set( - security_response_id, response_type_auto, response_code, location); -} - -static void _replace_security_response_id(zend_string **nonnull target_ptr_ptr) -{ - zend_string *security_response_id = _block_parameters->security_response_id; + zend_string *security_response_id = block_params->security_response_id; if (!security_response_id) { security_response_id = SECURITY_RESPONSE_ID_DEFAULT; } @@ -324,38 +257,45 @@ static void _replace_security_response_id(zend_string **nonnull target_ptr_ptr) } } -void dd_request_abort_redirect(void) +void dd_request_abort_redirect(struct block_params *nonnull block_params) { - if (_block_parameters->redirection_location == NULL || - ZSTR_LEN(_block_parameters->redirection_location) == 0) { - _request_abort_static_page( - DEFAULT_BLOCKING_RESPONSE_CODE, DEFAULT_RESPONSE_TYPE); + if (block_params->redirection_location == NULL || + ZSTR_LEN(block_params->redirection_location) == 0) { + mlog(dd_log_debug, + "Invalid redirect parameters: no redirection location"); + block_params->response_code = DEFAULT_BLOCKING_RESPONSE_CODE; + _request_abort_static_page(block_params); return; } - if (!_abort_prelude()) { + if (!_abort_prelude(block_params)) { mlog(dd_log_debug, "_abort_prelude has failed"); return; } - if (_block_parameters->security_response_id) { - _replace_security_response_id(&_block_parameters->redirection_location); + if (block_params->security_response_id) { + _replace_security_response_id( + block_params, &block_params->redirection_location); } char *line; - uint line_len = (uint)spprintf(&line, 0, "Location: %s", - ZSTR_VAL(_block_parameters->redirection_location)); + uint line_len = (uint)spprintf( + &line, 0, "Location: %s", ZSTR_VAL(block_params->redirection_location)); mlog_g(dd_log_debug, "Will forward to %s with status %d", - ZSTR_VAL(_block_parameters->redirection_location), - _block_parameters->response_code); + ZSTR_VAL(block_params->redirection_location), + block_params->response_code); + + SG(sapi_headers).http_response_code = + _is_valid_redirect_code(block_params->response_code) + ? block_params->response_code + : DEFAULT_REDIRECTION_RESPONSE_CODE; - SG(sapi_headers).http_response_code = _block_parameters->response_code; int res = sapi_header_op(SAPI_HEADER_REPLACE, &(sapi_header_line){.line = line, .line_len = line_len}); if (res == FAILURE) { mlog(dd_log_warning, "Could not forward to %s", - ZSTR_VAL(_block_parameters->redirection_location)); + ZSTR_VAL(block_params->redirection_location)); } efree(line); @@ -370,35 +310,49 @@ void dd_request_abort_redirect(void) mlog(dd_log_info, "Datadog blocked the request and attempted a redirection to %s. No " "action required. Security Response ID: %s", - ZSTR_VAL(_block_parameters->redirection_location), - _block_parameters->security_response_id - ? ZSTR_VAL(_block_parameters->security_response_id) + ZSTR_VAL(block_params->redirection_location), + block_params->security_response_id + ? ZSTR_VAL(block_params->security_response_id) : ""); } else { _emit_error("Datadog blocked the request and attempted a redirection " "to %s. No action required. Security Response ID: %s", - ZSTR_VAL(_block_parameters->redirection_location), - _block_parameters->security_response_id - ? ZSTR_VAL(_block_parameters->security_response_id) + ZSTR_VAL(block_params->redirection_location), + block_params->security_response_id + ? ZSTR_VAL(block_params->security_response_id) : ""); } } -zend_array *nonnull dd_request_abort_redirect_spec(void) +zend_array *nonnull dd_request_abort_redirect_spec( + struct block_params *nonnull block_params, const zend_array *nonnull server) { + if (block_params->redirection_location == NULL || + ZSTR_LEN(block_params->redirection_location) == 0) { + mlog(dd_log_debug, + "Invalid redirect parameters: no redirection location"); + block_params->response_code = DEFAULT_BLOCKING_RESPONSE_CODE; + return dd_request_abort_static_page_spec(block_params, server); + } + zend_array *arr = zend_new_array(2); + int response_code = _is_valid_redirect_code(block_params->response_code) + ? block_params->response_code + : DEFAULT_REDIRECTION_RESPONSE_CODE; + zval status; - ZVAL_LONG(&status, _block_parameters->response_code); + ZVAL_LONG(&status, response_code); zend_hash_add_new(arr, _status_zstr, &status); - if (_block_parameters->security_response_id) { - _replace_security_response_id(&_block_parameters->redirection_location); + if (block_params->security_response_id) { + _replace_security_response_id( + block_params, &block_params->redirection_location); } zend_array *headers = zend_new_array(1); zval location; - ZVAL_STR(&location, _block_parameters->redirection_location); + ZVAL_STR_COPY(&location, block_params->redirection_location); zend_hash_add_new(headers, _location_zstr, &location); zval headers_zv; ZVAL_ARR(&headers_zv, headers); @@ -408,11 +362,13 @@ zend_array *nonnull dd_request_abort_redirect_spec(void) } // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -void _request_abort_static_page(int response_code, int type) +void _request_abort_static_page(struct block_params *nonnull block_params) { - SG(sapi_headers).http_response_code = response_code; + SG(sapi_headers).http_response_code = block_params->response_code + ? block_params->response_code + : DEFAULT_BLOCKING_RESPONSE_CODE; - dd_response_type response_type = type; + dd_response_type response_type = block_params->response_type; if (response_type == response_type_auto) { zval *server = dd_php_get_autoglobal(TRACK_VARS_SERVER, LSTRARG("_SERVER")); @@ -438,9 +394,9 @@ void _request_abort_static_page(int response_code, int type) return; } - _replace_security_response_id(&body); + _replace_security_response_id(block_params, &body); - if (!_abort_prelude()) { + if (!_abort_prelude(block_params)) { mlog(dd_log_debug, "_abort_prelude has failed"); zend_string_release(body); return; @@ -458,35 +414,35 @@ void _request_abort_static_page(int response_code, int type) mlog(dd_log_info, "Datadog blocked the request and presented a static error page. No " "action required. Security Response ID: %s", - _block_parameters->security_response_id - ? ZSTR_VAL(_block_parameters->security_response_id) + block_params->security_response_id + ? ZSTR_VAL(block_params->security_response_id) : ""); } else { _emit_error("Datadog blocked the request and presented a static error " "page. No action required. Security Response ID: %s", - _block_parameters->security_response_id - ? ZSTR_VAL(_block_parameters->security_response_id) + block_params->security_response_id + ? ZSTR_VAL(block_params->security_response_id) : ""); } } -void dd_request_abort_static_page(void) +void dd_request_abort_static_page(struct block_params *nonnull block_params) { - _request_abort_static_page( - _block_parameters->response_code, _block_parameters->response_type); + _request_abort_static_page(block_params); } zend_array *nonnull dd_request_abort_static_page_spec( + struct block_params *nonnull block_params, const zend_array *nonnull _server) { zend_array *arr = zend_new_array(3); zval status; - ZVAL_LONG(&status, _block_parameters->response_code); + ZVAL_LONG(&status, block_params->response_code); zend_hash_add_new(arr, _status_zstr, &status); zend_array *headers = zend_new_array(2); - dd_response_type response_type = _block_parameters->response_type; + dd_response_type response_type = block_params->response_type; if (response_type == response_type_auto) { response_type = _get_response_type_from_accept_header(_server); } @@ -499,7 +455,7 @@ zend_array *nonnull dd_request_abort_static_page_spec( zend_hash_add_new(headers, _content_type_zstr, &content_type); zend_string *content = _get_html_blocking_template(); - _replace_security_response_id(&content); + _replace_security_response_id(block_params, &content); body_len = content->len; ZVAL_STR(&body, content); zend_hash_add_new(arr, _body_zstr, &body); @@ -508,7 +464,7 @@ zend_array *nonnull dd_request_abort_static_page_spec( zend_hash_add_new(headers, _content_type_zstr, &content_type); zend_string *content = _get_json_blocking_template(); - _replace_security_response_id(&content); + _replace_security_response_id(block_params, &content); body_len = content->len; ZVAL_STR(&body, content); zend_hash_add_new(arr, _body_zstr, &body); @@ -531,7 +487,7 @@ zend_array *nonnull dd_request_abort_static_page_spec( } static void _force_destroy_output_handlers(void); -static bool _abort_prelude(void) +static bool _abort_prelude(struct block_params *nonnull block_params) { if (OG(running)) { /* we were told to block from inside an output handler. In this case, @@ -548,16 +504,16 @@ static bool _abort_prelude(void) "Datadog blocked the request, but the response has already " "been partially committed. No action required. Security " "Response ID: %s", - _block_parameters->security_response_id - ? ZSTR_VAL(_block_parameters->security_response_id) + block_params->security_response_id + ? ZSTR_VAL(block_params->security_response_id) : ""); } else { _emit_error( "Datadog blocked the request, but the response has already " "been partially committed. No action required. Security " "Response ID: %s", - _block_parameters->security_response_id - ? ZSTR_VAL(_block_parameters->security_response_id) + block_params->security_response_id + ? ZSTR_VAL(block_params->security_response_id) : ""); } return false; @@ -592,6 +548,15 @@ static void _force_destroy_output_handlers(void) static void _run_rshutdowns(void); static void _suppress_error_reporting(void); +ATTR_FORMAT(2, 3) +static void _php_verror(int type, const char *format, ...) +{ + va_list args; + va_start(args, format); + php_verror(NULL, "", type, format, args); + va_end(args); +} + ATTR_FORMAT(1, 2) static void _emit_error(const char *format, ...) { @@ -600,6 +565,30 @@ static void _emit_error(const char *format, ...) va_list args; va_start(args, format); + va_list args2; + va_copy(args2, args); + // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + char buf[512]; + int len = vsnprintf(buf, sizeof(buf), format, args); + char *msg = NULL; + bool free_msg = false; + if (len >= (int)sizeof(buf)) { + msg = safe_emalloc(len, 1, 0); + if (vsnprintf(msg, len + 1, format, args2) < 0) { + char default_msg[] = "Datadog blocked the request."; + msg = default_msg; + } else { + free_msg = true; + } + } else if (len >= 0) { + msg = buf; + } else { + char default_msg[] = "Datadog blocked the request."; + msg = default_msg; + } + va_end(args2); + va_end(args); + if (PG(during_request_startup)) { /* if emitting error during startup, RSHUTDOWN will not run (except fpm) * so we need to run the same logic from here */ @@ -616,16 +605,20 @@ static void _emit_error(const char *format, ...) /* fpm children exit if we throw an error at this point. So emit * only warning and use other means to prevent the script from * executing */ - php_verror(NULL, "", E_WARNING, format, args); - va_end(args); + _php_verror(E_WARNING, "%s", msg); + if (free_msg) { + efree(msg); + } // fpm doesn't try to run the script if it sees this null SG(request_info).request_method = NULL; return; } #ifdef FRANKENPHP_SUPPORT if (strcmp(sapi_module.name, "frankenphp") == 0) { - php_verror(NULL, "", E_WARNING, format, args); - va_end(args); + _php_verror(E_WARNING, "%s", msg); + if (free_msg) { + efree(msg); + } _prepare_req_init_block(); return; } @@ -653,17 +646,12 @@ static void _emit_error(const char *format, ...) * be a possibility, but it bypasses the value of error_reporting and is * always logged */ { - va_list args2; - va_copy(args2, args); - php_verror(NULL, "", E_COMPILE_WARNING, format, args2); - va_end(args2); + _php_verror(E_COMPILE_WARNING, "%s", msg); } // not enough: EG(error_handling) = EH_SUPPRESS; _suppress_error_reporting(); - php_verror(NULL, "", E_ERROR, format, args); - - va_end(args); + _php_verror(E_ERROR, "%s", msg); __builtin_unreachable(); } @@ -719,7 +707,7 @@ static PHP_FUNCTION(datadog_appsec_testing_abort_static_page) if (zend_parse_parameters_none() == FAILURE) { return; } - dd_request_abort_static_page(); + dd_request_abort_static_page(&(struct block_params){0}); } // clang-format off @@ -808,8 +796,6 @@ void dd_request_abort_shutdown(void) #endif } -void dd_request_abort_rshutdown(void) { _block_parameters_free(); } - static zend_string *nonnull _get_json_blocking_template(void) { zend_string *json_template_file = diff --git a/appsec/src/extension/request_abort.h b/appsec/src/extension/request_abort.h index 7e78c5a2fc..220dace92b 100644 --- a/appsec/src/extension/request_abort.h +++ b/appsec/src/extension/request_abort.h @@ -13,23 +13,43 @@ typedef enum { response_type_json = 2, } dd_response_type; +struct block_params { + zend_string *nullable security_response_id; + dd_response_type response_type; + int response_code; + zend_string *nullable redirection_location; +}; + // NOLINTNEXTLINE(modernize-macro-to-enum) #define DEFAULT_BLOCKING_RESPONSE_CODE 403 #define DEFAULT_REDIRECTION_RESPONSE_CODE 303 #define DEFAULT_RESPONSE_TYPE response_type_auto -void dd_set_block_code_and_type(int code, dd_response_type type, zend_string *nullable security_response_id); -void dd_set_redirect_code_and_location( - int code, zend_string *nullable location, zend_string *nullable security_response_id); - void dd_request_abort_startup(void); -void dd_request_abort_rinit(void); void dd_request_abort_zend_ext_startup(void); void dd_request_abort_shutdown(void); -void dd_request_abort_rshutdown(void); + +// You generally don't want to call these functions directly. +// See dd_req_lifecycle_abort instead. // noreturn unless called from rinit on fpm -void dd_request_abort_static_page(void); +void dd_request_abort_static_page(struct block_params *nonnull block_params); zend_array *nonnull dd_request_abort_static_page_spec( - const zend_array *nonnull); -void dd_request_abort_redirect(void); -zend_array *nonnull dd_request_abort_redirect_spec(void); + struct block_params *nonnull block_params, + const zend_array *nonnull server); +void dd_request_abort_redirect(struct block_params *nonnull block_params); +zend_array *nonnull dd_request_abort_redirect_spec( + struct block_params *nonnull block_params, + const zend_array *nonnull server); + +static inline void dd_request_abort_destroy_block_params( + struct block_params *nonnull block_params) +{ + if (block_params->security_response_id) { + zend_string_release(block_params->security_response_id); + block_params->security_response_id = NULL; + } + if (block_params->redirection_location) { + zend_string_release(block_params->redirection_location); + block_params->redirection_location = NULL; + } +} diff --git a/appsec/src/extension/request_lifecycle.c b/appsec/src/extension/request_lifecycle.c index 828ad396ec..dfdeab165b 100644 --- a/appsec/src/extension/request_lifecycle.c +++ b/appsec/src/extension/request_lifecycle.c @@ -25,8 +25,7 @@ #include "attributes.h" static void _do_request_finish_php(bool ignore_verdict); -static zend_array *nullable _do_request_begin( - zval *nullable rbe_zv, bool user_req); +static zend_array *nullable _do_request_begin(zval *nullable rbe_zv); static void _do_request_begin_php(void); static zend_array *_do_request_finish_user_req(bool ignore_verdict, zend_array *nonnull superglob_equiv, int status_code, @@ -134,7 +133,7 @@ static void _do_request_begin_php(void) dd_request_body_buffered(get_DD_APPSEC_MAX_BODY_BUFF_SIZE()); zval req_body_zv; ZVAL_STR(&req_body_zv, req_body); - (void)_do_request_begin(&req_body_zv, false); + UNUSED(_do_request_begin(&req_body_zv)); } static zend_array *nullable _do_request_begin_user_req(zval *nullable rbe_zv) @@ -142,7 +141,7 @@ static zend_array *nullable _do_request_begin_user_req(zval *nullable rbe_zv) if (rbe_zv) { Z_TRY_ADDREF_P(rbe_zv); } - return _do_request_begin(rbe_zv, true); + return _do_request_begin(rbe_zv); } static bool _rem_cfg_path_changed(bool ignore_empty /* called from rinit */) @@ -181,7 +180,7 @@ static bool _rem_cfg_path_changed(bool ignore_empty /* called from rinit */) } static zend_array *nullable _do_request_begin( - zval *nullable rbe_zv /* needs free */, bool user_req) + zval *nullable rbe_zv /* needs free */) { // do this even on user req because frankenphp uses normal php output dd_entity_body_rinit(); @@ -234,34 +233,27 @@ static zend_array *nullable _do_request_begin( // we might have been disabled by request_init + zend_array *spec = NULL; if (res == dd_network) { mlog_g(dd_log_info, "request_init/config_sync failed with dd_network; closing " "connection to helper"); dd_helper_close_conn(); - } else if (res == dd_should_block) { - if (user_req) { - const zend_array *nonnull sv = _get_server_equiv(_superglob_equiv); - return dd_request_abort_static_page_spec(sv); - } - dd_request_abort_static_page(); - } else if (res == dd_should_redirect) { - if (user_req) { - return dd_request_abort_redirect_spec(); - } - dd_request_abort_redirect(); + } else if (res == dd_should_block || res == dd_should_redirect) { + spec = dd_req_lifecycle_abort( + REQUEST_STAGE_REQUEST_BEGIN, res, &req_info.req_info.block_params); } else if (res != dd_success && res != dd_should_record) { mlog_g( dd_log_info, "request init failed: %s", dd_result_to_string(res)); } - return NULL; + dd_request_abort_destroy_block_params(&req_info.req_info.block_params); + + return spec; } void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force) { - dd_request_abort_rshutdown(); - if (DDAPPSEC_G(enabled) == APPSEC_FULLY_DISABLED) { mlog_g(dd_log_debug, "Skipping all request shutdown actions because " "appsec is fully disabled"); @@ -319,10 +311,11 @@ static void _do_request_finish_php(bool ignore_verdict) { int verdict = dd_success; dd_conn *conn = dd_helper_mgr_cur_conn(); + struct req_shutdown_info ctx = {0}; if (conn && DDAPPSEC_G(active)) { const int status_code = SG(sapi_headers).http_response_code; - struct req_shutdown_info ctx = { + ctx = (struct req_shutdown_info){ .req_info.root_span = dd_req_lifecycle_get_cur_span(), .req_info.client_ip = dd_req_lifecycle_get_client_ip(), .status_code = status_code, @@ -355,14 +348,9 @@ static void _do_request_finish_php(bool ignore_verdict) _reset_globals(); - // TODO when blocking on shutdown, let the tracer handle flushing - if (verdict == dd_should_block) { - dd_trace_close_all_spans_and_flush(); - dd_request_abort_static_page(); - } else if (verdict == dd_should_redirect) { - dd_trace_close_all_spans_and_flush(); - dd_request_abort_redirect(); - } + dd_req_lifecycle_abort( + REQUEST_STAGE_REQUEST_END, verdict, &ctx.req_info.block_params); + dd_request_abort_destroy_block_params(&ctx.req_info.block_params); } static zend_array *_do_request_finish_user_req(bool ignore_verdict, @@ -371,9 +359,10 @@ static zend_array *_do_request_finish_user_req(bool ignore_verdict, { int verdict = dd_success; dd_conn *conn = dd_helper_mgr_cur_conn(); + struct req_shutdown_info ctx = {0}; if (conn && DDAPPSEC_G(active)) { - struct req_shutdown_info ctx = { + ctx = (struct req_shutdown_info){ .req_info.root_span = dd_req_lifecycle_get_cur_span(), .req_info.client_ip = dd_req_lifecycle_get_client_ip(), .status_code = status_code, @@ -403,15 +392,12 @@ static zend_array *_do_request_finish_user_req(bool ignore_verdict, dd_tags_add_tags(_cur_req_span, superglob_equiv); } - if (verdict == dd_should_block) { - const zend_array *nonnull sv = _get_server_equiv(superglob_equiv); - return dd_request_abort_static_page_spec(sv); - } - if (verdict == dd_should_redirect) { - return dd_request_abort_redirect_spec(); - } + zend_array *spec = dd_req_lifecycle_abort( + REQUEST_STAGE_REQUEST_END, verdict, &ctx.req_info.block_params); - return NULL; + dd_request_abort_destroy_block_params(&ctx.req_info.block_params); + + return spec; } static void _reset_globals(void) @@ -792,7 +778,8 @@ static void _set_blocking_function(ddtrace_user_req_listeners *nonnull self, ZVAL_COPY(&_blocking_function, blocking_function); } -void dd_req_call_blocking_function(dd_result res) +void _call_blocking_function( + dd_result res, struct block_params *nonnull block_params) { if (Z_TYPE(_blocking_function) == IS_UNDEF) { mlog(dd_log_debug, "dd_req_call_blocking_function called with no " @@ -816,9 +803,9 @@ void dd_req_call_blocking_function(dd_result res) const zend_array *nonnull sv = _get_server_equiv(_superglob_equiv); zend_array *spec = NULL; if (res == dd_should_block) { - spec = dd_request_abort_static_page_spec(sv); + spec = dd_request_abort_static_page_spec(block_params, sv); } else if (res == dd_should_redirect) { - spec = dd_request_abort_redirect_spec(); + spec = dd_request_abort_redirect_spec(block_params, sv); } else { mlog(dd_log_warning, "dd_req_call_blocking_function called with " @@ -863,6 +850,53 @@ ddtrace_user_req_listeners dd_user_req_listeners = { .finish_user_req = _finish_user_req, }; +zend_array *nullable dd_req_lifecycle_abort(enum request_stage stage, + dd_result result, struct block_params *nonnull block_params) +{ + if (result != dd_should_block && result != dd_should_redirect) { + return NULL; + } + + const bool user_req = dd_req_is_user_req(); + + zend_array *spec = NULL; + if (user_req) { + if (stage == REQUEST_STAGE_REQUEST_BEGIN || + stage == REQUEST_STAGE_REQUEST_END) { + const zend_array *nonnull server = + _get_server_equiv(_superglob_equiv); + if (result == dd_should_block) { + spec = dd_request_abort_static_page_spec(block_params, server); + } else if (result == dd_should_redirect) { + spec = dd_request_abort_redirect_spec(block_params, server); + } + } else { + assert(stage == REQUEST_STAGE_MID_REQUEST); + _call_blocking_function(result, block_params); + } + } else { // non-user req + if (stage == REQUEST_STAGE_REQUEST_END) { + dd_trace_close_all_spans_and_flush(); + } + if (result == dd_should_block) { + dd_request_abort_static_page(block_params); + } else if (result == dd_should_redirect) { + dd_request_abort_redirect(block_params); + } + } + + if (block_params->security_response_id) { + zend_string_release(block_params->security_response_id); + block_params->security_response_id = NULL; + } + if (block_params->redirection_location) { + zend_string_release(block_params->redirection_location); + block_params->redirection_location = NULL; + } + + return spec; +} + #define FNV_PRIME 1099511628211ULL #define FNV_OFFSET_BASIS 14695981039346656037ULL static inline uint64_t _hash_string( diff --git a/appsec/src/extension/request_lifecycle.h b/appsec/src/extension/request_lifecycle.h index 83a70276ca..ba152c373a 100644 --- a/appsec/src/extension/request_lifecycle.h +++ b/appsec/src/extension/request_lifecycle.h @@ -3,6 +3,7 @@ #include "ddappsec.h" #include "dddefs.h" #include "ddtrace.h" +#include "request_abort.h" extern ddtrace_user_req_listeners dd_user_req_listeners; @@ -10,7 +11,14 @@ bool dd_req_is_user_req(void); void dd_req_lifecycle_startup(void); void dd_req_lifecycle_rinit(bool force); void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force); -void dd_req_call_blocking_function(dd_result res); + +enum request_stage { + REQUEST_STAGE_REQUEST_BEGIN, + REQUEST_STAGE_MID_REQUEST, + REQUEST_STAGE_REQUEST_END, +}; +zend_array *nullable dd_req_lifecycle_abort(enum request_stage stage, + dd_result result, struct block_params *nonnull block_params); zend_object *nullable dd_req_lifecycle_get_cur_span(void); zend_string *nullable dd_req_lifecycle_get_client_ip(void); diff --git a/appsec/src/extension/user_tracking.c b/appsec/src/extension/user_tracking.c index ffd4ed64aa..3e1586967a 100644 --- a/appsec/src/extension/user_tracking.c +++ b/appsec/src/extension/user_tracking.c @@ -263,7 +263,8 @@ void dd_find_and_apply_verdict_for_user(zend_string *nullable user_id, Z_ARRVAL(data_zv), "usr.id", sizeof("usr.id") - 1, &user_id_zv); } - dd_result res = dd_request_exec(conn, &data_zv, false); + struct block_params block_params = {0}; + dd_result res = dd_request_exec(conn, &data_zv, false, &block_params); if (res == dd_network) { mlog_g(dd_log_info, "request_exec failed with dd_network; closing " "connection to helper"); @@ -276,17 +277,8 @@ void dd_find_and_apply_verdict_for_user(zend_string *nullable user_id, dd_tags_set_event_user_id(user_id); } - if (dd_req_is_user_req()) { - if (res == dd_should_block || res == dd_should_redirect) { - dd_req_call_blocking_function(res); - } - } else { - if (res == dd_should_block) { - dd_request_abort_static_page(); - } else if (res == dd_should_redirect) { - dd_request_abort_redirect(); - } - } + dd_req_lifecycle_abort(REQUEST_STAGE_MID_REQUEST, res, &block_params); + dd_request_abort_destroy_block_params(&block_params); } bool dd_parse_user_collection_mode( diff --git a/appsec/tests/extension/input_truncated_05.phpt b/appsec/tests/extension/input_truncated_05.phpt index 85c85f666d..971ca6afa7 100644 --- a/appsec/tests/extension/input_truncated_05.phpt +++ b/appsec/tests/extension/input_truncated_05.phpt @@ -12,7 +12,6 @@ use function datadog\appsec\push_addresses; include __DIR__ . '/inc/mock_helper.php'; -\datadog\appsec\testing\stop_for_debugger(); $helper = Helper::createInitedRun([ response_list(response_request_init([[['ok', []]]])), response_list(response_request_exec([[['ok', []]]])), // Test 1 @@ -26,7 +25,6 @@ rinit(); // Test 1: Flat array over limit $data_over = array_fill(0, 2047, 'y'); -//\datadog\appsec\testing\stop_for_debugger(); push_addresses(["test1" => $data_over]); // Test 2: two arrays -- the second is clipped diff --git a/appsec/tests/extension/req_abort_redirection_03.phpt b/appsec/tests/extension/req_abort_redirection_03.phpt index 17939ce522..4ffccaafe3 100644 --- a/appsec/tests/extension/req_abort_redirection_03.phpt +++ b/appsec/tests/extension/req_abort_redirection_03.phpt @@ -23,4 +23,4 @@ Status: 403 Forbidden Content-type: application/json --EXPECTF-- {"errors": [{"title": "You've been blocked", "detail": "Sorry, you cannot access this page. Please contact the customer service team. Security provided by Datadog."}], "security_response_id": ""} -Warning: datadog\appsec\testing\rinit(): Datadog blocked the request and presented a static error page. No action required. Security Response ID: in %s on line %d \ No newline at end of file +Warning: datadog\appsec\testing\rinit(): Datadog blocked the request and presented a static error page. No action required. Security Response ID: in %s on line %d