Block based on response status or headers

CMakeLists.txt

@@ -7,7 +7,7 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 cmake_policy(SET CMP0068 NEW)
 cmake_policy(SET CMP0135 NEW)
 
-set(NGINX_DATADOG_VERSION 1.5.0)
+set(NGINX_DATADOG_VERSION 1.6.0)
 project(ngx_http_datadog_module VERSION ${NGINX_DATADOG_VERSION})
 
 option(NGINX_DATADOG_ASM_ENABLED "Build with libddwaf" ON)

Makefile

@@ -122,7 +122,7 @@ build-musl-aux build-musl-cov-aux:
 		-DCMAKE_TOOLCHAIN_FILE=/sysroot/$(ARCH)-none-linux-musl/Toolchain.cmake \
 		-DNGINX_PATCH_AWAY_LIBC=ON \
 		-DCMAKE_BUILD_TYPE=$(BUILD_TYPE) \
- 		-DNGINX_VERSION="$(NGINX_VERSION)" \
+		-DNGINX_VERSION="$(NGINX_VERSION)" \
 		-DNGINX_DATADOG_ASM_ENABLED="$(WAF)" . \
 		-DNGINX_DATADOG_RUM_ENABLED="$(RUM)" . \
 		-DNGINX_COVERAGE=$(COVERAGE) \
@@ -142,7 +142,7 @@ endif
 		--env RESTY_VERSION=$(RESTY_VERSION) \
 		--env NGINX_VERSION=$(NGINX_VERSION) \
 		--env WAF=$(WAF) \
- 		--mount type=bind,source="$(PWD)",target=/mnt/repo \
+		--mount type=bind,source="$(PWD)",target=/mnt/repo \
 		$(DOCKER_REPOS):latest \
 		bash -c "cd /mnt/repo && ./bin/openresty/build_openresty.sh && make build-openresty-aux"
 
@@ -158,11 +158,15 @@ build-openresty-aux:
 
 .PHONY: test
 test:
-	python3 test/bin/run.py --platform ${ARCH} --image ${BASE_IMAGE} --module-path .musl-build/ngx_http_datadog_module.so -- --verbose --failfast $(TEST_ARGS)
+	python3 test/bin/run.py --platform $(DOCKER_PLATFORM) --image ${BASE_IMAGE} \
+		--module-path .musl-build/ngx_http_datadog_module.so -- \
+		--verbose $(TEST_ARGS)
 
 .PHONY: test-openresty
 test-openresty:
-	RESTY_TEST=ON python3 test/bin/run.py --platform ${ARCH} --image ${BASE_IMAGE} --module-path .openresty-build/ngx_http_datadog_module.so -- --verbose --failfast $(TEST_ARGS)
+	RESTY_TEST=ON python3 test/bin/run.py --platform $(DOCKER_PLATFORM) \
+	   --image ${BASE_IMAGE} --module-path .openresty-build/ngx_http_datadog_module.so -- \
+	   --verbose $(TEST_ARGS)
 
 .PHONY: build-and-test
 build-and-test: build-musl test

cmake/deps/nginx-module.cmake

@@ -66,6 +66,8 @@ target_include_directories(nginx_module
   ${nginx_SOURCE_DIR}/src/os/unix
   ${nginx_SOURCE_DIR}/objs
   ${nginx_SOURCE_DIR}/src/core
+  ${nginx_SOURCE_DIR}/src/stream
+  ${nginx_SOURCE_DIR}/src/http/v2
   ${nginx_SOURCE_DIR}/src/event/modules)
 
 # vim: set et sw=2 ts=2:

src/datadog_conf.h

