mirror of
https://github.com/netbirdio/netbird.git
synced 2026-04-24 11:16:38 +00:00
Fix CodeRabbit review issues from IPv6 overlay PR (#5839)
This commit is contained in:
@@ -50,7 +50,7 @@ func (a *Anonymizer) AnonymizeIP(ip netip.Addr) netip.Addr {
|
|||||||
ip.IsLinkLocalUnicast() ||
|
ip.IsLinkLocalUnicast() ||
|
||||||
ip.IsLinkLocalMulticast() ||
|
ip.IsLinkLocalMulticast() ||
|
||||||
ip.IsInterfaceLocalMulticast() ||
|
ip.IsInterfaceLocalMulticast() ||
|
||||||
ip.IsPrivate() ||
|
(ip.Is4() && ip.IsPrivate()) ||
|
||||||
ip.IsUnspecified() ||
|
ip.IsUnspecified() ||
|
||||||
ip.IsMulticast() ||
|
ip.IsMulticast() ||
|
||||||
isWellKnown(ip) ||
|
isWellKnown(ip) ||
|
||||||
|
|||||||
@@ -93,11 +93,13 @@ func (t *TunDevice) Create() (WGConfigurer, error) {
|
|||||||
if t.address.HasIPv6() {
|
if t.address.HasIPv6() {
|
||||||
nbiface6, err := luid.IPInterface(windows.AF_INET6)
|
nbiface6, err := luid.IPInterface(windows.AF_INET6)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Warnf("failed to get IPv6 interface for MTU: %v", err)
|
log.Warnf("failed to get IPv6 interface for MTU, continuing v4-only: %v", err)
|
||||||
|
t.address.ClearIPv6()
|
||||||
} else {
|
} else {
|
||||||
nbiface6.NLMTU = uint32(t.mtu)
|
nbiface6.NLMTU = uint32(t.mtu)
|
||||||
if err := nbiface6.Set(); err != nil {
|
if err := nbiface6.Set(); err != nil {
|
||||||
log.Warnf("failed to set IPv6 interface MTU: %v", err)
|
log.Warnf("failed to set IPv6 interface MTU, continuing v4-only: %v", err)
|
||||||
|
t.address.ClearIPv6()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ import (
|
|||||||
"github.com/netbirdio/netbird/client/internal/updater/installer"
|
"github.com/netbirdio/netbird/client/internal/updater/installer"
|
||||||
nbstatus "github.com/netbirdio/netbird/client/status"
|
nbstatus "github.com/netbirdio/netbird/client/status"
|
||||||
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
|
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
|
||||||
|
"github.com/netbirdio/netbird/shared/netiputil"
|
||||||
"github.com/netbirdio/netbird/util"
|
"github.com/netbirdio/netbird/util"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -1259,6 +1260,11 @@ func anonymizePeerConfig(config *mgmProto.PeerConfig, anonymizer *anonymize.Anon
|
|||||||
config.Address = anonymizer.AnonymizeIP(addr).String()
|
config.Address = anonymizer.AnonymizeIP(addr).String()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if v6Prefix, err := netiputil.DecodePrefix(config.GetAddressV6()); err == nil {
|
||||||
|
anonV6 := anonymizer.AnonymizeIP(v6Prefix.Addr())
|
||||||
|
config.AddressV6 = netiputil.EncodePrefix(netip.PrefixFrom(anonV6, v6Prefix.Bits()))
|
||||||
|
}
|
||||||
|
|
||||||
anonymizeSSHConfig(config.SshConfig)
|
anonymizeSSHConfig(config.SshConfig)
|
||||||
|
|
||||||
config.Dns = anonymizer.AnonymizeString(config.Dns)
|
config.Dns = anonymizer.AnonymizeString(config.Dns)
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"net"
|
"net"
|
||||||
|
"net/netip"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -16,6 +17,7 @@ import (
|
|||||||
"github.com/netbirdio/netbird/client/anonymize"
|
"github.com/netbirdio/netbird/client/anonymize"
|
||||||
"github.com/netbirdio/netbird/client/configs"
|
"github.com/netbirdio/netbird/client/configs"
|
||||||
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
|
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
|
||||||
|
"github.com/netbirdio/netbird/shared/netiputil"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestAnonymizeStateFile(t *testing.T) {
|
func TestAnonymizeStateFile(t *testing.T) {
|
||||||
@@ -168,7 +170,7 @@ func TestAnonymizeStateFile(t *testing.T) {
|
|||||||
assert.Equal(t, "100.64.0.1", state["protected_ip"]) // Protected IP unchanged
|
assert.Equal(t, "100.64.0.1", state["protected_ip"]) // Protected IP unchanged
|
||||||
assert.Equal(t, "8.8.8.8", state["well_known_ip"]) // Well-known IP unchanged
|
assert.Equal(t, "8.8.8.8", state["well_known_ip"]) // Well-known IP unchanged
|
||||||
assert.NotEqual(t, "2001:db8::1", state["ipv6_addr"])
|
assert.NotEqual(t, "2001:db8::1", state["ipv6_addr"])
|
||||||
assert.Equal(t, "fd00::1", state["private_ipv6"]) // Private IPv6 unchanged
|
assert.NotEqual(t, "fd00::1", state["private_ipv6"]) // ULA IPv6 anonymized (global ID is a fingerprint)
|
||||||
assert.NotEqual(t, "test.example.com", state["domain"])
|
assert.NotEqual(t, "test.example.com", state["domain"])
|
||||||
assert.True(t, strings.HasSuffix(state["domain"].(string), ".domain"))
|
assert.True(t, strings.HasSuffix(state["domain"].(string), ".domain"))
|
||||||
assert.Equal(t, "device.netbird.cloud", state["netbird_domain"]) // Netbird domain unchanged
|
assert.Equal(t, "device.netbird.cloud", state["netbird_domain"]) // Netbird domain unchanged
|
||||||
@@ -272,11 +274,13 @@ func mustMarshal(v any) json.RawMessage {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestAnonymizeNetworkMap(t *testing.T) {
|
func TestAnonymizeNetworkMap(t *testing.T) {
|
||||||
|
origV6Prefix := netip.MustParsePrefix("2001:db8:abcd::5/64")
|
||||||
networkMap := &mgmProto.NetworkMap{
|
networkMap := &mgmProto.NetworkMap{
|
||||||
PeerConfig: &mgmProto.PeerConfig{
|
PeerConfig: &mgmProto.PeerConfig{
|
||||||
Address: "203.0.113.5",
|
Address: "203.0.113.5",
|
||||||
Dns: "1.2.3.4",
|
AddressV6: netiputil.EncodePrefix(origV6Prefix),
|
||||||
Fqdn: "peer1.corp.example.com",
|
Dns: "1.2.3.4",
|
||||||
|
Fqdn: "peer1.corp.example.com",
|
||||||
SshConfig: &mgmProto.SSHConfig{
|
SshConfig: &mgmProto.SSHConfig{
|
||||||
SshPubKey: []byte("ssh-rsa AAAAB3NzaC1..."),
|
SshPubKey: []byte("ssh-rsa AAAAB3NzaC1..."),
|
||||||
},
|
},
|
||||||
@@ -350,6 +354,12 @@ func TestAnonymizeNetworkMap(t *testing.T) {
|
|||||||
require.NotEqual(t, "peer1.corp.example.com", peerCfg.Fqdn)
|
require.NotEqual(t, "peer1.corp.example.com", peerCfg.Fqdn)
|
||||||
require.True(t, strings.HasSuffix(peerCfg.Fqdn, ".domain"))
|
require.True(t, strings.HasSuffix(peerCfg.Fqdn, ".domain"))
|
||||||
|
|
||||||
|
// Verify AddressV6 is anonymized but preserves prefix length
|
||||||
|
anonV6Prefix, err := netiputil.DecodePrefix(peerCfg.AddressV6)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, origV6Prefix.Bits(), anonV6Prefix.Bits(), "prefix length must be preserved")
|
||||||
|
assert.NotEqual(t, origV6Prefix.Addr(), anonV6Prefix.Addr(), "IPv6 address must be anonymized")
|
||||||
|
|
||||||
// Verify SSH key is replaced
|
// Verify SSH key is replaced
|
||||||
require.Equal(t, []byte("ssh-placeholder-key"), peerCfg.SshConfig.SshPubKey)
|
require.Equal(t, []byte("ssh-placeholder-key"), peerCfg.SshConfig.SshPubKey)
|
||||||
|
|
||||||
@@ -784,8 +794,8 @@ COMMIT`
|
|||||||
assert.NotContains(t, anonNftables, "2001:db8::")
|
assert.NotContains(t, anonNftables, "2001:db8::")
|
||||||
assert.Contains(t, anonNftables, "2001:db8:ffff::") // Default anonymous v6 range
|
assert.Contains(t, anonNftables, "2001:db8:ffff::") // Default anonymous v6 range
|
||||||
|
|
||||||
// ULA addresses in nftables should remain unchanged (private)
|
// ULA addresses in nftables should be anonymized (global ID is a fingerprint)
|
||||||
assert.Contains(t, anonNftables, "fd00:1234::1")
|
assert.NotContains(t, anonNftables, "fd00:1234::1")
|
||||||
|
|
||||||
// IPv6 nftables structure preserved
|
// IPv6 nftables structure preserved
|
||||||
assert.Contains(t, anonNftables, "ip6 saddr")
|
assert.Contains(t, anonNftables, "ip6 saddr")
|
||||||
@@ -794,8 +804,8 @@ COMMIT`
|
|||||||
// Test ip6tables-save anonymization
|
// Test ip6tables-save anonymization
|
||||||
anonIp6tablesSave := anonymizer.AnonymizeString(ip6tablesSave)
|
anonIp6tablesSave := anonymizer.AnonymizeString(ip6tablesSave)
|
||||||
|
|
||||||
// ULA (private) IPv6 should remain unchanged
|
// ULA IPv6 should be anonymized (global ID is a fingerprint)
|
||||||
assert.Contains(t, anonIp6tablesSave, "fd00:1234::1/128")
|
assert.NotContains(t, anonIp6tablesSave, "fd00:1234::1/128")
|
||||||
|
|
||||||
// Public IPv6 addresses should be anonymized
|
// Public IPv6 addresses should be anonymized
|
||||||
assert.NotContains(t, anonIp6tablesSave, "2607:f8b0:4005::1")
|
assert.NotContains(t, anonIp6tablesSave, "2607:f8b0:4005::1")
|
||||||
|
|||||||
@@ -111,14 +111,24 @@ func (n *networkManagerDbusConfigurator) applyDNSConfig(config HostDNSConfig, st
|
|||||||
connSettings.cleanDeprecatedSettings()
|
connSettings.cleanDeprecatedSettings()
|
||||||
|
|
||||||
ipKey := networkManagerDbusIPv4Key
|
ipKey := networkManagerDbusIPv4Key
|
||||||
|
staleKey := networkManagerDbusIPv6Key
|
||||||
if config.ServerIP.Is6() {
|
if config.ServerIP.Is6() {
|
||||||
ipKey = networkManagerDbusIPv6Key
|
ipKey = networkManagerDbusIPv6Key
|
||||||
|
staleKey = networkManagerDbusIPv4Key
|
||||||
raw := config.ServerIP.As16()
|
raw := config.ServerIP.As16()
|
||||||
connSettings[ipKey][networkManagerDbusDNSKey] = dbus.MakeVariant([][]byte{raw[:]})
|
connSettings[ipKey][networkManagerDbusDNSKey] = dbus.MakeVariant([][]byte{raw[:]})
|
||||||
} else {
|
} else {
|
||||||
convDNSIP := binary.LittleEndian.Uint32(config.ServerIP.AsSlice())
|
convDNSIP := binary.LittleEndian.Uint32(config.ServerIP.AsSlice())
|
||||||
connSettings[ipKey][networkManagerDbusDNSKey] = dbus.MakeVariant([]uint32{convDNSIP})
|
connSettings[ipKey][networkManagerDbusDNSKey] = dbus.MakeVariant([]uint32{convDNSIP})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Clear stale DNS settings from the opposite address family to avoid
|
||||||
|
// leftover entries if the server IP family changed.
|
||||||
|
if staleSettings, ok := connSettings[staleKey]; ok {
|
||||||
|
delete(staleSettings, networkManagerDbusDNSKey)
|
||||||
|
delete(staleSettings, networkManagerDbusDNSPriorityKey)
|
||||||
|
delete(staleSettings, networkManagerDbusDNSSearchKey)
|
||||||
|
}
|
||||||
var (
|
var (
|
||||||
searchDomains []string
|
searchDomains []string
|
||||||
matchDomains []string
|
matchDomains []string
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestAddPeer(t *testing.T) {
|
func TestAddPeer(t *testing.T) {
|
||||||
@@ -46,7 +47,7 @@ func TestUpdatePeerState(t *testing.T) {
|
|||||||
ip := "10.10.10.10"
|
ip := "10.10.10.10"
|
||||||
fqdn := "peer-a.netbird.local"
|
fqdn := "peer-a.netbird.local"
|
||||||
status := NewRecorder("https://mgm")
|
status := NewRecorder("https://mgm")
|
||||||
_ = status.AddPeer(key, fqdn, ip, "")
|
require.NoError(t, status.AddPeer(key, fqdn, ip, ""))
|
||||||
|
|
||||||
peerState := State{
|
peerState := State{
|
||||||
PubKey: key,
|
PubKey: key,
|
||||||
|
|||||||
@@ -322,6 +322,8 @@ func (s *Server) Stop() error {
|
|||||||
}
|
}
|
||||||
s.sshServer = nil
|
s.sshServer = nil
|
||||||
s.listener = nil
|
s.listener = nil
|
||||||
|
extraListeners := s.extraListeners
|
||||||
|
s.extraListeners = nil
|
||||||
s.mu.Unlock()
|
s.mu.Unlock()
|
||||||
|
|
||||||
// Close outside the lock: session handlers need s.mu for unregisterSession.
|
// Close outside the lock: session handlers need s.mu for unregisterSession.
|
||||||
@@ -329,15 +331,11 @@ func (s *Server) Stop() error {
|
|||||||
log.Debugf("close SSH server: %v", err)
|
log.Debugf("close SSH server: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, ln := range s.extraListeners {
|
for _, ln := range extraListeners {
|
||||||
if err := ln.Close(); err != nil {
|
if err := ln.Close(); err != nil {
|
||||||
log.Debugf("close extra SSH listener: %v", err)
|
log.Debugf("close extra SSH listener: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
s.extraListeners = nil
|
|
||||||
|
|
||||||
s.sshServer = nil
|
|
||||||
s.listener = nil
|
|
||||||
|
|
||||||
s.mu.Lock()
|
s.mu.Lock()
|
||||||
maps.Clear(s.sessions)
|
maps.Clear(s.sessions)
|
||||||
|
|||||||
Reference in New Issue
Block a user