Skip to content

http: ignore Proxy-Authenticate unless the response comes from a proxy - #23002

Closed
MegaManSec wants to merge 1 commit into
curl:masterfrom
MegaManSec:proxy-digest-direct-poison
Closed

MegaManSec wants to merge 1 commit into
curl:masterfrom
MegaManSec:proxy-digest-direct-poison

Conversation

@MegaManSec

@MegaManSec MegaManSec commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

The 407 + Proxy-authenticate: handler in http_header_p() does not check where the response came from, unlike the Proxy-Connection: branch right above it. A host reached directly (NOPROXY exclusion, or a per-request empty proxy) can therefore set data->state.proxydigest from its own 407, and so can an origin reached through a CONNECT tunnel, since http_proxy.peer stays set for tunnels.

The next request to the real proxy then sends a preemptive Proxy-Authorization: Digest using that host's realm and nonce. For the tunneled case curl retries immediately and puts the header on the CONNECT.

Gate on bits.origin_is_proxy, which is what Curl_http_output_auth() already uses to decide whether to emit proxy auth headers, so the input and output sides agree. Proxy challenges for tunnels arrive in the CONNECT response and are handled by the proxy filters, so they are unaffected. This also covers the NTLM/Negotiate/Basic proxy state and CURLINFO_PROXYAUTH_AVAIL, which Curl_http_input_auth() sets on the same path.

digest_flush_stale() is additionally made to fail closed: with digest->origin == NULL both guards were false, so the state survived a peer change.

Test 1616 covers both vectors: a proxied request, a NOPROXY-excluded host answering 407, the proxy again, then a tunneled origin answering 407, then the proxy again. It fails with either vector unfixed.

@github-actions github-actions Bot added the tests label Sep 18, 2026
@MegaManSec
MegaManSec force-pushed the proxy-digest-direct-poison branch from e88ec61 to b8cd52b Compare September 18, 2026 22:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

CONNECT-tunneled origin responses can still poison proxy authentication state because the proxy peer remains present.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Prevents direct-origin 407 responses from contaminating HTTP proxy authentication state.

Changes:

  • Ignores proxy-authentication headers when no HTTP proxy peer exists.
  • Makes stale Digest state reset fail closed.
  • Adds regression test 1616.
File Description
lib/​http.c Gates proxy-auth challenge handling.
lib/​http_digest.c Strengthens Digest origin validation.
tests/​libtest/​lib1616.c Exercises proxy-state contamination.
tests/​libtest/​Makefile.inc Registers the libtest.
tests/​data/​test1616 Defines responses and expected requests.
tests/​data/​Makefile.am Registers test 1616.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/http.c Outdated
A 407 response was parsed for proxy authentication without checking
where it came from, so a host reached directly, or an origin reached
through a CONNECT tunnel, could set the proxy auth state. A later
request to the proxy then sent a Proxy-Authorization built from the
realm and nonce of that host, before the proxy challenged.

The proxy peer stays set for tunnels, so gate on origin_is_proxy, the
same condition Curl_http_output_auth() uses to emit these headers.

Also flush the Digest state when it has no origin.

Add test 1616.
@MegaManSec
MegaManSec force-pushed the proxy-digest-direct-poison branch from b8cd52b to b26865b Compare September 19, 2026 09:04
@MegaManSec MegaManSec changed the title http: ignore Proxy-Authenticate from a non-proxied connection http: ignore Proxy-Authenticate unless the response comes from a proxy Sep 19, 2026
@bagder bagder closed this in 4f888df Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants