Pull request: * dhcpd: send secondary dns as well

Merge in DNS/adguard-home from 1708-secondary-dns to master

Updates #1708.

Squashed commit of the following:

commit 4529452e31131763f00c9c834cc95638f1a3d142
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Nov 9 18:12:57 2020 +0300

    * dhcpd: send secondary dns as well

internal/dhcpd/check_other_dhcp.go

@@ -26,12 +26,16 @@ func CheckIfOtherDHCPServersPresentV4(ifaceName string) (bool, error) {
 		return false, fmt.Errorf("couldn't find interface by name %s: %w", ifaceName, err)
 	}
 
-	// get ipv4 address of an interface
+	ifaceIPNet, err := ifaceIPv4Addrs(iface)
-	ifaceIPNet := getIfaceIPv4(*iface)
+	if err != nil {
+		return false, fmt.Errorf("getting ipv4 addrs for iface %s: %w", ifaceName, err)
+	}
 	if len(ifaceIPNet) == 0 {
-		return false, fmt.Errorf("couldn't find IPv4 address of interface %s %+v", ifaceName, iface)
+		return false, fmt.Errorf("interface %s has no ipv4 addresses", ifaceName)
 	}
 
+	// TODO(a.garipov): Find out what this is about.  Perhaps this
+	// information is outdated or at least incomplete.
 	if runtime.GOOS == "darwin" {
 		return false, fmt.Errorf("can't find DHCP server: not supported on macOS")
 	}
