Skip to content

Commit

Permalink
net: fix IPv4 address selection
Browse files Browse the repository at this point in the history
Only apply RFC 6724's CommonPrefixLen rule for IPv4 source/destination
pairs that are members of the same IPv4 special purpose block.

Fixes golang#13283.

Change-Id: I2f7c26b408dd4675dfc5c1959e22d05b43bb8241
Reviewed-on: https://go-review.googlesource.com/16995
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
mdempsky committed Nov 17, 2015
1 parent e0e4ccb commit 4d4a266
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 0 deletions.
43 changes: 43 additions & 0 deletions src/net/addrselect.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,24 @@ func (s *byRFC6724) Less(i, j int) bool {
if da4 == db4 {
commonA := commonPrefixLen(SourceDA, DA)
commonB := commonPrefixLen(SourceDB, DB)

// CommonPrefixLen doesn't really make sense for IPv4, and even
// causes problems for common load balancing practices
// (e.g., https://golang.org/issue/13283). Glibc instead only
// uses CommonPrefixLen for IPv4 when the source and destination
// addresses are on the same subnet, but that requires extra
// work to find the netmask for our source addresses. As a
// simpler heuristic, we limit its use to when the source and
// destination belong to the same special purpose block.
if da4 {
if !sameIPv4SpecialPurposeBlock(SourceDA, DA) {
commonA = 0
}
if !sameIPv4SpecialPurposeBlock(SourceDB, DB) {
commonB = 0
}
}

if commonA > commonB {
return preferDA
}
Expand Down Expand Up @@ -386,3 +404,28 @@ func commonPrefixLen(a, b IP) (cpl int) {
}
return
}

// sameIPv4SpecialPurposeBlock reports whether a and b belong to the same
// address block reserved by the IANA IPv4 Special-Purpose Address Registry:
// http://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml
func sameIPv4SpecialPurposeBlock(a, b IP) bool {
a, b = a.To4(), b.To4()
if a == nil || b == nil || a[0] != b[0] {
return false
}
// IANA defines more special-purpose blocks, but these are the only
// ones likely to be relevant to typical Go systems.
switch a[0] {
case 10: // 10.0.0.0/8: Private-Use
return true
case 127: // 127.0.0.0/8: Loopback
return true
case 169: // 169.254.0.0/16: Link Local
return a[1] == 254 && b[1] == 254
case 172: // 172.16.0.0/12: Private-Use
return a[1]&0xf0 == 16 && b[1]&0xf0 == 16
case 192: // 192.168.0.0/16: Private-Use
return a[1] == 168 && b[1] == 168
}
return false
}
115 changes: 115 additions & 0 deletions src/net/addrselect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,57 @@ func TestSortByRFC6724(t *testing.T) {
},
reverse: true,
},

// Issue 13283. Having a 10/8 source address does not
// mean we should prefer 23/8 destination addresses.
{
in: []IPAddr{
{IP: ParseIP("54.83.193.112")},
{IP: ParseIP("184.72.238.214")},
{IP: ParseIP("23.23.172.185")},
{IP: ParseIP("75.101.148.21")},
{IP: ParseIP("23.23.134.56")},
{IP: ParseIP("23.21.50.150")},
},
srcs: []IP{
ParseIP("10.2.3.4"),
ParseIP("10.2.3.4"),
ParseIP("10.2.3.4"),
ParseIP("10.2.3.4"),
ParseIP("10.2.3.4"),
ParseIP("10.2.3.4"),
},
want: []IPAddr{
{IP: ParseIP("54.83.193.112")},
{IP: ParseIP("184.72.238.214")},
{IP: ParseIP("23.23.172.185")},
{IP: ParseIP("75.101.148.21")},
{IP: ParseIP("23.23.134.56")},
{IP: ParseIP("23.21.50.150")},
},
reverse: false,
},

// Prefer longer common prefixes, but only for IPv4 address
// pairs in the same special-purpose block.
{
in: []IPAddr{
{IP: ParseIP("1.2.3.4")},
{IP: ParseIP("10.55.0.1")},
{IP: ParseIP("10.66.0.1")},
},
srcs: []IP{
ParseIP("1.2.3.5"),
ParseIP("10.66.1.2"),
ParseIP("10.66.1.2"),
},
want: []IPAddr{
{IP: ParseIP("10.66.0.1")},
{IP: ParseIP("10.55.0.1")},
{IP: ParseIP("1.2.3.4")},
},
reverse: true,
},
}
for i, tt := range tests {
inCopy := make([]IPAddr, len(tt.in))
Expand Down Expand Up @@ -217,3 +268,67 @@ func TestRFC6724CommonPrefixLength(t *testing.T) {
}

}

func mustParseCIDRs(t *testing.T, blocks ...string) []*IPNet {
res := make([]*IPNet, len(blocks))
for i, block := range blocks {
var err error
_, res[i], err = ParseCIDR(block)
if err != nil {
t.Fatalf("ParseCIDR(%s) failed: %v", block, err)
}
}
return res
}

func TestSameIPv4SpecialPurposeBlock(t *testing.T) {
blocks := mustParseCIDRs(t,
"10.0.0.0/8",
"127.0.0.0/8",
"169.254.0.0/16",
"172.16.0.0/12",
"192.168.0.0/16",
)

addrs := []struct {
ip IP
block int // index or -1
}{
{IP{1, 2, 3, 4}, -1},
{IP{2, 3, 4, 5}, -1},
{IP{10, 2, 3, 4}, 0},
{IP{10, 6, 7, 8}, 0},
{IP{127, 0, 0, 1}, 1},
{IP{127, 255, 255, 255}, 1},
{IP{169, 254, 77, 99}, 2},
{IP{169, 254, 44, 22}, 2},
{IP{169, 255, 0, 1}, -1},
{IP{172, 15, 5, 6}, -1},
{IP{172, 16, 32, 41}, 3},
{IP{172, 31, 128, 9}, 3},
{IP{172, 32, 88, 100}, -1},
{IP{192, 168, 1, 1}, 4},
{IP{192, 168, 128, 42}, 4},
{IP{192, 169, 1, 1}, -1},
}

for i, addr := range addrs {
for j, block := range blocks {
got := block.Contains(addr.ip)
want := addr.block == j
if got != want {
t.Errorf("%d/%d. %s.Contains(%s): got %v, want %v", i, j, block, addr.ip, got, want)
}
}
}

for i, addr1 := range addrs {
for j, addr2 := range addrs {
got := sameIPv4SpecialPurposeBlock(addr1.ip, addr2.ip)
want := addr1.block >= 0 && addr1.block == addr2.block
if got != want {
t.Errorf("%d/%d. sameIPv4SpecialPurposeBlock(%s, %s): got %v, want %v", i, j, addr1.ip, addr2.ip, got, want)
}
}
}
}

0 comments on commit 4d4a266

Please sign in to comment.