From 1a7e8359499e0cdb72f7afa855cf9abbf67c9700 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 24 Mar 2026 12:35:58 +0100 Subject: [PATCH 1/5] Fix CodeRabbit findings: hasIPv6Changed restart loop, empty peerIPs panic, v6 validation --- client/iface/wgaddr/address.go | 3 +++ client/internal/engine.go | 25 ++++++++++++------------- client/internal/engine_test.go | 16 ++++++++++------ 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/client/iface/wgaddr/address.go b/client/iface/wgaddr/address.go index cc8afcf72f8..eae26ec85b9 100644 --- a/client/iface/wgaddr/address.go +++ b/client/iface/wgaddr/address.go @@ -71,6 +71,9 @@ func (addr *Address) SetIPv6FromCompact(raw []byte) error { if err != nil { return fmt.Errorf("decode v6 overlay address: %w", err) } + if !prefix.Addr().Is6() { + return fmt.Errorf("expected IPv6 address, got %s", prefix.Addr()) + } addr.IPv6 = prefix.Addr() addr.IPv6Net = prefix.Masked() return nil diff --git a/client/internal/engine.go b/client/internal/engine.go index 1d27df15827..2fc1617b4fd 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -1035,22 +1035,24 @@ func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error { } // hasIPv6Changed reports whether the IPv6 overlay address in the peer config -// differs from the current interface address (added, removed, or changed). +// differs from the configured address (added, removed, or changed). +// Compares against e.config.WgAddr (not the interface address, which may have +// been cleared by ClearIPv6 if OS assignment failed). func (e *Engine) hasIPv6Changed(conf *mgmProto.PeerConfig) bool { - current := e.wgInterface.Address() + current := e.config.WgAddr raw := conf.GetAddressV6() if len(raw) == 0 { return current.HasIPv6() } - addr, err := netiputil.DecodeAddr(raw) + prefix, err := netiputil.DecodePrefix(raw) if err != nil { log.Warnf("decode v6 overlay address: %v", err) return false } - return !current.HasIPv6() || current.IPv6 != addr + return !current.HasIPv6() || current.IPv6 != prefix.Addr() || current.IPv6Net != prefix.Masked() } func (e *Engine) receiveJobEvents() { @@ -1540,20 +1542,17 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error { peerIPs = append(peerIPs, allowedNetIP) } + if len(peerIPs) == 0 { + return fmt.Errorf("peer %s has no usable AllowedIPs", peerKey) + } + conn, err := e.createPeerConn(peerKey, peerIPs, peerConfig.AgentVersion) if err != nil { return fmt.Errorf("create peer connection: %w", err) } - var peerIPv6 string - ourV6Net := e.wgInterface.Address().IPv6Net - for _, pip := range peerIPs { - if pip.Addr().Is6() && pip.Bits() == 128 && ourV6Net.Contains(pip.Addr()) { - peerIPv6 = pip.Addr().String() - break - } - } - err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerIPs[0].Addr().String(), peerIPv6) + peerV4, peerV6 := splitAllowedIPs(peerConfig.GetAllowedIps(), e.wgInterface.Address().IPv6Net) + err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerV4, peerV6) if err != nil { log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err) } diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 182189f6a42..c3aa5b56da1 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -1728,7 +1728,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) { { name: "no v6 before, v6 added", current: v4Only, - confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")), + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")), expected: true, }, { @@ -1740,13 +1740,19 @@ func TestEngine_hasIPv6Changed(t *testing.T) { { name: "had v6, same v6", current: v4v6, - confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")), + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")), expected: false, }, { name: "had v6, different v6", current: v4v6, - confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::2")), + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::2/64")), + expected: true, + }, + { + name: "same v6 addr, different prefix length", + current: v4v6, + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/80")), expected: true, }, { @@ -1760,9 +1766,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { engine := &Engine{ - wgInterface: &MockWGIface{ - AddressFunc: func() wgaddr.Address { return tt.current }, - }, + config: &EngineConfig{WgAddr: tt.current}, } conf := &mgmtProto.PeerConfig{ AddressV6: tt.confV6, From 71962f88f8b71c01c046015d038a9d95b40dbdbc Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 24 Mar 2026 12:06:58 +0100 Subject: [PATCH 2/5] Add IPv6 reverse DNS and host configurator support --- client/internal/dns.go | 79 +++++++---- client/internal/dns/host_darwin.go | 1 + client/internal/dns/network_manager_unix.go | 15 ++- client/internal/dns/systemd_linux.go | 6 +- client/internal/dns/upstream_ios.go | 42 ++++-- client/internal/dns_test.go | 138 ++++++++++++++++++++ client/internal/engine.go | 15 ++- 7 files changed, 252 insertions(+), 44 deletions(-) create mode 100644 client/internal/dns_test.go diff --git a/client/internal/dns.go b/client/internal/dns.go index f5040ee49a2..a6604810f6f 100644 --- a/client/internal/dns.go +++ b/client/internal/dns.go @@ -12,52 +12,83 @@ import ( nbdns "github.com/netbirdio/netbird/dns" ) -func createPTRRecord(aRecord nbdns.SimpleRecord, prefix netip.Prefix) (nbdns.SimpleRecord, bool) { - ip, err := netip.ParseAddr(aRecord.RData) +func createPTRRecord(record nbdns.SimpleRecord, prefix netip.Prefix) (nbdns.SimpleRecord, bool) { + ip, err := netip.ParseAddr(record.RData) if err != nil { - log.Warnf("failed to parse IP address %s: %v", aRecord.RData, err) + log.Warnf("failed to parse IP address %s: %v", record.RData, err) return nbdns.SimpleRecord{}, false } + ip = ip.Unmap() if !prefix.Contains(ip) { return nbdns.SimpleRecord{}, false } - ipOctets := strings.Split(ip.String(), ".") - slices.Reverse(ipOctets) - rdnsName := dns.Fqdn(strings.Join(ipOctets, ".") + ".in-addr.arpa") + var rdnsName string + if ip.Is4() { + octets := strings.Split(ip.String(), ".") + slices.Reverse(octets) + rdnsName = dns.Fqdn(strings.Join(octets, ".") + ".in-addr.arpa") + } else { + // Expand to full 32 nibbles in reverse order (LSB first) per RFC 3596. + raw := ip.As16() + nibbles := make([]string, 32) + for i := 0; i < 16; i++ { + nibbles[31-i*2] = fmt.Sprintf("%x", raw[i]>>4) + nibbles[31-i*2-1] = fmt.Sprintf("%x", raw[i]&0x0f) + } + rdnsName = dns.Fqdn(strings.Join(nibbles, ".") + ".ip6.arpa") + } return nbdns.SimpleRecord{ Name: rdnsName, Type: int(dns.TypePTR), - Class: aRecord.Class, - TTL: aRecord.TTL, - RData: dns.Fqdn(aRecord.Name), + Class: record.Class, + TTL: record.TTL, + RData: dns.Fqdn(record.Name), }, true } -// generateReverseZoneName creates the reverse DNS zone name for a given network +// generateReverseZoneName creates the reverse DNS zone name for a given network. +// For IPv4 it produces an in-addr.arpa name, for IPv6 an ip6.arpa name. func generateReverseZoneName(network netip.Prefix) (string, error) { - networkIP := network.Masked().Addr() + networkIP := network.Masked().Addr().Unmap() + bits := network.Bits() + + if networkIP.Is4() { + // Round up to nearest byte. + octetsToUse := (bits + 7) / 8 + + octets := strings.Split(networkIP.String(), ".") + if octetsToUse > len(octets) { + return "", fmt.Errorf("invalid network mask size for reverse DNS: %d", bits) + } + + reverseOctets := make([]string, octetsToUse) + for i := 0; i < octetsToUse; i++ { + reverseOctets[octetsToUse-1-i] = octets[i] + } - if !networkIP.Is4() { - return "", fmt.Errorf("reverse DNS is only supported for IPv4 networks, got: %s", networkIP) + return dns.Fqdn(strings.Join(reverseOctets, ".") + ".in-addr.arpa"), nil } - // round up to nearest byte - octetsToUse := (network.Bits() + 7) / 8 + // IPv6: round up to nearest nibble (4-bit boundary). + nibblesToUse := (bits + 3) / 4 - octets := strings.Split(networkIP.String(), ".") - if octetsToUse > len(octets) { - return "", fmt.Errorf("invalid network mask size for reverse DNS: %d", network.Bits()) + raw := networkIP.As16() + allNibbles := make([]string, 32) + for i := 0; i < 16; i++ { + allNibbles[i*2] = fmt.Sprintf("%x", raw[i]>>4) + allNibbles[i*2+1] = fmt.Sprintf("%x", raw[i]&0x0f) } - reverseOctets := make([]string, octetsToUse) - for i := 0; i < octetsToUse; i++ { - reverseOctets[octetsToUse-1-i] = octets[i] + // Take the first nibblesToUse nibbles (network portion), reverse them. + used := make([]string, nibblesToUse) + for i := 0; i < nibblesToUse; i++ { + used[nibblesToUse-1-i] = allNibbles[i] } - return dns.Fqdn(strings.Join(reverseOctets, ".") + ".in-addr.arpa"), nil + return dns.Fqdn(strings.Join(used, ".") + ".ip6.arpa"), nil } // zoneExists checks if a zone with the given name already exists in the configuration @@ -71,7 +102,7 @@ func zoneExists(config *nbdns.Config, zoneName string) bool { return false } -// collectPTRRecords gathers all PTR records for the given network from A records +// collectPTRRecords gathers all PTR records for the given network from A and AAAA records. func collectPTRRecords(config *nbdns.Config, prefix netip.Prefix) []nbdns.SimpleRecord { var records []nbdns.SimpleRecord @@ -80,7 +111,7 @@ func collectPTRRecords(config *nbdns.Config, prefix netip.Prefix) []nbdns.Simple continue } for _, record := range zone.Records { - if record.Type != int(dns.TypeA) { + if record.Type != int(dns.TypeA) && record.Type != int(dns.TypeAAAA) { continue } diff --git a/client/internal/dns/host_darwin.go b/client/internal/dns/host_darwin.go index b3908f16313..0f4eb6bf821 100644 --- a/client/internal/dns/host_darwin.go +++ b/client/internal/dns/host_darwin.go @@ -298,6 +298,7 @@ func (s *systemConfigurator) getSystemDNSSettings() (SystemDNSSettings, error) { if ip, err := netip.ParseAddr(address); err == nil && !ip.IsUnspecified() { ip = ip.Unmap() serverAddresses = append(serverAddresses, ip) + // Prefer the first IPv4 server as ServerIP since our DNS listener is IPv4. if !dnsSettings.ServerIP.IsValid() && ip.Is4() { dnsSettings.ServerIP = ip } diff --git a/client/internal/dns/network_manager_unix.go b/client/internal/dns/network_manager_unix.go index e4ccc8cbd29..b5b21dc3914 100644 --- a/client/internal/dns/network_manager_unix.go +++ b/client/internal/dns/network_manager_unix.go @@ -110,8 +110,15 @@ func (n *networkManagerDbusConfigurator) applyDNSConfig(config HostDNSConfig, st connSettings.cleanDeprecatedSettings() - convDNSIP := binary.LittleEndian.Uint32(config.ServerIP.AsSlice()) - connSettings[networkManagerDbusIPv4Key][networkManagerDbusDNSKey] = dbus.MakeVariant([]uint32{convDNSIP}) + ipKey := networkManagerDbusIPv4Key + if config.ServerIP.Is6() { + ipKey = networkManagerDbusIPv6Key + raw := config.ServerIP.As16() + connSettings[ipKey][networkManagerDbusDNSKey] = dbus.MakeVariant([][]byte{raw[:]}) + } else { + convDNSIP := binary.LittleEndian.Uint32(config.ServerIP.AsSlice()) + connSettings[ipKey][networkManagerDbusDNSKey] = dbus.MakeVariant([]uint32{convDNSIP}) + } var ( searchDomains []string matchDomains []string @@ -146,8 +153,8 @@ func (n *networkManagerDbusConfigurator) applyDNSConfig(config HostDNSConfig, st n.routingAll = false } - connSettings[networkManagerDbusIPv4Key][networkManagerDbusDNSPriorityKey] = dbus.MakeVariant(priority) - connSettings[networkManagerDbusIPv4Key][networkManagerDbusDNSSearchKey] = dbus.MakeVariant(newDomainList) + connSettings[ipKey][networkManagerDbusDNSPriorityKey] = dbus.MakeVariant(priority) + connSettings[ipKey][networkManagerDbusDNSSearchKey] = dbus.MakeVariant(newDomainList) state := &ShutdownState{ ManagerType: networkManager, diff --git a/client/internal/dns/systemd_linux.go b/client/internal/dns/systemd_linux.go index d9854c03377..573dff540ef 100644 --- a/client/internal/dns/systemd_linux.go +++ b/client/internal/dns/systemd_linux.go @@ -90,8 +90,12 @@ func (s *systemdDbusConfigurator) supportCustomPort() bool { } func (s *systemdDbusConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error { + family := int32(unix.AF_INET) + if config.ServerIP.Is6() { + family = unix.AF_INET6 + } defaultLinkInput := systemdDbusDNSInput{ - Family: unix.AF_INET, + Family: family, Address: config.ServerIP.AsSlice(), } if err := s.callLinkMethod(systemdDbusSetDNSMethodSuffix, []systemdDbusDNSInput{defaultLinkInput}); err != nil { diff --git a/client/internal/dns/upstream_ios.go b/client/internal/dns/upstream_ios.go index 4d053a5a1ce..236c4d8e5be 100644 --- a/client/internal/dns/upstream_ios.go +++ b/client/internal/dns/upstream_ios.go @@ -21,6 +21,8 @@ type upstreamResolverIOS struct { *upstreamResolverBase lIP netip.Addr lNet netip.Prefix + lIPv6 netip.Addr + lNetV6 netip.Prefix interfaceName string } @@ -37,6 +39,8 @@ func newUpstreamResolver( upstreamResolverBase: upstreamResolverBase, lIP: wgIface.Address().IP, lNet: wgIface.Address().Network, + lIPv6: wgIface.Address().IPv6, + lNetV6: wgIface.Address().IPv6Net, interfaceName: wgIface.Name(), } ios.upstreamClient = ios @@ -65,11 +69,27 @@ func (u *upstreamResolverIOS) exchange(ctx context.Context, upstream string, r * } else { upstreamIP = upstreamIP.Unmap() } - if u.lNet.Contains(upstreamIP) || upstreamIP.IsPrivate() { - log.Debugf("using private client to query upstream: %s", upstream) - client, err = GetClientPrivate(u.lIP, u.interfaceName, timeout) - if err != nil { - return nil, 0, fmt.Errorf("error while creating private client: %s", err) + // TODO: IsPrivate is a rough heuristic. It misses public IPs routed through + // the tunnel (e.g. 9.9.9.9 via network route) and incorrectly matches local + // LAN private IPs. Replace with a check against the active route table or + // the set of routed prefixes from the network map. + needsPrivate := u.lNet.Contains(upstreamIP) || upstreamIP.IsPrivate() || + (u.lNetV6.IsValid() && u.lNetV6.Contains(upstreamIP)) + if needsPrivate { + var bindIP netip.Addr + switch { + case upstreamIP.Is6() && u.lIPv6.IsValid(): + bindIP = u.lIPv6 + case upstreamIP.Is4() && u.lIP.IsValid(): + bindIP = u.lIP + } + + if bindIP.IsValid() { + log.Debugf("using private client to query upstream: %s", upstream) + client, err = GetClientPrivate(bindIP, u.interfaceName, timeout) + if err != nil { + return nil, 0, fmt.Errorf("create private client: %s", err) + } } } @@ -86,16 +106,18 @@ func GetClientPrivate(ip netip.Addr, interfaceName string, dialTimeout time.Dura return nil, err } + proto, opt := unix.IPPROTO_IP, unix.IP_BOUND_IF + if ip.Is6() { + proto, opt = unix.IPPROTO_IPV6, unix.IPV6_BOUND_IF + } + dialer := &net.Dialer{ - LocalAddr: &net.UDPAddr{ - IP: ip.AsSlice(), - Port: 0, // Let the OS pick a free port - }, + LocalAddr: net.UDPAddrFromAddrPort(netip.AddrPortFrom(ip, 0)), Timeout: dialTimeout, Control: func(network, address string, c syscall.RawConn) error { var operr error fn := func(s uintptr) { - operr = unix.SetsockoptInt(int(s), unix.IPPROTO_IP, unix.IP_BOUND_IF, index) + operr = unix.SetsockoptInt(int(s), proto, opt, index) } if err := c.Control(fn); err != nil { diff --git a/client/internal/dns_test.go b/client/internal/dns_test.go new file mode 100644 index 00000000000..e15cc8fb728 --- /dev/null +++ b/client/internal/dns_test.go @@ -0,0 +1,138 @@ +package internal + +import ( + "net/netip" + "testing" + + "github.com/miekg/dns" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + nbdns "github.com/netbirdio/netbird/dns" +) + +func TestCreatePTRRecord_IPv4(t *testing.T) { + record := nbdns.SimpleRecord{ + Name: "peer1.netbird.cloud.", + Type: int(dns.TypeA), + Class: nbdns.DefaultClass, + TTL: 300, + RData: "100.64.0.5", + } + prefix := netip.MustParsePrefix("100.64.0.0/16") + + ptr, ok := createPTRRecord(record, prefix) + require.True(t, ok) + assert.Equal(t, "5.0.64.100.in-addr.arpa.", ptr.Name) + assert.Equal(t, int(dns.TypePTR), ptr.Type) + assert.Equal(t, "peer1.netbird.cloud.", ptr.RData) +} + +func TestCreatePTRRecord_IPv6(t *testing.T) { + record := nbdns.SimpleRecord{ + Name: "peer1.netbird.cloud.", + Type: int(dns.TypeAAAA), + Class: nbdns.DefaultClass, + TTL: 300, + RData: "fd00:1234:5678::1", + } + prefix := netip.MustParsePrefix("fd00:1234:5678::/48") + + ptr, ok := createPTRRecord(record, prefix) + require.True(t, ok) + assert.Equal(t, "1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.7.6.5.4.3.2.1.0.0.d.f.ip6.arpa.", ptr.Name) + assert.Equal(t, int(dns.TypePTR), ptr.Type) + assert.Equal(t, "peer1.netbird.cloud.", ptr.RData) +} + +func TestCreatePTRRecord_OutOfRange(t *testing.T) { + record := nbdns.SimpleRecord{ + Name: "peer1.netbird.cloud.", + Type: int(dns.TypeA), + RData: "10.0.0.1", + } + prefix := netip.MustParsePrefix("100.64.0.0/16") + + _, ok := createPTRRecord(record, prefix) + assert.False(t, ok) +} + +func TestGenerateReverseZoneName_IPv4(t *testing.T) { + tests := []struct { + prefix string + expected string + }{ + {"100.64.0.0/16", "64.100.in-addr.arpa."}, + {"10.0.0.0/8", "10.in-addr.arpa."}, + {"192.168.1.0/24", "1.168.192.in-addr.arpa."}, + } + + for _, tt := range tests { + t.Run(tt.prefix, func(t *testing.T) { + zone, err := generateReverseZoneName(netip.MustParsePrefix(tt.prefix)) + require.NoError(t, err) + assert.Equal(t, tt.expected, zone) + }) + } +} + +func TestGenerateReverseZoneName_IPv6(t *testing.T) { + tests := []struct { + prefix string + expected string + }{ + {"fd00:1234:5678::/48", "8.7.6.5.4.3.2.1.0.0.d.f.ip6.arpa."}, + {"fd00::/16", "0.0.d.f.ip6.arpa."}, + {"fd12:3456:789a:bcde::/64", "e.d.c.b.a.9.8.7.6.5.4.3.2.1.d.f.ip6.arpa."}, + } + + for _, tt := range tests { + t.Run(tt.prefix, func(t *testing.T) { + zone, err := generateReverseZoneName(netip.MustParsePrefix(tt.prefix)) + require.NoError(t, err) + assert.Equal(t, tt.expected, zone) + }) + } +} + +func TestCollectPTRRecords_BothFamilies(t *testing.T) { + config := &nbdns.Config{ + CustomZones: []nbdns.CustomZone{ + { + Domain: "netbird.cloud.", + Records: []nbdns.SimpleRecord{ + {Name: "peer1.netbird.cloud.", Type: int(dns.TypeA), RData: "100.64.0.1"}, + {Name: "peer1.netbird.cloud.", Type: int(dns.TypeAAAA), RData: "fd00::1"}, + {Name: "peer2.netbird.cloud.", Type: int(dns.TypeA), RData: "100.64.0.2"}, + }, + }, + }, + } + + v4Records := collectPTRRecords(config, netip.MustParsePrefix("100.64.0.0/16")) + assert.Len(t, v4Records, 2, "should collect 2 A record PTRs for the v4 prefix") + + v6Records := collectPTRRecords(config, netip.MustParsePrefix("fd00::/64")) + assert.Len(t, v6Records, 1, "should collect 1 AAAA record PTR for the v6 prefix") +} + +func TestAddReverseZone_IPv6(t *testing.T) { + config := &nbdns.Config{ + CustomZones: []nbdns.CustomZone{ + { + Domain: "netbird.cloud.", + Records: []nbdns.SimpleRecord{ + {Name: "peer1.netbird.cloud.", Type: int(dns.TypeAAAA), RData: "fd00:1234:5678::1"}, + }, + }, + }, + } + + addReverseZone(config, netip.MustParsePrefix("fd00:1234:5678::/48")) + + require.Len(t, config.CustomZones, 2) + reverseZone := config.CustomZones[1] + assert.Equal(t, "8.7.6.5.4.3.2.1.0.0.d.f.ip6.arpa.", reverseZone.Domain) + assert.Len(t, reverseZone.Records, 1) + assert.Equal(t, int(dns.TypePTR), reverseZone.Records[0].Type) +} diff --git a/client/internal/engine.go b/client/internal/engine.go index 2fc1617b4fd..e34bec00d05 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -28,11 +28,10 @@ import ( "github.com/netbirdio/netbird/client/firewall" firewallManager "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/iface" - "github.com/netbirdio/netbird/client/iface/wgaddr" - "github.com/netbirdio/netbird/shared/netiputil" "github.com/netbirdio/netbird/client/iface/device" nbnetstack "github.com/netbirdio/netbird/client/iface/netstack" "github.com/netbirdio/netbird/client/iface/udpmux" + "github.com/netbirdio/netbird/client/iface/wgaddr" "github.com/netbirdio/netbird/client/internal/acl" "github.com/netbirdio/netbird/client/internal/debug" "github.com/netbirdio/netbird/client/internal/dns" @@ -63,6 +62,7 @@ import ( mgm "github.com/netbirdio/netbird/shared/management/client" "github.com/netbirdio/netbird/shared/management/domain" mgmProto "github.com/netbirdio/netbird/shared/management/proto" + "github.com/netbirdio/netbird/shared/netiputil" auth "github.com/netbirdio/netbird/shared/relay/auth/hmac" relayClient "github.com/netbirdio/netbird/shared/relay/client" signal "github.com/netbirdio/netbird/shared/signal/client" @@ -1252,7 +1252,7 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error { protoDNSConfig = &mgmProto.DNSConfig{} } - dnsConfig := toDNSConfig(protoDNSConfig, e.wgInterface.Address().Network) + dnsConfig := toDNSConfig(protoDNSConfig, e.wgInterface.Address()) if err := e.dnsServer.UpdateDNSServer(serial, dnsConfig); err != nil { log.Errorf("failed to update dns server, err: %v", err) @@ -1407,7 +1407,9 @@ func toRouteDomains(myPubKey string, routes []*route.Route) []*dnsfwd.ForwarderE return entries } -func toDNSConfig(protoDNSConfig *mgmProto.DNSConfig, network netip.Prefix) nbdns.Config { +func toDNSConfig(protoDNSConfig *mgmProto.DNSConfig, addr wgaddr.Address) nbdns.Config { + network := addr.Network + networkV6 := addr.IPv6Net //nolint forwarderPort := uint16(protoDNSConfig.GetForwarderPort()) if forwarderPort == 0 { @@ -1464,6 +1466,9 @@ func toDNSConfig(protoDNSConfig *mgmProto.DNSConfig, network netip.Prefix) nbdns if len(dnsUpdate.CustomZones) > 0 { addReverseZone(&dnsUpdate, network) + if networkV6.IsValid() { + addReverseZone(&dnsUpdate, networkV6) + } } return dnsUpdate @@ -1789,7 +1794,7 @@ func (e *Engine) readInitialSettings() ([]*route.Route, *nbdns.Config, bool, err return nil, nil, false, err } routes := toRoutes(netMap.GetRoutes()) - dnsCfg := toDNSConfig(netMap.GetDNSConfig(), e.wgInterface.Address().Network) + dnsCfg := toDNSConfig(netMap.GetDNSConfig(), e.wgInterface.Address()) dnsFeatureFlag := toDNSFeatureFlag(netMap) return routes, &dnsCfg, dnsFeatureFlag, nil } From d81cd5d154c67d47b6e0864f32e39d973d3a5761 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 24 Mar 2026 12:06:58 +0100 Subject: [PATCH 3/5] Add IPv6 support to SSH server, client config, and netflow logger --- client/internal/engine_ssh.go | 54 ++++++++++++++---- .../internal/netflow/conntrack/conntrack.go | 23 +++++--- client/internal/netflow/logger/logger.go | 12 +++- client/internal/netflow/logger/logger_test.go | 2 +- client/internal/netflow/manager.go | 7 ++- client/internal/netflow/types/types.go | 3 + client/internal/rosenpass/manager.go | 2 +- client/ssh/config/manager.go | 11 +++- client/ssh/config/manager_test.go | 9 +-- client/ssh/server/server.go | 57 ++++++++++++++++--- 10 files changed, 136 insertions(+), 44 deletions(-) diff --git a/client/internal/engine_ssh.go b/client/internal/engine_ssh.go index 1419bc26266..9ef70bf6e27 100644 --- a/client/internal/engine_ssh.go +++ b/client/internal/engine_ssh.go @@ -41,6 +41,14 @@ func (e *Engine) setupSSHPortRedirection() error { } log.Infof("SSH port redirection enabled: %s:22 -> %s:22022", localAddr, localAddr) + if v6 := e.wgInterface.Address().IPv6; v6.IsValid() { + if err := e.firewall.AddInboundDNAT(v6, firewallManager.ProtocolTCP, 22, 22022); err != nil { + log.Warnf("failed to add IPv6 SSH port redirection: %v", err) + } else { + log.Infof("SSH port redirection enabled: [%s]:22 -> [%s]:22022", v6, v6) + } + } + return nil } @@ -137,12 +145,13 @@ func (e *Engine) extractPeerSSHInfo(remotePeers []*mgmProto.RemotePeerConfig) [] continue } - peerIP := e.extractPeerIP(peerConfig) + peerIP, peerIPv6 := e.extractPeerIPs(peerConfig) hostname := e.extractHostname(peerConfig) peerInfo = append(peerInfo, sshconfig.PeerSSHInfo{ Hostname: hostname, IP: peerIP, + IPv6: peerIPv6, FQDN: peerConfig.GetFqdn(), }) } @@ -150,16 +159,26 @@ func (e *Engine) extractPeerSSHInfo(remotePeers []*mgmProto.RemotePeerConfig) [] return peerInfo } -// extractPeerIP extracts IP address from peer's allowed IPs -func (e *Engine) extractPeerIP(peerConfig *mgmProto.RemotePeerConfig) string { - if len(peerConfig.GetAllowedIps()) == 0 { - return "" - } - - if prefix, err := netip.ParsePrefix(peerConfig.GetAllowedIps()[0]); err == nil { - return prefix.Addr().String() +// extractPeerIPs extracts IPv4 and IPv6 overlay addresses from peer's allowed IPs. +// Only considers host routes (/32, /128) within the overlay networks to avoid +// picking up routed prefixes or static routes like 2620:fe::fe/128. +func (e *Engine) extractPeerIPs(peerConfig *mgmProto.RemotePeerConfig) (v4, v6 netip.Addr) { + wgAddr := e.wgInterface.Address() + for _, allowedIP := range peerConfig.GetAllowedIps() { + prefix, err := netip.ParsePrefix(allowedIP) + if err != nil { + log.Warnf("failed to parse AllowedIP %q: %v", allowedIP, err) + continue + } + addr := prefix.Addr().Unmap() + switch { + case addr.Is4() && prefix.Bits() == 32 && wgAddr.Network.Contains(addr) && !v4.IsValid(): + v4 = addr + case addr.Is6() && prefix.Bits() == 128 && wgAddr.IPv6Net.IsValid() && wgAddr.IPv6Net.Contains(addr) && !v6.IsValid(): + v6 = addr + } } - return "" + return v4, v6 } // extractHostname extracts short hostname from FQDN @@ -208,7 +227,7 @@ func (e *Engine) GetPeerSSHKey(peerAddress string) ([]byte, bool) { fullStatus := statusRecorder.GetFullStatus() for _, peerState := range fullStatus.Peers { - if peerState.IP == peerAddress || peerState.FQDN == peerAddress { + if peerState.IP == peerAddress || peerState.FQDN == peerAddress || peerState.IPv6 == peerAddress { if len(peerState.SSHHostKey) > 0 { return peerState.SSHHostKey, true } @@ -262,6 +281,13 @@ func (e *Engine) startSSHServer(jwtConfig *sshserver.JWTConfig) error { return fmt.Errorf("start SSH server: %w", err) } + if v6 := wgAddr.IPv6; v6.IsValid() { + v6Addr := netip.AddrPortFrom(v6, sshserver.InternalSSHPort) + if err := server.AddListener(e.ctx, v6Addr); err != nil { + log.Warnf("failed to add IPv6 SSH listener: %v", err) + } + } + e.sshServer = server if netstackNet := e.wgInterface.GetNet(); netstackNet != nil { @@ -330,6 +356,12 @@ func (e *Engine) cleanupSSHPortRedirection() error { } log.Debugf("SSH port redirection removed: %s:22 -> %s:22022", localAddr, localAddr) + if v6 := e.wgInterface.Address().IPv6; v6.IsValid() { + if err := e.firewall.RemoveInboundDNAT(v6, firewallManager.ProtocolTCP, 22, 22022); err != nil { + log.Debugf("failed to remove IPv6 SSH port redirection: %v", err) + } + } + return nil } diff --git a/client/internal/netflow/conntrack/conntrack.go b/client/internal/netflow/conntrack/conntrack.go index a4ffa3a254d..084c642c24d 100644 --- a/client/internal/netflow/conntrack/conntrack.go +++ b/client/internal/netflow/conntrack/conntrack.go @@ -188,7 +188,7 @@ func (c *ConnTrack) handleEvent(event nfct.Event) { case nftypes.TCP, nftypes.UDP, nftypes.SCTP: srcPort = flow.TupleOrig.Proto.SourcePort dstPort = flow.TupleOrig.Proto.DestinationPort - case nftypes.ICMP: + case nftypes.ICMP, nftypes.ICMPv6: icmpType = flow.TupleOrig.Proto.ICMPType icmpCode = flow.TupleOrig.Proto.ICMPCode } @@ -231,8 +231,14 @@ func (c *ConnTrack) relevantFlow(mark uint32, srcIP, dstIP netip.Addr) bool { } // fallback if mark rules are not in place - wgnet := c.iface.Address().Network - return wgnet.Contains(srcIP) || wgnet.Contains(dstIP) + addr := c.iface.Address() + if addr.Network.Contains(srcIP) || addr.Network.Contains(dstIP) { + return true + } + if addr.IPv6Net.IsValid() { + return addr.IPv6Net.Contains(srcIP) || addr.IPv6Net.Contains(dstIP) + } + return false } // mapRxPackets maps packet counts to RX based on flow direction @@ -291,17 +297,16 @@ func (c *ConnTrack) inferDirection(mark uint32, srcIP, dstIP netip.Addr) nftypes } // fallback if marks are not set - wgaddr := c.iface.Address().IP - wgnetwork := c.iface.Address().Network + addr := c.iface.Address() switch { - case wgaddr == srcIP: + case addr.IP == srcIP || (addr.IPv6.IsValid() && addr.IPv6 == srcIP): return nftypes.Egress - case wgaddr == dstIP: + case addr.IP == dstIP || (addr.IPv6.IsValid() && addr.IPv6 == dstIP): return nftypes.Ingress - case wgnetwork.Contains(srcIP): + case addr.Network.Contains(srcIP) || (addr.IPv6Net.IsValid() && addr.IPv6Net.Contains(srcIP)): // netbird network -> resource network return nftypes.Ingress - case wgnetwork.Contains(dstIP): + case addr.Network.Contains(dstIP) || (addr.IPv6Net.IsValid() && addr.IPv6Net.Contains(dstIP)): // resource network -> netbird network return nftypes.Egress } diff --git a/client/internal/netflow/logger/logger.go b/client/internal/netflow/logger/logger.go index a033a2a7ccd..8f8e6878404 100644 --- a/client/internal/netflow/logger/logger.go +++ b/client/internal/netflow/logger/logger.go @@ -24,15 +24,17 @@ type Logger struct { cancel context.CancelFunc statusRecorder *peer.Status wgIfaceNet netip.Prefix + wgIfaceNetV6 netip.Prefix dnsCollection atomic.Bool exitNodeCollection atomic.Bool Store types.Store } -func New(statusRecorder *peer.Status, wgIfaceIPNet netip.Prefix) *Logger { +func New(statusRecorder *peer.Status, wgIfaceIPNet, wgIfaceIPNetV6 netip.Prefix) *Logger { return &Logger{ statusRecorder: statusRecorder, wgIfaceNet: wgIfaceIPNet, + wgIfaceNetV6: wgIfaceIPNetV6, Store: store.NewMemoryStore(), } } @@ -88,11 +90,11 @@ func (l *Logger) startReceiver() { var isSrcExitNode bool var isDestExitNode bool - if !l.wgIfaceNet.Contains(event.SourceIP) { + if !l.isOverlayIP(event.SourceIP) { event.SourceResourceID, isSrcExitNode = l.statusRecorder.CheckRoutes(event.SourceIP) } - if !l.wgIfaceNet.Contains(event.DestIP) { + if !l.isOverlayIP(event.DestIP) { event.DestResourceID, isDestExitNode = l.statusRecorder.CheckRoutes(event.DestIP) } @@ -136,6 +138,10 @@ func (l *Logger) UpdateConfig(dnsCollection, exitNodeCollection bool) { l.exitNodeCollection.Store(exitNodeCollection) } +func (l *Logger) isOverlayIP(ip netip.Addr) bool { + return l.wgIfaceNet.Contains(ip) || (l.wgIfaceNetV6.IsValid() && l.wgIfaceNetV6.Contains(ip)) +} + func (l *Logger) shouldStore(event *types.EventFields, isExitNode bool) bool { // check dns collection if !l.dnsCollection.Load() && event.Protocol == types.UDP && diff --git a/client/internal/netflow/logger/logger_test.go b/client/internal/netflow/logger/logger_test.go index 1144544d84e..ad2eedef2e4 100644 --- a/client/internal/netflow/logger/logger_test.go +++ b/client/internal/netflow/logger/logger_test.go @@ -12,7 +12,7 @@ import ( ) func TestStore(t *testing.T) { - logger := logger.New(nil, netip.Prefix{}) + logger := logger.New(nil, netip.Prefix{}, netip.Prefix{}) logger.Enable() event := types.EventFields{ diff --git a/client/internal/netflow/manager.go b/client/internal/netflow/manager.go index 7752c97b026..eff083dbfd4 100644 --- a/client/internal/netflow/manager.go +++ b/client/internal/netflow/manager.go @@ -35,11 +35,12 @@ type Manager struct { // NewManager creates a new netflow manager func NewManager(iface nftypes.IFaceMapper, publicKey []byte, statusRecorder *peer.Status) *Manager { - var prefix netip.Prefix + var prefix, prefixV6 netip.Prefix if iface != nil { prefix = iface.Address().Network + prefixV6 = iface.Address().IPv6Net } - flowLogger := logger.New(statusRecorder, prefix) + flowLogger := logger.New(statusRecorder, prefix, prefixV6) var ct nftypes.ConnTracker if runtime.GOOS == "linux" && iface != nil && !iface.IsUserspaceBind() { @@ -269,7 +270,7 @@ func toProtoEvent(publicKey []byte, event *nftypes.Event) *proto.FlowEvent { }, } - if event.Protocol == nftypes.ICMP { + if event.Protocol == nftypes.ICMP || event.Protocol == nftypes.ICMPv6 { protoEvent.FlowFields.ConnectionInfo = &proto.FlowFields_IcmpInfo{ IcmpInfo: &proto.ICMPInfo{ IcmpType: uint32(event.ICMPType), diff --git a/client/internal/netflow/types/types.go b/client/internal/netflow/types/types.go index f76146ba3b3..3f7d0d0add2 100644 --- a/client/internal/netflow/types/types.go +++ b/client/internal/netflow/types/types.go @@ -19,6 +19,7 @@ const ( ICMP = Protocol(1) TCP = Protocol(6) UDP = Protocol(17) + ICMPv6 = Protocol(58) SCTP = Protocol(132) ) @@ -30,6 +31,8 @@ func (p Protocol) String() string { return "TCP" case 17: return "UDP" + case 58: + return "ICMPv6" case 132: return "SCTP" default: diff --git a/client/internal/rosenpass/manager.go b/client/internal/rosenpass/manager.go index 1faa22dc5ca..c69ea9a6c60 100644 --- a/client/internal/rosenpass/manager.go +++ b/client/internal/rosenpass/manager.go @@ -75,7 +75,7 @@ func (m *Manager) addPeer(rosenpassPubKey []byte, rosenpassAddr string, wireGuar if err != nil { return fmt.Errorf("failed to parse rosenpass address: %w", err) } - peerAddr := fmt.Sprintf("%s:%s", wireGuardIP, strPort) + peerAddr := net.JoinHostPort(wireGuardIP, strPort) if pcfg.Endpoint, err = net.ResolveUDPAddr("udp", peerAddr); err != nil { return fmt.Errorf("failed to resolve peer endpoint address: %w", err) } diff --git a/client/ssh/config/manager.go b/client/ssh/config/manager.go index cc47fd2d2d1..c76f1a21229 100644 --- a/client/ssh/config/manager.go +++ b/client/ssh/config/manager.go @@ -3,6 +3,7 @@ package config import ( "context" "fmt" + "net/netip" "os" "path/filepath" "runtime" @@ -91,7 +92,8 @@ type Manager struct { // PeerSSHInfo represents a peer's SSH configuration information type PeerSSHInfo struct { Hostname string - IP string + IP netip.Addr + IPv6 netip.Addr FQDN string } @@ -211,8 +213,11 @@ func (m *Manager) buildPeerConfig(allHostPatterns []string) (string, error) { func (m *Manager) buildHostPatterns(peer PeerSSHInfo) []string { var hostPatterns []string - if peer.IP != "" { - hostPatterns = append(hostPatterns, peer.IP) + if peer.IP.IsValid() { + hostPatterns = append(hostPatterns, peer.IP.String()) + } + if peer.IPv6.IsValid() { + hostPatterns = append(hostPatterns, peer.IPv6.String()) } if peer.FQDN != "" { hostPatterns = append(hostPatterns, peer.FQDN) diff --git a/client/ssh/config/manager_test.go b/client/ssh/config/manager_test.go index dc3ad95b35f..bf7b0d1c024 100644 --- a/client/ssh/config/manager_test.go +++ b/client/ssh/config/manager_test.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "net/netip" "os" "path/filepath" "runtime" @@ -28,12 +29,12 @@ func TestManager_SetupSSHClientConfig(t *testing.T) { peers := []PeerSSHInfo{ { Hostname: "peer1", - IP: "100.125.1.1", + IP: netip.MustParseAddr("100.125.1.1"), FQDN: "peer1.nb.internal", }, { Hostname: "peer2", - IP: "100.125.1.2", + IP: netip.MustParseAddr("100.125.1.2"), FQDN: "peer2.nb.internal", }, } @@ -101,7 +102,7 @@ func TestManager_PeerLimit(t *testing.T) { for i := 0; i < MaxPeersForSSHConfig+10; i++ { peers = append(peers, PeerSSHInfo{ Hostname: fmt.Sprintf("peer%d", i), - IP: fmt.Sprintf("100.125.1.%d", i%254+1), + IP: netip.MustParseAddr(fmt.Sprintf("100.125.1.%d", i%254+1)), FQDN: fmt.Sprintf("peer%d.nb.internal", i), }) } @@ -136,7 +137,7 @@ func TestManager_ForcedSSHConfig(t *testing.T) { for i := 0; i < MaxPeersForSSHConfig+10; i++ { peers = append(peers, PeerSSHInfo{ Hostname: fmt.Sprintf("peer%d", i), - IP: fmt.Sprintf("100.125.1.%d", i%254+1), + IP: netip.MustParseAddr(fmt.Sprintf("100.125.1.%d", i%254+1)), FQDN: fmt.Sprintf("peer%d.nb.internal", i), }) } diff --git a/client/ssh/server/server.go b/client/ssh/server/server.go index 4431ae4230b..0ad8ef1272d 100644 --- a/client/ssh/server/server.go +++ b/client/ssh/server/server.go @@ -137,10 +137,11 @@ type sessionState struct { } type Server struct { - sshServer *ssh.Server - listener net.Listener - mu sync.RWMutex - hostKeyPEM []byte + sshServer *ssh.Server + listener net.Listener + extraListeners []net.Listener + mu sync.RWMutex + hostKeyPEM []byte // sessions tracks active SSH sessions (shell, command, SFTP). // These are created when a client opens a session channel and requests shell/exec/subsystem. @@ -254,6 +255,35 @@ func (s *Server) Start(ctx context.Context, addr netip.AddrPort) error { return nil } +// AddListener starts serving SSH on an additional address (e.g. IPv6). +// Must be called after Start. +func (s *Server) AddListener(ctx context.Context, addr netip.AddrPort) error { + s.mu.Lock() + srv := s.sshServer + if srv == nil { + s.mu.Unlock() + return errors.New("SSH server is not running") + } + + ln, addrDesc, err := s.createListener(ctx, addr) + if err != nil { + s.mu.Unlock() + return fmt.Errorf("create listener: %w", err) + } + + s.extraListeners = append(s.extraListeners, ln) + s.mu.Unlock() + + log.Infof("SSH server also listening on %s", addrDesc) + + go func() { + if err := srv.Serve(ln); err != nil && !errors.Is(err, ssh.ErrServerClosed) { + log.Errorf("SSH server error on %s: %v", addrDesc, err) + } + }() + return nil +} + func (s *Server) createListener(ctx context.Context, addr netip.AddrPort) (net.Listener, string, error) { if s.netstackNet != nil { ln, err := s.netstackNet.ListenTCPAddrPort(addr) @@ -294,6 +324,13 @@ func (s *Server) Stop() error { log.Debugf("close SSH server: %v", err) } + for _, ln := range s.extraListeners { + if err := ln.Close(); err != nil { + log.Debugf("close extra SSH listener: %v", err) + } + } + s.extraListeners = nil + s.sshServer = nil s.listener = nil @@ -746,11 +783,10 @@ func (s *Server) findSessionKeyByContext(ctx ssh.Context) sessionKey { func (s *Server) connectionValidator(_ ssh.Context, conn net.Conn) net.Conn { s.mu.RLock() - netbirdNetwork := s.wgAddress.Network - localIP := s.wgAddress.IP + wgAddr := s.wgAddress s.mu.RUnlock() - if !netbirdNetwork.IsValid() || !localIP.IsValid() { + if !wgAddr.Network.IsValid() || !wgAddr.IP.IsValid() { return conn } @@ -766,14 +802,17 @@ func (s *Server) connectionValidator(_ ssh.Context, conn net.Conn) net.Conn { log.Warnf("SSH connection rejected: invalid remote IP %s", tcpAddr.IP) return nil } + remoteIP = remoteIP.Unmap() // Block connections from our own IP (prevent local apps from connecting to ourselves) - if remoteIP == localIP { + if remoteIP == wgAddr.IP || wgAddr.IPv6.IsValid() && remoteIP == wgAddr.IPv6 { log.Warnf("SSH connection rejected from own IP %s", remoteIP) return nil } - if !netbirdNetwork.Contains(remoteIP) { + inV4 := wgAddr.Network.Contains(remoteIP) + inV6 := wgAddr.IPv6Net.IsValid() && wgAddr.IPv6Net.Contains(remoteIP) + if !inV4 && !inV6 { log.Warnf("SSH connection rejected from non-NetBird IP %s", remoteIP) return nil } From 3be5a5f230f18416407d43b646c58e66eb17dc26 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 24 Mar 2026 12:35:58 +0100 Subject: [PATCH 4/5] Fix CodeRabbit findings: hasIPv6Changed restart loop, empty peerIPs panic, v6 validation --- client/iface/wgaddr/address.go | 20 -------------------- client/internal/connect.go | 13 +++++++++++-- client/internal/engine.go | 25 ++++++++++++------------- client/internal/engine_test.go | 16 ++++++++++------ 4 files changed, 33 insertions(+), 41 deletions(-) diff --git a/client/iface/wgaddr/address.go b/client/iface/wgaddr/address.go index cc8afcf72f8..a9048476041 100644 --- a/client/iface/wgaddr/address.go +++ b/client/iface/wgaddr/address.go @@ -1,10 +1,7 @@ package wgaddr import ( - "fmt" "net/netip" - - "github.com/netbirdio/netbird/shared/netiputil" ) // Address WireGuard parsed address @@ -59,23 +56,6 @@ func (addr Address) IPv6Prefix() netip.Prefix { return netip.PrefixFrom(addr.IPv6, addr.IPv6Net.Bits()) } -// SetIPv6FromCompact decodes a compact prefix (5 or 17 bytes) and sets the IPv6 fields. -// Returns an error if the bytes are invalid. A nil or empty input is a no-op. -// -//nolint:recvcheck -func (addr *Address) SetIPv6FromCompact(raw []byte) error { - if len(raw) == 0 { - return nil - } - prefix, err := netiputil.DecodePrefix(raw) - if err != nil { - return fmt.Errorf("decode v6 overlay address: %w", err) - } - addr.IPv6 = prefix.Addr() - addr.IPv6Net = prefix.Masked() - return nil -} - // ClearIPv6 removes the IPv6 overlay address, leaving only v4. // //nolint:recvcheck // ClearIPv6 is the only mutating method on this otherwise value-type struct. diff --git a/client/internal/connect.go b/client/internal/connect.go index 4b1448e2197..f0836aa0a20 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -40,6 +40,7 @@ import ( "github.com/netbirdio/netbird/client/system" mgm "github.com/netbirdio/netbird/shared/management/client" mgmProto "github.com/netbirdio/netbird/shared/management/proto" + "github.com/netbirdio/netbird/shared/netiputil" "github.com/netbirdio/netbird/shared/relay/auth/hmac" relayClient "github.com/netbirdio/netbird/shared/relay/client" signal "github.com/netbirdio/netbird/shared/signal/client" @@ -529,8 +530,16 @@ func createEngineConfig(key wgtypes.Key, config *profilemanager.Config, peerConf } if !config.DisableIPv6 { - if err := wgAddr.SetIPv6FromCompact(peerConfig.GetAddressV6()); err != nil { - log.Warn(err) + if raw := peerConfig.GetAddressV6(); len(raw) > 0 { + prefix, err := netiputil.DecodePrefix(raw) + if err != nil { + log.Warnf("decode v6 overlay address: %v", err) + } else if !prefix.Addr().Is6() { + log.Warnf("expected IPv6 overlay address, got %s", prefix.Addr()) + } else { + wgAddr.IPv6 = prefix.Addr() + wgAddr.IPv6Net = prefix.Masked() + } } } diff --git a/client/internal/engine.go b/client/internal/engine.go index 1d27df15827..2fc1617b4fd 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -1035,22 +1035,24 @@ func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error { } // hasIPv6Changed reports whether the IPv6 overlay address in the peer config -// differs from the current interface address (added, removed, or changed). +// differs from the configured address (added, removed, or changed). +// Compares against e.config.WgAddr (not the interface address, which may have +// been cleared by ClearIPv6 if OS assignment failed). func (e *Engine) hasIPv6Changed(conf *mgmProto.PeerConfig) bool { - current := e.wgInterface.Address() + current := e.config.WgAddr raw := conf.GetAddressV6() if len(raw) == 0 { return current.HasIPv6() } - addr, err := netiputil.DecodeAddr(raw) + prefix, err := netiputil.DecodePrefix(raw) if err != nil { log.Warnf("decode v6 overlay address: %v", err) return false } - return !current.HasIPv6() || current.IPv6 != addr + return !current.HasIPv6() || current.IPv6 != prefix.Addr() || current.IPv6Net != prefix.Masked() } func (e *Engine) receiveJobEvents() { @@ -1540,20 +1542,17 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error { peerIPs = append(peerIPs, allowedNetIP) } + if len(peerIPs) == 0 { + return fmt.Errorf("peer %s has no usable AllowedIPs", peerKey) + } + conn, err := e.createPeerConn(peerKey, peerIPs, peerConfig.AgentVersion) if err != nil { return fmt.Errorf("create peer connection: %w", err) } - var peerIPv6 string - ourV6Net := e.wgInterface.Address().IPv6Net - for _, pip := range peerIPs { - if pip.Addr().Is6() && pip.Bits() == 128 && ourV6Net.Contains(pip.Addr()) { - peerIPv6 = pip.Addr().String() - break - } - } - err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerIPs[0].Addr().String(), peerIPv6) + peerV4, peerV6 := splitAllowedIPs(peerConfig.GetAllowedIps(), e.wgInterface.Address().IPv6Net) + err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerV4, peerV6) if err != nil { log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err) } diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 182189f6a42..c3aa5b56da1 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -1728,7 +1728,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) { { name: "no v6 before, v6 added", current: v4Only, - confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")), + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")), expected: true, }, { @@ -1740,13 +1740,19 @@ func TestEngine_hasIPv6Changed(t *testing.T) { { name: "had v6, same v6", current: v4v6, - confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")), + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")), expected: false, }, { name: "had v6, different v6", current: v4v6, - confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::2")), + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::2/64")), + expected: true, + }, + { + name: "same v6 addr, different prefix length", + current: v4v6, + confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/80")), expected: true, }, { @@ -1760,9 +1766,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { engine := &Engine{ - wgInterface: &MockWGIface{ - AddressFunc: func() wgaddr.Address { return tt.current }, - }, + config: &EngineConfig{WgAddr: tt.current}, } conf := &mgmtProto.PeerConfig{ AddressV6: tt.confV6, From 50c0bc583bb031db99ed19a58679da3a853d45cb Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 25 Mar 2026 10:57:40 +0100 Subject: [PATCH 5/5] Fix connect.go lint: use SetIPv6FromCompact instead of if-else chain --- client/internal/connect.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/client/internal/connect.go b/client/internal/connect.go index f0836aa0a20..4b1448e2197 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -40,7 +40,6 @@ import ( "github.com/netbirdio/netbird/client/system" mgm "github.com/netbirdio/netbird/shared/management/client" mgmProto "github.com/netbirdio/netbird/shared/management/proto" - "github.com/netbirdio/netbird/shared/netiputil" "github.com/netbirdio/netbird/shared/relay/auth/hmac" relayClient "github.com/netbirdio/netbird/shared/relay/client" signal "github.com/netbirdio/netbird/shared/signal/client" @@ -530,16 +529,8 @@ func createEngineConfig(key wgtypes.Key, config *profilemanager.Config, peerConf } if !config.DisableIPv6 { - if raw := peerConfig.GetAddressV6(); len(raw) > 0 { - prefix, err := netiputil.DecodePrefix(raw) - if err != nil { - log.Warnf("decode v6 overlay address: %v", err) - } else if !prefix.Addr().Is6() { - log.Warnf("expected IPv6 overlay address, got %s", prefix.Addr()) - } else { - wgAddr.IPv6 = prefix.Addr() - wgAddr.IPv6Net = prefix.Masked() - } + if err := wgAddr.SetIPv6FromCompact(peerConfig.GetAddressV6()); err != nil { + log.Warn(err) } }