diff --git a/.gitignore b/.gitignore index 1cd17c9..24e9a77 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,4 @@ coverage/ *.rpm *.deb *.tar.gz +.aider* diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index df1367d..f24ca80 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,7 +7,9 @@ stages: - package - verify +# ============================================================================= # Variables +# ============================================================================= variables: DOCKER_TLS_CERTDIR: "/certs" DOCKER_DRIVER: overlay2 @@ -23,7 +25,7 @@ build-packages: services: - docker:24-dind script: - # Build all packages (DEB + RPMs for el7, el8, el9) + # Build all packages (DEB + RPMs for el8, el9, el10) - docker build -f Dockerfile.package --target output --build-arg VERSION=$VERSION -t mod_reqin_log:packages . # Create output directories @@ -67,21 +69,6 @@ unit-tests: # Verify Stage - Test package installation on each target distribution # ============================================================================= -verify-rpm-el7: - stage: verify - image: docker:24 - services: - - docker:24-dind - needs: [build-packages] - script: - # Download artifacts - - apk add --no-cache curl - - curl -L -o /tmp/rpm-el7.rpm "$CI_PROJECT_URL/-/jobs/$CI_JOB_ID/artifacts/dist/rpm/mod_reqin_log-$VERSION-1.el7.x86_64.rpm" 2>/dev/null || echo "Artifact download via curl failed, trying alternative..." - - # Alternative: extract from build artifact - - docker run --rm -v $(pwd)/dist:/packages centos:7 sh -c "sed -i 's/mirror.centos.org/vault.centos.org/g' /etc/yum.repos.d/*.repo && sed -i 's/#baseurl/baseurl/g' /etc/yum.repos.d/*.repo && sed -i 's/metalink/#metalink/g' /etc/yum.repos.d/*.repo && yum install -y /packages/rpm/*.el7.*.rpm && httpd -M 2>&1 | grep reqin_log && echo 'RPM el7 verification OK'" - allow_failure: true - verify-rpm-el8: stage: verify image: docker:24 @@ -100,6 +87,15 @@ verify-rpm-el9: script: - docker run --rm -v $(pwd)/dist:/packages rockylinux:9 sh -c "dnf install -y /packages/rpm/*.el9.*.rpm && httpd -M 2>&1 | grep reqin_log && echo 'RPM el9 verification OK'" +verify-rpm-el10: + stage: verify + image: docker:24 + services: + - docker:24-dind + needs: [build-packages] + script: + - docker run --rm -v $(pwd)/dist:/packages almalinux:10 sh -c "dnf install -y /packages/rpm/*.el10.*.rpm && httpd -M 2>&1 | grep reqin_log && echo 'RPM el10 verification OK'" + verify-deb: stage: verify image: docker:24 diff --git a/Dockerfile.package b/Dockerfile.package index 670cb26..733393f 100644 --- a/Dockerfile.package +++ b/Dockerfile.package @@ -2,43 +2,13 @@ # ============================================================================= # mod_reqin_log - Dockerfile de packaging unifié (DEB + RPM avec fpm) # Builds RPMs for multiple RHEL-compatible versions: -# - AlmaLinux 7 / CentOS 7 (el7) - RHEL 7 compatible (using vault repos) # - Rocky Linux 8 (el8) - RHEL 8 compatible # - Rocky Linux 9 (el9) - RHEL 9 compatible +# - AlmaLinux 10 (el10) - RHEL 10 compatible # ============================================================================= # ============================================================================= -# Stage 1a: Builder CentOS 7 (RHEL 7 compatible, EOL - using vault) -# ============================================================================= -FROM centos:7 AS builder-el7 - -# CentOS 7 is EOL since June 2024, use vault.centos.org for repositories -RUN sed -i 's/mirror.centos.org/vault.centos.org/g' /etc/yum.repos.d/*.repo && \ - sed -i 's/#baseurl/baseurl/g' /etc/yum.repos.d/*.repo && \ - sed -i 's/metalink/#metalink/g' /etc/yum.repos.d/*.repo - -# Install build dependencies -RUN yum install -y \ - gcc \ - make \ - httpd \ - httpd-devel \ - apr-devel \ - apr-util-devel \ - python3 \ - curl \ - redhat-rpm-config \ - && yum clean all - -WORKDIR /build -COPY src/ src/ -COPY Makefile Makefile -COPY conf/ conf/ -RUN make APXS=/usr/bin/apxs -RUN ls -la modules/mod_reqin_log.so - -# ============================================================================= -# Stage 1b: Builder Rocky Linux 8 +# Stage 1a: Builder Rocky Linux 8 # ============================================================================= FROM rockylinux:8 AS builder-el8 @@ -63,7 +33,7 @@ RUN make APXS=/usr/bin/apxs RUN ls -la modules/mod_reqin_log.so # ============================================================================= -# Stage 1c: Builder Rocky Linux 9 +# Stage 1b: Builder Rocky Linux 9 # ============================================================================= FROM rockylinux:9 AS builder-el9 @@ -87,6 +57,31 @@ COPY conf/ conf/ RUN make APXS=/usr/bin/apxs RUN ls -la modules/mod_reqin_log.so +# ============================================================================= +# Stage 1c: Builder AlmaLinux 10 (RHEL 10 compatible) +# ============================================================================= +FROM almalinux:10 AS builder-el10 + +RUN dnf install -y epel-release && \ + dnf install -y --allowerasing \ + gcc \ + make \ + httpd \ + httpd-devel \ + apr-devel \ + apr-util-devel \ + python3 \ + curl \ + redhat-rpm-config \ + && dnf clean all + +WORKDIR /build +COPY src/ src/ +COPY Makefile Makefile +COPY conf/ conf/ +RUN make APXS=/usr/bin/apxs +RUN ls -la modules/mod_reqin_log.so + # ============================================================================= # Stage 2: Package builder - fpm pour DEB et RPM # ============================================================================= @@ -108,12 +103,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ # Copy binaries from each builder stage # ============================================================================= -# CentOS 7 (el7) -COPY --from=builder-el7 /build/modules/mod_reqin_log.so /tmp/pkgroot-el7/usr/lib64/httpd/modules/mod_reqin_log.so -COPY --from=builder-el7 /build/conf/mod_reqin_log.conf /tmp/pkgroot-el7/etc/httpd/conf.d/mod_reqin_log.conf -RUN chmod 755 /tmp/pkgroot-el7/usr/lib64/httpd/modules/mod_reqin_log.so && \ - chmod 644 /tmp/pkgroot-el7/etc/httpd/conf.d/mod_reqin_log.conf - # Rocky Linux 8 (el8) COPY --from=builder-el8 /build/modules/mod_reqin_log.so /tmp/pkgroot-el8/usr/lib64/httpd/modules/mod_reqin_log.so COPY --from=builder-el8 /build/conf/mod_reqin_log.conf /tmp/pkgroot-el8/etc/httpd/conf.d/mod_reqin_log.conf @@ -126,9 +115,15 @@ COPY --from=builder-el9 /build/conf/mod_reqin_log.conf /tmp/pkgroot-el9/etc/http RUN chmod 755 /tmp/pkgroot-el9/usr/lib64/httpd/modules/mod_reqin_log.so && \ chmod 644 /tmp/pkgroot-el9/etc/httpd/conf.d/mod_reqin_log.conf +# AlmaLinux 10 (el10) +COPY --from=builder-el10 /build/modules/mod_reqin_log.so /tmp/pkgroot-el10/usr/lib64/httpd/modules/mod_reqin_log.so +COPY --from=builder-el10 /build/conf/mod_reqin_log.conf /tmp/pkgroot-el10/etc/httpd/conf.d/mod_reqin_log.conf +RUN chmod 755 /tmp/pkgroot-el10/usr/lib64/httpd/modules/mod_reqin_log.so && \ + chmod 644 /tmp/pkgroot-el10/etc/httpd/conf.d/mod_reqin_log.conf + # DEB package (Debian paths) -COPY --from=builder-el9 /build/modules/mod_reqin_log.so /tmp/pkgroot-deb/usr/lib/apache2/modules/mod_reqin_log.so -COPY --from=builder-el9 /build/conf/mod_reqin_log.conf /tmp/pkgroot-deb/etc/apache2/conf-available/mod_reqin_log.conf +COPY --from=builder-el10 /build/modules/mod_reqin_log.so /tmp/pkgroot-deb/usr/lib/apache2/modules/mod_reqin_log.so +COPY --from=builder-el10 /build/conf/mod_reqin_log.conf /tmp/pkgroot-deb/etc/apache2/conf-available/mod_reqin_log.conf RUN chmod 755 /tmp/pkgroot-deb/usr/lib/apache2/modules/mod_reqin_log.so && \ chmod 644 /tmp/pkgroot-deb/etc/apache2/conf-available/mod_reqin_log.conf @@ -155,26 +150,9 @@ RUN mkdir -p /packages/deb && \ # Build RPM packages for each distribution # ============================================================================= -# CentOS 7 (el7) +# Rocky Linux 8 (el8) ARG VERSION=1.0.0 RUN mkdir -p /packages/rpm && \ - fpm -s dir -t rpm \ - -n mod_reqin_log \ - -v "${VERSION}" \ - --rpm-dist el7 \ - -C /tmp/pkgroot-el7 \ - --architecture "x86_64" \ - --description "Apache HTTPD module for logging HTTP requests as JSON to Unix socket" \ - --url "https://github.com/example/mod_reqin_log" \ - --license "Apache-2.0" \ - --vendor "Developer " \ - --depends "httpd" \ - -p /packages/rpm/mod_reqin_log-${VERSION}-1.el7.x86_64.rpm \ - usr/lib64/httpd/modules/mod_reqin_log.so \ - etc/httpd/conf.d/mod_reqin_log.conf - -# Rocky Linux 8 (el8) -RUN \ fpm -s dir -t rpm \ -n mod_reqin_log \ -v "${VERSION}" \ @@ -207,6 +185,23 @@ RUN \ usr/lib64/httpd/modules/mod_reqin_log.so \ etc/httpd/conf.d/mod_reqin_log.conf +# AlmaLinux 10 (el10) +RUN \ + fpm -s dir -t rpm \ + -n mod_reqin_log \ + -v "${VERSION}" \ + --rpm-dist el10 \ + -C /tmp/pkgroot-el10 \ + --architecture "x86_64" \ + --description "Apache HTTPD module for logging HTTP requests as JSON to Unix socket" \ + --url "https://github.com/example/mod_reqin_log" \ + --license "Apache-2.0" \ + --vendor "Developer " \ + --depends "httpd" \ + -p /packages/rpm/mod_reqin_log-${VERSION}-1.el10.x86_64.rpm \ + usr/lib64/httpd/modules/mod_reqin_log.so \ + etc/httpd/conf.d/mod_reqin_log.conf + # ============================================================================= # Stage 3: Output - Image finale avec les packages # ============================================================================= diff --git a/Makefile b/Makefile index 8859063..552bf3e 100644 --- a/Makefile +++ b/Makefile @@ -83,10 +83,10 @@ debug: clean all # Packaging (DEB + RPM with Docker + fpm) # Dockerfile.package builds all packages in a single multi-stage build: # - 1 DEB package (Debian/Ubuntu) -# - 3 RPM packages (el7, el8, el9 for RHEL/CentOS/Rocky compatibility) +# - 3 RPM packages (el8, el9, el10 for RHEL/Rocky/AlmaLinux compatibility) # ============================================================================= -## package: Build all packages (deb + rpm for el7, el8, el9) +## package: Build all packages (deb + rpm for el8, el9, el10) package: mkdir -p $(DIST_DIR)/deb $(DIST_DIR)/rpm docker build --target output -t mod_reqin_log:packager \ @@ -98,14 +98,14 @@ package: @echo "Packages created:" @echo " DEB:" @ls -la $(DIST_DIR)/deb/ - @echo " RPM (el7, el8, el9):" + @echo " RPM (el8, el9, el10):" @ls -la $(DIST_DIR)/rpm/ ## package-deb: Build DEB package (built together with RPMs in Dockerfile.package) package-deb: package @echo "DEB package built together with RPMs in Dockerfile.package" -## package-rpm: Build RPM packages (el7, el8, el9 built together in Dockerfile.package) +## package-rpm: Build RPM packages (el8, el9, el10 built together in Dockerfile.package) package-rpm: package @echo "RPM packages built together with DEB in Dockerfile.package" @@ -119,14 +119,6 @@ test-package-rpm: package docker run --rm -v $(PWD)/$(DIST_DIR)/rpm:/packages:ro rockylinux:9 \ sh -c "dnf install -y /packages/*.el9.*.rpm && echo 'RPM el9 install OK'" -## test-package-rpm-el7: Test el7 RPM installation -test-package-rpm-el7: package - docker run --rm -v $(PWD)/$(DIST_DIR)/rpm:/packages:ro centos:7 \ - sh -c "sed -i 's/mirror.centos.org/vault.centos.org/g' /etc/yum.repos.d/*.repo && \ - sed -i 's/#baseurl/baseurl/g' /etc/yum.repos.d/*.repo && \ - sed -i 's/metalink/#metalink/g' /etc/yum.repos.d/*.repo && \ - yum install -y /packages/*.el7.*.rpm && echo 'RPM el7 install OK'" - ## test-package-rpm-el8: Test el8 RPM installation test-package-rpm-el8: package docker run --rm -v $(PWD)/$(DIST_DIR)/rpm:/packages:ro rockylinux:8 \ @@ -137,8 +129,13 @@ test-package-rpm-el9: package docker run --rm -v $(PWD)/$(DIST_DIR)/rpm:/packages:ro rockylinux:9 \ sh -c "dnf install -y /packages/*.el9.*.rpm && echo 'RPM el9 install OK'" +## test-package-rpm-el10: Test el10 RPM installation +test-package-rpm-el10: package + docker run --rm -v $(PWD)/$(DIST_DIR)/rpm:/packages:ro almalinux:10 \ + sh -c "dnf install -y /packages/*.el10.*.rpm && echo 'RPM el10 install OK'" + ## test-package: Test all packages installation -test-package: test-package-deb test-package-rpm-el7 test-package-rpm-el8 test-package-rpm-el9 +test-package: test-package-deb test-package-rpm-el8 test-package-rpm-el9 test-package-rpm-el10 # Help target help: @@ -151,7 +148,7 @@ help: @echo " clean - Remove build artifacts" @echo " test - Run unit tests" @echo " debug - Build with debug symbols" - @echo " package - Build all packages (deb + rpm for el7, el8, el9)" + @echo " package - Build all packages (deb + rpm for el8, el9, el10)" @echo " package-deb - Build DEB package" @echo " package-rpm - Build RPM packages" @echo " test-package - Test package installation" diff --git a/architecture.yml b/architecture.yml index 5bb8199..35213d4 100644 --- a/architecture.yml +++ b/architecture.yml @@ -7,7 +7,7 @@ project: target: server: apache-httpd version: "2.4" - os: rocky-linux-8+ + os: rocky-linux-8+, almalinux-10+ build: toolchain: gcc apache_dev: httpd-devel (apxs) @@ -313,7 +313,7 @@ testing: location: tests/integration/test_integration.py env: server: apache-httpd 2.4 - os: rocky-linux-8+ + os: rocky-linux-8+, rocky-linux-9+, almalinux-10+ log_consumer: Unix socket server (Python threading) scenarios: - name: basic_logging @@ -368,10 +368,9 @@ ci: dind: true workflow_file: .gitlab-ci.yml rpm_strategy: > - Separate RPMs are built for each major RHEL/CentOS/Rocky version - (el7, el8, el9) due to glibc and httpd-devel incompatibilities + Separate RPMs are built for each major RHEL/CentOS/Rocky/AlmaLinux version + (el8, el9, el10) due to glibc and httpd-devel incompatibilities across major versions. A single RPM cannot work across all versions. - Note: CentOS 7 is EOL since June 2024, repositories use vault.centos.org. All packages (DEB + multi-RPM) are built from Dockerfile.package. stages: - name: build @@ -380,9 +379,9 @@ ci: dockerfile: Dockerfile.package artifacts: - dist/deb/*.deb - - dist/rpm/*.el7.*.rpm - dist/rpm/*.el8.*.rpm - dist/rpm/*.el9.*.rpm + - dist/rpm/*.el10.*.rpm - name: test description: > @@ -394,16 +393,15 @@ ci: description: > Verify package installation on each target distribution. jobs: - - name: verify-rpm-el7 - image: centos:7 - vault_repos: true - check: "httpd -M | grep reqin_log" - name: verify-rpm-el8 image: rockylinux:8 check: "httpd -M | grep reqin_log" - name: verify-rpm-el9 image: rockylinux:9 check: "httpd -M | grep reqin_log" + - name: verify-rpm-el10 + image: almalinux:10 + check: "httpd -M | grep reqin_log" - name: verify-deb image: debian:stable check: "ls -la /usr/lib/apache2/modules/mod_reqin_log.so" diff --git a/src/mod_reqin_log.c b/src/mod_reqin_log.c index 104ff2a..1f82685 100644 --- a/src/mod_reqin_log.c +++ b/src/mod_reqin_log.c @@ -15,6 +15,7 @@ #include "apr_time.h" #include "apr_lib.h" #include "ap_config.h" +#include "ap_mpm.h" #include #include @@ -237,10 +238,33 @@ static void format_iso8601(dynbuf_t *db, apr_time_t t) apr_time_exp_t tm; apr_time_exp_gmt(&tm, t); + /* Validate time components to prevent buffer overflow and invalid output. + * apr_time_exp_gmt should always produce valid values, but we validate + * defensively to catch any APR bugs or memory corruption. */ + int year = tm.tm_year + 1900; + int mon = tm.tm_mon + 1; + int day = tm.tm_mday; + int hour = tm.tm_hour; + int min = tm.tm_min; + int sec = tm.tm_sec; + + /* Clamp values to valid ranges to ensure fixed-width output (20 chars + null) */ + if (year < 0) year = 0; + if (year > 9999) year = 9999; + if (mon < 1) mon = 1; + if (mon > 12) mon = 12; + if (day < 1) day = 1; + if (day > 31) day = 31; + if (hour < 0) hour = 0; + if (hour > 23) hour = 23; + if (min < 0) min = 0; + if (min > 59) min = 59; + if (sec < 0) sec = 0; + if (sec > 61) sec = 61; + char time_str[32]; snprintf(time_str, sizeof(time_str), "%04d-%02d-%02dT%02d:%02d:%02dZ", - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, - tm.tm_hour, tm.tm_min, tm.tm_sec); + year, mon, day, hour, min, sec); dynbuf_append(db, time_str, -1); } @@ -249,6 +273,10 @@ static int parse_int_strict(const char *arg, int *out) char *end = NULL; long v; if (arg == NULL || *arg == '\0' || out == NULL) return -1; + + /* Reject leading whitespace (strtol skips it by default) */ + if (apr_isspace(*arg)) return -1; + errno = 0; v = strtol(arg, &end, 10); if (errno != 0 || end == arg || *end != '\0' || v < INT_MIN || v > INT_MAX) return -1; @@ -644,7 +672,8 @@ static void log_request(request_rec *r, reqin_log_config_t *cfg, reqin_log_child /* timestamp (nanoseconds since epoch) */ { apr_time_t now = apr_time_now(); - apr_uint64_t ns = (apr_uint64_t)now * 1000; + /* Cast to apr_uint64_t before multiplication to prevent overflow */ + apr_uint64_t ns = ((apr_uint64_t)now) * APR_UINT64_C(1000); char ts_buf[32]; snprintf(ts_buf, sizeof(ts_buf), "%" APR_UINT64_T_FMT, ns); dynbuf_append(&buf, "\"timestamp\":", 12); @@ -799,6 +828,8 @@ static int reqin_log_post_read_request(request_rec *r) static void reqin_log_child_init(apr_pool_t *p, server_rec *s) { reqin_log_server_conf_t *srv_conf = get_server_conf(s); + int threaded_mpm = 0; + int mpm_threads; if (srv_conf == NULL) { return; @@ -809,12 +840,31 @@ static void reqin_log_child_init(apr_pool_t *p, server_rec *s) srv_conf->child_state.last_error_report = 0; srv_conf->child_state.connect_failed = 0; + /* Detect if we're running in a threaded MPM (worker/event) */ + if (ap_mpm_query(AP_MPMQ_IS_THREADED, &mpm_threads) == APR_SUCCESS && + mpm_threads > 0) { + threaded_mpm = 1; + } + srv_conf->child_state.fd_mutex = NULL; if (apr_thread_mutex_create(&srv_conf->child_state.fd_mutex, APR_THREAD_MUTEX_DEFAULT, p) != APR_SUCCESS) { srv_conf->child_state.fd_mutex = NULL; - ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, - MOD_REQIN_LOG_NAME ": Failed to create mutex, continuing in degraded mode"); + + if (threaded_mpm) { + /* Critical error: cannot ensure thread safety in worker/event MPM */ + ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, + MOD_REQIN_LOG_NAME ": Failed to create mutex for thread safety. " + "Module cannot operate safely in threaded MPM. Disabling."); + /* Disable module by clearing config */ + srv_conf->config->enabled = 0; + return; + } else { + /* Prefork MPM: single thread per process, mutex not strictly needed */ + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, + MOD_REQIN_LOG_NAME ": Failed to create mutex, continuing in " + "degraded mode (prefork MPM detected)"); + } } if (srv_conf->config == NULL || !srv_conf->config->enabled || diff --git a/tests/unit/test_json_serialization.c b/tests/unit/test_json_serialization.c index 5da5345..16b1215 100644 --- a/tests/unit/test_json_serialization.c +++ b/tests/unit/test_json_serialization.c @@ -192,10 +192,10 @@ static void test_json_escape_control_chars(void **state) apr_pool_create(&pool, NULL); testbuf_init(&buf, pool, 256); - /* Test with bell character (0x07) */ - append_json_string(&buf, "test\bell"); + /* Test with bell character (0x07) - use octal literal */ + append_json_string(&buf, "test\007bell"); - /* Should contain unicode escape */ + /* Should contain unicode escape for bell (0x07) */ assert_true(strstr(buf.data, "\\u0007") != NULL); apr_pool_destroy(pool); diff --git a/tests/unit/test_module_real.c b/tests/unit/test_module_real.c index c2a5dc0..8219820 100644 --- a/tests/unit/test_module_real.c +++ b/tests/unit/test_module_real.c @@ -1,6 +1,6 @@ /* * test_module_real.c - Real unit tests for mod_reqin_log module - * + * * These tests compile with the actual module source code to test * real implementations, not mocks. */ @@ -12,18 +12,26 @@ #include #include #include +#include +#include #include #include #include #include +#include + +/* Maximum JSON log line size (64KB) - must match module definition */ +#define MAX_JSON_SIZE (64 * 1024) /* Include the module source to test real functions */ /* We need to extract and test specific functions */ /* ============================================================================ - * dynbuf_t structure and functions (copied from module for testing) + * Forward declarations for functions under test + * (These match the implementations in mod_reqin_log.c) * ============================================================================ */ +/* Dynamic string buffer */ typedef struct { char *data; apr_size_t len; @@ -31,6 +39,31 @@ typedef struct { apr_pool_t *pool; } dynbuf_t; +/* Module configuration structure */ +typedef struct { + int enabled; + const char *socket_path; + apr_array_header_t *headers; + int max_headers; + int max_header_value_len; + int reconnect_interval; + int error_report_interval; +} reqin_log_config_t; + +/* Function signatures for testing */ +static void dynbuf_init(dynbuf_t *db, apr_pool_t *pool, apr_size_t initial_capacity); +static void dynbuf_append(dynbuf_t *db, const char *str, apr_size_t len); +static void dynbuf_append_char(dynbuf_t *db, char c); +static void append_json_string(dynbuf_t *db, const char *str); +static void format_iso8601(dynbuf_t *db, apr_time_t t); +static int parse_int_strict(const char *arg, int *out); +static int is_sensitive_header(const char *name); +static char *truncate_header_value(apr_pool_t *pool, const char *value, int max_len); + +/* ============================================================================ + * dynbuf functions (copied from module for testing) + * ============================================================================ */ + static void dynbuf_init(dynbuf_t *db, apr_pool_t *pool, apr_size_t initial_capacity) { db->pool = pool; @@ -116,10 +149,33 @@ static void format_iso8601(dynbuf_t *db, apr_time_t t) apr_time_exp_t tm; apr_time_exp_gmt(&tm, t); + /* Validate time components to prevent buffer overflow and invalid output. + * apr_time_exp_gmt should always produce valid values, but we validate + * defensively to catch any APR bugs or memory corruption. */ + int year = tm.tm_year + 1900; + int mon = tm.tm_mon + 1; + int day = tm.tm_mday; + int hour = tm.tm_hour; + int min = tm.tm_min; + int sec = tm.tm_sec; + + /* Clamp values to valid ranges to ensure fixed-width output (20 chars + null) */ + if (year < 0) year = 0; + if (year > 9999) year = 9999; + if (mon < 1) mon = 1; + if (mon > 12) mon = 12; + if (day < 1) day = 1; + if (day > 31) day = 31; + if (hour < 0) hour = 0; + if (hour > 23) hour = 23; + if (min < 0) min = 0; + if (min > 59) min = 59; + if (sec < 0) sec = 0; + if (sec > 61) sec = 61; + char time_str[32]; snprintf(time_str, sizeof(time_str), "%04d-%02d-%02dT%02d:%02d:%02dZ", - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, - tm.tm_hour, tm.tm_min, tm.tm_sec); + year, mon, day, hour, min, sec); dynbuf_append(db, time_str, -1); } @@ -140,6 +196,55 @@ static char *truncate_header_value(apr_pool_t *pool, const char *value, int max_ return apr_pstrdup(pool, value); } +/* ============================================================================ + * Sensitive header detection (real logic from module) + * ============================================================================ */ + +static const char *DEFAULT_SENSITIVE_HEADERS[] = { + "Authorization", + "Cookie", + "Set-Cookie", + "X-Api-Key", + "X-Auth-Token", + "Proxy-Authorization", + "WWW-Authenticate", + NULL +}; + +static int is_sensitive_header(const char *name) +{ + if (name == NULL) { + return 0; + } + + for (int i = 0; DEFAULT_SENSITIVE_HEADERS[i] != NULL; i++) { + if (strcasecmp(name, DEFAULT_SENSITIVE_HEADERS[i]) == 0) { + return 1; + } + } + return 0; +} + +/* ============================================================================ + * Strict integer parsing (real logic from module) + * ============================================================================ */ + +static int parse_int_strict(const char *arg, int *out) +{ + char *end = NULL; + long v; + if (arg == NULL || *arg == '\0' || out == NULL) return -1; + + /* Reject leading whitespace (strtol skips it by default) */ + if (apr_isspace(*arg)) return -1; + + errno = 0; + v = strtol(arg, &end, 10); + if (errno != 0 || end == arg || *end != '\0' || v < INT_MIN || v > INT_MAX) return -1; + *out = (int)v; + return 0; +} + /* ============================================================================ * Test setup and teardown * ============================================================================ */ @@ -521,7 +626,7 @@ static void test_header_value_json_escape(void **state) dynbuf_init(&db, pool, 256); const char *header_value = "test\"value\\with\tspecial"; - + /* Simulate what the module does */ dynbuf_append(&db, "\"header_Test\":\"", -1); append_json_string(&db, header_value); @@ -535,6 +640,350 @@ static void test_header_value_json_escape(void **state) apr_pool_destroy(pool); } +/* ============================================================================ + * Test: ISO8601 format with year 0 (edge case) + * ============================================================================ */ +static void test_iso8601_format_year_zero(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, 64); + + /* Use time that would result in year 0 or negative after conversion */ + /* This tests the clamping logic */ + apr_time_t test_time = apr_time_from_sec(0); + format_iso8601(&db, test_time); + + /* Should produce valid ISO8601 with clamped values */ + assert_int_equal(strlen(db.data), 20); + assert_true(strstr(db.data, "Z") != NULL); + assert_true(db.data[4] == '-'); + assert_true(db.data[7] == '-'); + assert_true(db.data[10] == 'T'); + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: ISO8601 format with very large timestamp (overflow test) + * ============================================================================ */ +static void test_iso8601_format_large_timestamp(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, 64); + + /* Use maximum reasonable apr_time_t value */ + /* This tests that large timestamps don't cause buffer overflow */ + apr_time_t test_time = apr_time_from_sec(253402300799ULL); /* Year 9999 */ + format_iso8601(&db, test_time); + + /* Should produce valid ISO8601 with year 9999 or clamped */ + assert_int_equal(strlen(db.data), 20); + assert_true(strstr(db.data, "Z") != NULL); + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: ISO8601 format output length is always fixed + * ============================================================================ */ +static void test_iso8601_format_fixed_length(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, 64); + + /* Test various timestamps */ + apr_time_t times[] = { + apr_time_from_sec(0), + apr_time_from_sec(946684800), /* 2000-01-01 */ + apr_time_from_sec(1772107200), /* 2026-02-26 */ + apr_time_from_sec(253402300799ULL) /* Year 9999 */ + }; + + for (size_t i = 0; i < sizeof(times) / sizeof(times[0]); i++) { + db.len = 0; + db.data[0] = '\0'; + format_iso8601(&db, times[i]); + + /* ISO8601 format is always 20 characters: YYYY-MM-DDTHH:MM:SSZ */ + assert_int_equal(strlen(db.data), 20); + } + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: Timestamp calculation does not overflow + * ============================================================================ */ +static void test_timestamp_no_overflow(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + /* Test that converting apr_time_t (microseconds) to nanoseconds + * doesn't overflow for reasonable timestamps */ + + /* Current time in microseconds */ + apr_time_t now = apr_time_now(); + + /* Convert to nanoseconds (same as module does) */ + apr_uint64_t ns = ((apr_uint64_t)now) * APR_UINT64_C(1000); + + /* Should be a large but valid number */ + assert_true(ns > 0); + + /* Should be less than maximum apr_uint64_t */ + /* Current time ~1.7e9 seconds * 1e9 = 1.7e18 nanoseconds */ + /* APR_UINT64_MAX is ~1.8e19, so we have plenty of headroom */ + assert_true(ns < APR_UINT64_MAX); + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: JSON size limit enforcement (64KB) + * ============================================================================ */ +static void test_json_size_limit(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, 4096); + + /* Build JSON up to the limit */ + dynbuf_append(&db, "{", 1); + + /* Add a large header value to approach the limit */ + dynbuf_append(&db, "\"header_Test\":\"", -1); + + /* Add data up to just under the limit */ + size_t target_size = MAX_JSON_SIZE - 100; + char *large_data = apr_palloc(pool, target_size); + memset(large_data, 'A', target_size - 1); + large_data[target_size - 1] = '\0'; + + append_json_string(&db, large_data); + dynbuf_append(&db, "\"}", -1); + + /* Should have built the JSON without crash */ + assert_true(db.data[0] == '{'); + + /* Check that we can detect when limit is exceeded */ + if (db.len >= MAX_JSON_SIZE) { + /* Size limit reached - this is expected for large data */ + /* In real module this would be logged via ap_log_error */ + } + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: JSON size limit - buffer check before headers + * ============================================================================ */ +static void test_json_size_limit_before_headers(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, MAX_JSON_SIZE + 1024); + + /* Build base JSON fields */ + dynbuf_append(&db, "{\"time\":\"", -1); + format_iso8601(&db, apr_time_now()); + dynbuf_append(&db, "\",\"timestamp\":", -1); + + /* Add a very large timestamp field */ + char ts_buf[64]; + snprintf(ts_buf, sizeof(ts_buf), "%" APR_UINT64_T_FMT, + ((apr_uint64_t)apr_time_now()) * APR_UINT64_C(1000)); + dynbuf_append(&db, ts_buf, -1); + + dynbuf_append(&db, ",\"src_ip\":\"", -1); + + /* Add large IP-like string */ + char *large_ip = apr_palloc(pool, MAX_JSON_SIZE - 200); + memset(large_ip, '1', MAX_JSON_SIZE - 201); + large_ip[MAX_JSON_SIZE - 201] = '\0'; + dynbuf_append(&db, large_ip, -1); + + dynbuf_append(&db, "\",\"method\":\"GET\"}", -1); + + /* Should handle gracefully */ + assert_true(db.data[0] == '{'); + + /* Verify size check would trigger */ + if (db.len >= MAX_JSON_SIZE) { + /* This is expected - size limit check should catch this */ + } + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: dynbuf append with NULL string + * ============================================================================ */ +static void test_dynbuf_append_null(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, 64); + + /* Should handle NULL gracefully */ + dynbuf_append(&db, NULL, 10); + assert_int_equal(db.len, 0); + assert_string_equal(db.data, ""); + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: dynbuf append with len=-1 (strlen mode) + * ============================================================================ */ +static void test_dynbuf_append_strlen_mode(void **state) +{ + apr_pool_t *pool; + apr_pool_create(&pool, NULL); + + dynbuf_t db; + dynbuf_init(&db, pool, 64); + + dynbuf_append(&db, "hello", (apr_size_t)-1); + assert_int_equal(db.len, 5); + assert_string_equal(db.data, "hello"); + + apr_pool_destroy(pool); +} + +/* ============================================================================ + * Test: Sensitive header detection - blacklist + * ============================================================================ */ +static void test_sensitive_header_blacklist(void **state) +{ + (void)state; + + /* List of headers that should be blocked */ + const char *sensitive[] = { + "Authorization", + "Cookie", + "Set-Cookie", + "X-Api-Key", + "X-Auth-Token", + "Proxy-Authorization", + "WWW-Authenticate", + NULL + }; + + /* List of headers that should NOT be blocked */ + const char *non_sensitive[] = { + "X-Request-Id", + "User-Agent", + "Referer", + "Host", + "Content-Type", + "Accept", + NULL + }; + + /* Test sensitive headers */ + for (int i = 0; sensitive[i] != NULL; i++) { + assert_true(is_sensitive_header(sensitive[i])); + } + + /* Test non-sensitive headers */ + for (int i = 0; non_sensitive[i] != NULL; i++) { + assert_false(is_sensitive_header(non_sensitive[i])); + } +} + +/* ============================================================================ + * Test: Sensitive header detection - case insensitive + * ============================================================================ */ +static void test_sensitive_header_case_insensitive(void **state) +{ + (void)state; + + /* Should be blocked regardless of case */ + assert_true(is_sensitive_header("authorization")); + assert_true(is_sensitive_header("AUTHORIZATION")); + assert_true(is_sensitive_header("Authorization")); + assert_true(is_sensitive_header("cookie")); + assert_true(is_sensitive_header("COOKIE")); + assert_true(is_sensitive_header("x-api-key")); + assert_true(is_sensitive_header("X-API-KEY")); +} + +/* ============================================================================ + * Test: Sensitive header detection - NULL handling + * ============================================================================ */ +static void test_sensitive_header_null(void **state) +{ + (void)state; + + /* NULL should not be considered sensitive (returns 0) */ + assert_false(is_sensitive_header(NULL)); +} + +/* ============================================================================ + * Test: Parse integer strict - valid values + * ============================================================================ */ +static void test_parse_int_strict_valid(void **state) +{ + int out; + (void)state; + + assert_int_equal(parse_int_strict("0", &out), 0); + assert_int_equal(out, 0); + + assert_int_equal(parse_int_strict("10", &out), 0); + assert_int_equal(out, 10); + + assert_int_equal(parse_int_strict("-5", &out), 0); + assert_int_equal(out, -5); + + assert_int_equal(parse_int_strict("2147483647", &out), 0); + assert_int_equal(out, 2147483647); + + assert_int_equal(parse_int_strict("-2147483648", &out), 0); + assert_int_equal(out, -2147483648); +} + +/* ============================================================================ + * Test: Parse integer strict - invalid values + * ============================================================================ */ +static void test_parse_int_strict_invalid(void **state) +{ + int out; + (void)state; + + /* Invalid: empty string */ + assert_int_equal(parse_int_strict("", &out), -1); + + /* Invalid: NULL */ + assert_int_equal(parse_int_strict(NULL, &out), -1); + + /* Invalid: non-numeric */ + assert_int_equal(parse_int_strict("abc", &out), -1); + + /* Invalid: mixed */ + assert_int_equal(parse_int_strict("10abc", &out), -1); + + /* Invalid: whitespace */ + assert_int_equal(parse_int_strict(" 10", &out), -1); + assert_int_equal(parse_int_strict("10 ", &out), -1); +} + /* ============================================================================ * Main test runner * ============================================================================ */ @@ -545,7 +994,9 @@ int main(void) cmocka_unit_test_setup_teardown(test_dynbuf_init, setup, teardown), cmocka_unit_test_setup_teardown(test_dynbuf_append_basic, setup, teardown), cmocka_unit_test_setup_teardown(test_dynbuf_append_resize, setup, teardown), - + cmocka_unit_test_setup_teardown(test_dynbuf_append_null, setup, teardown), + cmocka_unit_test_setup_teardown(test_dynbuf_append_strlen_mode, setup, teardown), + /* JSON escaping tests */ cmocka_unit_test_setup_teardown(test_json_escape_empty, setup, teardown), cmocka_unit_test_setup_teardown(test_json_escape_null, setup, teardown), @@ -556,19 +1007,38 @@ int main(void) cmocka_unit_test_setup_teardown(test_json_escape_control_char, setup, teardown), cmocka_unit_test_setup_teardown(test_json_escape_user_agent, setup, teardown), cmocka_unit_test_setup_teardown(test_json_escape_combined_special, setup, teardown), - + /* ISO8601 formatting */ cmocka_unit_test_setup_teardown(test_iso8601_format, setup, teardown), - + cmocka_unit_test_setup_teardown(test_iso8601_format_year_zero, setup, teardown), + cmocka_unit_test_setup_teardown(test_iso8601_format_large_timestamp, setup, teardown), + cmocka_unit_test_setup_teardown(test_iso8601_format_fixed_length, setup, teardown), + + /* Timestamp overflow tests */ + cmocka_unit_test_setup_teardown(test_timestamp_no_overflow, setup, teardown), + + /* JSON size limit tests */ + cmocka_unit_test_setup_teardown(test_json_size_limit, setup, teardown), + cmocka_unit_test_setup_teardown(test_json_size_limit_before_headers, setup, teardown), + /* Header truncation tests */ cmocka_unit_test_setup_teardown(test_header_truncation_within, setup, teardown), cmocka_unit_test_setup_teardown(test_header_truncation_exceeds, setup, teardown), cmocka_unit_test_setup_teardown(test_header_truncation_null, setup, teardown), cmocka_unit_test_setup_teardown(test_header_truncation_empty, setup, teardown), - + + /* Sensitive header tests */ + cmocka_unit_test_setup_teardown(test_sensitive_header_blacklist, setup, teardown), + cmocka_unit_test_setup_teardown(test_sensitive_header_case_insensitive, setup, teardown), + cmocka_unit_test_setup_teardown(test_sensitive_header_null, setup, teardown), + + /* Parse integer strict tests */ + cmocka_unit_test_setup_teardown(test_parse_int_strict_valid, setup, teardown), + cmocka_unit_test_setup_teardown(test_parse_int_strict_invalid, setup, teardown), + /* Full JSON structure */ cmocka_unit_test_setup_teardown(test_full_json_line, setup, teardown), - + /* Header value escaping */ cmocka_unit_test_setup_teardown(test_header_value_json_escape, setup, teardown), };