From 76f9c3e0620af3d9b87a5b525ba24099efe93781 Mon Sep 17 00:00:00 2001 From: "Ryan.K" Date: Mon, 1 Dec 2025 23:23:40 +0800 Subject: [PATCH] fix: replace unsafe transmute with Request::from_ngx_http_request This fixes a critical memory safety issue that caused SIGSEGV crashes when processing OPTIONS requests. The issue was caused by unsafe mem::transmute assuming identical memory layouts between ngx_http_request_t and ngx::http::Request, which can break across different Nginx versions or compiler settings. Changes: - Replace mem::transmute with ngx-rust official API Request::from_ngx_http_request - Add null pointer checks before creating Request objects - Remove unnecessary unsafe keywords - Clean up temporary debugging scripts and test files - Update test configurations to match production environment - Bump version to 1.3.2 Fixes memory corruption in ngx_http_update_location_config when processing CORS preflight OPTIONS requests. --- .gitignore | 1 + Cargo.toml | 2 +- debian/changelog | 8 +++ src/ngx_module/mod.rs | 44 ++++++------ tests/Dockerfile.test | 156 +++++++++++++++++++++++++++++------------- tests/nginx.test.conf | 57 ++++++++++++++- 6 files changed, 197 insertions(+), 71 deletions(-) diff --git a/.gitignore b/.gitignore index dfeacf1..8a932cd 100644 --- a/.gitignore +++ b/.gitignore @@ -22,6 +22,7 @@ # Documentation files (exclude all .md except README.md) *.md !README.md +docs/ build/ # Generated files diff --git a/Cargo.toml b/Cargo.toml index 1fd9a58..c32b5f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nginx-x402" -version = "1.3.1" +version = "1.3.2" edition = "2021" description = "Pure Rust Nginx module for x402 HTTP micropayment protocol" authors = ["Ryan Kung "] diff --git a/debian/changelog b/debian/changelog index a5e504c..511d66c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +nginx-x402 (1.3.2-1) unstable; urgency=medium + + * Fix critical memory safety issue causing SIGSEGV crashes with OPTIONS requests + * Replace unsafe mem::transmute with ngx-rust official API Request::from_ngx_http_request + * Add null pointer checks before creating Request objects + * Fix crash in ngx_http_update_location_config when processing CORS preflight requests + * Improve code safety by using official ngx-rust APIs instead of unsafe transmute + nginx-x402 (1.3.1-1) unstable; urgency=medium * Version bump to 1.3.1 diff --git a/src/ngx_module/mod.rs b/src/ngx_module/mod.rs index 4f6c91e..b3f2145 100644 --- a/src/ngx_module/mod.rs +++ b/src/ngx_module/mod.rs @@ -69,9 +69,11 @@ pub use runtime::{ pub unsafe extern "C" fn x402_metrics_handler( r: *mut ngx::ffi::ngx_http_request_t, ) -> ngx::ffi::ngx_int_t { - use std::mem; - let req_ptr: *mut ngx::http::Request = mem::transmute(r); - let req_mut = &mut *req_ptr; + if r.is_null() { + return ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t; + } + + let req_mut = ngx::http::Request::from_ngx_http_request(r); match x402_metrics_handler_impl(req_mut) { ngx::core::Status::NGX_OK => ngx::ffi::NGX_OK as ngx::ffi::ngx_int_t, @@ -91,11 +93,7 @@ pub unsafe extern "C" fn x402_metrics_handler( /// # Safety /// /// The caller must ensure that `r` is a valid pointer to a `ngx_http_request_t`. -unsafe fn clear_x402_content_handler( - r: *mut ngx::ffi::ngx_http_request_t, - req_mut: &mut ngx::http::Request, - reason: &str, -) { +unsafe fn clear_x402_content_handler(r: *mut ngx::ffi::ngx_http_request_t, reason: &str) { use ngx::ffi::ngx_http_request_t; let r_raw = r.cast::(); if r_raw.is_null() { @@ -120,7 +118,7 @@ unsafe fn clear_x402_content_handler( (*r_raw).content_handler = None; use crate::ngx_module::logging::log_debug; log_debug( - Some(req_mut), + None, &format!( "[x402] Phase handler: Cleared x402 content handler {}", reason @@ -149,9 +147,14 @@ unsafe fn clear_x402_content_handler( pub unsafe extern "C" fn x402_phase_handler( r: *mut ngx::ffi::ngx_http_request_t, ) -> ngx::ffi::ngx_int_t { - use std::mem; - let req_ptr: *mut ngx::http::Request = mem::transmute(r); - let req_mut = &mut *req_ptr; + // Validate pointer + if r.is_null() { + return ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t; + } + + // Create Request object safely using ngx-rust's API + // Request is a zero-cost wrapper (repr(transparent)), so we can safely create it from the raw pointer + let req_mut = ngx::http::Request::from_ngx_http_request(r); use crate::ngx_module::logging::log_debug; use crate::ngx_module::module::get_module_config; @@ -178,7 +181,6 @@ pub unsafe extern "C" fn x402_phase_handler( let method_id = request_struct.method; let detected_method = unsafe { get_http_method(r) }; - // Debug: Log method ID and detected method for troubleshooting log_debug( Some(req_mut), &format!( @@ -202,7 +204,6 @@ pub unsafe extern "C" fn x402_phase_handler( unsafe { clear_x402_content_handler( r, - req_mut, &format!("for {} request to prevent payment verification", method), ); } @@ -217,11 +218,7 @@ pub unsafe extern "C" fn x402_phase_handler( ); // Clear content handler if it's x402_ngx_handler to prevent payment verification in CONTENT_PHASE unsafe { - clear_x402_content_handler( - r, - req_mut, - "for WebSocket request to prevent payment verification", - ); + clear_x402_content_handler(r, "for WebSocket request to prevent payment verification"); } return ngx::ffi::NGX_DECLINED as ngx::ffi::ngx_int_t; } @@ -299,7 +296,6 @@ pub unsafe extern "C" fn x402_phase_handler( unsafe { clear_x402_content_handler( r, - req_mut, "after payment verification to prevent duplicate verification", ); } @@ -348,9 +344,11 @@ pub unsafe extern "C" fn x402_phase_handler( pub unsafe extern "C" fn x402_ngx_handler( r: *mut ngx::ffi::ngx_http_request_t, ) -> ngx::ffi::ngx_int_t { - use std::mem; - let req_ptr: *mut ngx::http::Request = mem::transmute(r); - let req_mut = &mut *req_ptr; + if r.is_null() { + return ngx::ffi::NGX_ERROR as ngx::ffi::ngx_int_t; + } + + let req_mut = ngx::http::Request::from_ngx_http_request(r); let (status, _result) = x402_ngx_handler_impl(req_mut); match status { diff --git a/tests/Dockerfile.test b/tests/Dockerfile.test index 375a48a..e74e2b9 100644 --- a/tests/Dockerfile.test +++ b/tests/Dockerfile.test @@ -1,13 +1,14 @@ -# Single-stage Dockerfile for integration testing -# Builds nginx and module in the same environment to ensure compatibility -# Optimized for fast iteration: dependencies are cached separately from code +# Dockerfile for Debian integration testing +# Uses system nginx installed via apt-get (matches production environment) +# Builds module against system nginx to ensure compatibility -FROM ubuntu:22.04 +ARG DEBIAN_VERSION=bookworm +FROM debian:${DEBIAN_VERSION} # Avoid interactive prompts ENV DEBIAN_FRONTEND=noninteractive -# Install all dependencies in a single layer for better caching +# Install all dependencies including system nginx # This layer will be cached unless dependencies change RUN apt-get update && apt-get install -y \ curl \ @@ -22,41 +23,93 @@ RUN apt-get update && apt-get install -y \ wget \ python3 \ python3-pip \ + nginx \ && rm -rf /var/lib/apt/lists/* # Install Rust toolchain (cached separately from system packages) # This layer will be cached unless Rust version changes -RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y +# Using stable 1.91.1 to match production environment (deb package actually uses this) +# Production deb install log shows: "Using Rust version from rustup: 1.91.1" +# The nightly 1.86.0 shown in production is system default, but deb uses rustup stable +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && \ + /root/.cargo/bin/rustup toolchain install stable && \ + /root/.cargo/bin/rustup default stable && \ + echo "=== Installed Rust version ===" && \ + /root/.cargo/bin/rustc --version && \ + echo "=== Expected: rustc 1.91.1 (ed61e7d7e 2025-11-07) ===" ENV PATH="/root/.cargo/bin:${PATH}" -# Download and configure nginx (cached separately from code) +# Detect system nginx version and download matching source # This layer will be cached unless nginx version changes -RUN cd /tmp \ - && wget -q https://nginx.org/download/nginx-1.28.0.tar.gz \ - && tar -xzf nginx-1.28.0.tar.gz \ - && cd nginx-1.28.0 \ - && ./configure --prefix=/etc/nginx \ - --sbin-path=/usr/sbin/nginx \ - --modules-path=/usr/lib/nginx/modules \ - --conf-path=/etc/nginx/nginx.conf \ - --error-log-path=/var/log/nginx/error.log \ - --http-log-path=/var/log/nginx/access.log \ - --pid-path=/var/run/nginx.pid \ - --lock-path=/var/run/nginx.lock \ - --with-http_ssl_module \ - --with-http_realip_module \ - --with-http_gzip_static_module \ - --with-threads \ - --with-file-aio \ - && echo "Nginx configured successfully" - -# Build and install nginx (cached separately from module code) -# This layer will be cached unless nginx source or build config changes -RUN cd /tmp/nginx-1.28.0 \ - && make -j$(nproc) \ - && make install \ - && mkdir -p /var/cache/nginx /var/log/nginx \ - && useradd -r -s /bin/false nginx 2>/dev/null || true +RUN set -e && \ + # Detect nginx version from system installation + NGINX_VERSION=$(nginx -v 2>&1 | sed -n 's/.*nginx\/\([0-9]\+\.[0-9]\+\.[0-9]\+\).*/\1/p' | head -1) && \ + echo "Detected system nginx version: $NGINX_VERSION" && \ + # Get system nginx configure arguments + NGINX_CONFIGURE_ARGS_SYSTEM=$(nginx -V 2>&1 | grep -oE 'configure arguments:.*' | sed 's/configure arguments://' || echo "") && \ + echo "System nginx configure args: $NGINX_CONFIGURE_ARGS_SYSTEM" && \ + # Download matching nginx source + cd /tmp && \ + wget -q "https://nginx.org/download/nginx-$NGINX_VERSION.tar.gz" && \ + tar -xzf "nginx-$NGINX_VERSION.tar.gz" && \ + rm "nginx-$NGINX_VERSION.tar.gz" && \ + # Clean configure arguments (remove problematic modules, preserve signature-affecting flags) + # IMPORTANT: Only remove modules that cause build failures + # DO NOT remove --with-cc-opt, --with-ld-opt, --with-debug, or other flags + # that affect module signature and binary compatibility + CONFIGURE_ARGS_CLEAN=$(echo "$NGINX_CONFIGURE_ARGS_SYSTEM" | \ + sed 's/--with-http_rewrite_module//g' | \ + sed 's/--with-http_xslt_module=dynamic//g' | \ + sed 's/--with-http_perl_module=dynamic//g' | \ + sed 's/--with-http_image_filter_module=dynamic//g' | \ + sed 's/--with-http_geoip_module=dynamic//g' | \ + sed 's/--with-mail=dynamic//g' | \ + sed 's/--with-stream=dynamic//g' | \ + sed 's/--with-stream_geoip_module=dynamic//g' | \ + sed 's/--add-dynamic-module=[^ ]*//g' | \ + sed 's/ */ /g' | sed 's/^ *//' | sed 's/ *$//') && \ + # Handle PCRE: if --with-pcre-jit is present but --with-pcre is not, add --with-pcre (no path) + # This tells nginx to use system PCRE library (via pkg-config) instead of PCRE source + case "$CONFIGURE_ARGS_CLEAN" in \ + *--with-pcre-jit*) \ + case "$CONFIGURE_ARGS_CLEAN" in \ + *--with-pcre[[:space:]]*) \ + ;; \ + *) \ + CONFIGURE_ARGS_CLEAN="$CONFIGURE_ARGS_CLEAN --with-pcre" && \ + echo "Added --with-pcre (system library) for PCRE support" \ + ;; \ + esac \ + ;; \ + esac && \ + # Add --without-http_rewrite_module if not already present + if echo "$CONFIGURE_ARGS_CLEAN" | grep -qv -- "--without-http_rewrite_module"; then \ + CONFIGURE_ARGS_CLEAN="$CONFIGURE_ARGS_CLEAN --without-http_rewrite_module"; \ + fi && \ + # Configure nginx source with cleaned system arguments + cd "nginx-$NGINX_VERSION" && \ + if [ -z "$CONFIGURE_ARGS_CLEAN" ]; then \ + echo "ERROR: No configure arguments available from system nginx"; \ + exit 1; \ + fi && \ + echo "Configuring with system arguments:" && \ + echo "$CONFIGURE_ARGS_CLEAN" && \ + eval "./configure $CONFIGURE_ARGS_CLEAN" >/tmp/nginx-configure.log 2>&1 || { \ + echo "ERROR: Configure failed with system arguments"; \ + if [ -f /tmp/nginx-configure.log ]; then \ + echo "Configure log:"; \ + tail -50 /tmp/nginx-configure.log; \ + fi; \ + exit 1; \ + } && \ + # Verify configure succeeded + if [ ! -f "objs/ngx_auto_config.h" ]; then \ + echo "ERROR: Configure failed - ngx_auto_config.h not found"; \ + exit 1; \ + fi && \ + echo "✓ Nginx source configured successfully" && \ + # Store version for later use + echo "$NGINX_VERSION" > /tmp/nginx_version.txt # Set working directory WORKDIR /build @@ -71,25 +124,35 @@ COPY build.rs ./ COPY src ./src COPY nginx ./nginx -# Set environment variables for module build -# Use the nginx source directory we just configured -ENV NGINX_SOURCE_DIR=/tmp/nginx-1.28.0 ENV NGX_CONFIGURE_ARGS="--without-http_rewrite_module" ENV RUST_BACKTRACE=1 -# Debug: Check what files exist and their content -RUN echo "=== Debug: Check ngx_auto_config.h ===" && \ - if [ -f /tmp/nginx-1.28.0/objs/ngx_auto_config.h ]; then \ +# Debug: Check what files exist and their content +# Also set NGINX_SOURCE_DIR for the build step +RUN NGINX_VERSION=$(cat /tmp/nginx_version.txt) && \ + echo "=== Debug: Check ngx_auto_config.h ===" && \ + if [ -f "/tmp/nginx-$NGINX_VERSION/objs/ngx_auto_config.h" ]; then \ echo "✅ ngx_auto_config.h exists"; \ - grep -E "^#define NGX_PTR_SIZE|^#define NGX_SIG_ATOMIC_T_SIZE|^#define NGX_TIME_T_SIZE" /tmp/nginx-1.28.0/objs/ngx_auto_config.h | head -5; \ + grep -E "^#define NGX_PTR_SIZE|^#define NGX_SIG_ATOMIC_T_SIZE|^#define NGX_TIME_T_SIZE" "/tmp/nginx-$NGINX_VERSION/objs/ngx_auto_config.h" | head -5; \ else \ echo "❌ ngx_auto_config.h not found"; \ - fi - -# Build the module using the same nginx source (no vendored feature) + fi && \ + echo "=== System nginx info ===" && \ + nginx -v && \ + nginx -V 2>&1 | head -1 && \ + echo "=== Module will be built against ===" && \ + echo "NGINX_SOURCE_DIR=/tmp/nginx-$NGINX_VERSION" && \ + # Set NGINX_SOURCE_DIR in a way that persists for cargo build + echo "export NGINX_SOURCE_DIR=/tmp/nginx-$NGINX_VERSION" >> /root/.bashrc && \ + echo "/tmp/nginx-$NGINX_VERSION" > /tmp/nginx_source_dir.txt + +# Build the module using the nginx source configured with system nginx arguments # This ensures exact version and compilation environment match -# Cargo will cache dependencies automatically, so only source changes trigger rebuild -RUN cargo build --release --no-default-features +# Cargo will cache dependencies automatically, only source changes trigger rebuild +RUN NGINX_SOURCE_DIR=$(cat /tmp/nginx_source_dir.txt) && \ + export NGINX_SOURCE_DIR && \ + echo "Building module with NGINX_SOURCE_DIR=$NGINX_SOURCE_DIR" && \ + cargo build --release --no-default-features # Copy the built module RUN mkdir -p /usr/lib/nginx/modules \ @@ -102,7 +165,8 @@ COPY tests/nginx.test.conf /etc/nginx/nginx.conf RUN mkdir -p /var/www/html /var/log/nginx # Install Flask for mock backend -RUN pip3 install --no-cache-dir flask +# Debian 12+ requires --break-system-packages flag for pip installs +RUN pip3 install --break-system-packages --no-cache-dir flask # Copy mock backend server script COPY tests/mock-backend.py /usr/local/bin/mock-backend.py diff --git a/tests/nginx.test.conf b/tests/nginx.test.conf index c52da7e..d6ff645 100644 --- a/tests/nginx.test.conf +++ b/tests/nginx.test.conf @@ -16,9 +16,14 @@ http { error_log /var/log/nginx/error.log debug; # Upstream backend server (mock backend for proxy_pass testing) + # Match production environment: use keepalive for connection pooling upstream backend { server 127.0.0.1:9999; + keepalive 32; } + + # Rate limiting zones (match production environment) + limit_req_zone $binary_remote_addr zone=api_limit:10m rate=100r/s; server { listen 80; @@ -46,7 +51,57 @@ http { } # Protected location with x402 payment AND proxy_pass - # This tests that x402 handler works correctly even when proxy_pass is configured + # Match production environment configuration + location /api/ { + # Rate limiting (match production) + limit_req zone=api_limit burst=100 nodelay; + + # Enable x402 payment protection + x402 on; + + # Payment configuration + x402_amount 0.0001; + x402_pay_to 0x209693Bc6afc0C5328bA36FaF03C514EF312287C; + x402_facilitator_url https://x402.org/facilitator; + x402_network base-sepolia; + x402_facilitator_fallback error; + x402_description "Test API access payment with proxy"; + + # proxy_pass with keepalive support (match production) + proxy_pass http://backend; + proxy_http_version 1.1; + proxy_set_header Connection ""; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + + # WebSocket support (match production) + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection "upgrade"; + + proxy_redirect off; + } + + # Additional location without x402 (match production /mcp/ pattern) + location /api/no-x402/ { + limit_req zone=api_limit burst=100 nodelay; + proxy_pass http://backend; + proxy_http_version 1.1; + proxy_set_header Connection ""; + proxy_set_header Host $host; + proxy_redirect off; + } + + # Health check endpoint (match production) + location = /api/health { + proxy_pass http://backend; + access_log off; + } + + # Keep original protected-proxy location for backward compatibility location /api/protected-proxy { # Enable x402 payment protection x402 on;