@@ -82,46 +86,66 @@ func CheckIfOtherDHCPServersPresentV4(ifaceName string) (bool, error) {
 	}
 
 	for {
-		// wait for answer
+		ok, next, err := tryConn(req, c, iface)
-		log.Tracef("Waiting %v for an answer", defaultDiscoverTime)
+		if next {
-		// TODO: replicate dhclient's behaviour of retrying several times with progressively bigger timeouts
-		b := make([]byte, 1500)
-		_ = c.SetReadDeadline(time.Now().Add(defaultDiscoverTime))
-		n, _, err := c.ReadFrom(b)
-		if isTimeout(err) {
-			// timed out -- no DHCP servers
-			log.Debug("DHCPv4: didn't receive DHCP response")
-			return false, nil
-		}
-		if err != nil {
-			return false, fmt.Errorf("couldn't receive packet: %w", err)
-		}
-
-		log.Tracef("Received packet (%v bytes)", n)
-
-		response, err := dhcpv4.FromBytes(b[:n])
-		if err != nil {
-			log.Debug("DHCPv4: dhcpv4.FromBytes: %s", err)
 			continue
 		}
-
+		if ok {
-		log.Debug("DHCPv4: received message from server: %s", response.Summary())
+			return true, nil
-
+		}
-		if !(response.OpCode == dhcpv4.OpcodeBootReply &&
+		if err != nil {
-			response.HWType == iana.HWTypeEthernet &&
+			log.Debug("%s", err)
-			bytes.Equal(response.ClientHWAddr, iface.HardwareAddr) &&
-			bytes.Equal(response.TransactionID[:], req.TransactionID[:]) &&
-			response.Options.Has(dhcpv4.OptionDHCPMessageType)) {
-			log.Debug("DHCPv4: received message from server doesn't match our request")
-			continue
 		}
-
-		log.Tracef("The packet is from an active DHCP server")
-		// that's a DHCP server there
-		return true, nil
 	}
 }
 
+// TODO(a.garipov): Refactor further.  Inspect error handling, remove the next
+// parameter, address the TODO, etc.
+func tryConn(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, next bool, err error) {
+	// TODO: replicate dhclient's behavior of retrying several times with
+	// progressively longer timeouts.
+	log.Tracef("waiting %v for an answer", defaultDiscoverTime)
+
+	b := make([]byte, 1500)
+	_ = c.SetReadDeadline(time.Now().Add(defaultDiscoverTime))
+	n, _, err := c.ReadFrom(b)
+	if err != nil {
+		if isTimeout(err) {
+			log.Debug("dhcpv4: didn't receive dhcp response")
+
+			return false, false, nil
+		}
+
+		return false, false, fmt.Errorf("receiving packet: %w", err)
+	}
+
+	log.Tracef("received packet, %d bytes", n)
+
+	response, err := dhcpv4.FromBytes(b[:n])
+	if err != nil {
+		log.Debug("dhcpv4: encoding: %s", err)
+
+		return false, true, err
+	}
+
+	log.Debug("dhcpv4: received message from server: %s", response.Summary())
+
+	if !(response.OpCode == dhcpv4.OpcodeBootReply &&
+		response.HWType == iana.HWTypeEthernet &&
+		bytes.Equal(response.ClientHWAddr, iface.HardwareAddr) &&
+		bytes.Equal(response.TransactionID[:], req.TransactionID[:]) &&
+		response.Options.Has(dhcpv4.OptionDHCPMessageType)) {
+
+		log.Debug("dhcpv4: received message from server doesn't match our request")
+
+		return false, true, nil
+	}
+
+	log.Tracef("the packet is from an active dhcp server")
+
+	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) (bool, error) {
@@ -130,9 +154,12 @@ func CheckIfOtherDHCPServersPresentV6(ifaceName string) (bool, error) {
 		return false, fmt.Errorf("dhcpv6: net.InterfaceByName: %s: %w", ifaceName, err)
 	}
 
-	ifaceIPNet := getIfaceIPv6(*iface)
+	ifaceIPNet, err := ifaceIPv6Addrs(iface)
+	if err != nil {
+		return false, fmt.Errorf("getting ipv6 addrs for iface %s: %w", ifaceName, err)
+	}
 	if len(ifaceIPNet) == 0 {
-		return false, fmt.Errorf("dhcpv6: couldn't find IPv6 address of interface %s %+v", ifaceName, iface)
+		return false, fmt.Errorf("interface %s has no ipv6 addresses", ifaceName)
 	}
 
 	srcIP := ifaceIPNet[0]

internal/dhcpd/dhcp_http.go

@@ -335,7 +335,7 @@ func (s *Server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Reque
 	foundVal := "no"
 	if found4 {
 		foundVal = "yes"
-	} else if err != nil {
+	} else if err4 != nil {
 		foundVal = "error"
 		othSrv["error"] = err4.Error()
 	}

internal/dhcpd/helpers.go

@@ -14,26 +14,6 @@ func isTimeout(err error) bool {
 	return operr.Timeout()
 }
 
-// Get IPv4 address list
-func getIfaceIPv4(iface net.Interface) []net.IP {
-	addrs, err := iface.Addrs()
-	if err != nil {
-		return nil
-	}
-
-	var res []net.IP
-	for _, a := range addrs {
-		ipnet, ok := a.(*net.IPNet)
-		if !ok {
-			continue
-		}
-		if ipnet.IP.To4() != nil {
-			res = append(res, ipnet.IP.To4())
-		}
-	}
-	return res
-}
-
 func parseIPv4(text string) (net.IP, error) {
 	result := net.ParseIP(text)
 	if result == nil {

internal/dhcpd/v4.go

@@ -36,7 +36,7 @@ func (s *v4Server) WriteDiskConfig6(c *V6ServerConf) {
 }
 
 // Return TRUE if IP address is within range [start..stop]
-func ip4InRange(start net.IP, stop net.IP, ip net.IP) bool {
+func ip4InRange(start, stop, ip net.IP) bool {
 	if len(start) != 4 || len(stop) != 4 {
 		return false
 	}
@@ -335,7 +335,7 @@ func (s *v4Server) commitLease(l *Lease) {
 }
 
 // Process Discover request and return lease
-func (s *v4Server) processDiscover(req *dhcpv4.DHCPv4, resp *dhcpv4.DHCPv4) *Lease {
+func (s *v4Server) processDiscover(req, resp *dhcpv4.DHCPv4) *Lease {
 	mac := req.ClientHWAddr
 
 	s.leasesLock.Lock()
@@ -409,7 +409,7 @@ func (o *optFQDN) ToBytes() []byte {
 
 // Process Request request and return lease
 // Return false if we don't need to reply
-func (s *v4Server) processRequest(req *dhcpv4.DHCPv4, resp *dhcpv4.DHCPv4) (*Lease, bool) {
+func (s *v4Server) processRequest(req, resp *dhcpv4.DHCPv4) (*Lease, bool) {
 	var lease *Lease
 	mac := req.ClientHWAddr
 	hostname := req.Options.Get(dhcpv4.OptionHostName)
@@ -472,7 +472,7 @@ func (s *v4Server) processRequest(req *dhcpv4.DHCPv4, resp *dhcpv4.DHCPv4) (*Lea
 // Return 1: OK
 // Return 0: error; reply with Nak
 // Return -1: error; don't reply
-func (s *v4Server) process(req *dhcpv4.DHCPv4, resp *dhcpv4.DHCPv4) int {
+func (s *v4Server) process(req, resp *dhcpv4.DHCPv4) int {
 	var lease *Lease
 
 	resp.UpdateOption(dhcpv4.OptServerIdentifier(s.conf.dnsIPAddrs[0]))
@@ -554,24 +554,65 @@ func (s *v4Server) packetHandler(conn net.PacketConn, peer net.Addr, req *dhcpv4
 	}
 }
 
-// Start - start server
+// ifaceIPv4Addrs returns the interface's IPv4 addresses.
+func ifaceIPv4Addrs(iface *net.Interface) (ips []net.IP, err error) {
+	addrs, err := iface.Addrs()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, a := range addrs {
+		ipnet, ok := a.(*net.IPNet)
+		if !ok {
+			continue
+		}
+
+		if ip := ipnet.IP.To4(); ip != nil {
+			ips = append(ips, ip)
+		}
+	}
+
+	return ips, nil
+}
+
+// Start starts the IPv4 DHCP server.
 func (s *v4Server) Start() error {
 	if !s.conf.Enabled {
 		return nil
 	}
 
-	iface, err := net.InterfaceByName(s.conf.InterfaceName)
+	ifaceName := s.conf.InterfaceName
+	iface, err := net.InterfaceByName(ifaceName)
 	if err != nil {
-		return fmt.Errorf("dhcpv4: Couldn't find interface by name %s: %w", s.conf.InterfaceName, err)
+		return fmt.Errorf("dhcpv4: finding interface %s by name: %w", ifaceName, err)
 	}
 
 	log.Debug("dhcpv4: starting...")
-	s.conf.dnsIPAddrs = getIfaceIPv4(*iface)
+
-	if len(s.conf.dnsIPAddrs) == 0 {
+	dnsIPAddrs, err := ifaceIPv4Addrs(iface)
-		log.Debug("dhcpv4: no IPv6 address for interface %s", iface.Name)
+	if err != nil {
-		return nil
+		return fmt.Errorf("dhcpv4: getting ipv4 addrs for iface %s: %w", ifaceName, err)
 	}
 
+	switch len(dnsIPAddrs) {
+	case 0:
+		log.Debug("dhcpv4: no ipv4 address for interface %s", iface.Name)
+
+		return nil
+	case 1:
+		// Some Android devices use 8.8.8.8 if there is no secondary DNS
+		// server.  Fix that by setting the secondary DNS address to our
+		// address as well.
+		//
+		// See https://github.com/AdguardTeam/AdGuardHome/issues/1708.
+		log.Debug("dhcpv4: setting secondary dns ip to iself for interface %s", iface.Name)
+		dnsIPAddrs = append(dnsIPAddrs, dnsIPAddrs[0])
+	default:
+		// Go on.
+	}
+
+	s.conf.dnsIPAddrs = dnsIPAddrs
+
 	laddr := &net.UDPAddr{
 		IP:   net.ParseIP("0.0.0.0"),
 		Port: dhcpv4.ServerPort,
@@ -587,6 +628,7 @@ func (s *v4Server) Start() error {
 		err = s.srv.Serve()
 		log.Debug("dhcpv4: srv.Serve: %s", err)
 	}()
+
 	return nil
 }
 

internal/dhcpd/v6.go

@@ -537,24 +537,25 @@ func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6.
 	}
 }
 
-// Get IPv6 address list
+// ifaceIPv6Addrs returns the interface's IPv6 addresses.
-func getIfaceIPv6(iface net.Interface) []net.IP {
+func ifaceIPv6Addrs(iface *net.Interface) (ips []net.IP, err error) {
 	addrs, err := iface.Addrs()
 	if err != nil {
-		return nil
+		return nil, err
 	}
 
-	var res []net.IP
 	for _, a := range addrs {
 		ipnet, ok := a.(*net.IPNet)
 		if !ok {
 			continue
 		}
-		if ipnet.IP.To4() == nil {
+
-			res = append(res, ipnet.IP)
+		if ip := ipnet.IP.To16(); ip != nil {
+			ips = append(ips, ip)
 		}
 	}
-	return res
+
+	return ips, nil
 }
 
 // initialize RA module
@@ -578,23 +579,40 @@ func (s *v6Server) initRA(iface *net.Interface) error {
 	return s.ra.Init()
 }
 
-// Start - start server
+// Start starts the IPv6 DHCP server.
 func (s *v6Server) Start() error {
 	if !s.conf.Enabled {
 		return nil
 	}
 
-	iface, err := net.InterfaceByName(s.conf.InterfaceName)
+	ifaceName := s.conf.InterfaceName
+	iface, err := net.InterfaceByName(ifaceName)
 	if err != nil {
-		return fmt.Errorf("couldn't find interface by name %s: %w", s.conf.InterfaceName, err)
+		return fmt.Errorf("dhcpv6: finding interface %s by name: %w", ifaceName, err)
 	}
 
-	s.conf.dnsIPAddrs = getIfaceIPv6(*iface)
+	log.Debug("dhcpv4: starting...")
-	if len(s.conf.dnsIPAddrs) == 0 {
+
-		log.Debug("DHCPv6: no IPv6 address for interface %s", iface.Name)
+	dnsIPAddrs, err := ifaceIPv6Addrs(iface)
-		return nil
+	if err != nil {
+		return fmt.Errorf("dhcpv6: getting ipv6 addrs for iface %s: %w", ifaceName, err)
 	}
 
+	switch len(dnsIPAddrs) {
+	case 0:
+		log.Debug("dhcpv6: no ipv6 address for interface %s", iface.Name)
+
+		return nil
+	case 1:
+		// See the comment in (*v4Server).Start.
+		log.Debug("dhcpv6: setting secondary dns ip to iself for interface %s", iface.Name)
+		dnsIPAddrs = append(dnsIPAddrs, dnsIPAddrs[0])
+	default:
+		// Go on.
+	}
+
+	s.conf.dnsIPAddrs = dnsIPAddrs
+
 	err = s.initRA(iface)
 	if err != nil {
 		return err