Pull request: dhcpd: fix static leases

Updates #2541.
Updates #2834.

Squashed commit of the following:

commit d1580182db3b5866213e017405aa2cf8a6ee2f24
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Mar 17 14:51:43 2021 +0300

    all: doc changes; imp naming

commit f036b50a60ba030aa6866944fc7a7b8776ce01d4
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Mar 17 14:09:19 2021 +0300

    dhcpd: fix static leases

CHANGELOG.md

@@ -27,18 +27,20 @@ and this project adheres to
 
 - Go 1.15 support.  v0.107.0 will require at least Go 1.16 to build.
 
+### Fixed
+
+- Support for more than one `/24` subnet in DHCP ([#2541]).
+- Invalid filenames in the `mobileconfig` API responses ([#2835]).
+
 ### Removed
 
 - Go 1.14 support.
 
-### Fixed
-
-- Invalid filenames in the `mobileconfig` API responses ([#2835]).
-
 [#2385]: https://github.com/AdguardTeam/AdGuardHome/issues/2385
 [#2412]: https://github.com/AdguardTeam/AdGuardHome/issues/2412
 [#2498]: https://github.com/AdguardTeam/AdGuardHome/issues/2498
 [#2533]: https://github.com/AdguardTeam/AdGuardHome/issues/2533
+[#2541]: https://github.com/AdguardTeam/AdGuardHome/issues/2541
 [#2835]: https://github.com/AdguardTeam/AdGuardHome/issues/2835
 
 

internal/dhcpd/iprange.go

@@ -9,7 +9,8 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 )
 
-// ipRange is an inclusive range of IP addresses.
+// ipRange is an inclusive range of IP addresses.  A nil range is a range that
+// doesn't contain any IP addresses.
 //
 // It is safe for concurrent use.
 //
@@ -53,12 +54,16 @@ func newIPRange(start, end net.IP) (r *ipRange, err error) {
 
 // contains returns true if r contains ip.
 func (r *ipRange) contains(ip net.IP) (ok bool) {
+	if r == nil {
+		return false
+	}
+
 	ipInt := (&big.Int{}).SetBytes(ip.To16())
 
 	return r.containsInt(ipInt)
 }
 
-// containsInt returns true if r contains ipInt.
+// containsInt returns true if r contains ipInt.  For internal use only.
 func (r *ipRange) containsInt(ipInt *big.Int) (ok bool) {
 	return ipInt.Cmp(r.start) >= 0 && ipInt.Cmp(r.end) <= 0
 }
@@ -70,6 +75,10 @@ type ipPredicate func(ip net.IP) (ok bool)
 // find finds the first IP address in r for which p returns true.  ip is in the
 // 16-byte form.
 func (r *ipRange) find(p ipPredicate) (ip net.IP) {
+	if r == nil {
+		return nil
+	}
+
 	ip = make(net.IP, net.IPv6len)
 	_1 := big.NewInt(1)
 	for i := (&big.Int{}).Set(r.start); i.Cmp(r.end) <= 0; i.Add(i, _1) {
@@ -85,6 +94,10 @@ func (r *ipRange) find(p ipPredicate) (ip net.IP) {
 // offset returns the offset of ip from the beginning of r.  It returns 0 and
 // false if ip is not in r.
 func (r *ipRange) offset(ip net.IP) (offset uint, ok bool) {
+	if r == nil {
+		return 0, false
+	}
+
 	ip = ip.To16()
 	ipInt := (&big.Int{}).SetBytes(ip)
 	if !r.containsInt(ipInt) {

internal/dhcpd/v4.go

@@ -111,19 +111,34 @@ func (s *v4Server) FindMACbyIP(ip net.IP) net.HardwareAddr {
 	return nil
 }
 
+// defaultHwAddrLen is the default length of a hardware (MAC) address.
+const defaultHwAddrLen = 6
+
 // Add the specified IP to the black list for a time period
-func (s *v4Server) blacklistLease(lease *Lease) {
-	hw := make(net.HardwareAddr, 6)
-	lease.HWAddr = hw
-	lease.Hostname = ""
-	lease.Expiry = time.Now().Add(s.conf.leaseTime)
+func (s *v4Server) blocklistLease(l *Lease) {
+	l.HWAddr = make(net.HardwareAddr, defaultHwAddrLen)
+	l.Hostname = ""
+	l.Expiry = time.Now().Add(s.conf.leaseTime)
 }
 
 // rmLeaseByIndex removes a lease by its index in the leases slice.
 func (s *v4Server) rmLeaseByIndex(i int) {
+	n := len(s.leases)
+	if i >= n {
+		// TODO(a.garipov): Better error handling.
+		log.Debug("dhcpv4: can't remove lease at index %d: no such lease", i)
+
+		return
+	}
+
 	l := s.leases[i]
 	s.leases = append(s.leases[:i], s.leases[i+1:]...)
 
+	n = len(s.leases)
+	if n > 0 {
+		s.leases = s.leases[:n-1]
+	}
+
 	r := s.conf.ipRange
 	offset, ok := r.offset(l.IP)
 	if ok {
@@ -175,7 +190,7 @@ func (s *v4Server) addLease(l *Lease) {
 	}
 
 	s.leases = append(s.leases, l)
-	s.leasedOffsets.Set(uint(offset))
+	s.leasedOffsets.Set(offset)
 
 	log.Debug("dhcpv4: added lease %s (%s)", l.IP, l.HWAddr)
 }
@@ -303,7 +318,7 @@ func (s *v4Server) nextIP() (ip net.IP) {
 			return false
 		}
 
-		return !s.leasedOffsets.Test(uint(offset))
+		return !s.leasedOffsets.Test(offset)
 	})
 
 	return ip.To4()
@@ -325,7 +340,7 @@ func (s *v4Server) findExpiredLease() int {
 // nil if it couldn't allocate a new lease.
 func (s *v4Server) reserveLease(mac net.HardwareAddr) (l *Lease) {
 	l = &Lease{
-		HWAddr: make([]byte, 6),
+		HWAddr: make([]byte, len(mac)),
 	}
 
 	copy(l.HWAddr, mac)
@@ -380,7 +395,7 @@ func (s *v4Server) processDiscover(req, resp *dhcpv4.DHCPv4) *Lease {
 			toStore = true
 
 			if !s.addrAvailable(lease.IP) {
-				s.blacklistLease(lease)
+				s.blocklistLease(lease)
 				lease = nil
 				continue
 			}
@@ -646,6 +661,8 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) {
 	s := &v4Server{}
 	s.conf = conf
 
+	// TODO(a.garipov): Don't use a disabled server in other places or just
+	// use an interface.
 	if !conf.Enabled {
 		return s, nil
 	}