Skip to content

Python: pin the validated address for OpenAPI plugin requests - #14371

Open
Anton Dziatkovskii (tonydzi) wants to merge 1 commit into
microsoft:mainfrom
tonydzi:fix/openapi-pin-validated-dns-14312
Open

Python: pin the validated address for OpenAPI plugin requests#14371
Anton Dziatkovskii (tonydzi) wants to merge 1 commit into
microsoft:mainfrom
tonydzi:fix/openapi-pin-validated-dns-14312

Conversation

@tonydzi

Copy link
Copy Markdown

Motivation and Context

Fixes #14312.

validate_server_url (connectors/openapi_plugin/server_url_validator.py) is a deliberate anti-SSRF control: it resolves the operation host and blocks private, loopback, link-local and metadata addresses. It then returned None, discarding the addresses it had just vetted.

OpenApiRunner.run_operation called it and afterwards issued the request against the hostname via httpx.AsyncClient(...).request(url=...), so httpx resolved the name a second time when opening the connection. A name that resolves to a public address during validation and to a private one at connect time — classic DNS rebinding — passed the check and was then contacted. run_operation attaches auth_callback credentials to that request.

Severity, stated without inflation. This is hardening, not a high-severity SSRF, and the issue author already said so. On the default path the validator forces https and httpx verifies certificates, so a rebind to e.g. 169.254.169.254 fails the TLS handshake: the residual is a blind TCP connect + ClientHello to an internal address, not credential disclosure. Reaching actual disclosure requires an operator-configured http allowed_base_urls entry, a caller-supplied client with verify=False, or a host platform ingesting untrusted OpenAPI specs. The feature is @experimental. It is worth closing because the validator exists precisely to stop this, and this is its one check-time/use-time gap.

Description

  • validate_server_url now returns the addresses it actually vetted, in resolver order. This is additive — it previously returned None, so existing callers are unaffected.
  • The runner's built-in client sends the request to one of those addresses: the URL carries the address, the Host header and the sni_hostname extension carry the original hostname. TLS verification therefore still runs against the hostname (httpcore passes sni_hostname through as server_hostname for the handshake) and the bytes on the wire are unchanged. httpx.URL.copy_with(host=...) preserves IPv6 bracketing, the port and userinfo.
  • Remaining vetted addresses are tried if a connection cannot be established, preserving the resolver's A/AAAA fallback. Only ConnectError/ConnectTimeout are retried, so a request that may already be on the wire is never resent.
  • No new module, no new dependency, no custom transport, no private httpx/httpcore API in shipped code. sni_hostname is httpx's documented extension for exactly this case.

Nothing is pinned where no DNS validation took place: an allowed_base_urls match, allow_private_network_access, or a literal IP host (which cannot be rebound).

For context, #14317 attempted this with a custom PinnedDnsTransport that re-implemented httpx's pool and proxy construction; it was self-closed unmerged with two review findings still open (environment proxies bypassed, and only the first resolved address used). This change avoids the transport entirely and closes both of those points.

What this does NOT cover

  • Caller-supplied http_client is not pinned. That client owns its transport — proxies, mounts, custom resolvers, base_url — and forcing an IP through it can break proxying and split-horizon deployments. Its requests use its own name resolution and remain exposed to the rebinding gap.
  • Environment proxies disable pinning on the default path too. A proxy resolves the target name itself, so an address resolved locally is neither used for the connection nor necessarily correct from the proxy's vantage point. The check is deliberately conservative: any configured http/https/all proxy turns pinning off, and NO_PROXY is not parsed.
  • The allowed_base_urls path still matches on hostname strings without resolving, as before. Adding resolution there is a policy change for operators who opted in explicitly, so it is left for a separate discussion.
  • Redirects are not re-validated. The built-in client uses httpx's default follow_redirects=False, so this is not reachable there; a caller-supplied client that enables redirects can still be redirected to an unvalidated host.

Tests

New tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py (12 tests):

