IPAddress::IsReserved() is missing ranges |
|||
Issue descriptionNoticed by Martin in https://codereview.chromium.org/1860823005/ and some discussion there. The earlier patchset that introduced this captured more ranges: https://codereview.chromium.org/22538003/diff/14001/net/base/net_util.cc But some where lost when converting to the binary encoding: https://codereview.chromium.org/22538003/diff/21001/net/base/net_util.cc (Patchset 3 --> Patchset 4)
,
Apr 12 2016
Assigned to martijn@martijnc.be (owner:eroman is just a workaround for bug tracker not allowing that assignment).
,
Apr 12 2016
It was explicit and intentional that we didn't test for the global unicast/global; anything not explicitly reserved should not be assumed to be reserved (for forwards compatability)
,
Apr 12 2016
The IANA document covers the _entire_ IPv6 address space. There is nothing unallocated by those partition ranges. (I confirmed by intersecting all the rules, and you get a production for ::/0) Hence forward compatibility as you described in comment #3 is not possible. If a new range was carved out, it would necessarily have to come out of one of the existing reserved ranges. So either way we will consider it reserved until we then update our rules. Am I missing something? Or did I simply do the math wrong?
,
Apr 12 2016
My logic for claims in comment #4: All the allocations given in the IANA document are here: (http://www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml) 0000::/8 (00000000*) 0100::/8 (00000001*) 0200::/7 (0000001*) 0400::/6 (000001*) 0800::/5 (00001*) 1000::/4 (0001*) 2000::/3 (001*) Global Unicast 4000::/3 (010*) 6000::/3 (011*) 8000::/3 (100*) a000::/3 (101*) c000::/3 (110*) e000::/4 (1110*) f000::/5 (11110*) f800::/6 (111110*) fc00::/7 (1111110*) (Unique Local Unicast) fe00::/9 (111111100*) fe80::/10 (1111111010*) (Link-Scoped Unicast) fec0::/10 (1111111011*) ff00::/8 (11111111*) Multicast Taking the union of all those ranges you can reduce down to: 0000::/1 (0*) 8000::/1 (1*) Or if I may, ::/0 (*) Which is the entire IPv6 space. So in other words there is nothing left unallocated. Hence taking the exclusion is equivalent to "summing" the reserved ranges. The specific steps to do that reduction are: Combining 0000::/8 (00000000*) and 0100::/8 (00000001*) into 0000::/7 (0000000*) Combining 0200::/7 (0000001*) and 0000::/7 (0000000*) into 0000::/6 (000000*) Combining 0400::/6 (000001*) and 0000::/6 (000000*) into 0000::/5 (00000*) Combining 0800::/5 (00001*) and 0000::/5 (00000*) into 0000::/4 (0000*) Combining 1000::/4 (0001*) and 0000::/4 (0000*) into 0000::/3 (000*) Combining 2000::/3 (001*) and 0000::/3 (000*) into 0000::/2 (00*) Combining 4000::/3 (010*) and 6000::/3 (011*) into 4000::/2 (01*) Combining 8000::/3 (100*) and a000::/3 (101*) into 8000::/2 (10*) Combining fe80::/10 (1111111010*) and fec0::/10 (1111111011*) into fe80::/9 (111111101*) Combining fe00::/9 (111111100*) and fe80::/9 (111111101*) into fe00::/8 (11111110*) Combining ff00::/8 (11111111*) and fe00::/8 (11111110*) into fe00::/7 (1111111*) Combining fc00::/7 (1111110*) and fe00::/7 (1111111*) into fc00::/6 (111111*) Combining f800::/6 (111110*) and fc00::/6 (111111*) into f800::/5 (11111*) Combining f000::/5 (11110*) and f800::/5 (11111*) into f000::/4 (1111*) Combining e000::/4 (1110*) and f000::/4 (1111*) into e000::/3 (111*) Combining c000::/3 (110*) and e000::/3 (111*) into c000::/2 (11*) Combining 0000::/2 (00*) and 4000::/2 (01*) into 0000::/1 (0*) Combining 8000::/2 (10*) and c000::/2 (11*) into 8000::/1 (1*) In parenthesis I show the range as a bitstring pattern since I find that easier to visualize. In that production a * is a wildcard that expands to [01]* in regex notation.
,
Apr 13 2016
Ah, thanks Eric. I really suck at reading. The whitelist SGTM.
,
Apr 13 2016
No problem, I didn't realize it either until writing out the bitstring patterns :) For the record I should also point out that none of the reserved ranges overlap with the global unicast + multicast range, as one would expect. (That is the other necessary condition for a whitelist approach to be equivalent).
,
Apr 18 2016
Ryan, what is the caller's expectation around IPv4 mapped IPv6 addresses? Currently any IPv4 mapped IPv6 address is considered reserved, regardless of whether the embedded IPv4 address is considered reserved. Would any caller want to instead be told whether the underlying IPv4 address was reserved?
,
Apr 18 2016
I would argue no; I take the more restrictive approach until a demonstrated use case can be shown (e.g. why the non-mapped address couldn't/shouldn't be used)
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/821620aa901ab5fe90e89734fc1837c401ea8a37 commit 821620aa901ab5fe90e89734fc1837c401ea8a37 Author: martijn <martijn@martijnc.be> Date: Fri Apr 22 15:37:17 2016 Update IPAddress::IsReserved() ranges. BUG= 602773 Review URL: https://codereview.chromium.org/1886043002 Cr-Commit-Position: refs/heads/master@{#389114} [modify] https://crrev.com/821620aa901ab5fe90e89734fc1837c401ea8a37/net/base/ip_address.cc [modify] https://crrev.com/821620aa901ab5fe90e89734fc1837c401ea8a37/net/base/ip_address_unittest.cc
,
Jun 3 2016
Fixed by martijn@martijnc.be per above check-in. |
|||
►
Sign in to add a comment |
|||
Comment 1 by eroman@chromium.org
, Apr 12 2016Status: Available (was: Untriaged)