Skip to content

Commit

Permalink
OCPBUGS-10695: Use subnet mask to detect IP stack
Browse files Browse the repository at this point in the history
This PR hardens the logic of selecting node IP by adding more
sophisticated way of detecting IPv6 addresses. This is because there is
a class of IPv4-mapped-to-IPv6 addresses which are represented in the
same way as vanilla IPv4 addresses when using net.IP structures.

In order to make our logic smarter, we now analyse subnet mask to decide
if an address is IPv6 or IPv4. This is because we may have an address
`::ffff:192.168.0.160/64` which contains both `:` and `.` making the
regular logic unreliable. Knowing that the biggest mask for IPv4 is /32
we may detect if such an address is an IPv4-mapped-to-IPv6 address or
vanilla IPv4.

To solve the case of IPv4-mapped-to-IPv6 address in a huge v6 subnet
(e.g. `::ffff:192.168.0.160/16`) we are also checking for the capacity
of the struct storing subnet mask.

Contributes-to: OCPBUGS-10695
  • Loading branch information
mkowalski committed Mar 27, 2023
1 parent 1a0ba6c commit 54bd167
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 2 deletions.
6 changes: 5 additions & 1 deletion pkg/utils/addresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
for _, address := range addresses {
isVip := false
for _, vip := range vips {
if IsNetIPv6(*address.IPNet) != IsIPv6(vip) {
log.Debugf("Stack of address '%s' and VIP '%s' does not match. Skipping.", address, vip)
continue
}
if address.IP.String() == vip.String() {
log.Debugf("Address %s is VIP %s. Skipping.", address, vip)
isVip = true
Expand Down Expand Up @@ -191,7 +195,7 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
if len(matches) > 0 {
// Find an address of the opposite IP family on the same interface
for _, address := range addresses {
if IsIPv6(address.IP) != IsIPv6(matches[0]) {
if IsNetIPv6(*address.IPNet) != IsIPv6(matches[0]) {
matches = append(matches, address.IP)
break
}
Expand Down
1 change: 1 addition & 0 deletions pkg/utils/addresses_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func addIPv4RoutesDefaultEth1(routes map[int][]netlink.Route, rf RouteFilter) {
func addIPv6Addrs(addrs map[netlink.Link][]netlink.Addr, af AddressFilter) {
maybeAddAddress(addrs, af, lo, "127.0.0.1/8", false)
maybeAddAddress(addrs, af, lo, "::1/128", false)
maybeAddAddress(addrs, af, eth1, "::ffff:192.168.1.160/64", false)
maybeAddAddress(addrs, af, eth0, "fd00::5/64", false)
maybeAddAddress(addrs, af, eth0, "fe80::1234/64", false)
maybeAddAddress(addrs, af, eth1, "fd69::2/125", false)
Expand Down
27 changes: 26 additions & 1 deletion pkg/utils/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,39 @@ func SplitCIDR(s string) (string, string, error) {
}

func IsIPv4(ip net.IP) bool {
// func IsIPv4(ip string) bool {
return strings.Contains(ip.String(), ".")
}

func IsIPv6(ip net.IP) bool {
return strings.Contains(ip.String(), ":")
}

func IsNetIPv6(network net.IPNet) bool {
if IsIPv6(network.IP) {
return true
}

if IsIPv4(network.IP) {
// Sadly the fact that address has "." doesn't mean it's an IPv4 address. We need to check
// subnet masks to make sure this is really the case. If subnet mask is longer than 32 we
// are for sure dealing with IPv6 address. If it is longer, then we need to look at the
// total capacity of the mask as it can be simply an IPv4 address but could be as well an
// IPv6 in really enormous subnet.
m, n := network.Mask.Size()
if m > 8*net.IPv4len {
return true
} else {
if n > 8*net.IPv4len {
return true
}
}
return false
}

log.Debugf("Failed to find IP stack of '%+v'", &network)
return false
}

func IpInCidr(ipAddr, cidr string) (bool, error) {
_, ipNet, err := net.ParseCIDR(cidr)
if err != nil {
Expand Down
71 changes: 71 additions & 0 deletions pkg/utils/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,77 @@ var _ = Describe("IP stack detection", func() {
res := IsIPv6(net.ParseIP("this-is-not-an-ip"))
Expect(res).To(Equal(false))
})

})

Context("for IPv4-mapped-to-IPv6 addresses", func() {
// (mko) This test suite is intentionally returning wrong results. This is because the
// mapped form of those addresses is making them impossible to distinguish without
// knowing more properties of the link.
//
// Due to the fact how net.IPv4() initialization is implemented and the fact that it
// always adds the v4InV6Prefix header in a binary form, those addresses look like IPv4
// and in most cases behave like such.
//
// The tests here are saying that "::ffff:192.168.0.14" is IPv4 and not an IPv6 address
// whenever fed into our own IsIPv4 and IsIPv6 functions. For a proper answer returning
// IPv6 for those, we have IsNetIPv6 function which looks at IP address as well as subnet
// mask.

It("IsIPv4 returns true for IPv4-mapped-to-IPv6 address", func() {
res := IsIPv4(net.ParseIP("::ffff:192.168.0.14"))
Expect(res).To(Equal(true))
})
It("IsIPv4 returns true for binary IPv4-mapped-to-IPv6 address", func() {
ip := net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160})
res := IsIPv4(ip)
Expect(res).To(Equal(true))
})

It("IsIPv6 returns false for IPv4-mapped-to-IPv6 address", func() {
res := IsIPv6(net.ParseIP("::ffff:192.168.0.14"))
Expect(res).To(Equal(false))
})
It("IsIPv6 returns true for binary IPv4-mapped-to-IPv6 address", func() {
ip := net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160})
res := IsIPv6(ip)
Expect(res).To(Equal(false))
})
})

Context("using IsNetIPv6", func() {
It("returns false for IPv4 address", func() {
_, addr, _ := net.ParseCIDR("192.168.0.1/24")
res := IsNetIPv6(*addr)
Expect(res).To(Equal(false))
})
It("returns true for IPv6 address", func() {
_, addr, _ := net.ParseCIDR("fe80::84ca:4a24:ffbf:1778/64")
res := IsNetIPv6(*addr)
Expect(res).To(Equal(true))
})
It("returns true for IPv4-mapped-to-IPv6 address", func() {
// We must not use net.ParseCIDR function here because it cannot handle mapped IP
// addresses. For details look at https://github.com/golang/go/issues/51906 but TLDR
// is that feeding net.ParseCIDR with IPv4-mapped-to-IPv6 address outputs only "::".
addr := net.IPNet{
IP: net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160}),
Mask: net.IPMask([]byte{255, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0}),
}
res := IsNetIPv6(addr)
Expect(res).To(Equal(true))
})
It("returns true for IPv4-mapped-to-IPv6 address in huge subnet", func() {
// We must not use net.ParseCIDR function here because it cannot handle mapped IP
// addresses. For details look at https://github.com/golang/go/issues/51906 but TLDR
// is that feeding net.ParseCIDR with IPv4-mapped-to-IPv6 address outputs only "::".
addr := net.IPNet{
IP: net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160}),
Mask: net.IPMask([]byte{255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}),
}
res := IsNetIPv6(addr)
Expect(res).To(Equal(true))
})
})
})

Expand Down

0 comments on commit 54bd167

Please sign in to comment.