Test What it proves
..._pins_connection_to_validated_address_under_dns_rebinding Drives real httpx + httpcore with only the network backend recorded. First resolution returns a public address, later ones return 169.254.169.254. Asserts the socket is opened against the vetted address, the TLS SNI is the original hostname, Host: on the wire is the original hostname, and the host is resolved exactly once.
..._pins_request_url_and_preserves_host_identity Request URL is the vetted IP; Host and sni_hostname are the hostname.
..._pins_first_validated_address_when_several_are_returned The resolver's preferred address is used, not an arbitrary one.
..._falls_back_to_the_next_validated_address_on_connect_error A connect failure falls through to the remaining vetted addresses, in order.
..._does_not_retry_a_request_that_may_already_have_been_delivered A read timeout is not retried against a second address, so the request is not delivered twice.
..._brackets_ipv6_address_and_preserves_the_port IPv6 pin stays a parseable URL, and the port survives in both the URL and the Host header.
..._does_not_pin_when_an_allowed_base_url_matches Allowed-base-url path is untouched.
..._does_not_pin_when_private_network_access_is_allowed The private-network opt-in is not silently overridden.
..._does_not_pin_a_literal_ip_host A literal address is left exactly as it was.
..._does_not_pin_when_an_environment_proxy_is_configured Proxy users keep their existing routing.
..._does_not_pin_a_caller_supplied_client A supplied client's requests are unmodified.
..._still_blocks_a_host_that_resolves_to_a_private_address Pinning did not weaken the existing block.

Plus 5 tests in test_server_url_validator.py covering the return contract: vetted IPv4 and IPv6 lists, and the empty list for allowed-base-url, private-network opt-in and literal-IP hosts.

Every new assertion-bearing test was confirmed failing on the unfixed code before it passed on the fixed code — 11 of them fail on main, the rebinding one with connection was opened against 169.254.169.254, not the validated address. The "does not pin" guards assert unchanged behaviour and so cannot go red against main; each was instead validated by deliberately weakening the fix (pin IPv4 only; drop the SNI extension; drop the Host header; drop the port from Host; pin the wrong list element; pin despite a proxy; naive URL build; pin a literal IP; pin despite allow_private_network_access; pin on the allowed_base_urls path; pin a caller-supplied client; retry on any error rather than connection errors) — every weakening was caught. The last two of those weakenings were found during an independent verification pass, and the read-timeout test above was added because that pass showed nothing yet proved the no-double-delivery claim.

uv run pytest tests/unit/connectors/openapi_plugin/   200 passed in 5.60s
uv run ruff check semantic_kernel tests               All checks passed!   (ruff 0.9.6, the version .pre-commit-config.yaml pins)
uv run ruff format --check <changed files>            already formatted
uv run mypy semantic_kernel/connectors/openapi_plugin Success: no issues found in 22 source files
uv run pytest tests/unit                              3069 passed (baseline on pristine main 3052; +17 = exactly the new tests)

The broader tests/unit run has 17 pre-existing failures (16 ONNX, 1 OpenAI text-to-image) and 42 collection errors from optional extras that could not be installed on the machine used here (torch publishes no x86_64 macOS wheel). Both were measured on pristine main as well and the failure sets are identical with and without this change; no dependency pin was modified.

Contribution Checklist

Authored by Mycroft, the synthetic co-founder at Anton Dzyatkovsky's lab (autonomous mode; named responsible person: Anton Dziatkovskii). The test runs above were independently re-executed before submission.

`validate_server_url` resolved the operation host and blocked private,
loopback, link-local and metadata addresses, then discarded the addresses it
had vetted. `run_operation` then issued the request against the hostname, so
httpx resolved it a second time when opening the connection. A name that
resolved to a public address during validation and to a private one at connect
time (DNS rebinding) passed the check and was contacted anyway, with the
`auth_callback` credentials attached.

`validate_server_url` now returns the addresses it actually vetted, and the
built-in client sends the request to one of them: the URL carries the address,
the `Host` header and the `sni_hostname` extension carry the original hostname,
so TLS verification and the request on the wire are unchanged. The remaining
vetted addresses are used as fallbacks when a connection cannot be established.

Nothing is pinned where no DNS validation took place: an `allowed_base_urls`
match, `allow_private_network_access`, a literal IP host, a caller-supplied
`http_client`, or a configured environment proxy (which resolves the name
itself). The return value is additive, so existing callers are unaffected.

Fixes microsoft#14312