@@ -138,6 +138,8 @@ struct datadog_main_conf_t {
   // DD_APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP
   ngx_str_t appsec_obfuscation_value_regex = ngx_null_string;
 
+  std::size_t appsec_max_saved_output_data{NGX_CONF_UNSET_SIZE};
+
   // TODO: missing settings and their functionality
   // DD_TRACE_CLIENT_IP_RESOLVER_ENABLED (whether to collect headers and run the
   // client ip resolution. Also requires AppSec to be enabled or
@@ -225,6 +227,7 @@ struct datadog_loc_conf_t {
       allow_sampling_delegation_in_subrequests_directive;
 
 #ifdef WITH_WAF
+  // the thread pool used to run the WAF on
   ngx_thread_pool_t *waf_pool{nullptr};
 #endif
 

src/datadog_context.cpp

@@ -21,7 +21,8 @@ DatadogContext::DatadogContext(ngx_http_request_t *request,
                                ngx_http_core_loc_conf_t *core_loc_conf,
                                datadog_loc_conf_t *loc_conf)
 #ifdef WITH_WAF
-    : sec_ctx_{security::Context::maybe_create()}
+    : sec_ctx_{security::Context::maybe_create(
+          *loc_conf, security::Library::max_saved_output_data())}
 #endif
 {
   if (loc_conf->enable_tracing) {
@@ -82,9 +83,12 @@ ngx_int_t DatadogContext::on_header_filter(ngx_http_request_t *request) {
     return ngx_http_next_header_filter(request);
   }
 
+#if defined(WITH_RUM) || defined(WITH_WAF)
+  RequestTracing *trace{};
+#endif
 #ifdef WITH_RUM
   if (loc_conf->rum_enable) {
-    auto *trace = find_trace(request);
+    trace = find_trace(request);
     if (trace != nullptr) {
       auto rum_span = trace->active_span().create_child();
       rum_span.set_name("rum_sdk_injection.on_header");
@@ -97,6 +101,17 @@ ngx_int_t DatadogContext::on_header_filter(ngx_http_request_t *request) {
       rum_ctx_.on_header_filter(request, loc_conf, ngx_http_next_header_filter);
     }
   }
+#elif WITH_WAF
+  if (sec_ctx_) {
+    trace = find_trace(request);
+  }
+#endif
+
+#ifdef WITH_WAF
+  if (sec_ctx_ && trace) {
+    dd::Span &span = trace->active_span();
+    return sec_ctx_->header_filter(*request, span);
+  }
 #endif
 
   return ngx_http_next_header_filter(request);

src/ngx_header_writer.h

@@ -73,7 +73,8 @@ class NgxHeaderWriter : public datadog::tracing::DictWriter {
       }
 
       if (key.size() != h[i].key.len ||
-          ngx_strcasecmp((u_char *)key.data(), h[i].key.data) != 0) {
+          ngx_strncasecmp((u_char *)key.data(), h[i].key.data, key.size()) !=
+              0) {
         continue;
       }
 

src/ngx_http_datadog_module.cpp

@@ -375,6 +375,15 @@ static ngx_command_t datadog_commands[] = {
       offsetof(datadog_main_conf_t, appsec_obfuscation_value_regex),
       nullptr,
     },
+
+    {
+      ngx_string("datadog_appsec_max_saved_output_data"),
+      NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
+      ngx_conf_set_size_slot,
+      NGX_HTTP_MAIN_CONF_OFFSET,
+      offsetof(datadog_main_conf_t, appsec_max_saved_output_data),
+      nullptr,
+    },
 #endif
     DATADOG_RUM_DIRECTIVES
     ngx_null_command

src/security/blocking.cpp

@@ -8,6 +8,7 @@
 #include "util.h"
 
 extern "C" {
+#include <ngx_event_posted.h>
 #include <ngx_http.h>
 }
 
@@ -303,7 +304,8 @@ void BlockingService::initialize(std::optional<std::string_view> templ_html,
       new BlockingService(templ_html, templ_json));
 }
 
-void BlockingService::block(BlockSpecification spec, ngx_http_request_t &req) {
+ngx_int_t BlockingService::block(BlockSpecification spec,
+                                 ngx_http_request_t &req) {
   BlockResponse const resp = BlockResponse::resolve_content_type(spec, req);
   ngx_str_t *templ{};
   if (resp.ct == BlockResponse::ContentType::HTML) {
@@ -314,34 +316,55 @@ void BlockingService::block(BlockSpecification spec, ngx_http_request_t &req) {
     req.header_only = 1;
   }
 
-  ngx_http_discard_request_body(&req);
+  if (ngx_http_discard_request_body(&req) != NGX_OK) {
+    req.keepalive = 0;
+  }
+
+  req.header_sent = 0;
+  ngx_http_clean_header(&req);
 
-  // TODO: clear all current headers?
+  // ngx_http_filter_finalize_request neutralizes other filters with:
+  //   void *ctx = req.ctx[ngx_http_datadog_module.ctx_index];
+  //   ngx_memzero(req.ctx, sizeof(void *) * ngx_http_max_module);
+  //   req.ctx[ngx_http_datadog_module.ctx_index] = ctx;
+  //   req.filter_finalize = 1;
+  // This would prevent our blocking response from being changed by filters.
+  // However, at least the chunked body filter segfaults if its context is 0'ed.
+  // filter_finalize = 1 has its own problems, causing the connection->error to
+  // be set to 1.
 
   req.headers_out.status = resp.status;
+  req.headers_out.status_line.len = 0;
+  req.err_status = 0;
   req.headers_out.content_type = BlockResponse::content_type_header(resp.ct);
   req.headers_out.content_type_len = req.headers_out.content_type.len;
 
   if (!resp.location.empty()) {
     push_header(req, "Location"sv, resp.location);
   }
-  if (templ) {
-    req.headers_out.content_length_n = static_cast<off_t>(templ->len);
-  } else {
+  // Filters may change the response, invalidating this length
+  //   if (templ) {
+  //     req.headers_out.content_length_n = static_cast<off_t>(templ->len);
+  //   } else {
+  //     req.headers_out.content_length_n = 0;
+  //   }
+  if (req.header_only) {
     req.headers_out.content_length_n = 0;
+    req.chunked = 0;
   }
-
-  // TODO: bypass header filters?
-  auto res = ngx_http_send_header(&req);
-  if (res == NGX_ERROR || res > NGX_OK || req.header_only) {
-    ngx_http_finalize_request(&req, res);
-    return;
+  req.keepalive = 0;
+  req.lingering_close = 0;
+
+  ngx_int_t res = ngx_http_send_header(&req);
+  ngx_log_debug1(NGX_LOG_DEBUG, req.connection->log, 0,
+                 "block(): status %d returned by ngx_http_send_header", res);
+  if (res != NGX_OK || req.header_only) {
+    return res;
   }
 
   ngx_buf_t *b = static_cast<decltype(b)>(ngx_calloc_buf(req.pool));
   if (b == nullptr) {
-    ngx_http_finalize_request(&req, NGX_ERROR);
-    return;
+    return NGX_ERROR;
   }
 
   b->pos = templ->data;
@@ -352,9 +375,10 @@ void BlockingService::block(BlockSpecification spec, ngx_http_request_t &req) {
   ngx_chain_t out{};
   out.buf = b;
 
-  // TODO: bypass and call ngx_http_write_filter?
-  ngx_http_output_filter(&req, &out);
-  ngx_http_finalize_request(&req, NGX_DONE);
+  res = ngx_http_output_filter(&req, &out);
+  ngx_log_debug(NGX_LOG_DEBUG, req.connection->log, 0,
+                "block(): status %d returned by ngx_http_output_filter", res);
+  return res;
 }
 
 BlockingService::BlockingService(

src/security/blocking.h

@@ -1,10 +1,8 @@
 #pragma once
 
-#include <fstream>
 #include <memory>
 #include <optional>
-#include <sstream>
-#include <stdexcept>
+#include <string>
 #include <string_view>
 
 extern "C" {
@@ -37,7 +35,8 @@ class BlockingService {
 
   static BlockingService *get_instance() { return instance.get(); }
 
-  void block(BlockSpecification spec, ngx_http_request_t &req);
+  [[nodiscard]] ngx_int_t block(BlockSpecification spec,
+                                ngx_http_request_t &req);
 
  private:
   BlockingService(std::optional<std::string_view> templ_html_path,