feat: harden egress IP address validation#5604
Conversation
Consolodate down to a single implementation of Dialer in the dependencies.
In the standardlib prometheus scrape function use the HTTP client from the injected dependencies to provide IP validation at connection time.
Extend the PrivateIPValidator to include additional IP ranges that are defined as not being "Globally Reachable" by IANA.
appletreeisyellow
left a comment
appletreeisyellow
left a comment
There was a problem hiding this comment.
I have one question about a missing IP address that got removed in the PR. Otherwise, makes sense to me!
|
|
||
| func init() { | ||
| for _, cidr := range []string{ | ||
| "0.0.0.0/32", // Linux treats 0.0.0.0 as 127.0.0.1 |
There was a problem hiding this comment.
Seems like 0.0.0.0/32 is missing from the new list. I saw it is listed on https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml. Is it intentional to remove it?
There was a problem hiding this comment.
Yes, it is covered by 0.0.0.0/8
There was a problem hiding this comment.
Ah, good to know!
| // affect flux scripts. | ||
| "192.0.0.0/24", // IETF Protocol Assignments [RFC6890], Section 2.1 | ||
| "192.0.2.0/24", // Documentation (TEST-NET-1) [RFC5737] | ||
| "192.88.99.2/32", // 6a44-relay anycast address [RFC6751] |
There was a problem hiding this comment.
I was unfamiliar with most of these added ones, so I researched with claude and it had this to say about this one:
The commit blocks only the single host 192.88.99.2/32 citing RFC 6751 (6a44 relay
anycast). However, the entire 192.88.99.0/24 is in the IANA IPv4 Special-Purpose
Registry as "not globally reachable" per RFC 7526 (which deprecated 6to4 relay
anycast).
https://datatracker.ietf.org/doc/html/rfc7526#section-4 states "The prefix 192.88.99.0/24 MUST NOT be reassigned for other use except by a future IETF Standards Action." These shouldn't be routable, but since you already have 192.88.99.2/32, perhaps update?
There was a problem hiding this comment.
https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml has an empty space in the "Globally Reachable" so I went with not false. However as you point out there should be no real value in attempting to route to it. I'll update it in a follow-up.
| // The IPv4-mapped Address block is marked as not being globally | ||
| // reachable, but is also how Go stores IPv4 Addresses, so | ||
| // adding the range causes all IPv4 addresses to be blocked. | ||
| // "::ffff:0:0/96", IPv4-mapped Address [RFC4291] |
There was a problem hiding this comment.
Nice catch to avoid a regression. :)
There was a problem hiding this comment.
I'll let you decide if I worked that out all on my own or if I spent a baffling hour trying to work out why all my traffic was being blocked.
3 participants
Some changes to strengthen the protection from IP address validation. The two near-identical Dialer definitions have been reduced to a single implementation that is shared. The PrivateIPValidator's list of addresses has been expanded with a number of additional ranges specified as not being globally routable by IANA. The prometheus scrape builtin function has been updated to use the HTTP client specified by the dependencies mechanism.
Helps: influxdata/idpe#19461
Checklist
Dear Author 👋, the following checks should be completed (or explicitly dismissed) before merging.
experimental/docs/Spec.mdhas been updatedDear Reviewer(s) 👋, you are responsible (among others) for ensuring the completeness and quality of the above before approval.