Python: block cloud metadata endpoints and IPv6-embedded IPv4 in server_url_validator - #14242
Python: block cloud metadata endpoints and IPv6-embedded IPv4 in server_url_validator#14242ErenAta16 (ErenAta16) wants to merge 4 commits into
Conversation
…er_url_validator server_url_validator is the default SSRF guard for OpenAPI plugin calls, and the URL it sees is shaped by the model through server variables and path parameters. Two categories get through it: 168.63.129.16 -- Azure WireServer, reachable from every Azure VM. It is publicly routable, so none of the _try_classify_ipv4 branches match it. NAT64, 6to4 and Teredo addresses. _try_classify_ipv6 unwraps ::ffff: via ipv4_mapped but does not decode the other encodings, so 64:ff9b::169.254.169.254 and 2002:a9fe:a9fe:: both pass. Add an explicit metadata denylist, checked before the range logic and also when allow_private_network_access is set: reaching a host on your own network and reaching the instance's credential endpoint are different requests, and only the first is what that option is for. RFC1918 and loopback still pass with the option on. Decode the embedded IPv4 for NAT64 (RFC 6052 / RFC 8215), 6to4 (RFC 3056) and Teredo (RFC 4380) and classify that. Only the /96 embedding is decoded: guessing the shorter lengths inside the local-use prefix reads bytes that are not the embedded address, which rejected legitimate NAT64 targets like 64:ff9b:1::1.1.1.1 -- there is a test pinning that those stay allowed. Two existing tests moved off 169.254.169.254 onto a plain link-local address. It is still blocked, now with the more specific metadata message, and the new tests assert on it directly. Closes microsoft#14240
There was a problem hiding this comment.
🟡 Not ready to approve
NAT64 decoding currently applies to all addresses in 64:ff9b:1::/48, which can cause false-positive blocking for non-/96 embeddings and should be restricted to the /96-embedding shape.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR hardens the Python OpenAPI plugin SSRF guard (server_url_validator) by explicitly blocking known cloud metadata/credential endpoints (including Azure WireServer) and by detecting IPv6 encodings that embed an IPv4 target (NAT64/6to4/Teredo/IPv4-mapped) before categorization.
Changes:
- Add an explicit denylist for cloud metadata endpoints and enforce it even when
allow_private_network_access=True. - Decode IPv4 targets embedded in certain IPv6 forms and classify the decoded IPv4 for non-public ranges.
- Expand unit tests to cover cloud metadata endpoints,
allow_private_network_accessbehavior, and IPv6 embedded-IPv4 cases.
File summaries
| File | Description |
|---|---|
python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py |
Adds cloud metadata endpoint blocking and IPv6 embedded-IPv4 decoding for SSRF protection. |
python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py |
Adds/adjusts unit tests for metadata endpoint blocking and embedded IPv4-in-IPv6 rejection/allow cases. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| if any(address in network for network in _NAT64_NETWORKS): | ||
| candidates.append(ipaddress.IPv4Address(bytes(packed[offset] for offset in _NAT64_IPV4_OFFSETS))) | ||
|
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
This review comment is correct and I should have answered it sooner. The false positive is real, and it fires on the ordinary case rather than a contrived one. But the suggested remedy would open a hole in the exact thing this PR exists to close, so I want to lay out the measurements before changing anything. The bug, confirmed. RFC 6052 section 2.2 puts the embedded IPv4 in different byte positions per prefix length, and only But restricting the decode to the Today those are blocked, though only by accident: the decode reads a zero suffix and rejects Decoding every RFC 6052 length instead does not work either. I tried it. Each wrong interpretation still yields some IPv4, and those spurious readings classify as blocked often enough to be useless: Skipping all-zero readings recovers the What I think the fix is. Split the two prefixes, because they are not the same kind of thing.
No public address gets mislabelled The cost is explicit and worth stating in the code: legitimate NAT64 traffic through a local-use prefix is refused. For a server-URL allowlist in an OpenAPI plugin I think that is the right side to err on, and it is a stated policy rather than an accident that reports the wrong reason. If anyone disagrees, the alternative is making the local-use prefix length configurable, which seems like a lot of surface for this. I will push that unless someone prefers a different call. Thanks to whoever wired up the review here, the byte-offset observation was exactly right. |
RFC 6052 only places the embedded IPv4 in bytes 12-15 for a /96 embedding. For the shorter lengths those bytes hold the suffix, which the RFC says SHOULD be zero, so decoding them regardless read 0.0.0.0 out of an ordinary public target and rejected it as "unspecified". Declining to decode instead would have swung the error the other way: the metadata address embedded at /48, /56 or /64 would then have passed the check entirely. The prefix length is translator configuration (RFC 6052 section 3.3) and is not carried in the address, so neither direction can be inferred. The well-known prefix 64:ff9b::/96 has a fixed length and keeps its decode. The RFC 8215 local-use prefix is now blocked as a whole instead, which is uniform in both directions and states its reason.
|
Pushed as
Red/green on the new test, reverting only and with the change in place the whole file passes, 84 tests. One test moved rather than being added: |
`IPv6Address.sixtofour` and `.teredo` already do this, and they return None outside their own prefixes, so they carry the membership test with them. That drops two network constants, one offset tuple and the hand-written XOR for the Teredo client address, which is the piece most likely to be restated wrongly. Checked against the previous hand-rolled version on 40000 random addresses per form: no disagreement. The approach is taken from microsoft#14267, which covers the same ground.
|
Flagging an overlap so a reviewer does not read the same fix twice. #14267 covers the same two bypasses this PR does, Azure WireServer being publicly routable and IPv4 smuggled inside NAT64/6to4/Teredo addresses. It was opened on 4 August; this PR and the issue behind it, #14240, are from 29 July. I am not asking anyone to prefer this one. If #14267 is the better patch, say so and I will close this immediately. What would be unhelpful is both sitting in the queue looking like separate work when only one of them needs reviewing. The same thing happened one PR over: #14268 overlaps #14241, which came from #14239 on the same date. |
Motivation and Context
Fixes #14240.
server_url_validatoris the default SSRF guard for OpenAPI plugin calls (allow_private_network_accessdefaults toFalse), and the URL it validates is shaped by the model through server variables and path parameters. Two categories get past it.A publicly routable metadata endpoint.
_try_classify_ipv4works by octet ranges, so168.63.129.16— Azure's WireServer, reachable from every Azure VM — matches nothing:169.254.169.254(AWS/GCP/Azure/OCI/DO IMDS)169.254.170.2(AWS ECS task creds)169.254.170.23(AWS EKS Pod Identity)169.254.42.42(Scaleway)100.100.100.200(Alibaba)192.0.0.192(Oracle Cloud Classic)168.63.129.16(Azure WireServer)IPv6 forms carrying an IPv4 target.
_try_classify_ipv6unwraps::ffff:throughipv4_mappedbut doesn't decode the tunnel encodings:::ffff:169.254.169.254(IPv4-mapped)64:ff9b::169.254.169.254(NAT64, RFC 6052)2002:a9fe:a9fe::(6to4, RFC 3056)Description
CLOUD_METADATA_ADDRESSESis checked before the range logic, and also whenallow_private_network_accessis set. That option reads as "let me reach hosts on my own network"; reaching the instance's credential endpoint is a different request. RFC1918 and loopback still pass with it on:allow_private_network_access=True10.0.0.5127.0.0.11.1.1.1_embedded_ipv4sdecodes NAT64, 6to4, Teredo and IPv4-mapped, andtry_categorize_non_public_addressclassifies whatever comes out, reporting e.g.link-local (embedded in IPv6).Only the /96 NAT64 embedding is decoded. I started with all the RFC 6052 §2.2 lengths for the RFC 8215 local-use
64:ff9b:1::/48prefix and it produced false positives — the shorter-prefix offsets read bytes that aren't the embedded address, yielding0.0.0.0, so64:ff9b:1::1.1.1.1(a legitimate NAT64 address pointing at a public IPv4) came back blocked asunspecified. Reading only bytes 12-15 covers the well-known prefix exactly and the local-use prefix for the /96 embedding, with no false positives.test_ipv6_forms_carrying_a_public_ipv4_are_allowedpins that.How did you test it?
tests/unit/connectors/openapi_plugin/test_server_url_validator.py:allow_private_network_access=True)allow_private_network_access=True, so the option isn't accidentally neutered169.254.169.254, plus a Teredo address built by XOR-ing the low 32 bits64:ff9b::1.1.1.1,64:ff9b:1::1.1.1.1,2002:0101:0101::,2606:4700:4700::1111) that must stay allowedStashing only
server_url_validator.pygives 12 failures; with the change the file is 77 passed.ruff checkandruff format --checkclean.Notes for the reviewer
Two existing tests moved off
169.254.169.254onto169.254.10.10. The address is still blocked — it now reports the metadata message instead oflink-local, so theirmatch=no longer applied. The new tests assert on it directly with a stricter match.Behaviour change worth calling out: anyone currently using
allow_private_network_access=Trueto reach a metadata endpoint will now get an error. That felt right for a control whose stated purpose is SSRF prevention, but I'm happy to put it behind its own flag instead.Checklist