New issue
Advanced search Search tips

Issue 602773 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

IPAddress::IsReserved() is missing ranges

Project Member Reported by eroman@chromium.org, Apr 12 2016

Issue description

Noticed 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)

 

Comment 1 by eroman@chromium.org, Apr 12 2016

Cc: rsleevi@chromium.org
Status: Available (was: Untriaged)
Copy pasting from patchset 3 (and confirmed with the IANA spec [1]), the reserved ranges this function intends to test are:

"0000::/8", "::1/128", "0100::/8", "0200::/7", "0400::/6", "0800::/5",
"1000::/4", "4000::/3", "6000::/3", "8000::/3", "a000::/3", "c000::/3",
"e000::/4", "f000::/5", "f800::/6", "FC00::/7", "fe00::/9", "fe80::/10",
"fec0::/10"

The above is a non-minimal encoding (::1/128 for instance is already covered by ::/8, and other simplifications are possible) but nevertheless it serves as a baseline for intent.

To summarize, we want to capture ALL of the ranges whose allocation is labeled "Reserved by IETF" in the IANA document, as well as the "Unique Local Unicast" and "Link-Scoped Unicast" allocations (as they are private address space).

Since the IANA document partitions the entire IPv6 address space, a simpler way to express this is by exclusion: anything that isn't unicast or multicast.

So when fixing this we can simply test these two ranges:

"2000::/3", "ff00::/8"

(global unicast and global respectively)

Not sure that IsReserved() is the best name either.

[1] http://www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml

Comment 2 by eroman@chromium.org, Apr 12 2016

Owner: eroman@chromium.org
Status: Assigned (was: Available)
Assigned to martijn@martijnc.be

(owner:eroman is just a workaround for bug tracker not allowing that assignment).
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)

Comment 4 by eroman@chromium.org, 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?

Comment 5 by eroman@chromium.org, 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.

Comment 6 by sleevi@google.com, Apr 13 2016

Ah, thanks Eric. I really suck at reading. The whitelist SGTM.

Comment 7 by eroman@chromium.org, 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).

Comment 8 by eroman@chromium.org, 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?
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)
Project Member

Comment 10 by bugdroid1@chromium.org, 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

Owner: ----
Status: Fixed (was: Assigned)
Fixed by martijn@martijnc.be per above check-in.

Sign in to add a comment