Pull request: 4722 dhcp http panic

Merge in DNS/adguard-home from 4722-dhcp-http-panic to master

Updates #4722.

Squashed commit of the following:

commit 8a8db48
Merge: 39b344f b74b92f
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 20:11:23 2022 +0300

    Merge branch 'master' into 4722-dhcp-http-panic

commit 39b344f
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 18:33:56 2022 +0300

    dhcpd: imp code, fmt

commit a36d70d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 17:38:17 2022 +0300

    dhcpd: imp names, docs

commit 600d63d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 17:36:17 2022 +0300

    dhcpd: rename files, imp tags

commit 44f5507
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 16:40:26 2022 +0300

    dhcpd: add mock

commit cfc3cfb
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 16:15:27 2022 +0300

    all: use ptr instead of value

commit ec526c2
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 14:57:10 2022 +0300

    all: log changes

commit 0eca09f
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 14:50:32 2022 +0300

    dhcpd: let v4 be unconfigured

commit 59636e9
Merge: 9238ca0 bc1503a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 14:50:17 2022 +0300

    Merge branch 'master' into 4722-dhcp-http-panic

commit 9238ca0
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Sep 7 18:28:56 2022 +0300

    dhcpd: imp conf

commit 5f801c9
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 6 16:31:13 2022 +0300

    dhcpd: hide behind iface

commit a95c274
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Aug 31 16:24:02 2022 +0300

    dhcpd: separate os files

CHANGELOG.md

@@ -15,6 +15,11 @@ and this project adheres to
 ## [v0.108.0] - 2022-12-01 (APPROX.)
 -->
 
