feat: nginx-ntlm-modulev2 — full rewrite with security hardening for nginx ≥ 1.25

README.md

@@ -1,7 +1,21 @@
-# nginx-ntlm-module
+# nginx-ntlm-modulev2
 
 The NTLM module allows proxying requests with [NTLM Authentication](https://en.wikipedia.org/wiki/Integrated_Windows_Authentication). The upstream connection is bound to the client connection once the client sends a request with the "Authorization" header field value starting with "Negotiate" or "NTLM". Further client requests will be proxied through the same upstream connection, keeping the authentication context.
 
+This is a full rewrite of the original [nginx-ntlm-module](https://github.com/Securepoint/nginx-ntlm-module), targeting **nginx ≥ 1.25** only. It is a 1:1 drop-in replacement for the original module in terms of configuration directives and behaviour, but does not support older nginx releases.
+
+## Security improvements over v1
+
+- **Session-hijack via cleanup-OOM (Critical)** — If `ngx_pool_cleanup_add()` returns NULL the upstream connection is no longer cached. Previously the connection was inserted into the cache with a stale `client_connection` pointer that nginx might later recycle for a different client, allowing `get_ntlm_peer` to hand an already-authenticated session to the wrong client (ABA pointer-reuse attack).
+
+- **Stale-credential reuse (High)** — When a keep-alive client connection sends new `NTLM`/`Negotiate` credentials after the initial handshake has completed (`c->requests >= 2`), the old authenticated upstream session is now evicted and closed, forcing a fresh negotiation. Previously the old session was silently reused regardless of the new credentials.
+
+- **Atomic item release** — A dedicated `ngx_http_upstream_ntlm_item_release()` helper is the single canonical code path for returning a cache item to the free list. It atomically clears `in_cache`, `peer_connection`, and `client_connection` before any queue operation, preventing any concurrent handler from operating on a partially-released item.
+
+- **`c->data` segfault on nginx ≥ 1.25** — Upstream connections idle in the NTLM cache store the cache-item pointer in `c->read->data` instead of `c->data`. nginx's `ngx_http_upstream_handler()` requires `c->data` to hold the request pointer; overwriting it with the cache item caused segfaults on nginx ≥ 1.25.
+
+- **Synchronous cleanup** — The client-connection pool cleanup handler closes the bound upstream connection synchronously. Earlier versions posted an event which could fire after the cache slot had already been reused, causing queue corruption and segfaults.
+
 ## How to use
 
 > Syntax:  ntlm [connections];  
@@ -13,6 +27,7 @@ The NTLM module allows proxying requests with [NTLM Authentication](https://en.w
 upstream http_backend {
     server 127.0.0.1:8080;
 
+    keepalive 16;
     ntlm;
 }
 
@@ -29,6 +44,7 @@ server {
 ```
 
 The connections parameter sets the maximum number of connections to the upstream servers that are preserved in the cache.
+If you configure explicit upstream keepalive, declare `keepalive` **before** `ntlm` in the same `upstream` block; nginx applies wrappers by directive chaining, and this order keeps NTLM as the outermost peer wrapper so authenticated connection pinning is preserved.
 
 > Syntax:  ntlm_timeout timeout;  
 > Default: ntlm_timeout 60s;  
@@ -54,14 +70,14 @@ Follow the instructions from [Building nginx from Sources](http://nginx.org/en/d
 
 ```bash 
 ./configure \
-    --add-module=../nginx-ntlm-module
+    --add-module=../nginx-ntlm-modulev2
 ```
 
-To build this as dynamic module run this command
+To build this as a dynamic module run:
 
 ```bash
 ./configure \
-    --add-dynamic-module=../nginx-ntlm-module
+    --add-dynamic-module=../nginx-ntlm-modulev2
 ```
 
 ## Tests
@@ -72,7 +88,7 @@ In order to run the tests you need nodejs and perl installed on your system
 # install the backend packages
 npm install -C t/backend
 
-# instal the test framework
+# install the test framework
 cpan Test::Nginx
 
 # set the path to your nginx location
@@ -86,22 +102,53 @@ prove -r t
 
 | nginx version | Notes |
 |---------------|-------|
-| < 1.9.1       | Not supported — the upstream peer API used by this module was introduced in nginx 1.9.1. |
-| 1.9.1 – 1.24.x | Supported. |
-| ≥ 1.25.x      | Supported. Versions in the 1.25/1.26/1.27/1.28 series (e.g. 1.28.3) changed internal assumptions about `ngx_connection_t->data` in the upstream event handler (`ngx_http_upstream_handler` now expects `c->data` to hold the request pointer). Earlier releases of this module reused `c->data` to store the NTLM cache item on idle connections, which caused segfaults with these nginx versions. This was fixed in the module — see [PR #4](https://github.com/Securepoint/nginx-ntlm-module/pull/4). Use a module build from the current `main` branch when running nginx ≥ 1.25. |
+| < 1.25        | Not supported. |
+| ≥ 1.25        | Supported. This module targets nginx 1.25 and later. It correctly stores cache-item pointers in `c->read->data` (not `c->data`) to avoid the segfault regression introduced in the 1.25/1.26/1.27/1.28 series. |
+| master (1.31+) | Supported. See [nginx master compatibility](#nginx-master-compatibility) below. |
+
+## nginx master compatibility
+
+nginx master (≥ 1.31) introduced **automatic keepalive injection**: the built-in
+`ngx_http_upstream_keepalive_module` now installs a peer wrapper for every
+upstream in its `init_main_conf` hook — even when no explicit `keepalive N`
+directive is present.  If the NTLM module's peer wrapper was installed during
+`init_upstream` (as it was in earlier versions of this module), the keepalive
+wrapper ended up on the *outside*, intercepting `free_peer` calls before NTLM
+could pin the upstream connection.  The keepalive module then cached the
+connection itself and set `pc->connection = NULL`, causing NTLM's `free_peer`
+to bail out via the `c == NULL` guard — so NTLM never registered the connection
+in its own cache.  Subsequent requests were round-robined across servers rather
+than staying pinned to the authenticated upstream.
+
+**Fix (shipped in this version):** The peer-init wrapping was moved from
+`ngx_http_upstream_init_ntlm` (`init_upstream`) to a new
+`ngx_http_upstream_ntlm_init_main_conf` (`init_main_conf`) hook.
+`--add-module` extensions are assigned higher module indices than all built-in
+modules, so NTLM's `init_main_conf` always runs *after* the keepalive module's
+`init_main_conf`.  This keeps NTLM as the outermost peer wrapper regardless of
+nginx version.
 
 ## Acknowledgments
 
-- This module is using most of the code from the original nginx keepalive module.
+- This module is based on the original nginx-ntlm-module by Gabriel Hodoroaga.
 - DO NOT USE THIS IN PRODUCTION. [**Nginx Plus**](https://www.nginx.com/products/nginx/) has support for NTLM. 
 
 ## Authors 
 
-* Gabriel Hodoroaga ([hodo.dev](https://hodo.dev))
-
-## TODO
-
-- [x] Add tests
-- [x] Add support for multiple workers
-- [x] Drop the upstream connection when the client connection drops.
-- [ ] Add travis ci
+* Gabriel Hodoroaga ([hodo.dev](https://hodo.dev)) — original module
+* Securepoint — v2 rewrite and security hardening
+
+## Changelog
+
+### v2
+- Full rewrite targeting nginx ≥ 1.25
+- Fix session hijack via cleanup-OOM (Critical)
+- Fix stale-credential reuse (High)
+- Fix `c->data` segfault on nginx ≥ 1.25
+- Add atomic item-release helper
+- Synchronous client-connection cleanup (no posted events)
+- Add `ntlm_time` and `ntlm_requests` directives
+- Add `notify` peer callback pass-through
+- Fix NTLM pinning broken on nginx master (≥ 1.31): move peer-init wrapping
+  to `init_main_conf` so NTLM is always the outermost peer wrapper, even when
+  the keepalive module auto-injects its own wrapper

docker/alpine/dynamic/Dockerfile

@@ -1,8 +1,8 @@
-FROM alpine:3.13 AS builder
+FROM alpine:3.20 AS builder
 
-ARG NGINX_VERSION=1.19.3
+ARG NGINX_VERSION=1.28.0
 
-# install buidl tools
+# install build tools
 RUN set -x \
     && addgroup -g 101 -S nginx \
     && adduser -S -D -H -u 101 -h /var/cache/nginx -s /sbin/nologin -G nginx -g nginx nginx \
@@ -11,12 +11,11 @@ RUN set -x \
     libc-dev \
     make \
     openssl-dev \
-    pcre-dev \
+    pcre2-dev \
     zlib-dev \
     linux-headers \
     libxslt-dev \
     gd-dev \
-    geoip-dev \
     perl-dev \
     libedit-dev \
     curl \
@@ -27,7 +26,7 @@ WORKDIR /build
 # download nginx 
 RUN curl -OL http://nginx.org/download/nginx-${NGINX_VERSION}.tar.gz \
     && tar -xvzf nginx-${NGINX_VERSION}.tar.gz && rm nginx-${NGINX_VERSION}.tar.gz \
-    && git clone https://github.com/gabihodoroaga/nginx-ntlm-module.git
+    && git clone https://github.com/Securepoint/nginx-ntlm-module.git nginx-ntlm-modulev2
 
 RUN cd nginx-${NGINX_VERSION}/ \
     && ./configure \
@@ -38,7 +37,7 @@ RUN cd nginx-${NGINX_VERSION}/ \
         --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 \ 
+        --lock-path=/var/run/nginx.lock \
         --http-client-body-temp-path=/var/cache/nginx/client_temp \
         --http-proxy-temp-path=/var/cache/nginx/proxy_temp \
         --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp \
@@ -48,7 +47,7 @@ RUN cd nginx-${NGINX_VERSION}/ \
         --user=nginx \
         --group=nginx \
         --with-compat \
-        --with-file-aio \ 
+        --with-file-aio \
         --with-threads \
         --with-http_addition_module \
         --with-http_auth_request_module \
@@ -73,11 +72,11 @@ RUN cd nginx-${NGINX_VERSION}/ \
         --with-stream_ssl_preread_module \
         --with-cc-opt='-Os -fomit-frame-pointer' \
         --with-ld-opt=-Wl,--as-needed \
-        # add the ntlm module
-        --add-dynamic-module=../nginx-ntlm-module \
+        # add the ntlm v2 module as a dynamic module
+        --add-dynamic-module=../nginx-ntlm-modulev2 \
     && make modules
 
-FROM nginx:1.19.3-alpine
+FROM nginx:1.28.0-alpine
 
 COPY --from=builder /build/nginx-${NGINX_VERSION}/objs/ngx_http_upstream_ntlm_module.so /etc/nginx/modules/ngx_http_upstream_ntlm_module.so
 

docker/alpine/static/Dockerfile

@@ -1,9 +1,9 @@
-FROM alpine:3.13
+FROM alpine:3.20
 
-ENV NGINX_VERSION 1.19.3
+ENV NGINX_VERSION 1.28.0
 
 
-# install buidl tools
+# install build tools
 RUN set -x \
     && addgroup -g 101 -S nginx \
     && adduser -S -D -H -u 101 -h /var/cache/nginx -s /sbin/nologin -G nginx -g nginx nginx \
@@ -12,12 +12,11 @@ RUN set -x \
     libc-dev \
     make \
     openssl-dev \
-    pcre-dev \
+    pcre2-dev \
     zlib-dev \
     linux-headers \
     libxslt-dev \
     gd-dev \
-    geoip-dev \
     perl-dev \
     libedit-dev \
     curl \
@@ -28,7 +27,7 @@ WORKDIR /build
 # download nginx 
 RUN curl -OL http://nginx.org/download/nginx-${NGINX_VERSION}.tar.gz \
     && tar -xvzf nginx-${NGINX_VERSION}.tar.gz && rm nginx-${NGINX_VERSION}.tar.gz \
-    && git clone https://github.com/gabihodoroaga/nginx-ntlm-module.git
+    && git clone https://github.com/Securepoint/nginx-ntlm-module.git nginx-ntlm-modulev2
 
 RUN cd nginx-${NGINX_VERSION}/ \
     && ./configure \
@@ -39,7 +38,7 @@ RUN cd nginx-${NGINX_VERSION}/ \
         --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 \ 
+        --lock-path=/var/run/nginx.lock \
         --http-client-body-temp-path=/var/cache/nginx/client_temp \
         --http-proxy-temp-path=/var/cache/nginx/proxy_temp \
         --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp \
@@ -49,7 +48,7 @@ RUN cd nginx-${NGINX_VERSION}/ \
         --user=nginx \
         --group=nginx \
         --with-compat \
-        --with-file-aio \ 
+        --with-file-aio \
         --with-threads \
         --with-http_addition_module \
         --with-http_auth_request_module \
@@ -74,8 +73,8 @@ RUN cd nginx-${NGINX_VERSION}/ \
         --with-stream_ssl_preread_module \
         --with-cc-opt='-Os -fomit-frame-pointer' \
         --with-ld-opt=-Wl,--as-needed \
-        # add the ntlm module
-        --add-module=../nginx-ntlm-module \
+        # add the ntlm v2 module
+        --add-module=../nginx-ntlm-modulev2 \
     && make \
     && make install \
     && mkdir -p /var/cache/nginx
@@ -85,7 +84,7 @@ WORKDIR /
 # clean up 
 RUN apk del .build-deps \
     && rm -rf /build \
-    && apk add pcre
+    && apk add pcre2
 
 WORKDIR /etc/nginx/html
 

docker/openresty/alpine/dynamic/Dockerfile

@@ -48,7 +48,7 @@ ARG RESTY_CONFIG_OPTIONS="\
     --with-stream \
     --with-stream_ssl_module \
     --with-threads \
-    --add-dynamic-module=../nginx-ntlm-module \
+    --add-dynamic-module=../nginx-ntlm-modulev2 \
     "
 ARG RESTY_CONFIG_OPTIONS_MORE=""
 ARG RESTY_LUAJIT_OPTIONS="--with-luajit-xcflags='-DLUAJIT_NUMMODE=2 -DLUAJIT_ENABLE_LUA52COMPAT'"
@@ -139,8 +139,8 @@ RUN apk add --no-cache --virtual .build-deps \
     && cd /tmp \
     && curl -fSL https://openresty.org/download/openresty-${RESTY_VERSION}.tar.gz -o openresty-${RESTY_VERSION}.tar.gz \
     && tar xzf openresty-${RESTY_VERSION}.tar.gz \
-    # clone the ntlm repository  
-    && git clone https://github.com/gabihodoroaga/nginx-ntlm-module.git \ 
+    # clone the ntlm v2 repository  
+    && git clone https://github.com/Securepoint/nginx-ntlm-module.git nginx-ntlm-modulev2 \ 
     # end
     && cd /tmp/openresty-${RESTY_VERSION} \
     && eval ./configure -j${RESTY_J} ${_RESTY_CONFIG_DEPS} ${RESTY_CONFIG_OPTIONS} ${RESTY_CONFIG_OPTIONS_MORE} ${RESTY_LUAJIT_OPTIONS} \

docker/openresty/alpine/static/Dockerfile

@@ -48,7 +48,7 @@ ARG RESTY_CONFIG_OPTIONS="\
     --with-stream \
     --with-stream_ssl_module \
     --with-threads \
-    --add-module=../nginx-ntlm-module \
+    --add-module=../nginx-ntlm-modulev2 \
     "
 ARG RESTY_CONFIG_OPTIONS_MORE=""
 ARG RESTY_LUAJIT_OPTIONS="--with-luajit-xcflags='-DLUAJIT_NUMMODE=2 -DLUAJIT_ENABLE_LUA52COMPAT'"
@@ -139,8 +139,8 @@ RUN apk add --no-cache --virtual .build-deps \
     && cd /tmp \
     && curl -fSL https://openresty.org/download/openresty-${RESTY_VERSION}.tar.gz -o openresty-${RESTY_VERSION}.tar.gz \
     && tar xzf openresty-${RESTY_VERSION}.tar.gz \
-    # clone the ntlm repository  
-    && git clone https://github.com/gabihodoroaga/nginx-ntlm-module.git \ 
+    # clone the ntlm v2 repository  
+    && git clone https://github.com/Securepoint/nginx-ntlm-module.git nginx-ntlm-modulev2 \ 
     # end
     && cd /tmp/openresty-${RESTY_VERSION} \
     && eval ./configure -j${RESTY_J} ${_RESTY_CONFIG_DEPS} ${RESTY_CONFIG_OPTIONS} ${RESTY_CONFIG_OPTIONS_MORE} ${RESTY_LUAJIT_OPTIONS} \