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>
This commit is contained in:
Antonio Ojea 2023-06-17 10:39:23 +00:00
parent c984d53b31
commit e4f93d8a82
2 changed files with 55 additions and 4 deletions

View File

@ -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
}

View File

@ -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 {