From cad2fe1f39f7ce850692feb4a857477717325337 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Fri, 9 May 2025 13:56:27 +0200 Subject: [PATCH 1/3] Return with the correct copied length (#3804) --- sharedsock/sock_linux.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sharedsock/sock_linux.go b/sharedsock/sock_linux.go index 74ac6c163..1c22e7869 100644 --- a/sharedsock/sock_linux.go +++ b/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 From d5b52e86b6386e7dab2d370d9f49da7e96893e3b Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 9 May 2025 14:01:21 +0200 Subject: [PATCH 2/3] [client] Ignore irrelevant route changes to tracked network monitor routes (#3796) --- .../networkmonitor/check_change_bsd.go | 2 +- .../networkmonitor/check_change_windows.go | 56 ++- .../check_change_windows_test.go | 404 ++++++++++++++++++ client/internal/networkmonitor/monitor.go | 5 +- .../routemanager/systemops/systemops.go | 15 + .../systemops/systemops_windows.go | 11 +- 6 files changed, 464 insertions(+), 29 deletions(-) create mode 100644 client/internal/networkmonitor/check_change_windows_test.go diff --git a/client/internal/networkmonitor/check_change_bsd.go b/client/internal/networkmonitor/check_change_bsd.go index bb327a877..f5eb2c739 100644 --- a/client/internal/networkmonitor/check_change_bsd.go +++ b/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) diff --git a/client/internal/networkmonitor/check_change_windows.go b/client/internal/networkmonitor/check_change_windows.go index 582865738..814584863 100644 --- a/client/internal/networkmonitor/check_change_windows.go +++ b/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 := "" - 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") } diff --git a/client/internal/networkmonitor/check_change_windows_test.go b/client/internal/networkmonitor/check_change_windows_test.go new file mode 100644 index 000000000..29ff34dca --- /dev/null +++ b/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) + }) + } +} diff --git a/client/internal/networkmonitor/monitor.go b/client/internal/networkmonitor/monitor.go index 5896b66b6..accdd9c9d 100644 --- a/client/internal/networkmonitor/monitor.go +++ b/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 diff --git a/client/internal/routemanager/systemops/systemops.go b/client/internal/routemanager/systemops/systemops.go index 5c117b94d..fd511fc20 100644 --- a/client/internal/routemanager/systemops/systemops.go +++ b/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 { diff --git a/client/internal/routemanager/systemops/systemops_windows.go b/client/internal/routemanager/systemops/systemops_windows.go index ad325e123..f66161595 100644 --- a/client/internal/routemanager/systemops/systemops_windows.go +++ b/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 } From 2f34e984b0051c5b0ad1992d88b93c62165bf765 Mon Sep 17 00:00:00 2001 From: hakansa <43675540+hakansa@users.noreply.github.com> Date: Fri, 9 May 2025 15:06:34 +0300 Subject: [PATCH 3/3] [client] Add TCP support to DNS forwarder service listener (#3790) [client] Add TCP support to DNS forwarder service listener --- client/internal/dnsfwd/forwarder.go | 104 ++++++++++++++++++++++------ client/internal/dnsfwd/manager.go | 14 ++++ 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/client/internal/dnsfwd/forwarder.go b/client/internal/dnsfwd/forwarder.go index 8f6a31f47..45b479632 100644 --- a/client/internal/dnsfwd/forwarder.go +++ b/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) } diff --git a/client/internal/dnsfwd/manager.go b/client/internal/dnsfwd/manager.go index e4a23450f..91abce823 100644 --- a/client/internal/dnsfwd/manager.go +++ b/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) }