diff --git a/pkg/utils/addresses.go b/pkg/utils/addresses.go index b5c35395..efe18495 100644 --- a/pkg/utils/addresses.go +++ b/pkg/utils/addresses.go @@ -140,7 +140,7 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM } var routeMap map[int][]netlink.Route - matches := make([]net.IP, 0) + matches := make([]net.IPNet, 0) for link, addresses := range addrMap { addrLoop: for _, address := range addresses { @@ -168,9 +168,12 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM containmentNet := net.IPNet{IP: address.IP, Mask: route.Dst.Mask} for _, vip := range vips { log.Debugf("Checking whether address %s with route %s contains VIP %s", address, route, vip) + if IsNetIPv6(containmentNet) != IsIPv6(vip) { + log.Debugf("Stack of address '%s' and VIP '%s' does not match. Skipping.", address, vip) + } if containmentNet.Contains(vip) { log.Debugf("Address %s with route %s contains VIP %s", address, route, vip) - matches = append(matches, address.IP) + matches = append(matches, *address.IPNet) break addrLoop } } @@ -179,9 +182,12 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM } else { for _, vip := range vips { log.Debugf("Checking whether address %s contains VIP %s", address, vip) + if IsNetIPv6(*address.IPNet) != IsIPv6(vip) { + log.Debugf("Stack of address '%s' and VIP '%s' does not match. Skipping.", address, vip) + } if address.Contains(vip) { log.Debugf("Address %s contains VIP %s", address, vip) - matches = append(matches, address.IP) + matches = append(matches, *address.IPNet) break addrLoop } } @@ -191,8 +197,8 @@ 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]) { - matches = append(matches, address.IP) + if IsNetIPv6(*address.IPNet) != IsNetIPv6(matches[0]) { + matches = append(matches, *address.IPNet) break } } @@ -202,9 +208,9 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM // Ensure we return addresses in the desired order sort.SliceStable(matches, func(i, j int) bool { // Very simplified since we only ever have two items in this list - return IsIPv6(matches[i]) == preferIPv6 + return IsNetIPv6(matches[i]) == preferIPv6 }) - return matches, nil + return Mapper(matches, func(ipnet net.IPNet) net.IP { return ipnet.IP }), nil } // defaultRoute returns true if the passed route is a default route @@ -218,7 +224,7 @@ func AddressesDefault(preferIPv6 bool, af AddressFilter) ([]net.IP, error) { } type FoundAddress struct { - Address net.IP + Network net.IPNet Priority int LinkIndex int GatewayOnSubnet bool @@ -245,7 +251,7 @@ func addressesDefaultInternal(preferIPv6 bool, af AddressFilter, getAddrs addres log.Debugf("Address %s is on interface %s with default route", address, link.Attrs().Name) // We should only have one default route per interface addrs = append(addrs, FoundAddress{ - Address: address.IP, + Network: *address.IPNet, Priority: routeMap[linkIndex][0].Priority, LinkIndex: linkIndex, GatewayOnSubnet: address.IPNet.Contains(routeMap[linkIndex][0].Gw), @@ -262,13 +268,13 @@ func addressesDefaultInternal(preferIPv6 bool, af AddressFilter, getAddrs addres sort.SliceStable(addrs, func(i, j int) bool { if addrs[i].Priority == addrs[j].Priority { if addrs[i].LinkIndex == addrs[j].LinkIndex { - if IsIPv6(addrs[i].Address) == IsIPv6(addrs[j].Address) { + if IsNetIPv6(addrs[i].Network) == IsNetIPv6(addrs[j].Network) { if addrs[i].GatewayOnSubnet == addrs[j].GatewayOnSubnet { - return !addrs[i].Address.IsPrivate() + return !addrs[i].Network.IP.IsPrivate() } return addrs[i].GatewayOnSubnet } - return IsIPv6(addrs[i].Address) == preferIPv6 + return IsNetIPv6(addrs[i].Network) == preferIPv6 } return addrs[i].LinkIndex < addrs[j].LinkIndex } @@ -278,11 +284,11 @@ func addressesDefaultInternal(preferIPv6 bool, af AddressFilter, getAddrs addres foundv4 := false foundv6 := false for _, addr := range addrs { - if (IsIPv6(addr.Address) && foundv6) || (!IsIPv6(addr.Address) && foundv4) { + if (IsNetIPv6(addr.Network) && foundv6) || (!IsNetIPv6(addr.Network) && foundv4) { continue } - matches = append(matches, addr.Address) - if IsIPv6(addr.Address) { + matches = append(matches, addr.Network.IP) + if IsNetIPv6(addr.Network) { foundv6 = true } else { foundv4 = true diff --git a/pkg/utils/addresses_test.go b/pkg/utils/addresses_test.go index b664a314..6cf8cbec 100644 --- a/pkg/utils/addresses_test.go +++ b/pkg/utils/addresses_test.go @@ -98,6 +98,7 @@ func addIPv6Addrs(addrs map[netlink.Link][]netlink.Addr, af AddressFilter) { maybeAddAddress(addrs, af, lo, "::1/128", false) maybeAddAddress(addrs, af, eth0, "fd00::5/64", false) maybeAddAddress(addrs, af, eth0, "fe80::1234/64", false) + maybeAddAddress(addrs, af, eth0, "::ffff:192.168.1.160/64", false) maybeAddAddress(addrs, af, eth1, "fd69::2/125", false) maybeAddAddress(addrs, af, eth1, "fd01::3/64", true) maybeAddAddress(addrs, af, eth1, "fd01::4/64", true) diff --git a/pkg/utils/network.go b/pkg/utils/network.go index e6babf3f..652df0f3 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,33 @@ 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 shorter, 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. + // + // To simplify this logic because subnet longer than 32 can happen if the capacity of mask + // matches IPv6, we can simply look at the capacity of the mask and determine IP stack. + _, n := network.Mask.Size() + if n == 32 { + return false + } + if n == 128 { + return true + } + } + + 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..fbc01dd2 100644 --- a/pkg/utils/network_test.go +++ b/pkg/utils/network_test.go @@ -43,6 +43,93 @@ 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 addresses", func() { + addrs := []string{ + "192.168.0.1/24", + "192.168.0.1/32", + "192.168.0.1/0", + "0.0.0.0/0", + } + for _, addr := range addrs { + _, addr, _ := net.ParseCIDR(addr) + res := IsNetIPv6(*addr) + Expect(res).To(Equal(false)) + } + }) + It("returns true for IPv6 address", func() { + addrs := []string{ + "fe80::84ca:4a24:ffbf:1778/64", + "::/0", + "::1/128", + "::ffff:192.168.0.14/64", + "::ffff:192.168.0.14/16", + } + for _, addr := range addrs { + _, addr, _ := net.ParseCIDR(addr) + res := IsNetIPv6(*addr) + Expect(res).To(Equal(true)) + } + }) + It("returns true for IPv4-mapped-to-IPv6 address", func() { + // Because of non-obvious behaviour of net library when it comes to + // IPv4-mapped-to-IPv6 addresses (more in https://github.com/golang/go/issues/51906) + // we are adding tests where we explicitly create net.IPNet struct from a binary + // input. + addrs := []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}), + }, + { + 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}), + }, + } + + for _, addr := range addrs { + res := IsNetIPv6(addr) + Expect(res).To(Equal(true)) + } + }) }) }) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index f6f305c6..9b0d37f4 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -7,13 +7,14 @@ import ( "fmt" "io" "io/ioutil" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" "net/http" "os" "strings" "time" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "github.com/sirupsen/logrus" ) @@ -135,3 +136,11 @@ func GetClientConfig(kubeApiServerUrl, kubeconfigPath string) (*rest.Config, err config.Timeout = kubeClientTimeout return config, err } + +func Mapper[T, U any](data []T, f func(T) U) []U { + res := make([]U, 0, len(data)) + for _, e := range data { + res = append(res, f(e)) + } + return res +}