+### Security
+
+- Weaker cipher suites that use the CBC (cipher block chaining) mode of
+  operation have been disabled ([#2993]).
+
 ### Added
 
 - The new optional `dns.ipset_file` property in the configuration file.  It
@@ -26,13 +31,13 @@ and this project adheres to
 - The minimum DHCP message size is reassigned back to BOOTP's constraint of 300
   bytes ([#4904]).
 
-### Security
+### Fixed
 
-- Weaker cipher suites that use the CBC (cipher block chaining) mode of
-  operation have been disabled ([#2993]).
+- Panic when adding a static lease within the disabled DHCP server ([#4722]).
 
 [#2993]: https://github.com/AdguardTeam/AdGuardHome/issues/2993
 [#4686]: https://github.com/AdguardTeam/AdGuardHome/issues/4686
+[#4722]: https://github.com/AdguardTeam/AdGuardHome/issues/4722
 [#4904]: https://github.com/AdguardTeam/AdGuardHome/issues/4904
 
 

internal/dhcpd/broadcast_bsd.go

@@ -9,11 +9,10 @@ import (
 // broadcast sends resp to the broadcast address specific for network interface.
 func (c *dhcpConn) broadcast(respData []byte, peer *net.UDPAddr) (n int, err error) {
 	// Despite the fact that server4.NewIPv4UDPConn explicitly sets socket
-	// options to allow broadcasting, it also binds the connection to a
-	// specific interface.  On FreeBSD and OpenBSD net.UDPConn.WriteTo
-	// causes errors while writing to the addresses that belong to another
-	// interface.  So, use the broadcast address specific for the interface
-	// bound.
+	// options to allow broadcasting, it also binds the connection to a specific
+	// interface.  On FreeBSD and OpenBSD net.UDPConn.WriteTo causes errors
+	// while writing to the addresses that belong to another interface.  So, use
+	// the broadcast address specific for the interface bound.
 	peer.IP = c.bcastIP
 
 	return c.udpConn.WriteTo(respData, peer)

internal/dhcpd/server.go → internal/dhcpd/config.go

@@ -1,10 +1,40 @@
 package dhcpd
 
 import (
+	"fmt"
 	"net"
 	"time"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
+	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/netutil"
 )
 
+// ServerConfig is the configuration for the DHCP server.  The order of YAML
+// fields is important, since the YAML configuration file follows it.
+type ServerConfig struct {
+	// Called when the configuration is changed by HTTP request
+	ConfigModified func() `yaml:"-"`
+
+	// Register an HTTP handler
+	HTTPRegister aghhttp.RegisterFunc `yaml:"-"`
+
+	Enabled       bool   `yaml:"enabled"`
+	InterfaceName string `yaml:"interface_name"`
+
+	// LocalDomainName is the domain name used for DHCP hosts.  For example,
+	// a DHCP client with the hostname "myhost" can be addressed as "myhost.lan"
+	// when LocalDomainName is "lan".
+	LocalDomainName string `yaml:"local_domain_name"`
+
+	Conf4 V4ServerConf `yaml:"dhcpv4"`
+	Conf6 V6ServerConf `yaml:"dhcpv6"`
+
+	WorkDir    string `yaml:"-"`
+	DBFilePath string `yaml:"-"`
+}
+
 // DHCPServer - DHCP server interface
 type DHCPServer interface {
 	// ResetLeases resets leases.
@@ -80,6 +110,86 @@ type V4ServerConf struct {
 	notify func(uint32)
 }
 
+// errNilConfig is an error returned by validation method if the config is nil.
+const errNilConfig errors.Error = "nil config"
+
+// ensureV4 returns a 4-byte version of ip.  An error is returned if the passed
+// ip is not an IPv4.
+func ensureV4(ip net.IP) (ip4 net.IP, err error) {
+	if ip == nil {
+		return nil, fmt.Errorf("%v is not an IP address", ip)
+	}
+
+	ip4 = ip.To4()
+	if ip4 == nil {
+		return nil, fmt.Errorf("%v is not an IPv4 address", ip)
+	}
+
+	return ip4, nil
+}
+
+// Validate returns an error if c is not a valid configuration.
+//
+// TODO(e.burkov):  Don't set the config fields when the server itself will stop
+// containing the config.
+func (c *V4ServerConf) Validate() (err error) {
+	defer func() { err = errors.Annotate(err, "dhcpv4: %w") }()
+
+	if c == nil {
+		return errNilConfig
+	}
+
+	var gatewayIP net.IP
+	gatewayIP, err = ensureV4(c.GatewayIP)
+	if err != nil {
+		// Don't wrap an errors since it's inforative enough as is and there is
+		// an annotation deferred already.
+		return err
+	}
+
+	if c.SubnetMask == nil {
+		return fmt.Errorf("invalid subnet mask: %v", c.SubnetMask)
+	}
+
+	subnetMask := net.IPMask(netutil.CloneIP(c.SubnetMask.To4()))
+	c.subnet = &net.IPNet{
+		IP:   gatewayIP,
+		Mask: subnetMask,
+	}
+	c.broadcastIP = aghnet.BroadcastFromIPNet(c.subnet)
+
+	c.ipRange, err = newIPRange(c.RangeStart, c.RangeEnd)
+	if err != nil {
+		// Don't wrap an errors since it's inforative enough as is and there is
+		// an annotation deferred already.
+		return err
+	}
+
+	if c.ipRange.contains(gatewayIP) {
+		return fmt.Errorf("gateway ip %v in the ip range: %v-%v",
+			gatewayIP,
+			c.RangeStart,
+			c.RangeEnd,
+		)
+	}
+
+	if !c.subnet.Contains(c.RangeStart) {
+		return fmt.Errorf("range start %v is outside network %v",
+			c.RangeStart,
+			c.subnet,
+		)
+	}
+
+	if !c.subnet.Contains(c.RangeEnd) {
+		return fmt.Errorf("range end %v is outside network %v",
+			c.RangeEnd,
+			c.subnet,
+		)
+	}
+
+	return nil
+}
+
 // V6ServerConf - server configuration
 type V6ServerConf struct {
 	Enabled       bool   `yaml:"-" json:"-"`

internal/dhcpd/db.go

@@ -32,7 +32,7 @@ func normalizeIP(ip net.IP) net.IP {
 }
 
 // Load lease table from DB
-func (s *Server) dbLoad() (err error) {
+func (s *server) dbLoad() (err error) {
 	dynLeases := []*Lease{}
 	staticLeases := []*Lease{}
 	v6StaticLeases := []*Lease{}
@@ -132,7 +132,7 @@ func normalizeLeases(staticLeases, dynLeases []*Lease) []*Lease {
 }
 
 // Store lease table in DB
-func (s *Server) dbStore() (err error) {
+func (s *server) dbStore() (err error) {
 	// Use an empty slice here as opposed to nil so that it doesn't write
 	// "null" into the database file if leases are empty.
 	leases := []leaseJSON{}