Repository navigation
Commit a287ee2
authored
fix(signing)!: stop
fix(signing)!: stop `allow_private` from admitting IANA special-use ranges
Closes #1357
`validate_resolved_ip` applied its whole blocked set behind one boolean, so
`allow_private=True` — documented for dialling a test's own loopback origin —
also admitted 23 named IANA ranges that are not private address space, among
them the RFC 6598 carrier-grade NAT and RFC 7526/7535/7450/7534/7343 ranges PR
#974 added, plus the unallocated IPv6 space `is_reserved` covers.
A caller that enabled private destinations lost the special-use blocklist with
them, on the paths where the dialled URL is publisher- or buyer-supplied.
## What changed
The blocked set splits at the reachability line, and each half gets its own
gate:
* `allow_private` admits **private destinations** — RFC 1918, RFC 4193
unique-local, loopback. These are the ranges an operator controls and the
ones the flag's docstring describes.
* `allow_special_use` admits **special-use ranges** — link-local, multicast,
the unspecified address, RFC 5737/3849 documentation space, RFC 2544
benchmarking, RFC 6890 protocol assignments, `240.0.0.0/4`, 6to4, Teredo,
NAT64, and the `_EXTRA_BLOCKED_NETWORKS` anycast and identifier ranges.
None is a destination an operator deploys a service to.
* `BLOCKED_METADATA_IPS` stays blocked under both, as before.
`_PRIVATE_DESTINATION_NETWORKS` holds the admitted set, next to
`_EXTRA_BLOCKED_NETWORKS` and with the same kind of per-range justification.
`_is_blocked_by_base_policy` spells the pre-split union once, so the two gates
partition that union rather than each restating part of it.
`resolve_and_validate_host`, `validate_jwks_uri`, `build_ip_pinned_transport`,
`build_async_ip_pinned_transport` and `abuild_ip_pinned_transport` take the new
keyword and forward it. The higher-level agent-resolution and brand.json APIs
(`allow_private_destinations`) keep the hardened default: a publisher- or
brand-supplied URL has no legitimate special-use destination, so no escape
hatch is threaded to them.
The `JwksFetcher` and `AsyncJwksFetcher` Protocols are unchanged. Widening them
breaks every custom fetcher an adopter has written, and the built-in
fetchers reaching the hardened default is the behavior they should have. An
adopter needing the relaxation on its own transport calls
`resolve_and_validate_host` or `build_ip_pinned_transport` directly, which is
what the custom-fetcher section of the package docstring already points at.
The package docstring's custom-fetcher section names both gates and their
defaults, next to the primitives it lists.
## Why API DESIGN and not the ranges
Nothing is mis-classified. `_EXTRA_BLOCKED_NETWORKS` is complete, the six
`ipaddress` flags are the right predicate, and the empirical verification in
that table's comment holds. The defect is the scope of one parameter, so the
change is to the parameter.
A narrower fix — making `_EXTRA_BLOCKED_NETWORKS` unconditional and leaving the
flags behind `allow_private` — closes six of the 23 named ranges and leaves 17
open,
because `ipaddress.is_private` is itself far wider than RFC 1918: it reports
True for documentation, benchmarking, protocol-assignment, 6to4, Teredo and
`240.0.0.0/4` space. Any gate keyed on that flag admits all of it. Naming the
private ranges positively is what makes the relaxation as narrow as the flag's
name.
## Why breaking, and the migration
`allow_private=True` refuses addresses it previously accepted, so this carries
`!` and a `BREAKING CHANGE:` footer. A caller that relied on it to reach
link-local, multicast, carrier-grade NAT, documentation or benchmarking space
passes `allow_special_use=True` alongside it and keeps the previous behavior
exactly.
The alternative — adding `allow_special_use` while leaving `allow_private`
relaxing everything — is additive and fixes nothing: the hole stays open for
every existing caller, and the new parameter has no effect unless
`allow_private` is already off. The behavior change is the fix.
Callers using the flag for what it is documented for are unaffected.
`DockerLocalhostRewrite`, whose `allow_private_destinations=True` requirement
is documented in `docs/webhooks/migration-from-fragmented-senders.md`, rewrites
to `host.docker.internal` or `172.17.0.1`; both resolve into RFC 1918 space and
keep working.
## Tests
`tests/conformance/signing/test_jwks.py`, table-driven over the 23 named ranges
so a
future relaxation fails per range rather than in aggregate:
* `test_ssrf_special_use_ranges_stay_blocked_under_allow_private` — each range
refused with `allow_private=True`.
* `test_ssrf_special_use_ranges_admitted_only_by_their_own_flag` — each range
admitted with `allow_special_use=True`, which is the migration path.
* `test_ssrf_private_destinations_honour_allow_private` — RFC 1918, RFC 4193
and loopback still admitted by `allow_private`, still refused without it.
* `test_private_destination_networks_are_a_subset_of_the_base_policy` — walks
`_PRIVATE_DESTINATION_NETWORKS` and asserts the base policy blocks every
bound. This is what keeps the default posture identical to before the split:
a range added to the private set that the union does not cover turns a
refusal into an acceptance for `allow_private=False`.
* `test_private_and_special_use_sets_are_disjoint` — one address, one gate.
* `test_ssrf_metadata_blocked_under_both_relaxations` — every entry in
`BLOCKED_METADATA_IPS` refused under `allow_private`, under
`allow_special_use`, and under both. Each one sits inside a range one of the
gates admits (`169.254.169.254` in link-local, `100.100.100.200` in RFC 6598,
`192.0.0.192` in RFC 6890, `fd00:ec2::254` in RFC 4193), so the unconditional
check has to run ahead of both. Moving it behind them fails all four cases.
The two tests that asserted the old behavior,
`test_ssrf_6to4_relay_honours_allow_private_override` and
`test_ssrf_cgnat_honours_allow_private_override`, are replaced by the rows
above; both ranges now appear in the stay-blocked table.
Verified against the pinned base: 50 of the new parametrized cases fail without
the change and pass with it.
`make lint typecheck validate-generated test-type-checks
check-type-ignore-contract` pass. `tests/conformance/signing/`,
`tests/test_adagents.py`, `tests/test_canonical_reference_resolver.py`,
`tests/test_a2a_push_security.py`, the webhook-delivery and
webhook-destination-policy suites, and every test naming `allow_private` or
`allow_private_destinations` pass.
## What an adopter can now do
Ask for private destinations without asking for anything else:
```python
from adcp.signing import resolve_and_validate_host
# A test dialling its own loopback origin.
resolve_and_validate_host("https://127.0.0.1:8443/webhook", allow_private=True)
# The same posture refuses a buyer-supplied URL that resolves into shared
# carrier space, which allow_private previously accepted.
resolve_and_validate_host("https://buyer.example/webhook", allow_private=True)
# SSRFValidationError: resolved IP 100.64.0.1 is in a reserved range
# (IANA special-use; allow_private does not admit it)
```
Before this change the only way to express that was to re-enumerate the ranges
outside the SDK: `_EXTRA_BLOCKED_NETWORKS` and `BLOCKED_METADATA_IPS` are
module-private and absent from `adcp.signing.__all__`, so an adopter could not
even reuse the tables it was duplicating.
The refusal message names the gate that admits the address, so an operator
hitting it in a dev deployment reads which knob to turn.
BREAKING CHANGE: `allow_private=True` now admits only RFC 1918, RFC 4193 unique-local and loopback destinations. The IANA special-use ranges it used to admit — link-local, multicast, the unspecified address, RFC 6598 carrier-grade NAT, documentation and benchmarking space, RFC 6890 protocol assignments, 240.0.0.0/4, 6to4, Teredo, NAT64, the IANA anycast and identifier ranges and unallocated IPv6 — are refused unless the caller also passes the new `allow_special_use=True`; cloud metadata addresses stay refused under both. Direct callers of `validate_resolved_ip`, `resolve_and_validate_host`, `validate_jwks_uri`, `build_ip_pinned_transport`, `build_async_ip_pinned_transport` or the deprecated `abuild_ip_pinned_transport` that dial those ranges migrate by adding `allow_special_use=True` next to `allow_private=True`. The higher-level `allow_private_destinations=True` surfaces — webhook delivery and `WebhookSender`, the audit sink, agent and brand.json resolution, and the built-in JWKS and revocation fetchers — forward only `allow_private` and get no special-use relaxation: deployments that reached a special-use address through them must move the destination into RFC 1918, RFC 4193 or loopback space, or supply their own client or transport built with `allow_special_use=True` where that surface accepts one. Loopback, RFC 1918 and RFC 4193 destinations, including the `DockerLocalhostRewrite` targets, are unaffected.allow_private from admitting IANA special-use ranges (#1377)1 parent a0fbee3 commit a287ee2
4 files changed
Lines changed: 289 additions & 30 deletions
File tree
- src/adcp/signing
- tests/conformance/signing
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
93 | 105 | | |
94 | 106 | | |
95 | 107 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
293 | 293 | | |
294 | 294 | | |
295 | 295 | | |
| 296 | + | |
296 | 297 | | |
297 | 298 | | |
298 | 299 | | |
| |||
317 | 318 | | |
318 | 319 | | |
319 | 320 | | |
| 321 | + | |
320 | 322 | | |
321 | 323 | | |
322 | 324 | | |
| |||
326 | 328 | | |
327 | 329 | | |
328 | 330 | | |
| 331 | + | |
329 | 332 | | |
330 | 333 | | |
331 | 334 | | |
| |||
342 | 345 | | |
343 | 346 | | |
344 | 347 | | |
| 348 | + | |
345 | 349 | | |
346 | 350 | | |
347 | 351 | | |
| |||
351 | 355 | | |
352 | 356 | | |
353 | 357 | | |
| 358 | + | |
354 | 359 | | |
355 | 360 | | |
356 | 361 | | |
| |||
371 | 376 | | |
372 | 377 | | |
373 | 378 | | |
| 379 | + | |
374 | 380 | | |
375 | 381 | | |
376 | 382 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
109 | 154 | | |
110 | 155 | | |
111 | 156 | | |
| |||
210 | 255 | | |
211 | 256 | | |
212 | 257 | | |
| 258 | + | |
213 | 259 | | |
214 | 260 | | |
215 | 261 | | |
| |||
218 | 264 | | |
219 | 265 | | |
220 | 266 | | |
221 | | - | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
222 | 273 | | |
223 | 274 | | |
224 | 275 | | |
| |||
268 | 319 | | |
269 | 320 | | |
270 | 321 | | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
271 | 349 | | |
272 | 350 | | |
273 | 351 | | |
274 | 352 | | |
| 353 | + | |
275 | 354 | | |
276 | 355 | | |
277 | 356 | | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
282 | 372 | | |
283 | 373 | | |
284 | 374 | | |
285 | 375 | | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
296 | 387 | | |
297 | 388 | | |
298 | 389 | | |
299 | 390 | | |
300 | 391 | | |
301 | 392 | | |
| 393 | + | |
302 | 394 | | |
303 | 395 | | |
304 | 396 | | |
| |||
318 | 410 | | |
319 | 411 | | |
320 | 412 | | |
321 | | - | |
322 | | - | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
323 | 421 | | |
324 | 422 | | |
325 | 423 | | |
| |||
365 | 463 | | |
366 | 464 | | |
367 | 465 | | |
368 | | - | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
369 | 469 | | |
370 | 470 | | |
371 | 471 | | |
| |||
0 commit comments