[SECURITY] egress: prevent credential-destination-confusion via Host header spoofing

[Pangjiping]

Summary

Fix a credential-destination-confusion vulnerability in the Credential Vault mitmproxy addon where flow.request.pretty_host (derived from the client-controlled Host header) was used to match credential bindings. A sandbox workload could spoof the Host header to a protected binding domain while directing the TCP connection to an attacker-controlled host, causing credentials to be injected into the wrong request.

Root Cause

_request_host() in components/egress/mitmscripts/system.py preferred pretty_host over host:

# BEFORE (vulnerable)
def _request_host(flow):
    host = flow.request.pretty_host or flow.request.host or ""
    return host.rstrip(".").lower()

Fix

1. Rewrite _request_host() to prefer the actual upstream authority:

   • flow.request.host when it is an FQDN (direct match)
   • TLS SNI from flow.client_conn.sni for transparent HTTPS (certificate-verified)
   • pretty_host only as last resort for transparent HTTP with IP destination

2. Add _credential_destination_mismatch() guard in request() that rejects credential injection when the Host header diverges from the actual upstream destination or TLS SNI, logging a warning.

Test Coverage

Added 7 regression tests:

Test	Scenario	Expected
test_host_header_spoof_fqdn_mismatch_blocks_injection	host=attacker FQDN, pretty_host=protected domain	Blocked
test_transparent_https_sni_mismatch_blocks_injection	host=IP, SNI=attacker, pretty_host=protected	Blocked
test_transparent_https_sni_match_allows_injection	host=IP, SNI=protected, pretty_host=protected	Allowed
test_matching_host_and_pretty_host_allows_injection	host=pretty_host=protected	Allowed
test_request_host_prefers_fqdn_over_pretty_host	FQDN host takes priority	Verified
test_request_host_uses_sni_when_host_is_ip	SNI preferred over pretty_host when host is IP	Verified
test_request_host_falls_back_to_pretty_host_for_http	Transparent HTTP fallback	Verified

All 11 tests pass (4 existing + 7 new).

Residual Risk

Transparent HTTP with IP destination and no SNI still falls back to pretty_host. This is mitigated by egress network policy (defaultAction=deny) and DNS enforcement. A follow-up could consider blocking http scheme bindings entirely on the Go validation side.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 45dd2d557e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b723cfbd85

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 843dc20a1d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6590df777

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d76d3527a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[jwx0925]

The HTTPS part of this fix looks reasonable to me: it avoids trusting pretty_host, uses the actual upstream destination/SNI, blocks SNI/Host mismatches, and rejects HTTPS IP-destination flows without SNI.

The HTTP part is still problematic though. In transparent HTTP mode, flow.request.host is typically the original destination IP, while flow.request.pretty_host comes from the client-controlled Host header. Falling back to pretty_host for HTTP IP-destination flows keeps credential injection dependent on a spoofable header, so the destination-confusion risk remains for HTTP bindings.

I think we should either disallow HTTP credential bindings, or reject IP-destination/no-SNI flows instead of falling back to pretty_host.

[Pangjiping]

@jwx0925 Thanks — agreed on the HTTP part. You're right that falling back to pretty_host for transparent HTTP IP-destination flows keeps credential injection dependent on a spoofable Host header, so the destination-confusion risk remains for HTTP bindings.

I looked at both options you suggested:

1. Reject IP-dest/no-SNI HTTP flows in the proxy — this breaks the transparent-HTTP credential-vault E2E (tests/go/credential_vault_e2e_test.go + scripts/credential-vault-e2e-target.sh): the target runs a plain-HTTP echo server on port 80, and the sandbox curls it via --resolve <host>:80:<container-ip>, so the proxy legitimately sees an IP destination with no SNI and a Host header carrying the bound FQDN. Blocking here would stop injection on that path entirely.
2. Reject http scheme bindings at the Go validation layer (components/egress/pkg/credentialvault/vault.go) — cleaner: it closes the hole at binding-creation time without weakening the proxy, and makes the intent explicit (HTTP credential bindings simply aren't allowed).

I'd prefer option 2 as the proper fix, but it's a behavior change to the public binding API and out of scope for this security patch, so I've left it as a tracked follow-up. For now I've added a FIXME in _credential_destination_mismatch() documenting the residual risk, the correct fix location, and why we can't block it in the proxy. The HTTPS paths you flagged are fully closed in this PR. Meanwhile the HTTP residual is mitigated by egress network policy (defaultAction=deny) + DNS enforcement as noted in the description.