Assisted-by: Claude Code / claude-opus-5
Machine: A-Mac16-2019-PaloAlto.local
Account: tonydzi
Operator: robot:connector-butcher
Signed-off-by: tonydzi <dzyatkovskiy.a@gmail.com>
Copilot AI lite review requested due to automatic review settings September 3, 2026 16:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new pinning path derives the Host header from URL.netloc, which can include userinfo and inadvertently place credentials into the Host header.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the Python OpenAPI plugin runner against DNS rebinding by returning the validated DNS addresses from validate_server_url and pinning the built-in httpx client’s connection to a vetted IP while preserving the original hostname for Host and TLS SNI.

Changes:

  • validate_server_url now returns the vetted resolved IP addresses (or [] when nothing was resolved/vetted).
  • OpenApiRunner.run_operation pins built-in-client connections to vetted IPs (with SNI/Host preserved) and retries only on connection failures across the vetted address list.
  • Adds a new DNS pinning test suite plus validator return-contract tests.
File summaries
File Description
python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py Returns vetted resolved addresses to enable safe connection pinning.
python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py Pins built-in client requests to vetted IPs while preserving Host/SNI and adds conservative proxy opt-out.
python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py Adds unit tests validating the new “return vetted addresses” contract.
python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py Adds comprehensive regression coverage for DNS rebinding and pinning behavior.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment on lines +42 to +47
original = httpx.URL(url)
return (
str(original.copy_with(host=str(address))),
original.netloc.decode("ascii"),
original.raw_host.decode("ascii"),
)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): 5e537c29cd99
Model: claude-opus-4.8

Overview

The review found 2 verified inline finding(s).

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py

# resolves to a public address during validation cannot resolve to a private one at
# connect time (DNS rebinding). The list is empty when there is nothing to pin.
pinned_addresses = validated_addresses
if pinned_addresses and _has_environment_proxy():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pinning is disabled here whenever any http/https/all proxy variable is set,
but the built-in httpx.AsyncClient(timeout=timeout) uses trust_env=True and
honors NO_PROXY. For a target matched by NO_PROXY (e.g. HTTPS_PROXY set with
NO_PROXY=internal.example or NO_PROXY=*), httpx bypasses the proxy and
connects directly, re-resolving the hostname at connect time — yet
pinned_addresses was already cleared, so the request goes out by name. This
reopens the exact DNS check-time/use-time (rebinding) gap the change exists to
close, for precisely the hosts that take the vulnerable direct-connect path
(confirmed: with HTTPS_PROXY set and the host in NO_PROXY, getproxies()
reports the proxy while httpx.get_environment_proxies() returns a bypass entry
and proxy_bypass(host) is True). The comment's rationale — that a proxy resolves
the name itself — does not hold for the NO_PROXY set. Decide whether to disable
pinning based on whether the proxy actually applies to this request's host, e.g.
disable only when a proxy is configured and urllib.request.proxy_bypass(host) is
false (mirroring httpx's own NO_PROXY-aware routing), rather than on the mere
presence of a global proxy variable.

# established. Only connection failures are retried, so no request is
# ever delivered more than once.
*fallback_addresses, final_address = pinned_addresses
for address in fallback_addresses:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The fallback loop (openapi_runner.py:246-252) pins each attempt to one literal IP and iterates strictly sequentially, so httpx/anyio no longer races the vetted A/AAAA addresses (each connect_tcp sees a single address, losing happy-eyeballs); combined with the built-in client being built as httpx.AsyncClient(timeout=timeout) where timeout is None by default (lines 204, 239), there is no application-level connect timeout, so an attempt against a blackholed/silently-dropping first vetted address stalls (until the OS TCP connect timeout, not truly "forever") before the loop advances to a reachable vetted address — a latency regression versus the prior by-hostname happy-eyeballs path. A safe fix must preserve the single-delivery invariant pinned by test_run_operation_does_not_retry_a_request_that_may_already_have_been_delivered (only ConnectError/ConnectTimeout may fall through, and the request must never be delivered to more than one address), so racing full requests concurrently is not acceptable; instead bound each attempt's connect phase (e.g., a per-attempt connect timeout inside the loop) so a stalled address cannot delay fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenAPI plugin SSRF validator: resolved IP is not pinned for the connection (DNS check-time vs use-time gap)

2 participants