From c8092fe34d62f00ee6c224c0eddbdb5a58ec3bcc Mon Sep 17 00:00:00 2001
From: Ainar Garipov <a.garipov@adguard.com>
Date: Thu, 29 Apr 2021 14:52:22 +0300
Subject: [PATCH] Pull request: dhcpd: normalize hostnames of invalid static
 leases

Updates #3027.

Squashed commit of the following:

commit 171ebaecb4b96ebeb2d0fa350460c65f244e94ae
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Apr 29 14:45:04 2021 +0300

    all: imp docs

commit 48b13ed5b435042a0622a66b60407116ae5aa288
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Apr 29 14:34:28 2021 +0300

    dhcpd: normalize hostnames of invalid static leases
---
 CHANGELOG.md         |  2 ++
 internal/dhcpd/v4.go | 81 ++++++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a6fc8a33..6d7d372d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,9 +19,11 @@ and this project adheres to
 
 ### Fixed
 
+- Normalization of perviously-saved invalid static DHCP leases ([#3027]).
 - Validation of IPv6 addresses with zones in system resolvers ([#3022]).
 
 [#3022]: https://github.com/AdguardTeam/AdGuardHome/issues/3022
+[#3027]: https://github.com/AdguardTeam/AdGuardHome/issues/3027
 
 
 
diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go
index 5cbfc370..cb59fcb4 100644
--- a/internal/dhcpd/v4.go
+++ b/internal/dhcpd/v4.go
@@ -51,6 +51,8 @@ func (s *v4Server) WriteDiskConfig6(c *V6ServerConf) {
 
 // ResetLeases - reset leases
 func (s *v4Server) ResetLeases(leases []*Lease) {
+	var err error
+
 	if !s.conf.Enabled {
 		return
 	}
@@ -60,9 +62,14 @@ func (s *v4Server) ResetLeases(leases []*Lease) {
 	s.leases = nil
 
 	for _, l := range leases {
-		err := s.addLease(l)
+		l.Hostname, err = s.validHostnameForClient(l.Hostname, l.IP)
 		if err != nil {
-			// TODO(a.garipov): Better error handling.
+			log.Info("dhcpv4: warning: previous hostname %q is invalid: %s", l.Hostname, err)
+		}
+
+		err = s.addLease(l)
+		if err != nil {
+			// TODO(a.garipov): Wrap and bubble up the error.
 			log.Error(
 				"dhcpv4: reset: re-adding a lease for %s (%s): %s",
 				l.IP,
@@ -562,7 +569,8 @@ func (o *optFQDN) ToBytes() []byte {
 	return b
 }
 
-// normalizeHostname normalizes a hostname sent by the client.
+// normalizeHostname normalizes a hostname sent by the client.  If err is not
+// nil, norm is an empty string.
 func normalizeHostname(name string) (norm string, err error) {
 	if name == "" {
 		return "", nil
@@ -603,6 +611,37 @@ func (s *v4Server) validateHostname(name string) (err error) {
 	return nil
 }
 
+// validHostnameForClient accepts the hostname sent by the client and returns
+// either a normalized version of that hostname or a new hostname generated from
+// the client's IP address.
+//
+// hostname is always a non-empty valid hostname.  If err is not nil, it
+// describes the issues encountered when normalizing cliHostname.
+func (s *v4Server) validHostnameForClient(
+	cliHostname string,
+	ip net.IP,
+) (hostname string, err error) {
+	hostname, err = normalizeHostname(cliHostname)
+	if err == nil {
+		err = s.validateHostname(hostname)
+		if err != nil {
+			// Go on and assign a hostname made from the IP below,
+			// returning the error that we've got.
+			hostname = ""
+		}
+	}
+
+	if hostname == "" {
+		hostname = aghnet.GenerateHostname(ip)
+	}
+
+	if hostname != cliHostname {
+		log.Info("dhcpv4: normalized hostname %q into %q", cliHostname, hostname)
+	}
+
+	return hostname, err
+}
+
 // validateLease returns an error if the lease is invalid.
 func (s *v4Server) validateLease(l *Lease) (err error) {
 	defer agherr.Annotate("validating lease: %s", &err)
@@ -682,43 +721,11 @@ func (s *v4Server) processRequest(req, resp *dhcpv4.DHCPv4) (lease *Lease, ok bo
 
 	if !lease.IsStatic() {
 		cliHostname := req.HostName()
-
-		var hostname string
-		hostname, err = normalizeHostname(cliHostname)
+		lease.Hostname, err = s.validHostnameForClient(cliHostname, reqIP)
 		if err != nil {
-			log.Error("dhcpv4: cannot normalize hostname for %s: %s", mac, err)
-
-			// Go on and assign a hostname made from the IP.
+			log.Info("dhcpv4: warning: client hostname %q is invalid: %s", cliHostname, err)
 		}
 
-		if hostname != "" {
-			if cliHostname != hostname {
-				log.Debug(
-					"dhcpv4: normalized hostname %q into %q",
-					cliHostname,
-					hostname,
-				)
-			}
-
-			if lease.Hostname != hostname {
-				// Either a new lease or an old lease with a new
-				// hostname, so validate.
-				err = s.validateHostname(hostname)
-				if err != nil {
-					log.Error("dhcpv4: validating %s: %s", mac, err)
-
-					// Go on and assign a hostname made from
-					// the IP below.
-					hostname = ""
-				}
-			}
-		}
-
-		if hostname == "" {
-			hostname = aghnet.GenerateHostname(reqIP)
-		}
-
-		lease.Hostname = hostname
 		s.commitLease(lease)
 	} else if len(lease.Hostname) != 0 {
 		o := &optFQDN{