diff --git a/ngx_http_upstream_ntlm_module.c b/ngx_http_upstream_ntlm_module.c index 2a106ff..64c262f 100644 --- a/ngx_http_upstream_ntlm_module.c +++ b/ngx_http_upstream_ntlm_module.c @@ -258,6 +258,31 @@ static ngx_int_t ngx_http_upstream_get_ntlm_peer(ngx_peer_connection_t *pc, if (item->client_connection == hndp->client_connection) { c = item->peer_connection; + /* + * If the current request carries new NTLM/Negotiate credentials + * and the cached upstream connection has already served at least + * two requests (i.e. the NTLM three-way handshake is complete), + * this is a re-authentication attempt for a potentially different + * identity. Evict and close the old authenticated session rather + * than silently reusing it. + * + * When c->requests == 1 the handshake is still in progress + * (Type-1 sent, Type-3 is the next expected request); in that + * case reuse must be allowed so the exchange can complete on the + * same upstream TCP connection. + */ + if (hndp->ntlm_init && c->requests >= 2) { + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, pc->log, 0, + "ntlm: new credentials on established session, evicting upstream connection %p (requests=%ui)", + c, c->requests); + item->in_cache = 0; + item->peer_connection = NULL; + ngx_queue_remove(q); + ngx_queue_insert_head(&hndp->conf->free, q); + ngx_http_upstream_ntlm_close(c); + break; + } + /* * Clear ownership fields BEFORE touching the queues so that any * handler that checks in_cache (close_handler, cleanup) sees the @@ -408,12 +433,28 @@ static void ngx_http_upstream_free_ntlm_peer(ngx_peer_connection_t *pc, if (cleanup_item == NULL) { cln = ngx_pool_cleanup_add(item->client_connection->pool, 0); if (cln == NULL) { - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, pc->log, 0, - "ntlm free peer ngx_pool_cleanup_add returned null"); - } else { - cln->handler = ngx_http_upstream_client_conn_cleanup; - cln->data = item; + ngx_log_error(NGX_LOG_ERR, pc->log, 0, + "ntlm: failed to allocate cleanup handler," + " not caching upstream connection"); + /* + * Without a cleanup handler we cannot close the upstream + * connection when the client disconnects. The cached item + * would hold a stale client_connection pointer that nginx may + * later recycle for a completely different client, allowing + * get_ntlm_peer to hand an already-authenticated upstream + * session to the wrong client (session hijack via pointer + * reuse). Undo the cache insertion and treat this connection + * as uncacheable. + */ + item->in_cache = 0; + item->peer_connection = NULL; + item->client_connection = NULL; + ngx_queue_remove(q); + ngx_queue_insert_head(&hndp->conf->free, q); + goto invalid; } + cln->handler = ngx_http_upstream_client_conn_cleanup; + cln->data = item; } pc->connection = NULL; diff --git a/t/001-sanity.t b/t/001-sanity.t index 25c3801..991f33e 100644 --- a/t/001-sanity.t +++ b/t/001-sanity.t @@ -6,6 +6,8 @@ repeat_each(2); my $string_gen = String::Random->new; our $random_token = $string_gen->randpattern("CcCcCcCCcC"); +our $random_token2; +do { $random_token2 = $string_gen->randpattern("CcCcCcCCcC"); } while $random_token2 eq $random_token; # Start the nodejs backend my $pid = fork(); @@ -137,3 +139,33 @@ also ["========","X-NGX-NTLM-AUTH: ","X-NGX-NTLM-AUTH: "] --- no_error_log [error] + + +=== TEST 5: New NTLM credentials on an established connection must evict the old session +After the initial NTLM handshake completes (upstream connection has served at +least 2 requests), presenting new credentials on the same keep-alive client +connection must not reuse the old authenticated upstream session. The module +must evict the old session and obtain a fresh upstream connection so the new +credentials are negotiated independently. +--- http_config + upstream backend { + server localhost:19841; + server localhost:19842; + ntlm; + } +--- config + location /t { + proxy_pass http://backend; + proxy_http_version 1.1; + proxy_set_header Connection ""; + } +--- pipelined_requests eval +["GET /t", "GET /t", "GET /t"] +--- more_headers eval +["Authorization: NTLM " . $::random_token, "", "Authorization: NTLM " . $::random_token2] +--- response_body eval +["OK", "OK", "OK"] +--- response_headers eval +["X-NGX-NTLM-AUTH: " . $::random_token, "X-NGX-NTLM-AUTH: " . $::random_token, "X-NGX-NTLM-AUTH: " . $::random_token2] +--- no_error_log +[error]