From 54bd167699e2a001b04a0b672171529aef4a5a5d Mon Sep 17 00:00:00 2001 From: Mat Kowalski Date: Mon, 27 Mar 2023 12:35:36 +0200 Subject: [PATCH] OCPBUGS-10695: Use subnet mask to detect IP stack 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 --- pkg/utils/addresses.go | 6 +++- pkg/utils/addresses_test.go | 1 + pkg/utils/network.go | 27 +++++++++++++- pkg/utils/network_test.go | 71 +++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/pkg/utils/addresses.go b/pkg/utils/addresses.go index b5c35395..50f17717 100644 --- a/pkg/utils/addresses.go +++ b/pkg/utils/addresses.go @@ -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 @@ -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 } diff --git a/pkg/utils/addresses_test.go b/pkg/utils/addresses_test.go index b664a314..39c547f5 100644 --- a/pkg/utils/addresses_test.go +++ b/pkg/utils/addresses_test.go @@ -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) diff --git a/pkg/utils/network.go b/pkg/utils/network.go index e6babf3f..dc774c49 100644 --- a/pkg/utils/network.go +++ b/pkg/utils/network.go @@ -17,7 +17,6 @@ func SplitCIDR(s string) (string, string, error) { } func IsIPv4(ip net.IP) bool { - // func IsIPv4(ip string) bool { return strings.Contains(ip.String(), ".") } @@ -25,6 +24,32 @@ 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 { diff --git a/pkg/utils/network_test.go b/pkg/utils/network_test.go index b2a34e12..be0e2561 100644 --- a/pkg/utils/network_test.go +++ b/pkg/utils/network_test.go @@ -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)) + }) }) })