From e4f93d8a82d3d784b456bc6d4955f73a93b4c050 Mon Sep 17 00:00:00 2001
From: Antonio Ojea <aojea@google.com>
Date: Sat, 17 Jun 2023 10:39:23 +0000
Subject: [PATCH] ipallocator bug if ips has leading zeros

The ipallocator for the new IPAddress object use the golang big.Int
library for some math operations, like adding an offset to an IP
address.

We use the bytes array to convert between big.Int and IP addresses,
however, IP addresses are always represented as 4 or 16 bytes arrays.
Big int bytes representations just return the byte array until the
most representative number, this requires that we need to prepend
these extra bytes for IPs with leading zeros.

Change-Id: I9d539f582cae1f9f4e373b28c5b94d7a342f09c7
Signed-off-by: Antonio Ojea <aojea@google.com>
---
 .../core/service/ipallocator/ipallocator.go   | 18 ++++++--
 .../service/ipallocator/ipallocator_test.go   | 41 +++++++++++++++++++
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/pkg/registry/core/service/ipallocator/ipallocator.go b/pkg/registry/core/service/ipallocator/ipallocator.go
index 8618d541aff..d407ddf5fda 100644
--- a/pkg/registry/core/service/ipallocator/ipallocator.go
+++ b/pkg/registry/core/service/ipallocator/ipallocator.go
@@ -484,11 +484,21 @@ func (dry dryRunAllocator) EnableMetrics() {
 // TODO: move it to k8s.io/utils/net, this is the same as current AddIPOffset()
 // but using netip.Addr instead of net.IP
 func addOffsetAddress(address netip.Addr, offset uint64) (netip.Addr, error) {
-	addressBig := big.NewInt(0).SetBytes(address.AsSlice())
-	r := big.NewInt(0).Add(addressBig, big.NewInt(int64(offset)))
-	addr, ok := netip.AddrFromSlice(r.Bytes())
+	addressBytes := address.AsSlice()
+	addressBig := big.NewInt(0).SetBytes(addressBytes)
+	r := big.NewInt(0).Add(addressBig, big.NewInt(int64(offset))).Bytes()
+	// r must be 4 or 16 bytes depending of the ip family
+	// bigInt conversion to bytes will not take this into consideration
+	// and drop the leading zeros, so we have to take this into account.
+	lenDiff := len(addressBytes) - len(r)
+	if lenDiff > 0 {
+		r = append(make([]byte, lenDiff), r...)
+	} else if lenDiff < 0 {
+		return netip.Addr{}, fmt.Errorf("invalid address %v", r)
+	}
+	addr, ok := netip.AddrFromSlice(r)
 	if !ok {
-		return netip.Addr{}, fmt.Errorf("invalid address %v", r.Bytes())
+		return netip.Addr{}, fmt.Errorf("invalid address %v", r)
 	}
 	return addr, nil
 }
diff --git a/pkg/registry/core/service/ipallocator/ipallocator_test.go b/pkg/registry/core/service/ipallocator/ipallocator_test.go
index 5e80115278a..6c5c73cde76 100644
--- a/pkg/registry/core/service/ipallocator/ipallocator_test.go
+++ b/pkg/registry/core/service/ipallocator/ipallocator_test.go
@@ -587,6 +587,18 @@ func Test_addOffsetAddress(t *testing.T) {
 			offset:  128,
 			want:    netip.MustParseAddr("192.168.0.128"),
 		},
+		{
+			name:    "IPv4 with leading zeros",
+			address: netip.MustParseAddr("0.0.1.8"),
+			offset:  138,
+			want:    netip.MustParseAddr("0.0.1.146"),
+		},
+		{
+			name:    "IPv6 with leading zeros",
+			address: netip.MustParseAddr("00fc::1"),
+			offset:  255,
+			want:    netip.MustParseAddr("fc::100"),
+		},
 		{
 			name:    "IPv6 offset 255",
 			address: netip.MustParseAddr("2001:db8:1::101"),
@@ -643,6 +655,16 @@ func Test_broadcastAddress(t *testing.T) {
 			subnet: netip.MustParsePrefix("fd00:1:2:3::/64"),
 			want:   netip.MustParseAddr("fd00:1:2:3:FFFF:FFFF:FFFF:FFFF"),
 		},
+		{
+			name:   "ipv6 00fc::/112",
+			subnet: netip.MustParsePrefix("00fc::/112"),
+			want:   netip.MustParseAddr("fc::ffff"),
+		},
+		{
+			name:   "ipv6 fc00::/112",
+			subnet: netip.MustParsePrefix("fc00::/112"),
+			want:   netip.MustParseAddr("fc00::ffff"),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -888,6 +910,25 @@ func Test_ipIterator_Number(t *testing.T) {
 	}
 }
 
+func TestAllocateNextFC(t *testing.T) {
+	_, cidr, err := netutils.ParseCIDRSloppy("fc::/112")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Logf("CIDR %s", cidr)
+
+	r, err := newTestAllocator(cidr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer r.Destroy()
+	ip, err := r.AllocateNext()
+	if err != nil {
+		t.Fatalf("wrong ip %s : %v", ip, err)
+	}
+	t.Log(ip.String())
+}
+
 func BenchmarkIPAllocatorAllocateNextIPv4Size1048574(b *testing.B) {
 	_, cidr, err := netutils.ParseCIDRSloppy("10.0.0.0/12")
 	if err != nil {