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 <e.burkov@adguard.com>
Date:   Thu Aug 12 17:02:02 2021 +0300

    all: imp code, docs

commit 5b598fc18a9b69a0256569f4c691bb6a2193dfbd
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Aug 12 16:28:12 2021 +0300

    all: mv logic, imp code, docs, log changes

commit e3e1577a668fe3e5c61d075c390e4bd7268181ba
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Aug 12 14:15:10 2021 +0300

    dhcpd: imp checkother

commit 3cc8b058195c30a7ef0b7741ee8463270d9e47ff
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Wed Aug 11 13:20:18 2021 +0300

    all: imp bsd support

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
 
 
 

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)
+}

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,
+// defaultDiscoverTime is the
-// and waits for a response for a period defined by defaultDiscoverTime
+const defaultDiscoverTime = 3 * time.Second
-func CheckIfOtherDHCPServersPresentV4(ifaceName string) (ok bool, err error) {
+
+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)
+	ok4, err4 = checkOtherDHCPv4(iface)
-	if err != nil {
+	ok6, err6 = checkOtherDHCPv6(iface)
-		return false, fmt.Errorf("getting ipv4 addrs for iface %s: %w", ifaceName, err)
+
-	}
+	return ok4, ok6, err4, err6
-	if len(ifaceIPNet) == 0 {
+}
-		return false, fmt.Errorf("interface %s has no ipv4 addresses", ifaceName)
+
+// 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
+	for _, a := range addrs {
-	// information is outdated or at least incomplete.
+		switch a := a.(type) {
-	if runtime.GOOS == "darwin" {
+		case *net.IPAddr:
-		return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV4")
+			subnet = &net.IPNet{
+				IP:   a.IP,
+				Mask: a.IP.DefaultMask(),
+			}
+		case *net.IPNet:
+			subnet = a
+		default:
+			continue
 		}
 
-	srcIP := ifaceIPNet[0]
+		if ip4 := subnet.IP.To4(); ip4 != nil {
-	src := netutil.JoinHostPort(srcIP.String(), 68)
+			subnet.IP = ip4
-	dst := "255.255.255.255:67"
 
-	hostname, _ := os.Hostname()
+			return subnet, nil
-
-	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)
 	}
 
-	if !udpAddr.IP.To4().Equal(srcIP) {
+	return nil, fmt.Errorf("interface %s has no ipv4 addresses", iface.Name)
-		return false, fmt.Errorf("resolved UDP address is not %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
 	}
 
-	// resolve 255.255.255.255:67
+	// Resolve broadcast addr.
-	dstAddr, err := net.ResolveUDPAddr("udp4", dst)
+	dst := netutil.IPPort{
-	if err != nil {
+		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
+	var hostname string
-	log.Tracef("Listening to udp4 %+v", udpAddr)
+	if hostname, err = os.Hostname(); err != nil {
-	c, err := nclient4.NewRawUDPConn(ifaceName, 68)
+		return false, fmt.Errorf("couldn't get hostname: %w", err)
-	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()) }()
 	}
 
-	// send to 255.255.255.255:67
+	return discover4(iface, dstAddr, hostname)
-	_, err = c.WriteTo(req.ToBytes(), dstAddr)
+}
-	if err != nil {
+
-		return false, fmt.Errorf("couldn't send a packet to %s: %w", dst, err)
+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,
+// checkOtherDHCPv6 sends a DHCP request to the specified network interface, and
-// and waits for a response for a period defined by defaultDiscoverTime
+// waits for a response for a period defined by defaultDiscoverTime.
-func CheckIfOtherDHCPServersPresentV6(ifaceName string) (ok bool, err error) {
+func checkOtherDHCPv6(iface *net.Interface) (ok bool, err error) {
-	iface, err := net.InterfaceByName(ifaceName)
+	ifaceIPNet, err := IfaceIPAddrs(iface, IPVersion6)
 	if err != nil {
-		return false, fmt.Errorf("dhcpv6: net.InterfaceByName: %s: %w", ifaceName, err)
+		return false, fmt.Errorf("getting ipv6 addrs for iface %s: %w", iface.Name, err)
-	}
-
-	ifaceIPNet, err := ifaceIPAddrs(iface, ipVersion6)
-	if err != nil {
-		return false, fmt.Errorf("getting ipv6 addrs for iface %s: %w", ifaceName, 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()) }()
-	}
 
 	_, 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
+}

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")
+}

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
+}

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
+		iface   NetIface
-		ipv     ipVersion
+		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
+		iface   NetIface
-		ipv     ipVersion
+		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)
 		})

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
+}

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
+// getNetworkSetupHardwareReports parses the output of the `networksetup
-// it returns a map where the key is the interface name, and the value is the "hardware port"
+// -listallhardwareports` command it returns a map where the key is the
-// returns nil if it fails to parse the output
+// 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 {

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

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")
+}

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)
+		})
+	}
+}

internal/aghnet/net_others.go

@@ -1,5 +1,5 @@
-//go:build !(linux || darwin || freebsd)
+//go:build !(linux || darwin || freebsd || openbsd)
-// +build !linux,!darwin,!freebsd
+// +build !linux,!darwin,!freebsd,!openbsd
 
 package aghnet
 

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)
+		})
+	}
+}

internal/dhcpd/checkother_windows.go

@@ -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")
-}

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.
 	//

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)

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))
+	ifaceName := strings.TrimSpace(string(body))
-	if interfaceName == "" {
+	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)
+	if isStaticIP, serr := aghnet.IfaceHasStaticIP(ifaceName); serr != nil {
-
-	isStaticIP, err := aghnet.IfaceHasStaticIP(interfaceName)
-	if err != 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 {
+	found4, found6, err4, err6 := aghnet.CheckOtherDHCP(ifaceName)
-		result.V4.OtherServer.Found = "yes"
+	if err4 != nil {
-	} else if err4 != nil {
 		result.V4.OtherServer.Found = "error"
 		result.V4.OtherServer.Error = err4.Error()
+	} else if found4 {
+		result.V4.OtherServer.Found = "yes"
 	}
-
+	if err6 != nil {
-	found6, err6 := CheckIfOtherDHCPServersPresentV6(interfaceName)
-
-	if found6 {
-		result.V6.OtherServer.Found = "yes"
-	} else 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
 	}
 }

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,
 	}
-
+	s.conf.broadcastIP = aghnet.BroadcastFromIPNet(s.conf.subnet)
-	bcastIP := netutil.CloneIP(routerIP)
-	for i, b := range subnetMask {
-		bcastIP[i] |= ^b
-	}
-	s.conf.broadcastIP = bcastIP
 
 	s.conf.ipRange, err = newIPRange(conf.RangeStart, conf.RangeEnd)
 	if err != nil {

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
-}

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)
 	}

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'\