[client] Add TCP support to DNS forwarder service listener (#3790)

[client] Add TCP support to DNS forwarder service listener

client/internal/dnsfwd/forwarder.go

@@ -33,6 +33,8 @@ type DNSForwarder struct {
 
 	dnsServer *dns.Server
 	mux       *dns.ServeMux
+	tcpServer *dns.Server
+	tcpMux    *dns.ServeMux
 
 	mutex      sync.RWMutex
 	fwdEntries []*ForwarderEntry
@@ -50,22 +52,41 @@ func NewDNSForwarder(listenAddress string, ttl uint32, firewall firewall.Manager
 }
 
 func (f *DNSForwarder) Listen(entries []*ForwarderEntry) error {
-	log.Infof("listen DNS forwarder on address=%s", f.listenAddress)
-	mux := dns.NewServeMux()
+	log.Infof("starting DNS forwarder on address=%s", f.listenAddress)
 
-	dnsServer := &dns.Server{
+	// UDP server
+	mux := dns.NewServeMux()
+	f.mux = mux
+	f.dnsServer = &dns.Server{
 		Addr:    f.listenAddress,
 		Net:     "udp",
 		Handler: mux,
 	}
-	f.dnsServer = dnsServer
-	f.mux = mux
+	// TCP server
+	tcpMux := dns.NewServeMux()
+	f.tcpMux = tcpMux
+	f.tcpServer = &dns.Server{
+		Addr:    f.listenAddress,
+		Net:     "tcp",
+		Handler: tcpMux,
+	}
 
 	f.UpdateDomains(entries)
 
-	return dnsServer.ListenAndServe()
-}
+	errCh := make(chan error, 2)
 
+	go func() {
+		log.Infof("DNS UDP listener running on %s", f.listenAddress)
+		errCh <- f.dnsServer.ListenAndServe()
+	}()
+	go func() {
+		log.Infof("DNS TCP listener running on %s", f.listenAddress)
+		errCh <- f.tcpServer.ListenAndServe()
+	}()
+
+	// return the first error we get (e.g. bind failure or shutdown)
+	return <-errCh
+}
 func (f *DNSForwarder) UpdateDomains(entries []*ForwarderEntry) {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
@@ -77,31 +98,41 @@ func (f *DNSForwarder) UpdateDomains(entries []*ForwarderEntry) {
 	}
 
 	oldDomains := filterDomains(f.fwdEntries)
-
 	for _, d := range oldDomains {
 		f.mux.HandleRemove(d.PunycodeString())
+		f.tcpMux.HandleRemove(d.PunycodeString())
 	}
 
 	newDomains := filterDomains(entries)
 	for _, d := range newDomains {
-		f.mux.HandleFunc(d.PunycodeString(), f.handleDNSQuery)
+		f.mux.HandleFunc(d.PunycodeString(), f.handleDNSQueryUDP)
+		f.tcpMux.HandleFunc(d.PunycodeString(), f.handleDNSQueryTCP)
 	}
 
 	f.fwdEntries = entries
-
 	log.Debugf("Updated domains from %v to %v", oldDomains, newDomains)
 }
 
 func (f *DNSForwarder) Close(ctx context.Context) error {
-	if f.dnsServer == nil {
-		return nil
+	var result *multierror.Error
+
+	if f.dnsServer != nil {
+		if err := f.dnsServer.ShutdownContext(ctx); err != nil {
+			result = multierror.Append(result, fmt.Errorf("UDP shutdown: %w", err))
+		}
 	}
-	return f.dnsServer.ShutdownContext(ctx)
+	if f.tcpServer != nil {
+		if err := f.tcpServer.ShutdownContext(ctx); err != nil {
+			result = multierror.Append(result, fmt.Errorf("TCP shutdown: %w", err))
+		}
+	}
+
+	return nberrors.FormatErrorOrNil(result)
 }
 
-func (f *DNSForwarder) handleDNSQuery(w dns.ResponseWriter, query *dns.Msg) {
+func (f *DNSForwarder) handleDNSQuery(w dns.ResponseWriter, query *dns.Msg) *dns.Msg {
 	if len(query.Question) == 0 {
-		return
+		return nil
 	}
 	question := query.Question[0]
 	log.Tracef("received DNS request for DNS forwarder: domain=%v type=%v class=%v",
@@ -123,20 +154,53 @@ func (f *DNSForwarder) handleDNSQuery(w dns.ResponseWriter, query *dns.Msg) {
 		if err := w.WriteMsg(resp); err != nil {
 			log.Errorf("failed to write DNS response: %v", err)
 		}
-		return
+		return nil
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), upstreamTimeout)
 	defer cancel()
 	ips, err := net.DefaultResolver.LookupNetIP(ctx, network, domain)
 	if err != nil {
-		f.handleDNSError(w, resp, domain, err)
-		return
+		f.handleDNSError(w, query, resp, domain, err)
+		return nil
 	}
 
 	f.updateInternalState(domain, ips)
 	f.addIPsToResponse(resp, domain, ips)
 
+	return resp
+}
+
+func (f *DNSForwarder) handleDNSQueryUDP(w dns.ResponseWriter, query *dns.Msg) {
+
+	resp := f.handleDNSQuery(w, query)
+	if resp == nil {
+		return
+	}
+
+	opt := query.IsEdns0()
+	maxSize := dns.MinMsgSize
+	if opt != nil {
+		// client advertised a larger EDNS0 buffer
+		maxSize = int(opt.UDPSize())
+	}
+
+	// if our response is too big, truncate and set the TC bit
+	if resp.Len() > maxSize {
+		resp.Truncate(maxSize)
+	}
+
+	if err := w.WriteMsg(resp); err != nil {
+		log.Errorf("failed to write DNS response: %v", err)
+	}
+}
+
+func (f *DNSForwarder) handleDNSQueryTCP(w dns.ResponseWriter, query *dns.Msg) {
+	resp := f.handleDNSQuery(w, query)
+	if resp == nil {
+		return
+	}
+
 	if err := w.WriteMsg(resp); err != nil {
 		log.Errorf("failed to write DNS response: %v", err)
 	}
@@ -179,7 +243,7 @@ func (f *DNSForwarder) updateFirewall(matchingEntries []*ForwarderEntry, prefixe
 }
 
 // handleDNSError processes DNS lookup errors and sends an appropriate error response
-func (f *DNSForwarder) handleDNSError(w dns.ResponseWriter, resp *dns.Msg, domain string, err error) {
+func (f *DNSForwarder) handleDNSError(w dns.ResponseWriter, query, resp *dns.Msg, domain string, err error) {
 	var dnsErr *net.DNSError
 
 	switch {
@@ -191,7 +255,7 @@ func (f *DNSForwarder) handleDNSError(w dns.ResponseWriter, resp *dns.Msg, domai
 		}
 
 		if dnsErr.Server != "" {
-			log.Warnf("failed to resolve query for domain=%s server=%s: %v", domain, dnsErr.Server, err)
+			log.Warnf("failed to resolve query for type=%s domain=%s server=%s: %v", dns.TypeToString[query.Question[0].Qtype], domain, dnsErr.Server, err)
 		} else {
 			log.Warnf(errResolveFailed, domain, err)
 		}

client/internal/dnsfwd/manager.go

@@ -33,6 +33,7 @@ type Manager struct {
 	statusRecorder *peer.Status
 
 	fwRules      []firewall.Rule
+	tcpRules     []firewall.Rule
 	dnsForwarder *DNSForwarder
 }
 
@@ -107,6 +108,13 @@ func (m *Manager) allowDNSFirewall() error {
 	}
 	m.fwRules = dnsRules
 
+	tcpRules, err := m.firewall.AddPeerFiltering(nil, net.IP{0, 0, 0, 0}, firewall.ProtocolTCP, nil, dport, firewall.ActionAccept, "")
+	if err != nil {
+		log.Errorf("failed to add allow DNS router rules, err: %v", err)
+		return err
+	}
+	m.tcpRules = tcpRules
+
 	return nil
 }
 
@@ -117,7 +125,13 @@ func (m *Manager) dropDNSFirewall() error {
 			mErr = multierror.Append(mErr, fmt.Errorf("failed to delete DNS router rules, err: %v", err))
 		}
 	}
+	for _, rule := range m.tcpRules {
+		if err := m.firewall.DeletePeerRule(rule); err != nil {
+			mErr = multierror.Append(mErr, fmt.Errorf("failed to delete DNS router rules, err: %v", err))
+		}
+	}
 
 	m.fwRules = nil
+	m.tcpRules = nil
 	return nberrors.FormatErrorOrNil(mErr)
 }

client/internal/networkmonitor/check_change_bsd.go

@@ -19,7 +19,7 @@ import (
 func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop) error {
 	fd, err := unix.Socket(syscall.AF_ROUTE, syscall.SOCK_RAW, syscall.AF_UNSPEC)
 	if err != nil {
-		return fmt.Errorf("failed to open routing socket: %v", err)
+		return fmt.Errorf("open routing socket: %v", err)
 	}
 	defer func() {
 		err := unix.Close(fd)

client/internal/networkmonitor/check_change_windows.go

@@ -13,7 +13,7 @@ import (
 func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop) error {
 	routeMonitor, err := systemops.NewRouteMonitor(ctx)
 	if err != nil {
-		return fmt.Errorf("failed to create route monitor: %w", err)
+		return fmt.Errorf("create route monitor: %w", err)
 	}
 	defer func() {
 		if err := routeMonitor.Stop(); err != nil {
@@ -38,35 +38,49 @@ func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop) er
 }
 
 func routeChanged(route systemops.RouteUpdate, nexthopv4, nexthopv6 systemops.Nexthop) bool {
-	intf := "<nil>"
-	if route.Interface != nil {
-		intf = route.Interface.Name
-		if isSoftInterface(intf) {
-			log.Debugf("Network monitor: ignoring default route change for soft interface %s", intf)
-			return false
-		}
+	if intf := route.NextHop.Intf; intf != nil && isSoftInterface(intf.Name) {
+		log.Debugf("Network monitor: ignoring default route change for next hop with soft interface %s", route.NextHop)
+		return false
+	}
+
+	// TODO: for the empty nexthop ip (on-link), determine the family differently
+	nexthop := nexthopv4
+	if route.NextHop.IP.Is6() {
+		nexthop = nexthopv6
 	}
 
 	switch route.Type {
-	case systemops.RouteModified:
-		// TODO: get routing table to figure out if our route is affected for modified routes
-		log.Infof("Network monitor: default route changed: via %s, interface %s", route.NextHop, intf)
-		return true
-	case systemops.RouteAdded:
-		if route.NextHop.Is4() && route.NextHop != nexthopv4.IP || route.NextHop.Is6() && route.NextHop != nexthopv6.IP {
-			log.Infof("Network monitor: default route added: via %s, interface %s", route.NextHop, intf)
-			return true
-		}
+	case systemops.RouteModified, systemops.RouteAdded:
+		return handleRouteAddedOrModified(route, nexthop)
 	case systemops.RouteDeleted:
-		if nexthopv4.Intf != nil && route.NextHop == nexthopv4.IP || nexthopv6.Intf != nil && route.NextHop == nexthopv6.IP {
-			log.Infof("Network monitor: default route removed: via %s, interface %s", route.NextHop, intf)
-			return true
-		}
+		return handleRouteDeleted(route, nexthop)
 	}
 
 	return false
 }
 
+func handleRouteAddedOrModified(route systemops.RouteUpdate, nexthop systemops.Nexthop) bool {
+	// For added/modified routes, we care about different next hops
+	if !nexthop.Equal(route.NextHop) {
+		action := "changed"
+		if route.Type == systemops.RouteAdded {
+			action = "added"
+		}
+		log.Infof("Network monitor: default route %s: via %s", action, route.NextHop)
+		return true
+	}
+	return false
+}
+
+func handleRouteDeleted(route systemops.RouteUpdate, nexthop systemops.Nexthop) bool {
+	// For deleted routes, we care about our tracked next hop being deleted
+	if nexthop.Equal(route.NextHop) {
+		log.Infof("Network monitor: default route removed: via %s", route.NextHop)
+		return true
+	}
+	return false
+}
+
 func isSoftInterface(name string) bool {
 	return strings.Contains(strings.ToLower(name), "isatap") || strings.Contains(strings.ToLower(name), "teredo")
 }

client/internal/networkmonitor/check_change_windows_test.go

@@ -0,0 +1,404 @@
+package networkmonitor
+
+import (
+	"net"
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/netbirdio/netbird/client/internal/routemanager/systemops"
+)
+
+func TestRouteChanged(t *testing.T) {
+	tests := []struct {
+		name      string
+		route     systemops.RouteUpdate
+		nexthopv4 systemops.Nexthop
+		nexthopv6 systemops.Nexthop
+		expected  bool
+	}{
+		{
+			name: "soft interface should be ignored",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Name: "ISATAP-Interface", // isSoftInterface checks name
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.2"),
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: false,
+		},
+		{
+			name: "modified route with different v4 nexthop IP should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.2"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: true,
+		},
+		{
+			name: "modified route with same v4 nexthop (IP and Intf Index) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: false,
+		},
+		{
+			name: "added route with different v6 nexthop IP should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteAdded,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("2001:db8::2"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "added route with same v6 nexthop (IP and Intf Index) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteAdded,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "deleted route matching tracked v4 nexthop (IP and Intf Index) should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: true,
+		},
+		{
+			name: "deleted route not matching tracked v4 nexthop (different IP) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.3"), // Different IP
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: false,
+		},
+		{
+			name: "modified v4 route with same IP, different Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "modified v4 route with same IP, one Intf nil, other non-nil should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: nil, // Intf is nil
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"}, // Tracked Intf is not nil
+			},
+			expected: true,
+		},
+		{
+			name: "added v4 route with same IP, different Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteAdded,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "deleted v4 route with same IP, different Intf Index should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{ // This is the route being deleted
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv4: systemops.Nexthop{ // This is our tracked nexthop
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+			},
+			expected: false, // Because nexthopv4.Equal(route.NextHop) will be false
+		},
+		{
+			name: "modified v6 route with different IP, same Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::3"), // Different IP
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "modified v6 route with same IP, different Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "modified v6 route with same IP, same Intf Index should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: false,
+		},
+		{
+			name: "deleted v6 route matching tracked nexthop (IP and Intf Index) should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "deleted v6 route not matching tracked nexthop (different IP) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::3"), // Different IP
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: false,
+		},
+		{
+			name: "deleted v6 route not matching tracked nexthop (same IP, different Intf Index) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{ // This is the route being deleted
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{ // This is our tracked nexthop
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+			},
+			expected: false,
+		},
+		{
+			name: "unknown route type should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteUpdateType(99), // Unknown type
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.2"), // Different from route.NextHop
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := routeChanged(tt.route, tt.nexthopv4, tt.nexthopv6)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsSoftInterface(t *testing.T) {
+	tests := []struct {
+		name     string
+		ifname   string
+		expected bool
+	}{
+		{
+			name:     "ISATAP interface should be detected",
+			ifname:   "ISATAP tunnel adapter",
+			expected: true,
+		},
+		{
+			name:     "lowercase soft interface should be detected",
+			ifname:   "isatap.{14A5CF17-CA72-43EC-B4EA-B4B093641B7D}",
+			expected: true,
+		},
+		{
+			name:     "Teredo interface should be detected",
+			ifname:   "Teredo Tunneling Pseudo-Interface",
+			expected: true,
+		},
+		{
+			name:     "regular interface should not be detected as soft",
+			ifname:   "eth0",
+			expected: false,
+		},
+		{
+			name:     "another regular interface should not be detected as soft",
+			ifname:   "wlan0",
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := isSoftInterface(tt.ifname)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

client/internal/networkmonitor/monitor.go

@@ -118,9 +118,12 @@ func (nw *NetworkMonitor) Stop() {
 }
 
 func (nw *NetworkMonitor) checkChanges(ctx context.Context, event chan struct{}, nexthop4 systemops.Nexthop, nexthop6 systemops.Nexthop) {
+	defer close(event)
 	for {
 		if err := checkChangeFn(ctx, nexthop4, nexthop6); err != nil {
-			close(event)
+			if !errors.Is(err, context.Canceled) {
+				log.Errorf("Network monitor: failed to check for changes: %v", err)
+			}
 			return
 		}
 		// prevent blocking

client/internal/routemanager/systemops/systemops.go

@@ -1,6 +1,7 @@
 package systemops
 
 import (
+	"fmt"
 	"net"
 	"net/netip"
 	"sync"
@@ -15,6 +16,20 @@ type Nexthop struct {
 	Intf *net.Interface
 }
 
+// Equal checks if two nexthops are equal.
+func (n Nexthop) Equal(other Nexthop) bool {
+	return n.IP == other.IP && (n.Intf == nil && other.Intf == nil ||
+		n.Intf != nil && other.Intf != nil && n.Intf.Index == other.Intf.Index)
+}
+
+// String returns a string representation of the nexthop.
+func (n Nexthop) String() string {
+	if n.Intf == nil {
+		return n.IP.String()
+	}
+	return fmt.Sprintf("%s @ %d (%s)", n.IP.String(), n.Intf.Index, n.Intf.Name)
+}
+
 type ExclusionCounter = refcounter.Counter[netip.Prefix, struct{}, Nexthop]
 
 type SysOps struct {

client/internal/routemanager/systemops/systemops_windows.go

@@ -33,8 +33,7 @@ type RouteUpdateType int
 type RouteUpdate struct {
 	Type        RouteUpdateType
 	Destination netip.Prefix
-	NextHop     netip.Addr
-	Interface   *net.Interface
+	NextHop     Nexthop
 }
 
 // RouteMonitor provides a way to monitor changes in the routing table.
@@ -231,15 +230,15 @@ func (rm *RouteMonitor) parseUpdate(row *MIB_IPFORWARD_ROW2, notificationType MI
 		intf, err := net.InterfaceByIndex(idx)
 		if err != nil {
 			log.Warnf("failed to get interface name for index %d: %v", idx, err)
-			update.Interface = &net.Interface{
+			update.NextHop.Intf = &net.Interface{
 				Index: idx,
 			}
 		} else {
-			update.Interface = intf
+			update.NextHop.Intf = intf
 		}
 	}
 
-	log.Tracef("Received route update with destination %v, next hop %v, interface %v", row.DestinationPrefix, row.NextHop, update.Interface)
+	log.Tracef("Received route update with destination %v, next hop %v, interface %v", row.DestinationPrefix, row.NextHop, update.NextHop.Intf)
 	dest := parseIPPrefix(row.DestinationPrefix, idx)
 	if !dest.Addr().IsValid() {
 		return RouteUpdate{}, fmt.Errorf("invalid destination: %v", row)
@@ -262,7 +261,7 @@ func (rm *RouteMonitor) parseUpdate(row *MIB_IPFORWARD_ROW2, notificationType MI
 
 	update.Type = updateType
 	update.Destination = dest
-	update.NextHop = nexthop
+	update.NextHop.IP = nexthop
 
 	return update, nil
 }

management/server/account.go

@@ -603,11 +603,15 @@ func (am *DefaultAccountManager) DeleteAccount(ctx context.Context, accountID, u
 	}
 
 	for _, otherUser := range account.Users {
-		if otherUser.IsServiceUser {
+		if otherUser.Id == userID {
 			continue
 		}
 
-		if otherUser.Id == userID {
+		if otherUser.IsServiceUser {
+			err = am.deleteServiceUser(ctx, accountID, userID, otherUser)
+			if err != nil {
+				return err
+			}
 			continue
 		}
 

management/server/account_test.go

@@ -853,6 +853,42 @@ func TestAccountManager_DeleteAccount(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	account.Users["service-user-1"] = &types.User{
+		Id:            "service-user-1",
+		Role:          types.UserRoleAdmin,
+		IsServiceUser: true,
+		Issued:        types.UserIssuedAPI,
+		PATs: map[string]*types.PersonalAccessToken{
+			"pat-1": {
+				ID:          "pat-1",
+				UserID:      "service-user-1",
+				Name:        "service-user-1",
+				HashedToken: "hashedToken",
+				CreatedAt:   time.Now(),
+			},
+		},
+	}
+	account.Users[userId] = &types.User{
+		Id:            "service-user-2",
+		Role:          types.UserRoleUser,
+		IsServiceUser: true,
+		Issued:        types.UserIssuedAPI,
+		PATs: map[string]*types.PersonalAccessToken{
+			"pat-2": {
+				ID:          "pat-2",
+				UserID:      userId,
+				Name:        userId,
+				HashedToken: "hashedToken",
+				CreatedAt:   time.Now(),
+			},
+		},
+	}
+
+	err = manager.Store.SaveAccount(context.Background(), account)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	err = manager.DeleteAccount(context.Background(), account.Id, userId)
 	if err != nil {
 		t.Fatal(err)
@@ -862,6 +898,14 @@ func TestAccountManager_DeleteAccount(t *testing.T) {
 	if err == nil {
 		t.Fatal(fmt.Errorf("expected to get an error when trying to get deleted account, got %v", getAccount))
 	}
+
+	pats, err := manager.Store.GetUserPATs(context.Background(), store.LockingStrengthShare, "service-user-1")
+	require.NoError(t, err)
+	assert.Len(t, pats, 0)
+
+	pats, err = manager.Store.GetUserPATs(context.Background(), store.LockingStrengthShare, userId)
+	require.NoError(t, err)
+	assert.Len(t, pats, 0)
 }
 
 func BenchmarkTest_GetAccountWithclaims(b *testing.B) {

management/server/store/sql_store.go

@@ -1683,18 +1683,26 @@ func (s *SqlStore) SavePolicy(ctx context.Context, lockStrength LockingStrength,
 }
 
 func (s *SqlStore) DeletePolicy(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) error {
-	result := s.db.Clauses(clause.Locking{Strength: string(lockStrength)}).
-		Delete(&types.Policy{}, accountAndIDQueryCondition, accountID, policyID)
-	if err := result.Error; err != nil {
-		log.WithContext(ctx).Errorf("failed to delete policy from store: %s", err)
-		return status.Errorf(status.Internal, "failed to delete policy from store")
-	}
+	return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+		if err := tx.Where("policy_id = ?", policyID).Delete(&types.PolicyRule{}).Error; err != nil {
+			return fmt.Errorf("delete policy rules: %w", err)
+		}
 
-	if result.RowsAffected == 0 {
-		return status.NewPolicyNotFoundError(policyID)
-	}
+		result := tx.Clauses(clause.Locking{Strength: string(lockStrength)}).
+			Where(accountAndIDQueryCondition, accountID, policyID).
+			Delete(&types.Policy{})
 
-	return nil
+		if err := result.Error; err != nil {
+			log.WithContext(ctx).Errorf("failed to delete policy from store: %s", err)
+			return status.Errorf(status.Internal, "failed to delete policy from store")
+		}
+
+		if result.RowsAffected == 0 {
+			return status.NewPolicyNotFoundError(policyID)
+		}
+
+		return nil
+	})
 }
 
 // GetAccountPostureChecks retrieves posture checks for an account.

sharedsock/sock_linux.go

@@ -234,7 +234,7 @@ func (s *SharedSocket) read(receiver receiver) {
 }
 
 // ReadFrom reads packets received in the packetDemux channel
-func (s *SharedSocket) ReadFrom(b []byte) (n int, addr net.Addr, err error) {
+func (s *SharedSocket) ReadFrom(b []byte) (int, net.Addr, error) {
 	var pkt rcvdPacket
 	select {
 	case <-s.ctx.Done():
@@ -263,8 +263,7 @@ func (s *SharedSocket) ReadFrom(b []byte) (n int, addr net.Addr, err error) {
 
 	decodedLayers := make([]gopacket.LayerType, 0, 3)
 
-	err = parser.DecodeLayers(pkt.buf, &decodedLayers)
-	if err != nil {
+	if err := parser.DecodeLayers(pkt.buf, &decodedLayers); err != nil {
 		return 0, nil, err
 	}
 
@@ -273,8 +272,8 @@ func (s *SharedSocket) ReadFrom(b []byte) (n int, addr net.Addr, err error) {
 		Port: int(udp.SrcPort),
 	}
 
-	copy(b, payload)
-	return int(udp.Length), remoteAddr, nil
+	n := copy(b, payload)
+	return n, remoteAddr, nil
 }
 
 // WriteTo builds a UDP packet and writes it using the specific IP version writer