Resolve AAAA records in resolve_public_host so IPv6-only hosts work#3229
Merged
Resolve AAAA records in resolve_public_host so IPv6-only hosts work#3229
Conversation
resolve_public_host() previously only looked up A records via gethostbynamel(), so any host with AAAA-only records was rejected even if it was a perfectly public IPv6 address. The current callers (CIMD discovery, SSE proxy) all gate on the false return and stay safe, but the helper documents itself as an SSRF guard for general use, and a future caller forgetting the false-check would silently lose protection on IPv6 deployments. Add a dns_get_record(DNS_AAAA) lookup alongside gethostbynamel(), validate every returned address against private/reserved ranges, and prefer IPv4 when both exist (mirrors wp_safe_remote_get's default). Extract the IPv4-mapped IPv6 byte-pattern check into is_ipv4_mapped_ipv6() so the literal path and the AAAA path can share it, and add unit tests for that helper.
There was a problem hiding this comment.
Pull request overview
This PR strengthens the Activitypub\resolve_public_host() SSRF-guard helper so IPv6-only hostnames are no longer rejected, by adding AAAA resolution/validation alongside the existing IPv4 (A-record) path.
Changes:
- Add AAAA record lookup (
dns_get_record(..., DNS_AAAA)) and validate all returned IPv6 addresses as public. - Prefer IPv4 over IPv6 when both exist, while still rejecting if any resolved address is private/reserved.
- Extract IPv4-mapped-IPv6 detection into a dedicated
is_ipv4_mapped_ipv6()helper and add unit tests for it.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
includes/functions-request.php |
Extends resolve_public_host() to resolve/validate AAAA records and introduces is_ipv4_mapped_ipv6() helper. |
tests/phpunit/tests/includes/class-test-functions-request.php |
Adds PHPUnit coverage for the new IPv4-mapped-IPv6 detector helper. |
.github/changelog/harden-resolve-public-host-ipv6 |
Adds a patch-level security changelog entry describing the hardening. |
Introduce an `activitypub_pre_resolve_public_host` filter so tests can inject pre-resolved address sets without making real DNS queries. Add tests covering the new code paths the previous coverage missed: IPv4 preference when both record types exist, fall-through to IPv6 when only AAAA resolves, split-horizon rejection (one private address fails the whole answer) for both IPv4 and IPv6, IPv4-mapped IPv6 rejection in the AAAA path, and the empty-resolution case.
…tests The previous stub helper registered a filter without removing it, so the injected addresses persisted across test boundaries. Track every registered callback and remove them all in tear_down() so the suite stays order-independent.
- is_ipv4_mapped_ipv6() short-circuits on non-IPv6 input before inet_pton(), so feeding it 'not-an-ip' or '' (which the unit tests do) no longer triggers a warning. - The activitypub_pre_resolve_public_host filter return value now has its ipv4 / ipv6 keys verified to be arrays before iteration; a misbehaving filter callback can't crash the foreach with a type error.
The closure has no parameters, so registering it with `accepted_args = 2` would have WordPress invoke it with two arguments the callable doesn't expect. Falling back to the add_filter() default keeps the call-site argument count aligned with the closure shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes:
resolve_public_host()previously only looked up A records viagethostbynamel(), so any host with AAAA-only records was rejected even if it was a perfectly public IPv6 address. The current callers (OAuth client discovery, the SSE eventStream proxy) all gate on thefalsereturn and stay safe today, but the helper documents itself as a general SSRF guard and a future caller that forgets to check forfalsewould silently lose protection on IPv6-only deployments.dns_get_record($host, DNS_AAAA)lookup alongsidegethostbynamel(). Every returned IPv4 address is validated againstFILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE; every returned IPv6 address is checked the same way and explicitly rejected if it falls in::ffff:0:0/96(IPv4-mapped IPv6, which PHP's filter flags don't catch consistently across builds).wp_safe_remote_get()'s default. Fall through to IPv6 when only AAAA records resolve. The SSE proxy's existing[ip]:portformatting handles either.is_ipv4_mapped_ipv6) so the literal path and the AAAA path share it.This is opt-in defense-in-depth: the surfaces that call
resolve_public_host()are only active behind feature flags (the ActivityPub API option for OAuth, SSE for the proxy). Default-config sites are not affected.Other information:
Testing instructions:
activitypub_api) enabled, register a CIMDclient_idwhose host has only AAAA records. Discovery should succeed where it previously rejected the host. Confirm a CIMD URL whose AAAA points at::1,fc00::/7, or any link-local/private IPv6 is rejected withactivitypub_client_unsafe_host.eventStreamat an IPv6-only public peer. The relay should connect. An IPv6-only peer whose AAAA resolves to a private/loopback address should produceactivitypub_proxy_unsafe_host.Changelog entry
Changelog Entry Details
Significance
Type
Message
Hardened outbound request safety to cover IPv6-only third-party hosts.