Return error from EncodePrefix instead of silently clamping bits

This commit is contained in:
Viktor Liu
2026-04-10 06:51:55 +02:00
parent 456298864c
commit 6c5ff88569
6 changed files with 72 additions and 41 deletions

View File

@@ -1262,7 +1262,9 @@ func anonymizePeerConfig(config *mgmProto.PeerConfig, anonymizer *anonymize.Anon
if v6Prefix, err := netiputil.DecodePrefix(config.GetAddressV6()); err == nil { if v6Prefix, err := netiputil.DecodePrefix(config.GetAddressV6()); err == nil {
anonV6 := anonymizer.AnonymizeIP(v6Prefix.Addr()) anonV6 := anonymizer.AnonymizeIP(v6Prefix.Addr())
config.AddressV6 = netiputil.EncodePrefix(netip.PrefixFrom(anonV6, v6Prefix.Bits())) if b, err := netiputil.EncodePrefix(netip.PrefixFrom(anonV6, v6Prefix.Bits())); err == nil {
config.AddressV6 = b
}
} }
anonymizeSSHConfig(config.SshConfig) anonymizeSSHConfig(config.SshConfig)

View File

@@ -20,6 +20,13 @@ import (
"github.com/netbirdio/netbird/shared/netiputil" "github.com/netbirdio/netbird/shared/netiputil"
) )
func mustEncodePrefix(t *testing.T, p netip.Prefix) []byte {
t.Helper()
b, err := netiputil.EncodePrefix(p)
require.NoError(t, err)
return b
}
func TestAnonymizeStateFile(t *testing.T) { func TestAnonymizeStateFile(t *testing.T) {
testState := map[string]json.RawMessage{ testState := map[string]json.RawMessage{
"null_state": json.RawMessage("null"), "null_state": json.RawMessage("null"),
@@ -278,7 +285,7 @@ func TestAnonymizeNetworkMap(t *testing.T) {
networkMap := &mgmProto.NetworkMap{ networkMap := &mgmProto.NetworkMap{
PeerConfig: &mgmProto.PeerConfig{ PeerConfig: &mgmProto.PeerConfig{
Address: "203.0.113.5", Address: "203.0.113.5",
AddressV6: netiputil.EncodePrefix(origV6Prefix), AddressV6: mustEncodePrefix(t, origV6Prefix),
Dns: "1.2.3.4", Dns: "1.2.3.4",
Fqdn: "peer1.corp.example.com", Fqdn: "peer1.corp.example.com",
SshConfig: &mgmProto.SSHConfig{ SshConfig: &mgmProto.SSHConfig{

View File

@@ -1701,6 +1701,13 @@ func getPeers(e *Engine) int {
return len(e.peerStore.PeersPubKey()) return len(e.peerStore.PeersPubKey())
} }
func mustEncodePrefix(t *testing.T, p netip.Prefix) []byte {
t.Helper()
b, err := netiputil.EncodePrefix(p)
require.NoError(t, err)
return b
}
func TestEngine_hasIPv6Changed(t *testing.T) { func TestEngine_hasIPv6Changed(t *testing.T) {
v4Only := wgaddr.MustParseWGAddress("100.64.0.1/16") v4Only := wgaddr.MustParseWGAddress("100.64.0.1/16")
@@ -1723,7 +1730,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
{ {
name: "no v6 before, v6 added", name: "no v6 before, v6 added",
current: v4Only, current: v4Only,
confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")), confV6: mustEncodePrefix(t, netip.MustParsePrefix("fd00::1/64")),
expected: true, expected: true,
}, },
{ {
@@ -1735,19 +1742,19 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
{ {
name: "had v6, same v6", name: "had v6, same v6",
current: v4v6, current: v4v6,
confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")), confV6: mustEncodePrefix(t, netip.MustParsePrefix("fd00::1/64")),
expected: false, expected: false,
}, },
{ {
name: "had v6, different v6", name: "had v6, different v6",
current: v4v6, current: v4v6,
confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::2/64")), confV6: mustEncodePrefix(t, netip.MustParsePrefix("fd00::2/64")),
expected: true, expected: true,
}, },
{ {
name: "same v6 addr, different prefix length", name: "same v6 addr, different prefix length",
current: v4v6, current: v4v6,
confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/80")), confV6: mustEncodePrefix(t, netip.MustParsePrefix("fd00::1/80")),
expected: true, expected: true,
}, },
{ {

View File

@@ -119,7 +119,9 @@ func toPeerConfig(peer *nbpeer.Peer, network *types.Network, dnsName string, set
if peer.SupportsIPv6() && peer.IPv6.IsValid() && network.NetV6.IP != nil { if peer.SupportsIPv6() && peer.IPv6.IsValid() && network.NetV6.IP != nil {
ones, _ := network.NetV6.Mask.Size() ones, _ := network.NetV6.Mask.Size()
v6Prefix := netip.PrefixFrom(peer.IPv6.Unmap(), ones) v6Prefix := netip.PrefixFrom(peer.IPv6.Unmap(), ones)
peerConfig.AddressV6 = netiputil.EncodePrefix(v6Prefix) if b, err := netiputil.EncodePrefix(v6Prefix); err == nil {
peerConfig.AddressV6 = b
}
} }
return peerConfig return peerConfig
@@ -344,9 +346,9 @@ func populateSourcePrefixes(fwRule *proto.FirewallRule, rule *types.FirewallRule
return nil return nil
} }
fwRule.SourcePrefixes = [][]byte{ // IPv4Unspecified/0 is always valid, error is impossible.
netiputil.EncodePrefix(netip.PrefixFrom(netip.IPv4Unspecified(), 0)), v4Wildcard, _ := netiputil.EncodePrefix(netip.PrefixFrom(netip.IPv4Unspecified(), 0))
} fwRule.SourcePrefixes = [][]byte{v4Wildcard}
if !includeIPv6 { if !includeIPv6 {
return nil return nil
@@ -354,9 +356,9 @@ func populateSourcePrefixes(fwRule *proto.FirewallRule, rule *types.FirewallRule
v6Rule := goproto.Clone(fwRule).(*proto.FirewallRule) v6Rule := goproto.Clone(fwRule).(*proto.FirewallRule)
v6Rule.PeerIP = "::" //nolint:staticcheck // populated for backward compatibility v6Rule.PeerIP = "::" //nolint:staticcheck // populated for backward compatibility
v6Rule.SourcePrefixes = [][]byte{ // IPv6Unspecified/0 is always valid, error is impossible.
netiputil.EncodePrefix(netip.PrefixFrom(netip.IPv6Unspecified(), 0)), v6Wildcard, _ := netiputil.EncodePrefix(netip.PrefixFrom(netip.IPv6Unspecified(), 0))
} v6Rule.SourcePrefixes = [][]byte{v6Wildcard}
if shouldUsePortRange(v6Rule) { if shouldUsePortRange(v6Rule) {
v6Rule.PortInfo = rule.PortRange.ToProto() v6Rule.PortInfo = rule.PortRange.ToProto()
} }

View File

@@ -14,15 +14,14 @@ import (
) )
// EncodePrefix encodes a netip.Prefix into compact bytes. // EncodePrefix encodes a netip.Prefix into compact bytes.
// The address is always unmapped before encoding. If unmapping produces a v4 // The address is always unmapped before encoding.
// address, the prefix length is clamped to 32. func EncodePrefix(p netip.Prefix) ([]byte, error) {
func EncodePrefix(p netip.Prefix) []byte {
addr := p.Addr().Unmap() addr := p.Addr().Unmap()
bits := p.Bits() bits := p.Bits()
if addr.Is4() && bits > 32 { if addr.Is4() && bits > 32 {
bits = 32 return nil, fmt.Errorf("invalid prefix length %d for IPv4 address %s (max 32)", bits, addr)
} }
return append(addr.AsSlice(), byte(bits)) return append(addr.AsSlice(), byte(bits)), nil
} }
// DecodePrefix decodes compact bytes into a netip.Prefix. // DecodePrefix decodes compact bytes into a netip.Prefix.
@@ -43,7 +42,7 @@ func DecodePrefix(b []byte) (netip.Prefix, error) {
bits := int(b[len(b)-1]) bits := int(b[len(b)-1])
if addr.Is4() { if addr.Is4() {
if bits > 32 { if bits > 32 {
bits = 32 return netip.Prefix{}, fmt.Errorf("invalid prefix length %d for v4-mapped address (max 32)", bits)
} }
} else if bits > 128 { } else if bits > 128 {
return netip.Prefix{}, fmt.Errorf("invalid IPv6 prefix length %d (max 128)", bits) return netip.Prefix{}, fmt.Errorf("invalid IPv6 prefix length %d (max 128)", bits)
@@ -62,7 +61,9 @@ func EncodeAddr(a netip.Addr) []byte {
if a.Is4() { if a.Is4() {
bits = 32 bits = 32
} }
return EncodePrefix(netip.PrefixFrom(a, bits)) // Host prefix lengths are always valid for the address family, so error is impossible.
b, _ := EncodePrefix(netip.PrefixFrom(a, bits))
return b
} }
// DecodeAddr decodes compact prefix bytes and returns only the address, // DecodeAddr decodes compact prefix bytes and returns only the address,

View File

@@ -59,7 +59,8 @@ func TestEncodeDecodePrefix(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
p := netip.MustParsePrefix(tt.prefix) p := netip.MustParsePrefix(tt.prefix)
b := EncodePrefix(p) b, err := EncodePrefix(p)
require.NoError(t, err)
assert.Equal(t, tt.size, len(b), "encoded size") assert.Equal(t, tt.size, len(b), "encoded size")
decoded, err := DecodePrefix(b) decoded, err := DecodePrefix(b)
@@ -72,7 +73,8 @@ func TestEncodeDecodePrefix(t *testing.T) {
func TestEncodePrefixUnmaps(t *testing.T) { func TestEncodePrefixUnmaps(t *testing.T) {
// v4-mapped v6 address should encode as v4 // v4-mapped v6 address should encode as v4
mapped := netip.MustParsePrefix("::ffff:10.1.2.3/32") mapped := netip.MustParsePrefix("::ffff:10.1.2.3/32")
b := EncodePrefix(mapped) b, err := EncodePrefix(mapped)
require.NoError(t, err)
assert.Equal(t, 5, len(b), "v4-mapped should encode as 5 bytes") assert.Equal(t, 5, len(b), "v4-mapped should encode as 5 bytes")
decoded, err := DecodePrefix(b) decoded, err := DecodePrefix(b)
@@ -80,24 +82,26 @@ func TestEncodePrefixUnmaps(t *testing.T) {
assert.Equal(t, netip.MustParsePrefix("10.1.2.3/32"), decoded) assert.Equal(t, netip.MustParsePrefix("10.1.2.3/32"), decoded)
} }
func TestEncodePrefixUnmapsClampsBits(t *testing.T) { func TestEncodePrefixUnmapsRejectsInvalidBits(t *testing.T) {
// v4-mapped v6 with bits > 32 should clamp to /32 // v4-mapped v6 with bits > 32 should return an error
mapped := netip.MustParsePrefix("::ffff:10.1.2.3/128") mapped128 := netip.MustParsePrefix("::ffff:10.1.2.3/128")
b := EncodePrefix(mapped) _, err := EncodePrefix(mapped128)
require.Error(t, err)
// v4-mapped v6 with bits=96 should also return an error
mapped96 := netip.MustParsePrefix("::ffff:10.0.0.0/96")
_, err = EncodePrefix(mapped96)
require.Error(t, err)
// v4-mapped v6 with bits=32 should succeed
mapped32 := netip.MustParsePrefix("::ffff:10.1.2.3/32")
b, err := EncodePrefix(mapped32)
require.NoError(t, err)
assert.Equal(t, 5, len(b), "v4-mapped should encode as 5 bytes") assert.Equal(t, 5, len(b), "v4-mapped should encode as 5 bytes")
decoded, err := DecodePrefix(b) decoded, err := DecodePrefix(b)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, netip.MustParsePrefix("10.1.2.3/32"), decoded) assert.Equal(t, netip.MustParsePrefix("10.1.2.3/32"), decoded)
// v4-mapped v6 with bits=96 should also clamp to /32
mapped96 := netip.MustParsePrefix("::ffff:10.0.0.0/96")
b96 := EncodePrefix(mapped96)
assert.Equal(t, 5, len(b96))
decoded96, err := DecodePrefix(b96)
require.NoError(t, err)
assert.Equal(t, 32, decoded96.Bits())
} }
func TestDecodeAddr(t *testing.T) { func TestDecodeAddr(t *testing.T) {
@@ -147,16 +151,24 @@ func TestDecodePrefixInvalidBits(t *testing.T) {
} }
func TestDecodePrefixUnmapsV6Input(t *testing.T) { func TestDecodePrefixUnmapsV6Input(t *testing.T) {
// If someone encodes a v4-mapped v6 as 17 bytes, decode should unmap it
// and clamp the prefix length to 32 for v4
addr := netip.MustParseAddr("::ffff:192.168.1.1") addr := netip.MustParseAddr("::ffff:192.168.1.1")
// v4-mapped v6 with bits > 32 should return an error
raw := addr.As16() raw := addr.As16()
b := make([]byte, 17) bInvalid := make([]byte, 17)
copy(b, raw[:]) copy(bInvalid, raw[:])
b[16] = 128 bInvalid[16] = 128
decoded, err := DecodePrefix(b) _, err := DecodePrefix(bInvalid)
require.Error(t, err, "v4-mapped address with /128 prefix should be rejected")
assert.Contains(t, err.Error(), "invalid prefix length")
// v4-mapped v6 with valid /32 should decode and unmap correctly
bValid := make([]byte, 17)
copy(bValid, raw[:])
bValid[16] = 32
decoded, err := DecodePrefix(bValid)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, decoded.Addr().Is4(), "should be unmapped to v4") assert.True(t, decoded.Addr().Is4(), "should be unmapped to v4")
assert.Equal(t, netip.MustParsePrefix("192.168.1.1/32"), decoded) assert.Equal(t, netip.MustParsePrefix("192.168.1.1/32"), decoded)