From 506b459842d47e1536548fb10441aef1e534bf14 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Thu, 12 Aug 2021 17:33:53 +0300 Subject: [PATCH] Pull request: 3225 bsd dhcp Merge in DNS/adguard-home from 3225-bsd-dhcp to master Closes #3225. Closes #3417. Squashed commit of the following: commit e7ea691824c7ebc8cafd8c9e206679346cbc8592 Author: Eugene Burkov Date: Thu Aug 12 17:02:02 2021 +0300 all: imp code, docs commit 5b598fc18a9b69a0256569f4c691bb6a2193dfbd Author: Eugene Burkov Date: Thu Aug 12 16:28:12 2021 +0300 all: mv logic, imp code, docs, log changes commit e3e1577a668fe3e5c61d075c390e4bd7268181ba Author: Eugene Burkov Date: Thu Aug 12 14:15:10 2021 +0300 dhcpd: imp checkother commit 3cc8b058195c30a7ef0b7741ee8463270d9e47ff Author: Eugene Burkov Date: Wed Aug 11 13:20:18 2021 +0300 all: imp bsd support --- CHANGELOG.md | 5 +- internal/aghnet/dhcp.go | 6 + .../checkother.go => aghnet/dhcp_unix.go} | 202 +++++++++++------- internal/aghnet/dhcp_windows.go | 13 ++ internal/aghnet/interfaces.go | 118 ++++++++++ .../v46_test.go => aghnet/interfaces_test.go} | 46 ++-- internal/aghnet/net.go | 16 ++ internal/aghnet/net_darwin.go | 11 +- internal/aghnet/net_linux.go | 1 - internal/aghnet/net_openbsd.go | 70 ++++++ internal/aghnet/net_openbsd_test.go | 52 +++++ internal/aghnet/net_others.go | 4 +- internal/aghnet/net_test.go | 50 +++++ internal/dhcpd/checkother_windows.go | 14 -- internal/dhcpd/dhcpd.go | 1 - internal/dhcpd/helpers.go | 8 - internal/dhcpd/http.go | 41 ++-- internal/dhcpd/v4.go | 14 +- internal/dhcpd/v46.go | 113 ---------- internal/dhcpd/v6.go | 8 +- scripts/make/go-lint.sh | 1 + 21 files changed, 522 insertions(+), 272 deletions(-) create mode 100644 internal/aghnet/dhcp.go rename internal/{dhcpd/checkother.go => aghnet/dhcp_unix.go} (60%) create mode 100644 internal/aghnet/dhcp_windows.go create mode 100644 internal/aghnet/interfaces.go rename internal/{dhcpd/v46_test.go => aghnet/interfaces_test.go} (84%) create mode 100644 internal/aghnet/net_openbsd.go create mode 100644 internal/aghnet/net_openbsd_test.go delete mode 100644 internal/dhcpd/checkother_windows.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 58371b2d..9d07fc3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ and this project adheres to - Settable timeouts for querying the upstream servers ([#2280]). - Configuration file parameters to change group and user ID on startup on Unix ([#2763]). -- Experimental OpenBSD support for AMD64 and 64-bit ARM CPUs ([#2439]). +- Experimental OpenBSD support for AMD64 and 64-bit ARM CPUs ([#2439], [#3225]). - Support for custom port in DNS-over-HTTPS profiles for Apple's devices ([#3172]). - `darwin/arm64` support ([#2443]). @@ -64,6 +64,7 @@ and this project adheres to ### Fixed +- Discovering other DHCP servers on `darwin` and `freebsd` ([#3417]). - Switching listening address to unspecified one when bound to a single specified IPv4 address on Darwin (macOS) ([#2807]). - Incomplete HTTP response for static IP address. @@ -112,6 +113,7 @@ and this project adheres to [#3194]: https://github.com/AdguardTeam/AdGuardHome/issues/3194 [#3198]: https://github.com/AdguardTeam/AdGuardHome/issues/3198 [#3217]: https://github.com/AdguardTeam/AdGuardHome/issues/3217 +[#3225]: https://github.com/AdguardTeam/AdGuardHome/issues/3225 [#3256]: https://github.com/AdguardTeam/AdGuardHome/issues/3256 [#3257]: https://github.com/AdguardTeam/AdGuardHome/issues/3257 [#3289]: https://github.com/AdguardTeam/AdGuardHome/issues/3289 @@ -119,6 +121,7 @@ and this project adheres to [#3343]: https://github.com/AdguardTeam/AdGuardHome/issues/3343 [#3351]: https://github.com/AdguardTeam/AdGuardHome/issues/3351 [#3372]: https://github.com/AdguardTeam/AdGuardHome/issues/3372 +[#3417]: https://github.com/AdguardTeam/AdGuardHome/issues/3417 diff --git a/internal/aghnet/dhcp.go b/internal/aghnet/dhcp.go new file mode 100644 index 00000000..327a5656 --- /dev/null +++ b/internal/aghnet/dhcp.go @@ -0,0 +1,6 @@ +package aghnet + +// CheckOtherDHCP tries to discover another DHCP server in the network. +func CheckOtherDHCP(ifaceName string) (ok4, ok6 bool, err4, err6 error) { + return checkOtherDHCP(ifaceName) +} diff --git a/internal/dhcpd/checkother.go b/internal/aghnet/dhcp_unix.go similarity index 60% rename from internal/dhcpd/checkother.go rename to internal/aghnet/dhcp_unix.go index 39e60955..a4a3820d 100644 --- a/internal/dhcpd/checkother.go +++ b/internal/aghnet/dhcp_unix.go @@ -1,95 +1,132 @@ //go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris // +build aix darwin dragonfly freebsd linux netbsd openbsd solaris -package dhcpd +package aghnet import ( "bytes" "fmt" "net" "os" - "runtime" "time" - "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" "github.com/insomniacslk/dhcp/dhcpv4" - "github.com/insomniacslk/dhcp/dhcpv4/nclient4" "github.com/insomniacslk/dhcp/dhcpv6" "github.com/insomniacslk/dhcp/dhcpv6/nclient6" "github.com/insomniacslk/dhcp/iana" ) -// CheckIfOtherDHCPServersPresentV4 sends a DHCP request to the specified network interface, -// and waits for a response for a period defined by defaultDiscoverTime -func CheckIfOtherDHCPServersPresentV4(ifaceName string) (ok bool, err error) { +// defaultDiscoverTime is the +const defaultDiscoverTime = 3 * time.Second + +func checkOtherDHCP(ifaceName string) (ok4, ok6 bool, err4, err6 error) { iface, err := net.InterfaceByName(ifaceName) if err != nil { - return false, fmt.Errorf("couldn't find interface by name %s: %w", ifaceName, err) + err = fmt.Errorf("couldn't find interface by name %s: %w", ifaceName, err) + err4, err6 = err, err + + return false, false, err4, err6 } - ifaceIPNet, err := ifaceIPAddrs(iface, ipVersion4) - if err != nil { - return false, fmt.Errorf("getting ipv4 addrs for iface %s: %w", ifaceName, err) - } - if len(ifaceIPNet) == 0 { - return false, fmt.Errorf("interface %s has no ipv4 addresses", ifaceName) + ok4, err4 = checkOtherDHCPv4(iface) + ok6, err6 = checkOtherDHCPv6(iface) + + return ok4, ok6, err4, err6 +} + +// ifaceIPv4Subnet returns the first suitable IPv4 subnetwork iface has. +func ifaceIPv4Subnet(iface *net.Interface) (subnet *net.IPNet, err error) { + var addrs []net.Addr + if addrs, err = iface.Addrs(); err != nil { + return nil, err } - // TODO(a.garipov): Find out what this is about. Perhaps this - // information is outdated or at least incomplete. - if runtime.GOOS == "darwin" { - return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV4") + for _, a := range addrs { + switch a := a.(type) { + case *net.IPAddr: + subnet = &net.IPNet{ + IP: a.IP, + Mask: a.IP.DefaultMask(), + } + case *net.IPNet: + subnet = a + default: + continue + } + + if ip4 := subnet.IP.To4(); ip4 != nil { + subnet.IP = ip4 + + return subnet, nil + } } - srcIP := ifaceIPNet[0] - src := netutil.JoinHostPort(srcIP.String(), 68) - dst := "255.255.255.255:67" + return nil, fmt.Errorf("interface %s has no ipv4 addresses", iface.Name) +} - hostname, _ := os.Hostname() - - req, err := dhcpv4.NewDiscovery(iface.HardwareAddr) - if err != nil { - return false, fmt.Errorf("dhcpv4.NewDiscovery: %w", err) - } - req.Options.Update(dhcpv4.OptClientIdentifier(iface.HardwareAddr)) - req.Options.Update(dhcpv4.OptHostName(hostname)) - - // resolve 0.0.0.0:68 - udpAddr, err := net.ResolveUDPAddr("udp4", src) - if err != nil { - return false, fmt.Errorf("couldn't resolve UDP address %s: %w", src, err) +// checkOtherDHCPv4 sends a DHCP request to the specified network interface, and +// waits for a response for a period defined by defaultDiscoverTime. +func checkOtherDHCPv4(iface *net.Interface) (ok bool, err error) { + var subnet *net.IPNet + if subnet, err = ifaceIPv4Subnet(iface); err != nil { + return false, err } - if !udpAddr.IP.To4().Equal(srcIP) { - return false, fmt.Errorf("resolved UDP address is not %s: %w", src, err) - } - - // resolve 255.255.255.255:67 - dstAddr, err := net.ResolveUDPAddr("udp4", dst) - if err != nil { + // Resolve broadcast addr. + dst := netutil.IPPort{ + IP: BroadcastFromIPNet(subnet), + Port: 67, + }.String() + var dstAddr *net.UDPAddr + if dstAddr, err = net.ResolveUDPAddr("udp4", dst); err != nil { return false, fmt.Errorf("couldn't resolve UDP address %s: %w", dst, err) } - // bind to 0.0.0.0:68 - log.Tracef("Listening to udp4 %+v", udpAddr) - c, err := nclient4.NewRawUDPConn(ifaceName, 68) - if err != nil { - return false, fmt.Errorf("couldn't listen on :68: %w", err) - } - if c != nil { - defer func() { err = errors.WithDeferred(err, c.Close()) }() + var hostname string + if hostname, err = os.Hostname(); err != nil { + return false, fmt.Errorf("couldn't get hostname: %w", err) } - // send to 255.255.255.255:67 - _, err = c.WriteTo(req.ToBytes(), dstAddr) - if err != nil { - return false, fmt.Errorf("couldn't send a packet to %s: %w", dst, err) + return discover4(iface, dstAddr, hostname) +} + +func discover4(iface *net.Interface, dstAddr *net.UDPAddr, hostname string) (ok bool, err error) { + var req *dhcpv4.DHCPv4 + if req, err = dhcpv4.NewDiscovery(iface.HardwareAddr); err != nil { + return false, fmt.Errorf("dhcpv4.NewDiscovery: %w", err) + } + + req.Options.Update(dhcpv4.OptClientIdentifier(iface.HardwareAddr)) + req.Options.Update(dhcpv4.OptHostName(hostname)) + req.SetBroadcast() + + // Bind to 0.0.0.0:68. + // + // On OpenBSD binding to the port 68 competes with dhclient's binding, + // so that all incoming packets are ignored and the discovering process + // is spoiled. + // + // It's also known that listening on the specified interface's address + // ignores broadcasted packets when reading. + var c net.PacketConn + if c, err = net.ListenPacket("udp4", ":68"); err != nil { + return false, fmt.Errorf("couldn't listen on :68: %w", err) + } + defer func() { err = errors.WithDeferred(err, c.Close()) }() + + // Send to resolved broadcast. + if _, err = c.WriteTo(req.ToBytes(), dstAddr); err != nil { + return false, fmt.Errorf("couldn't send a packet to %s: %w", dstAddr, err) } for { + if err = c.SetDeadline(time.Now().Add(defaultDiscoverTime)); err != nil { + return false, fmt.Errorf("setting deadline: %w", err) + } + var next bool ok, next, err = tryConn4(req, c, iface) if next { @@ -116,12 +153,10 @@ func tryConn4(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, n log.Tracef("dhcpv4: waiting %v for an answer", defaultDiscoverTime) b := make([]byte, 1500) - err = c.SetDeadline(time.Now().Add(defaultDiscoverTime)) - if err != nil { - return false, false, fmt.Errorf("setting deadline: %w", err) - } - n, _, err := c.ReadFrom(b) + if n > 0 { + log.Debug("received %d bytes: %v", n, b) + } if err != nil { if isTimeout(err) { log.Debug("dhcpv4: didn't receive dhcp response") @@ -159,31 +194,21 @@ func tryConn4(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, n return true, false, nil } -// CheckIfOtherDHCPServersPresentV6 sends a DHCP request to the specified network interface, -// and waits for a response for a period defined by defaultDiscoverTime -func CheckIfOtherDHCPServersPresentV6(ifaceName string) (ok bool, err error) { - iface, err := net.InterfaceByName(ifaceName) +// checkOtherDHCPv6 sends a DHCP request to the specified network interface, and +// waits for a response for a period defined by defaultDiscoverTime. +func checkOtherDHCPv6(iface *net.Interface) (ok bool, err error) { + ifaceIPNet, err := IfaceIPAddrs(iface, IPVersion6) if err != nil { - return false, fmt.Errorf("dhcpv6: net.InterfaceByName: %s: %w", ifaceName, err) - } - - ifaceIPNet, err := ifaceIPAddrs(iface, ipVersion6) - if err != nil { - return false, fmt.Errorf("getting ipv6 addrs for iface %s: %w", ifaceName, err) + return false, fmt.Errorf("getting ipv6 addrs for iface %s: %w", iface.Name, err) } if len(ifaceIPNet) == 0 { - return false, fmt.Errorf("interface %s has no ipv6 addresses", ifaceName) + return false, fmt.Errorf("interface %s has no ipv6 addresses", iface.Name) } srcIP := ifaceIPNet[0] src := netutil.JoinHostPort(srcIP.String(), 546) dst := "[ff02::1:2]:547" - req, err := dhcpv6.NewSolicit(iface.HardwareAddr) - if err != nil { - return false, fmt.Errorf("dhcpv6: dhcpv6.NewSolicit: %w", err) - } - udpAddr, err := net.ResolveUDPAddr("udp6", src) if err != nil { return false, fmt.Errorf("dhcpv6: Couldn't resolve UDP address %s: %w", src, err) @@ -198,18 +223,25 @@ func CheckIfOtherDHCPServersPresentV6(ifaceName string) (ok bool, err error) { return false, fmt.Errorf("dhcpv6: Couldn't resolve UDP address %s: %w", dst, err) } + return discover6(iface, udpAddr, dstAddr) +} + +func discover6(iface *net.Interface, udpAddr, dstAddr *net.UDPAddr) (ok bool, err error) { + req, err := dhcpv6.NewSolicit(iface.HardwareAddr) + if err != nil { + return false, fmt.Errorf("dhcpv6: dhcpv6.NewSolicit: %w", err) + } + log.Debug("DHCPv6: Listening to udp6 %+v", udpAddr) - c, err := nclient6.NewIPv6UDPConn(ifaceName, dhcpv6.DefaultClientPort) + c, err := nclient6.NewIPv6UDPConn(iface.Name, dhcpv6.DefaultClientPort) if err != nil { return false, fmt.Errorf("dhcpv6: Couldn't listen on :546: %w", err) } - if c != nil { - defer func() { err = errors.WithDeferred(err, c.Close()) }() - } + defer func() { err = errors.WithDeferred(err, c.Close()) }() _, err = c.WriteTo(req.ToBytes(), dstAddr) if err != nil { - return false, fmt.Errorf("dhcpv6: Couldn't send a packet to %s: %w", dst, err) + return false, fmt.Errorf("dhcpv6: Couldn't send a packet to %s: %w", dstAddr, err) } for { @@ -288,3 +320,15 @@ func tryConn6(req *dhcpv6.Message, c net.PacketConn) (ok, next bool, err error) return true, false, nil } + +// isTimeout returns true if err is an operation timeout error from net package. +// +// TODO(e.burkov): Consider moving into netutil. +func isTimeout(err error) (ok bool) { + var operr *net.OpError + if errors.As(err, &operr) { + return operr.Timeout() + } + + return false +} diff --git a/internal/aghnet/dhcp_windows.go b/internal/aghnet/dhcp_windows.go new file mode 100644 index 00000000..6d1ba231 --- /dev/null +++ b/internal/aghnet/dhcp_windows.go @@ -0,0 +1,13 @@ +//go:build windows +// +build windows + +package aghnet + +import "github.com/AdguardTeam/AdGuardHome/internal/aghos" + +func checkOtherDHCP(ifaceName string) (ok4, ok6 bool, err4, err6 error) { + return false, + false, + aghos.Unsupported("CheckIfOtherDHCPServersPresentV4"), + aghos.Unsupported("CheckIfOtherDHCPServersPresentV6") +} diff --git a/internal/aghnet/interfaces.go b/internal/aghnet/interfaces.go new file mode 100644 index 00000000..ce7f217c --- /dev/null +++ b/internal/aghnet/interfaces.go @@ -0,0 +1,118 @@ +package aghnet + +import ( + "fmt" + "net" + "time" + + "github.com/AdguardTeam/golibs/log" +) + +// IPVersion is a documentational alias for int. Use it when the integer means +// IP version. +type IPVersion = int + +// IP version constants. +const ( + IPVersion4 IPVersion = 4 + IPVersion6 IPVersion = 6 +) + +// NetIface is the interface for network interface methods. +type NetIface interface { + Addrs() ([]net.Addr, error) +} + +// IfaceIPAddrs returns the interface's IP addresses. +func IfaceIPAddrs(iface NetIface, ipv IPVersion) (ips []net.IP, err error) { + addrs, err := iface.Addrs() + if err != nil { + return nil, err + } + + for _, a := range addrs { + var ip net.IP + switch a := a.(type) { + case *net.IPAddr: + ip = a.IP + case *net.IPNet: + ip = a.IP + default: + continue + } + + // Assume that net.(*Interface).Addrs can only return valid IPv4 + // and IPv6 addresses. Thus, if it isn't an IPv4 address, it + // must be an IPv6 one. + switch ipv { + case IPVersion4: + if ip4 := ip.To4(); ip4 != nil { + ips = append(ips, ip4) + } + case IPVersion6: + if ip6 := ip.To4(); ip6 == nil { + ips = append(ips, ip) + } + default: + return nil, fmt.Errorf("invalid ip version %d", ipv) + } + } + + return ips, nil +} + +// IfaceDNSIPAddrs returns IP addresses of the interface suitable to send to +// clients as DNS addresses. If err is nil, addrs contains either no addresses +// or at least two. +// +// It makes up to maxAttempts attempts to get the addresses if there are none, +// each time using the provided backoff. Sometimes an interface needs a few +// seconds to really ititialize. +// +// See https://github.com/AdguardTeam/AdGuardHome/issues/2304. +func IfaceDNSIPAddrs( + iface NetIface, + ipv IPVersion, + maxAttempts int, + backoff time.Duration, +) (addrs []net.IP, err error) { + var n int + for n = 1; n <= maxAttempts; n++ { + addrs, err = IfaceIPAddrs(iface, ipv) + if err != nil { + return nil, fmt.Errorf("getting ip addrs: %w", err) + } + + if len(addrs) > 0 { + break + } + + log.Debug("dhcpv%d: attempt %d: no ip addresses", ipv, n) + + time.Sleep(backoff) + } + + switch len(addrs) { + case 0: + // Don't return errors in case the users want to try and enable + // the DHCP server later. + t := time.Duration(n) * backoff + log.Error("dhcpv%d: no ip for iface after %d attempts and %s", ipv, n, t) + + return nil, nil + case 1: + // Some Android devices use 8.8.8.8 if there is not a secondary + // DNS server. Fix that by setting the secondary DNS address to + // the same address. + // + // See https://github.com/AdguardTeam/AdGuardHome/issues/1708. + log.Debug("dhcpv%d: setting secondary dns ip to itself", ipv) + addrs = append(addrs, addrs[0]) + default: + // Go on. + } + + log.Debug("dhcpv%d: got addresses %s after %d attempts", ipv, addrs, n) + + return addrs, nil +} diff --git a/internal/dhcpd/v46_test.go b/internal/aghnet/interfaces_test.go similarity index 84% rename from internal/dhcpd/v46_test.go rename to internal/aghnet/interfaces_test.go index a07ccff3..2b70429c 100644 --- a/internal/dhcpd/v46_test.go +++ b/internal/aghnet/interfaces_test.go @@ -1,4 +1,4 @@ -package dhcpd +package aghnet import ( "net" @@ -14,7 +14,7 @@ type fakeIface struct { err error } -// Addrs implements the netIface interface for *fakeIface. +// Addrs implements the NetIface interface for *fakeIface. func (iface *fakeIface) Addrs() (addrs []net.Addr, err error) { if iface.err != nil { return nil, iface.err @@ -34,51 +34,51 @@ func TestIfaceIPAddrs(t *testing.T) { testCases := []struct { name string - iface netIface - ipv ipVersion + iface NetIface + ipv IPVersion want []net.IP wantErr error }{{ name: "ipv4_success", iface: &fakeIface{addrs: []net.Addr{addr4}, err: nil}, - ipv: ipVersion4, + ipv: IPVersion4, want: []net.IP{ip4}, wantErr: nil, }, { name: "ipv4_success_with_ipv6", iface: &fakeIface{addrs: []net.Addr{addr6, addr4}, err: nil}, - ipv: ipVersion4, + ipv: IPVersion4, want: []net.IP{ip4}, wantErr: nil, }, { name: "ipv4_error", iface: &fakeIface{addrs: []net.Addr{addr4}, err: errTest}, - ipv: ipVersion4, + ipv: IPVersion4, want: nil, wantErr: errTest, }, { name: "ipv6_success", iface: &fakeIface{addrs: []net.Addr{addr6}, err: nil}, - ipv: ipVersion6, + ipv: IPVersion6, want: []net.IP{ip6}, wantErr: nil, }, { name: "ipv6_success_with_ipv4", iface: &fakeIface{addrs: []net.Addr{addr6, addr4}, err: nil}, - ipv: ipVersion6, + ipv: IPVersion6, want: []net.IP{ip6}, wantErr: nil, }, { name: "ipv6_error", iface: &fakeIface{addrs: []net.Addr{addr6}, err: errTest}, - ipv: ipVersion6, + ipv: IPVersion6, want: nil, wantErr: errTest, }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, gotErr := ifaceIPAddrs(tc.iface, tc.ipv) + got, gotErr := IfaceIPAddrs(tc.iface, tc.ipv) require.True(t, errors.Is(gotErr, tc.wantErr)) assert.Equal(t, tc.want, got) }) @@ -91,7 +91,7 @@ type waitingFakeIface struct { n int } -// Addrs implements the netIface interface for *waitingFakeIface. +// Addrs implements the NetIface interface for *waitingFakeIface. func (iface *waitingFakeIface) Addrs() (addrs []net.Addr, err error) { if iface.err != nil { return nil, iface.err @@ -117,63 +117,63 @@ func TestIfaceDNSIPAddrs(t *testing.T) { testCases := []struct { name string - iface netIface - ipv ipVersion + iface NetIface + ipv IPVersion want []net.IP wantErr error }{{ name: "ipv4_success", iface: &fakeIface{addrs: []net.Addr{addr4}, err: nil}, - ipv: ipVersion4, + ipv: IPVersion4, want: []net.IP{ip4, ip4}, wantErr: nil, }, { name: "ipv4_success_with_ipv6", iface: &fakeIface{addrs: []net.Addr{addr6, addr4}, err: nil}, - ipv: ipVersion4, + ipv: IPVersion4, want: []net.IP{ip4, ip4}, wantErr: nil, }, { name: "ipv4_error", iface: &fakeIface{addrs: []net.Addr{addr4}, err: errTest}, - ipv: ipVersion4, + ipv: IPVersion4, want: nil, wantErr: errTest, }, { name: "ipv4_wait", iface: &waitingFakeIface{addrs: []net.Addr{addr4}, err: nil, n: 1}, - ipv: ipVersion4, + ipv: IPVersion4, want: []net.IP{ip4, ip4}, wantErr: nil, }, { name: "ipv6_success", iface: &fakeIface{addrs: []net.Addr{addr6}, err: nil}, - ipv: ipVersion6, + ipv: IPVersion6, want: []net.IP{ip6, ip6}, wantErr: nil, }, { name: "ipv6_success_with_ipv4", iface: &fakeIface{addrs: []net.Addr{addr6, addr4}, err: nil}, - ipv: ipVersion6, + ipv: IPVersion6, want: []net.IP{ip6, ip6}, wantErr: nil, }, { name: "ipv6_error", iface: &fakeIface{addrs: []net.Addr{addr6}, err: errTest}, - ipv: ipVersion6, + ipv: IPVersion6, want: nil, wantErr: errTest, }, { name: "ipv6_wait", iface: &waitingFakeIface{addrs: []net.Addr{addr6}, err: nil, n: 1}, - ipv: ipVersion6, + ipv: IPVersion6, want: []net.IP{ip6, ip6}, wantErr: nil, }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, gotErr := ifaceDNSIPAddrs(tc.iface, tc.ipv, 2, 0) + got, gotErr := IfaceDNSIPAddrs(tc.iface, tc.ipv, 2, 0) require.True(t, errors.Is(gotErr, tc.wantErr)) assert.Equal(t, tc.want, got) }) diff --git a/internal/aghnet/net.go b/internal/aghnet/net.go index 6e68da03..393778ad 100644 --- a/internal/aghnet/net.go +++ b/internal/aghnet/net.go @@ -294,3 +294,19 @@ func CollectAllIfacesAddrs() (addrs []string, err error) { return addrs, nil } + +// BroadcastFromIPNet calculates the broadcast IP address for n. +func BroadcastFromIPNet(n *net.IPNet) (dc net.IP) { + dc = netutil.CloneIP(n.IP) + + mask := n.Mask + if mask == nil { + mask = dc.DefaultMask() + } + + for i, b := range mask { + dc[i] |= ^b + } + + return dc +} diff --git a/internal/aghnet/net_darwin.go b/internal/aghnet/net_darwin.go index 4da5d56c..3c504988 100644 --- a/internal/aghnet/net_darwin.go +++ b/internal/aghnet/net_darwin.go @@ -48,9 +48,14 @@ func getCurrentHardwarePortInfo(ifaceName string) (hardwarePortInfo, error) { return getHardwarePortInfo(hardwarePort) } -// getNetworkSetupHardwareReports parses the output of the `networksetup -listallhardwareports` command -// it returns a map where the key is the interface name, and the value is the "hardware port" -// returns nil if it fails to parse the output +// getNetworkSetupHardwareReports parses the output of the `networksetup +// -listallhardwareports` command it returns a map where the key is the +// interface name, and the value is the "hardware port" returns nil if it fails +// to parse the output +// +// TODO(e.burkov): There should be more proper approach than parsing the +// command output. For example, see +// https://developer.apple.com/documentation/systemconfiguration. func getNetworkSetupHardwareReports() map[string]string { _, out, err := aghos.RunCommand("networksetup", "-listallhardwareports") if err != nil { diff --git a/internal/aghnet/net_linux.go b/internal/aghnet/net_linux.go index a646178d..0bdfbeb7 100644 --- a/internal/aghnet/net_linux.go +++ b/internal/aghnet/net_linux.go @@ -48,7 +48,6 @@ func (rc *recurrentChecker) checkFile(sourcePath, desired string) ( if err != nil { return nil, false, err } - defer func() { err = errors.WithDeferred(err, f.Close()) }() var r io.Reader diff --git a/internal/aghnet/net_openbsd.go b/internal/aghnet/net_openbsd.go new file mode 100644 index 00000000..c62689a9 --- /dev/null +++ b/internal/aghnet/net_openbsd.go @@ -0,0 +1,70 @@ +//go:build openbsd +// +build openbsd + +package aghnet + +import ( + "bufio" + "fmt" + "io" + "net" + "os" + "strings" + + "github.com/AdguardTeam/AdGuardHome/internal/aghio" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" + "github.com/AdguardTeam/golibs/errors" +) + +func canBindPrivilegedPorts() (can bool, err error) { + return aghos.HaveAdminRights() +} + +// maxCheckedFileSize is the maximum acceptable length of the /etc/hostname.* +// files. +const maxCheckedFileSize = 1024 * 1024 + +func ifaceHasStaticIP(ifaceName string) (ok bool, err error) { + const filenameFmt = "/etc/hostname.%s" + + filename := fmt.Sprintf(filenameFmt, ifaceName) + var f *os.File + if f, err = os.Open(filename); err != nil { + if errors.Is(err, os.ErrNotExist) { + err = nil + } + + return false, err + } + defer func() { err = errors.WithDeferred(err, f.Close()) }() + + var r io.Reader + r, err = aghio.LimitReader(f, maxCheckedFileSize) + if err != nil { + return false, err + } + + return hostnameIfStaticConfig(r) +} + +// hostnameIfStaticConfig checks if the interface is configured by +// /etc/hostname.* to have a static IP. +// +// TODO(e.burkov): The platform-dependent functions to check the static IP +// address configured are rather similar. Think about unifying common parts. +func hostnameIfStaticConfig(r io.Reader) (has bool, err error) { + s := bufio.NewScanner(r) + for s.Scan() { + line := strings.TrimSpace(s.Text()) + fields := strings.Fields(line) + if len(fields) >= 2 && fields[0] == "inet" && net.ParseIP(fields[1]) != nil { + return true, s.Err() + } + } + + return false, s.Err() +} + +func ifaceSetStaticIP(string) (err error) { + return aghos.Unsupported("setting static ip") +} diff --git a/internal/aghnet/net_openbsd_test.go b/internal/aghnet/net_openbsd_test.go new file mode 100644 index 00000000..5b005a6b --- /dev/null +++ b/internal/aghnet/net_openbsd_test.go @@ -0,0 +1,52 @@ +//go:build openbsd +// +build openbsd + +package aghnet + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHostnameIfStaticConfig(t *testing.T) { + const nl = "\n" + + testCases := []struct { + name string + rcconfData string + wantHas bool + }{{ + name: "simple", + rcconfData: `inet 127.0.0.253` + nl, + wantHas: true, + }, { + name: "case_sensitiveness", + rcconfData: `InEt 127.0.0.253` + nl, + wantHas: false, + }, { + name: "comments_and_trash", + rcconfData: `# comment 1` + nl + + `` + nl + + `# inet 127.0.0.253` + nl + + `inet` + nl, + wantHas: false, + }, { + name: "incorrect_config", + rcconfData: `inet6 127.0.0.253` + nl + + `inet 256.256.256.256` + nl, + wantHas: false, + }} + + for _, tc := range testCases { + r := strings.NewReader(tc.rcconfData) + t.Run(tc.name, func(t *testing.T) { + has, err := hostnameIfStaticConfig(r) + require.NoError(t, err) + + assert.Equal(t, tc.wantHas, has) + }) + } +} diff --git a/internal/aghnet/net_others.go b/internal/aghnet/net_others.go index 033a40cc..de88ed80 100644 --- a/internal/aghnet/net_others.go +++ b/internal/aghnet/net_others.go @@ -1,5 +1,5 @@ -//go:build !(linux || darwin || freebsd) -// +build !linux,!darwin,!freebsd +//go:build !(linux || darwin || freebsd || openbsd) +// +build !linux,!darwin,!freebsd,!openbsd package aghnet diff --git a/internal/aghnet/net_test.go b/internal/aghnet/net_test.go index 3cd2fd6a..0773f67f 100644 --- a/internal/aghnet/net_test.go +++ b/internal/aghnet/net_test.go @@ -1,8 +1,10 @@ package aghnet import ( + "net" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -14,3 +16,51 @@ func TestGetValidNetInterfacesForWeb(t *testing.T) { require.NotEmptyf(t, iface.Addresses, "no addresses found for %s", iface.Name) } } + +func TestBroadcastFromIPNet(t *testing.T) { + known6 := net.IP{ + 1, 2, 3, 4, + 5, 6, 7, 8, + 9, 10, 11, 12, + 13, 14, 15, 16, + } + + testCases := []struct { + name string + subnet *net.IPNet + want net.IP + }{{ + name: "full", + subnet: &net.IPNet{ + IP: net.IP{192, 168, 0, 1}, + Mask: net.IPMask{255, 255, 15, 0}, + }, + want: net.IP{192, 168, 240, 255}, + }, { + name: "ipv6_no_mask", + subnet: &net.IPNet{ + IP: known6, + }, + want: known6, + }, { + name: "ipv4_no_mask", + subnet: &net.IPNet{ + IP: net.IP{192, 168, 1, 2}, + }, + want: net.IP{192, 168, 1, 255}, + }, { + name: "unspecified", + subnet: &net.IPNet{ + IP: net.IP{0, 0, 0, 0}, + Mask: net.IPMask{0, 0, 0, 0}, + }, + want: net.IPv4bcast, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + bc := BroadcastFromIPNet(tc.subnet) + assert.True(t, bc.Equal(tc.want), bc) + }) + } +} diff --git a/internal/dhcpd/checkother_windows.go b/internal/dhcpd/checkother_windows.go deleted file mode 100644 index c417f643..00000000 --- a/internal/dhcpd/checkother_windows.go +++ /dev/null @@ -1,14 +0,0 @@ -//go:build windows -// +build windows - -package dhcpd - -import "github.com/AdguardTeam/AdGuardHome/internal/aghos" - -func CheckIfOtherDHCPServersPresentV4(ifaceName string) (bool, error) { - return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV4") -} - -func CheckIfOtherDHCPServersPresentV6(ifaceName string) (bool, error) { - return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV6") -} diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index b77b1e5b..341aa3d6 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -15,7 +15,6 @@ import ( ) const ( - defaultDiscoverTime = time.Second * 3 // leaseExpireStatic is used to define the Expiry field for static // leases. // diff --git a/internal/dhcpd/helpers.go b/internal/dhcpd/helpers.go index 28856b5f..df157a49 100644 --- a/internal/dhcpd/helpers.go +++ b/internal/dhcpd/helpers.go @@ -6,14 +6,6 @@ import ( "net" ) -func isTimeout(err error) bool { - operr, ok := err.(*net.OpError) - if !ok { - return false - } - return operr.Timeout() -} - func tryTo4(ip net.IP) (ip4 net.IP, err error) { if ip == nil { return nil, fmt.Errorf("%v is not an IP address", ip) diff --git a/internal/dhcpd/http.go b/internal/dhcpd/http.go index 4c10bd96..990d3c7c 100644 --- a/internal/dhcpd/http.go +++ b/internal/dhcpd/http.go @@ -398,60 +398,63 @@ func (s *Server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Reque msg := fmt.Sprintf("failed to read request body: %s", err) log.Error(msg) http.Error(w, msg, http.StatusBadRequest) + return } - interfaceName := strings.TrimSpace(string(body)) - if interfaceName == "" { + ifaceName := strings.TrimSpace(string(body)) + if ifaceName == "" { msg := "empty interface name specified" log.Error(msg) http.Error(w, msg, http.StatusBadRequest) + return } result := dhcpSearchResult{ V4: dhcpSearchV4Result{ - OtherServer: dhcpSearchOtherResult{}, + OtherServer: dhcpSearchOtherResult{ + Found: "no", + }, StaticIP: dhcpStaticIPStatus{ Static: "yes", }, }, V6: dhcpSearchV6Result{ - OtherServer: dhcpSearchOtherResult{}, + OtherServer: dhcpSearchOtherResult{ + Found: "no", + }, }, } - found4, err4 := CheckIfOtherDHCPServersPresentV4(interfaceName) - - isStaticIP, err := aghnet.IfaceHasStaticIP(interfaceName) - if err != nil { + if isStaticIP, serr := aghnet.IfaceHasStaticIP(ifaceName); serr != nil { result.V4.StaticIP.Static = "error" - result.V4.StaticIP.Error = err.Error() + result.V4.StaticIP.Error = serr.Error() } else if !isStaticIP { result.V4.StaticIP.Static = "no" - result.V4.StaticIP.IP = aghnet.GetSubnet(interfaceName).String() + // TODO(e.burkov): The returned IP should only be of version 4. + result.V4.StaticIP.IP = aghnet.GetSubnet(ifaceName).String() } - if found4 { - result.V4.OtherServer.Found = "yes" - } else if err4 != nil { + found4, found6, err4, err6 := aghnet.CheckOtherDHCP(ifaceName) + if err4 != nil { result.V4.OtherServer.Found = "error" result.V4.OtherServer.Error = err4.Error() + } else if found4 { + result.V4.OtherServer.Found = "yes" } - - found6, err6 := CheckIfOtherDHCPServersPresentV6(interfaceName) - - if found6 { - result.V6.OtherServer.Found = "yes" - } else if err6 != nil { + if err6 != nil { result.V6.OtherServer.Found = "error" result.V6.OtherServer.Error = err6.Error() + } else if found6 { + result.V6.OtherServer.Found = "yes" } w.Header().Set("Content-Type", "application/json") err = json.NewEncoder(w).Encode(result) if err != nil { httpError(r, w, http.StatusInternalServerError, "Failed to marshal DHCP found json: %s", err) + return } } diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go index 7848811f..8d2832a1 100644 --- a/internal/dhcpd/v4.go +++ b/internal/dhcpd/v4.go @@ -971,7 +971,12 @@ func (s *v4Server) Start() (err error) { log.Debug("dhcpv4: starting...") - dnsIPAddrs, err := ifaceDNSIPAddrs(iface, ipVersion4, defaultMaxAttempts, defaultBackoff) + dnsIPAddrs, err := aghnet.IfaceDNSIPAddrs( + iface, + aghnet.IPVersion4, + defaultMaxAttempts, + defaultBackoff, + ) if err != nil { return fmt.Errorf("interface %s: %w", ifaceName, err) } @@ -1061,12 +1066,7 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) { IP: routerIP, Mask: subnetMask, } - - bcastIP := netutil.CloneIP(routerIP) - for i, b := range subnetMask { - bcastIP[i] |= ^b - } - s.conf.broadcastIP = bcastIP + s.conf.broadcastIP = aghnet.BroadcastFromIPNet(s.conf.subnet) s.conf.ipRange, err = newIPRange(conf.RangeStart, conf.RangeEnd) if err != nil { diff --git a/internal/dhcpd/v46.go b/internal/dhcpd/v46.go index 6b39fe5d..5e44eca3 100644 --- a/internal/dhcpd/v46.go +++ b/internal/dhcpd/v46.go @@ -1,125 +1,12 @@ package dhcpd import ( - "fmt" - "net" "time" - - "github.com/AdguardTeam/golibs/log" ) -// ipVersion is a documentational alias for int. Use it when the integer means -// IP version. -type ipVersion = int - -// IP version constants. -const ( - ipVersion4 ipVersion = 4 - ipVersion6 ipVersion = 6 -) - -// netIface is the interface for network interface methods. -type netIface interface { - Addrs() ([]net.Addr, error) -} - -// ifaceIPAddrs returns the interface's IP addresses. -func ifaceIPAddrs(iface netIface, ipv ipVersion) (ips []net.IP, err error) { - addrs, err := iface.Addrs() - if err != nil { - return nil, err - } - - for _, a := range addrs { - var ip net.IP - switch a := a.(type) { - case *net.IPAddr: - ip = a.IP - case *net.IPNet: - ip = a.IP - default: - continue - } - - // Assume that net.(*Interface).Addrs can only return valid IPv4 - // and IPv6 addresses. Thus, if it isn't an IPv4 address, it - // must be an IPv6 one. - switch ipv { - case ipVersion4: - if ip4 := ip.To4(); ip4 != nil { - ips = append(ips, ip4) - } - case ipVersion6: - if ip6 := ip.To4(); ip6 == nil { - ips = append(ips, ip) - } - default: - return nil, fmt.Errorf("invalid ip version %d", ipv) - } - } - - return ips, nil -} - // Currently used defaults for ifaceDNSAddrs. const ( defaultMaxAttempts int = 10 defaultBackoff time.Duration = 500 * time.Millisecond ) - -// ifaceDNSIPAddrs returns IP addresses of the interface suitable to send to -// clients as DNS addresses. If err is nil, addrs contains either no addresses -// or at least two. -// -// It makes up to maxAttempts attempts to get the addresses if there are none, -// each time using the provided backoff. Sometimes an interface needs a few -// seconds to really ititialize. -// -// See https://github.com/AdguardTeam/AdGuardHome/issues/2304. -func ifaceDNSIPAddrs( - iface netIface, - ipv ipVersion, - maxAttempts int, - backoff time.Duration, -) (addrs []net.IP, err error) { - var n int - for n = 1; n <= maxAttempts; n++ { - addrs, err = ifaceIPAddrs(iface, ipv) - if err != nil { - return nil, fmt.Errorf("getting ip addrs: %w", err) - } - - if len(addrs) > 0 { - break - } - - log.Debug("dhcpv%d: attempt %d: no ip addresses", ipv, n) - - time.Sleep(backoff) - } - - switch len(addrs) { - case 0: - // Don't return errors in case the users want to try and enable - // the DHCP server later. - t := time.Duration(n) * backoff - log.Error("dhcpv%d: no ip for iface after %d attempts and %s", ipv, n, t) - - return nil, nil - case 1: - // Some Android devices use 8.8.8.8 if there is not a secondary - // DNS server. Fix that by setting the secondary DNS address to - // the same address. - // - // See https://github.com/AdguardTeam/AdGuardHome/issues/1708. - log.Debug("dhcpv%d: setting secondary dns ip to itself", ipv) - addrs = append(addrs, addrs[0]) - default: - // Go on. - } - - log.Debug("dhcpv%d: got addresses %s after %d attempts", ipv, addrs, n) - - return addrs, nil -} diff --git a/internal/dhcpd/v6.go b/internal/dhcpd/v6.go index b4a4424f..92cf59ce 100644 --- a/internal/dhcpd/v6.go +++ b/internal/dhcpd/v6.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" @@ -607,7 +608,12 @@ func (s *v6Server) Start() (err error) { log.Debug("dhcpv6: starting...") - dnsIPAddrs, err := ifaceDNSIPAddrs(iface, ipVersion6, defaultMaxAttempts, defaultBackoff) + dnsIPAddrs, err := aghnet.IfaceDNSIPAddrs( + iface, + aghnet.IPVersion6, + defaultMaxAttempts, + defaultBackoff, + ) if err != nil { return fmt.Errorf("interface %s: %w", ifaceName, err) } diff --git a/scripts/make/go-lint.sh b/scripts/make/go-lint.sh index d3585f19..81fcc706 100644 --- a/scripts/make/go-lint.sh +++ b/scripts/make/go-lint.sh @@ -114,6 +114,7 @@ underscores() { -e '_bsd.go'\ -e '_darwin.go'\ -e '_freebsd.go'\ + -e '_openbsd.go'\ -e '_linux.go'\ -e '_little.go'\ -e '_others.go'\