CAMEL-24371: camel-a2a - fix WebhookUrlValidator address classification and host matching - #26583
Conversation
…on and host matching (apache#25406) WebhookUrlValidator classified webhook hosts in two places that did not agree. A host written as an IP literal was checked against a string prefix list, while a host reached through a name was classified with the InetAddress predicates. Those predicates do not cover the same ground: isSiteLocalAddress reports the deprecated fec0::/10 block and not the fc00::/7 unique local addresses that replaced it, so the same address was accepted or rejected depending on how it was written. The literal pre-check also prefix-matched the raw host string without establishing that the host was an IP literal, so any name beginning with fc or fd, such as fcm.googleapis.com, was rejected outright. Both paths now resolve the host and classify the resulting address with one shared raw-byte classifier. InetAddress.getByName already parses bracketed IPv6 literals without touching DNS, so the separate literal path is no longer needed. The classifier additionally recognises fc00::/7, IPv4-compatible IPv6, the NAT64 well-known prefix 64:ff9b::/96, 6to4 under 2002::/16 and the shared address space 100.64.0.0/10. NAT64 and 6to4 addresses are classified by the IPv4 address they embed, so a translation prefix carrying a globally routable address stays allowed. A package-private resolver seam lets the resolved-host path be tested without DNS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 54041cc)
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
Reviewed the security fix thoroughly. Byte arithmetic for all IPv6 transition mechanisms verified correct:
- ULA (fc00::/7):
(bytes[0] & 0xfe) == 0xfccorrectly catches bothfc00::andfd00::, bypassingisSiteLocalAddress()'s limitation to the deprecatedfec0::/10block. - NAT64 prefix constant:
64:ff9b::→0x0064,0xff9bin network byte order →{0x00, 0x64, 0xff, 0x9b, ...}. Matches exactly. - CG-NAT (100.64.0.0/10):
(bytes[0] & 0xff) == 100 && (bytes[1] & 0xc0) == 0x40covers100.64.0.0–100.127.255.255with exact boundary values confirmed. - Embedded IPv4 recursion: always extracts exactly 4 bytes →
getByAddress()returnsInet4Address→nonGlobalReason()callsipv4Reason(), notipv6Reason()— no unbounded recursion possible. - allowLocal bypass:
::ffff:127.0.0.1is notisLoopbackAddress()forInet6Addressin the JVM → falls through tononGlobalReason()→embeddedIpv4Reason()→ blocked regardless ofallowLocal. The comment on line 149–150 documents this intentional design. resolvingTo()test helper:InetAddress.getByName()on IP literals (both v4 and v6) is a pure parse — no DNS involved. Tests are DNS-independent.- Teredo (2001::/32): not covered, but defensible — Teredo is deprecated (RFC 7059), and SSRF via a Teredo tunnel requires an active server + cooperating client, which is not a realistic webhook attack vector.
Test coverage is comprehensive, including boundary cases for adjacent address blocks and the resolver-injection path for hostname-resolved addresses. The original bug (name-prefix false positive blocking fcm.googleapis.com) and the isSiteLocalAddress gap on fc00::/7 are both properly fixed.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 9 tested, 29 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
Backport of #25406 to
camel-4.22.x.Straight cherry-pick of
54041cce545322f7ff1bb362ecc02999b3d96ba0, with the upgrade-guide changes dropped — those live onmainonly.WebhookUrlValidatorclassified a webhook host in two places that did not agree. A host written as an IP literal was matched against a string prefix list, while a host reached through a name was classified with theInetAddresspredicates — and those do not cover the same ground, sinceisSiteLocalAddressreports the deprecatedfec0::/10block and not thefc00::/7unique local addresses that replaced it. The same address was therefore accepted or rejected depending on how it was written. The literal pre-check was also wrong in the other direction: it prefix-matched the raw host string without first establishing that the host was an IP literal, so any name beginning withfcorfd(e.g.fcm.googleapis.com) was refused outright.Both paths now resolve the host and classify the resulting address with one shared raw-byte classifier.
Verified locally:
mvn install -DskipITsincomponents/camel-ai/camel-a2a— 519 tests, 0 failures (30 inWebhookUrlValidatorTest).🤖 Generated with Claude Code