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 8a8db48c3bd4f6bb7fabe65b5b7b162f0986fc76
Merge: 39b344f9 b74b92fc
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 39b344f97180af17ab22041e5655a27bcc99c29e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Sep 13 18:33:56 2022 +0300

    dhcpd: imp code, fmt

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

    dhcpd: imp names, docs

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

    dhcpd: rename files, imp tags

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

    dhcpd: add mock

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

    all: use ptr instead of value

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

    all: log changes

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

    dhcpd: let v4 be unconfigured

commit 59636e9ff48aea989d7bdfd216b37899b57137d2
Merge: 9238ca0a bc1503af
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 9238ca0a1e190ddc344f01959f474932809f086a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Sep 7 18:28:56 2022 +0300

    dhcpd: imp conf

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

    dhcpd: hide behind iface

commit a95c2741a7e3e5bfe8775bf937a3709217b76da0
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
+- Panic when adding a static lease within the disabled DHCP server ([#4722]).
-  operation have been disabled ([#2993]).
 
 [#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
+	// options to allow broadcasting, it also binds the connection to a specific
-	// specific interface.  On FreeBSD and OpenBSD net.UDPConn.WriteTo
+	// interface.  On FreeBSD and OpenBSD net.UDPConn.WriteTo causes errors
-	// causes errors while writing to the addresses that belong to another
+	// while writing to the addresses that belong to another interface.  So, use
-	// interface.  So, use the broadcast address specific for the interface
+	// the broadcast address specific for the interface bound.
-	// bound.
 	peer.IP = c.bcastIP
 
 	return c.udpConn.WriteTo(respData, peer)

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

internal/dhcpd/dhcpd.go

@@ -6,12 +6,11 @@ import (
 	"fmt"
 	"net"
 	"path/filepath"
-	"runtime"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
+	"github.com/AdguardTeam/golibs/timeutil"
 )
 
 const (
@@ -21,9 +20,19 @@ const (
 	// TODO(e.burkov): Remove it when static leases determining mechanism
 	// will be improved.
 	leaseExpireStatic = 1
+
+	// DefaultDHCPLeaseTTL is the default time-to-live for leases.
+	DefaultDHCPLeaseTTL = uint32(timeutil.Day / time.Second)
+
+	// DefaultDHCPTimeoutICMP is the default timeout for waiting ICMP responses.
+	DefaultDHCPTimeoutICMP = 1000
 )
 
-var webHandlersRegistered = false
+// Currently used defaults for ifaceDNSAddrs.
+const (
+	defaultMaxAttempts int           = 10
+	defaultBackoff     time.Duration = 500 * time.Millisecond
+)
 
 // Lease contains the necessary information about a DHCP lease
 type Lease struct {
@@ -119,30 +128,6 @@ func (l *Lease) UnmarshalJSON(data []byte) (err error) {
 	return nil
 }
 
-// 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:"-"`
-}
-
 // OnLeaseChangedT is a callback for lease changes.
 type OnLeaseChangedT func(flags int)
 
@@ -156,8 +141,68 @@ const (
 	LeaseChangedDBStore
 )
 
-// Server - the current state of the DHCP server
+// GetLeasesFlags are the flags for GetLeases.
-type Server struct {
+type GetLeasesFlags uint8
+
+// GetLeasesFlags values
+const (
+	LeasesDynamic GetLeasesFlags = 0b01
+	LeasesStatic  GetLeasesFlags = 0b10
+
+	LeasesAll = LeasesDynamic | LeasesStatic
+)
+
+// Interface is the DHCP server that deals with both IP address families.
+type Interface interface {
+	Start() (err error)
+	Stop() (err error)
+	Enabled() (ok bool)
+
+	Leases(flags GetLeasesFlags) (leases []*Lease)
+	SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT)
+	FindMACbyIP(ip net.IP) (mac net.HardwareAddr)
+
+	WriteDiskConfig(c *ServerConfig)
+}
+
+// MockInterface is a mock Interface implementation.
+//
+// TODO(e.burkov):  Move to aghtest when the API stabilized.
+type MockInterface struct {
+	OnStart             func() (err error)
+	OnStop              func() (err error)
+	OnEnabled           func() (ok bool)
+	OnLeases            func(flags GetLeasesFlags) (leases []*Lease)
+	OnSetOnLeaseChanged func(f OnLeaseChangedT)
+	OnFindMACbyIP       func(ip net.IP) (mac net.HardwareAddr)
+	OnWriteDiskConfig   func(c *ServerConfig)
+}
+
+var _ Interface = (*MockInterface)(nil)
+
+// Start implements the Interface for *MockInterface.
+func (s *MockInterface) Start() (err error) { return s.OnStart() }
+
+// Stop implements the Interface for *MockInterface.
+func (s *MockInterface) Stop() (err error) { return s.OnStop() }
+
+// Enabled implements the Interface for *MockInterface.
+func (s *MockInterface) Enabled() (ok bool) { return s.OnEnabled() }
+
+// Leases implements the Interface for *MockInterface.
+func (s *MockInterface) Leases(flags GetLeasesFlags) (ls []*Lease) { return s.OnLeases(flags) }
+
+// SetOnLeaseChanged implements the Interface for *MockInterface.
+func (s *MockInterface) SetOnLeaseChanged(f OnLeaseChangedT) { s.OnSetOnLeaseChanged(f) }
+
+// FindMACbyIP implements the Interface for *MockInterface.
+func (s *MockInterface) FindMACbyIP(ip net.IP) (mac net.HardwareAddr) { return s.OnFindMACbyIP(ip) }
+
+// WriteDiskConfig implements the Interface for *MockInterface.
+func (s *MockInterface) WriteDiskConfig(c *ServerConfig) { s.OnWriteDiskConfig(c) }
+
+// server is the DHCP service that handles DHCPv4, DHCPv6, and HTTP API.
+type server struct {
 	srv4 DHCPServer
 	srv6 DHCPServer
 
@@ -169,27 +214,15 @@ type Server struct {
 	onLeaseChanged []OnLeaseChangedT
 }
 
-// GetLeasesFlags are the flags for GetLeases.
+// type check
-type GetLeasesFlags uint8
+var _ Interface = (*server)(nil)
 
-// GetLeasesFlags values
+// Create initializes and returns the DHCP server handling both address
-const (
+// families.  It also registers the corresponding HTTP API endpoints.
-	LeasesDynamic GetLeasesFlags = 0b0001
+//
-	LeasesStatic  GetLeasesFlags = 0b0010
+// TODO(e.burkov):  Don't register handlers, see TODO on [aghhttp.RegisterFunc].
-
+func Create(conf *ServerConfig) (s *server, err error) {
-	LeasesAll = LeasesDynamic | LeasesStatic
+	s = &server{
-)
-
-// ServerInterface is an interface for servers.
-type ServerInterface interface {
-	Enabled() (ok bool)
-	Leases(flags GetLeasesFlags) (leases []*Lease)
-	SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT)
-}
-
-// Create - create object
-func Create(conf *ServerConfig) (s *Server, err error) {
-	s = &Server{
 		conf: &ServerConfig{
 			ConfigModified: conf.ConfigModified,
 
@@ -204,35 +237,20 @@ func Create(conf *ServerConfig) (s *Server, err error) {
 		},
 	}
 
-	if !webHandlersRegistered && s.conf.HTTPRegister != nil {
+	s.registerHandlers()
-		if runtime.GOOS == "windows" {
-			// Our DHCP server doesn't work on Windows yet, so
-			// signal that to the front with an HTTP 501.
-			//
-			// TODO(a.garipov): This needs refactoring.  We
-			// shouldn't even try and initialize a DHCP server on
-			// Windows, but there are currently too many
-			// interconnected parts--such as HTTP handlers and
-			// frontend--to make that work properly.
-			s.registerNotImplementedHandlers()
-		} else {
-			s.registerHandlers()
-		}
-
-		webHandlersRegistered = true
-	}
 
 	v4conf := conf.Conf4
-	v4conf.Enabled = s.conf.Enabled
-	if len(v4conf.RangeStart) == 0 {
-		v4conf.Enabled = false
-	}
-
 	v4conf.InterfaceName = s.conf.InterfaceName
 	v4conf.notify = s.onNotify
-	s.srv4, err = v4Create(v4conf)
+	v4conf.Enabled = s.conf.Enabled && len(v4conf.RangeStart) != 0
+
+	s.srv4, err = v4Create(&v4conf)
 	if err != nil {
-		return nil, fmt.Errorf("creating dhcpv4 srv: %w", err)
+		if v4conf.Enabled {
+			return nil, fmt.Errorf("creating dhcpv4 srv: %w", err)
+		}
+
+		log.Error("creating dhcpv4 srv: %s", err)
 	}
 
 	v6conf := conf.Conf6
@@ -265,12 +283,12 @@ func Create(conf *ServerConfig) (s *Server, err error) {
 }
 
 // Enabled returns true when the server is enabled.
-func (s *Server) Enabled() (ok bool) {
+func (s *server) Enabled() (ok bool) {
 	return s.conf.Enabled
 }
 
 // resetLeases resets all leases in the lease database.
-func (s *Server) resetLeases() (err error) {
+func (s *server) resetLeases() (err error) {
 	err = s.srv4.ResetLeases(nil)
 	if err != nil {
 		return err
@@ -287,7 +305,7 @@ func (s *Server) resetLeases() (err error) {
 }
 
 // server calls this function after DB is updated
-func (s *Server) onNotify(flags uint32) {
+func (s *server) onNotify(flags uint32) {
 	if flags == LeaseChangedDBStore {
 		err := s.dbStore()
 		if err != nil {
@@ -301,31 +319,28 @@ func (s *Server) onNotify(flags uint32) {
 }
 
 // SetOnLeaseChanged - set callback
-func (s *Server) SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT) {
+func (s *server) SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT) {
 	s.onLeaseChanged = append(s.onLeaseChanged, onLeaseChanged)
 }
 
-func (s *Server) notify(flags int) {
+func (s *server) notify(flags int) {
-	if len(s.onLeaseChanged) == 0 {
-		return
-	}
-
 	for _, f := range s.onLeaseChanged {
 		f(flags)
 	}
 }
 
 // WriteDiskConfig - write configuration
-func (s *Server) WriteDiskConfig(c *ServerConfig) {
+func (s *server) WriteDiskConfig(c *ServerConfig) {
 	c.Enabled = s.conf.Enabled
 	c.InterfaceName = s.conf.InterfaceName
 	c.LocalDomainName = s.conf.LocalDomainName
+
 	s.srv4.WriteDiskConfig4(&c.Conf4)
 	s.srv6.WriteDiskConfig6(&c.Conf6)
 }
 
 // Start will listen on port 67 and serve DHCP requests.
-func (s *Server) Start() (err error) {
+func (s *server) Start() (err error) {
 	err = s.srv4.Start()
 	if err != nil {
 		return err
@@ -340,7 +355,7 @@ func (s *Server) Start() (err error) {
 }
 
 // Stop closes the listening UDP socket
-func (s *Server) Stop() (err error) {
+func (s *server) Stop() (err error) {
 	err = s.srv4.Stop()
 	if err != nil {
 		return err
@@ -356,12 +371,12 @@ func (s *Server) Stop() (err error) {
 
 // Leases returns the list of active IPv4 and IPv6 DHCP leases.  It's safe for
 // concurrent use.
-func (s *Server) Leases(flags GetLeasesFlags) (leases []*Lease) {
+func (s *server) Leases(flags GetLeasesFlags) (leases []*Lease) {
 	return append(s.srv4.GetLeases(flags), s.srv6.GetLeases(flags)...)
 }
 
 // FindMACbyIP - find a MAC address by IP address in the currently active DHCP leases
-func (s *Server) FindMACbyIP(ip net.IP) net.HardwareAddr {
+func (s *server) FindMACbyIP(ip net.IP) net.HardwareAddr {
 	if ip.To4() != nil {
 		return s.srv4.FindMACbyIP(ip)
 	}
@@ -369,6 +384,6 @@ func (s *Server) FindMACbyIP(ip net.IP) net.HardwareAddr {
 }
 
 // AddStaticLease - add static v4 lease
-func (s *Server) AddStaticLease(l *Lease) error {
+func (s *server) AddStaticLease(l *Lease) error {
 	return s.srv4.AddStaticLease(l)
 }

internal/dhcpd/dhcpd_unix_test.go

@@ -1,5 +1,4 @@
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+//go:build darwin || freebsd || linux || openbsd
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
 
 package dhcpd
 
@@ -26,13 +25,13 @@ func testNotify(flags uint32) {
 // Leases database store/load.
 func TestDB(t *testing.T) {
 	var err error
-	s := Server{
+	s := server{
 		conf: &ServerConfig{
 			DBFilePath: dbFilename,
 		},
 	}
 
-	s.srv4, err = v4Create(V4ServerConf{
+	s.srv4, err = v4Create(&V4ServerConf{
 		Enabled:    true,
 		RangeStart: net.IP{192, 168, 10, 100},
 		RangeEnd:   net.IP{192, 168, 10, 200},
@@ -88,32 +87,6 @@ func TestDB(t *testing.T) {
 	assert.Equal(t, leases[0].Expiry.Unix(), ll[1].Expiry.Unix())
 }
 
-func TestIsValidSubnetMask(t *testing.T) {
-	testCases := []struct {
-		mask net.IP
-		want bool
-	}{{
-		mask: net.IP{255, 255, 255, 0},
-		want: true,
-	}, {
-		mask: net.IP{255, 255, 254, 0},
-		want: true,
-	}, {
-		mask: net.IP{255, 255, 252, 0},
-		want: true,
-	}, {
-		mask: net.IP{255, 255, 253, 0},
-	}, {
-		mask: net.IP{255, 255, 255, 1},
-	}}
-
-	for _, tc := range testCases {
-		t.Run(tc.mask.String(), func(t *testing.T) {
-			assert.Equal(t, tc.want, isValidSubnetMask(tc.mask))
-		})
-	}
-}
-
 func TestNormalizeLeases(t *testing.T) {
 	dynLeases := []*Lease{{
 		HWAddr: net.HardwareAddr{1, 2, 3, 4},
@@ -174,7 +147,7 @@ func TestV4Server_badRange(t *testing.T) {
 				notify:     testNotify,
 			}
 
-			_, err := v4Create(conf)
+			_, err := v4Create(&conf)
 			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
 		})
 	}

internal/dhcpd/helpers.go

@@ -1,36 +0,0 @@
-package dhcpd
-
-import (
-	"encoding/binary"
-	"fmt"
-	"net"
-)
-
-func tryTo4(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
-}
-
-// Return TRUE if subnet mask is correct (e.g. 255.255.255.0)
-func isValidSubnetMask(mask net.IP) bool {
-	var n uint32
-	n = binary.BigEndian.Uint32(mask)
-	for i := 0; i != 32; i++ {
-		if n == 0 {
-			break
-		}
-		if (n & 0x80000000) == 0 {
-			return false
-		}
-		n <<= 1
-	}
-	return true
-}

internal/dhcpd/http_unix.go

@@ -1,3 +1,5 @@
+//go:build darwin || freebsd || linux || openbsd
+
 package dhcpd
 
 import (
@@ -8,14 +10,12 @@ import (
 	"net/http"
 	"os"
 	"strings"
-	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
-	"github.com/AdguardTeam/golibs/timeutil"
 )
 
 type v4ServerConfJSON struct {
@@ -26,12 +26,12 @@ type v4ServerConfJSON struct {
 	LeaseDuration uint32 `json:"lease_duration"`
 }
 
-func v4JSONToServerConf(j *v4ServerConfJSON) V4ServerConf {
+func (j *v4ServerConfJSON) toServerConf() *V4ServerConf {
 	if j == nil {
-		return V4ServerConf{}
+		return &V4ServerConf{}
 	}
 
-	return V4ServerConf{
+	return &V4ServerConf{
 		GatewayIP:     j.GatewayIP,
 		SubnetMask:    j.SubnetMask,
 		RangeStart:    j.RangeStart,
@@ -66,7 +66,7 @@ type dhcpStatusResponse struct {
 	Enabled      bool         `json:"enabled"`
 }
 
-func (s *Server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) {
 	status := &dhcpStatusResponse{
 		Enabled:   s.conf.Enabled,
 		IfaceName: s.conf.InterfaceName,
@@ -81,6 +81,7 @@ func (s *Server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) {
 	status.StaticLeases = s.Leases(LeasesStatic)
 
 	w.Header().Set("Content-Type", "application/json")
+
 	err := json.NewEncoder(w).Encode(status)
 	if err != nil {
 		aghhttp.Error(
@@ -93,28 +94,26 @@ func (s *Server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (s *Server) enableDHCP(ifaceName string) (code int, err error) {
+func (s *server) enableDHCP(ifaceName string) (code int, err error) {
 	var hasStaticIP bool
 	hasStaticIP, err = aghnet.IfaceHasStaticIP(ifaceName)
 	if err != nil {
 		if errors.Is(err, os.ErrPermission) {
-			// ErrPermission may happen here on Linux systems where
+			// ErrPermission may happen here on Linux systems where AdGuard Home
-			// AdGuard Home is installed using Snap.  That doesn't
+			// is installed using Snap.  That doesn't necessarily mean that the
-			// necessarily mean that the machine doesn't have
+			// machine doesn't have a static IP, so we can assume that it has
-			// a static IP, so we can assume that it has and go on.
+			// and go on.  If the machine doesn't, we'll get an error later.
-			// If the machine doesn't, we'll get an error later.
 			//
 			// See https://github.com/AdguardTeam/AdGuardHome/issues/2667.
 			//
-			// TODO(a.garipov): I was thinking about moving this
+			// TODO(a.garipov): I was thinking about moving this into
-			// into IfaceHasStaticIP, but then we wouldn't be able
+			// IfaceHasStaticIP, but then we wouldn't be able to log it.  Think
-			// to log it.  Think about it more.
+			// about it more.
 			log.Info("error while checking static ip: %s; "+
 				"assuming machine has static ip and going on", err)
 			hasStaticIP = true
 		} else if errors.Is(err, aghnet.ErrNoStaticIPInfo) {
-			// Couldn't obtain a definitive answer.  Assume static
+			// Couldn't obtain a definitive answer.  Assume static IP an go on.
-			// IP an go on.
 			log.Info("can't check for static ip; " +
 				"assuming machine has static ip and going on")
 			hasStaticIP = true
@@ -149,34 +148,39 @@ type dhcpServerConfigJSON struct {
 	Enabled       aghalg.NullBool   `json:"enabled"`
 }
 
-func (s *Server) handleDHCPSetConfigV4(
+func (s *server) handleDHCPSetConfigV4(
 	conf *dhcpServerConfigJSON,
-) (srv4 DHCPServer, enabled bool, err error) {
+) (srv DHCPServer, enabled bool, err error) {
 	if conf.V4 == nil {
 		return nil, false, nil
 	}
 
-	v4Conf := v4JSONToServerConf(conf.V4)
+	v4Conf := conf.V4.toServerConf()
 	v4Conf.Enabled = conf.Enabled == aghalg.NBTrue
 	if len(v4Conf.RangeStart) == 0 {
 		v4Conf.Enabled = false
 	}
 
-	enabled = v4Conf.Enabled
 	v4Conf.InterfaceName = conf.InterfaceName
 
-	c4 := V4ServerConf{}
+	// Set the default values for the fields not configurable via web API.
-	s.srv4.WriteDiskConfig4(&c4)
+	c4 := &V4ServerConf{
+		notify:      s.onNotify,
+		ICMPTimeout: s.conf.Conf4.ICMPTimeout,
+		Options:     s.conf.Conf4.Options,
+	}
+
+	s.srv4.WriteDiskConfig4(c4)
 	v4Conf.notify = c4.notify
 	v4Conf.ICMPTimeout = c4.ICMPTimeout
 	v4Conf.Options = c4.Options
 
-	srv4, err = v4Create(v4Conf)
+	srv4, err := v4Create(v4Conf)
 
-	return srv4, enabled, err
+	return srv4, srv4.enabled(), err
 }
 
-func (s *Server) handleDHCPSetConfigV6(
+func (s *server) handleDHCPSetConfigV6(
 	conf *dhcpServerConfigJSON,
 ) (srv6 DHCPServer, enabled bool, err error) {
 	if conf.V6 == nil {
@@ -205,7 +209,7 @@ func (s *Server) handleDHCPSetConfigV6(
 	return srv6, enabled, err
 }
 
-func (s *Server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) {
 	conf := &dhcpServerConfigJSON{}
 	conf.Enabled = aghalg.BoolToNullBool(s.conf.Enabled)
 	conf.InterfaceName = s.conf.InterfaceName
@@ -287,7 +291,7 @@ type netInterfaceJSON struct {
 	Addrs6       []net.IP `json:"ipv6_addresses"`
 }
 
-func (s *Server) handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) {
 	response := map[string]netInterfaceJSON{}
 
 	ifaces, err := net.Interfaces()
@@ -410,7 +414,7 @@ type dhcpSearchResult struct {
 // . Search for another DHCP server running
 // . Check if a static IP is configured for the network interface
 // Respond with results
-func (s *Server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Request) {
 	// This use of ReadAll is safe, because request's body is now limited.
 	body, err := io.ReadAll(r.Body)
 	if err != nil {
@@ -482,7 +486,7 @@ func (s *Server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Reque
 	}
 }
 
-func (s *Server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request) {
 	l := &Lease{}
 	err := json.NewDecoder(r.Body).Decode(l)
 	if err != nil {
@@ -497,20 +501,16 @@ func (s *Server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	ip4 := l.IP.To4()
+	var srv DHCPServer
-	if ip4 == nil {
+	if ip4 := l.IP.To4(); ip4 != nil {
+		l.IP = ip4
+		srv = s.srv4
+	} else {
 		l.IP = l.IP.To16()
-
+		srv = s.srv6
-		err = s.srv6.AddStaticLease(l)
-		if err != nil {
-			aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
-		}
-
-		return
 	}
 
-	l.IP = ip4
+	err = srv.AddStaticLease(l)
-	err = s.srv4.AddStaticLease(l)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
 
@@ -518,7 +518,7 @@ func (s *Server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request
 	}
 }
 
-func (s *Server) handleDHCPRemoveStaticLease(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleDHCPRemoveStaticLease(w http.ResponseWriter, r *http.Request) {
 	l := &Lease{}
 	err := json.NewDecoder(r.Body).Decode(l)
 	if err != nil {
@@ -555,14 +555,7 @@ func (s *Server) handleDHCPRemoveStaticLease(w http.ResponseWriter, r *http.Requ
 	}
 }
 
-const (
+func (s *server) handleReset(w http.ResponseWriter, r *http.Request) {
-	// DefaultDHCPLeaseTTL is the default time-to-live for leases.
-	DefaultDHCPLeaseTTL = uint32(timeutil.Day / time.Second)
-	// DefaultDHCPTimeoutICMP is the default timeout for waiting ICMP responses.
-	DefaultDHCPTimeoutICMP = 1000
-)
-
-func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) {
 	err := s.Stop()
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusInternalServerError, "stopping dhcp: %s", err)
@@ -586,7 +579,7 @@ func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) {
 		DBFilePath: s.conf.DBFilePath,
 	}
 
-	v4conf := V4ServerConf{
+	v4conf := &V4ServerConf{
 		LeaseDuration: DefaultDHCPLeaseTTL,
 		ICMPTimeout:   DefaultDHCPTimeoutICMP,
 		notify:        s.onNotify,
@@ -602,7 +595,7 @@ func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) {
 	s.conf.ConfigModified()
 }
 
-func (s *Server) handleResetLeases(w http.ResponseWriter, r *http.Request) {
+func (s *server) handleResetLeases(w http.ResponseWriter, r *http.Request) {
 	err := s.resetLeases()
 	if err != nil {
 		msg := "resetting leases: %s"
@@ -612,7 +605,11 @@ func (s *Server) handleResetLeases(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (s *Server) registerHandlers() {
+func (s *server) registerHandlers() {
+	if s.conf.HTTPRegister == nil {
+		return
+	}
+
 	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", s.handleDHCPStatus)
 	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", s.handleDHCPInterfaces)
 	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", s.handleDHCPSetConfig)
@@ -622,44 +619,3 @@ func (s *Server) registerHandlers() {
 	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", s.handleReset)
 	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset_leases", s.handleResetLeases)
 }
-
-// jsonError is a generic JSON error response.
-//
-// TODO(a.garipov): Merge together with the implementations in .../home and
-// other packages after refactoring the web handler registering.
-type jsonError struct {
-	// Message is the error message, an opaque string.
-	Message string `json:"message"`
-}
-
-// notImplemented returns a handler that replies to any request with an HTTP 501
-// Not Implemented status and a JSON error with the provided message msg.
-//
-// TODO(a.garipov): Either take the logger from the server after we've
-// refactored logging or make this not a method of *Server.
-func (s *Server) notImplemented(msg string) (f func(http.ResponseWriter, *http.Request)) {
-	return func(w http.ResponseWriter, _ *http.Request) {
-		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusNotImplemented)
-
-		err := json.NewEncoder(w).Encode(&jsonError{
-			Message: msg,
-		})
-		if err != nil {
-			log.Debug("writing 501 json response: %s", err)
-		}
-	}
-}
-
-func (s *Server) registerNotImplementedHandlers() {
-	h := s.notImplemented("dhcp is not supported on windows")
-
-	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", h)
-	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", h)
-	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", h)
-	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/find_active_dhcp", h)
-	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/add_static_lease", h)
-	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/remove_static_lease", h)
-	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", h)
-	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset_leases", h)
-}

internal/dhcpd/http_windows.go

@@ -0,0 +1,55 @@
+//go:build windows
+
+package dhcpd
+
+import (
+	"encoding/json"
+	"net/http"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
+	"github.com/AdguardTeam/golibs/log"
+)
+
+// jsonError is a generic JSON error response.
+//
+// TODO(a.garipov): Merge together with the implementations in .../home and
+// other packages after refactoring the web handler registering.
+type jsonError struct {
+	// Message is the error message, an opaque string.
+	Message string `json:"message"`
+}
+
+// notImplemented is a handler that replies to any request with an HTTP 501 Not
+// Implemented status and a JSON error with the provided message msg.
+//
+// TODO(a.garipov): Either take the logger from the server after we've
+// refactored logging or make this not a method of *Server.
+func (s *server) notImplemented(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusNotImplemented)
+
+	err := json.NewEncoder(w).Encode(&jsonError{
+		Message: aghos.Unsupported("dhcp").Error(),
+	})
+	if err != nil {
+		log.Debug("writing 501 json response: %s", err)
+	}
+}
+
+// registerHandlers sets the handlers for DHCP HTTP API that always respond with
+// an HTTP 501, since DHCP server doesn't work on Windows yet.
+//
+// TODO(a.garipov): This needs refactoring.  We shouldn't even try and
+// initialize a DHCP server on Windows, but there are currently too many
+// interconnected parts--such as HTTP handlers and frontend--to make that work
+// properly.
+func (s *server) registerHandlers() {
+	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/find_active_dhcp", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/add_static_lease", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/remove_static_lease", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", s.notImplemented)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset_leases", s.notImplemented)
+}

internal/dhcpd/http_windows_test.go

@@ -1,23 +1,28 @@
+//go:build windows
+
 package dhcpd
 
 import (
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestServer_notImplemented(t *testing.T) {
-	s := &Server{}
+	s := &server{}
-	h := s.notImplemented("never!")
 
 	w := httptest.NewRecorder()
 	r, err := http.NewRequest(http.MethodGet, "/unsupported", nil)
 	require.NoError(t, err)
 
-	h(w, r)
+	s.notImplemented(w, r)
 	assert.Equal(t, http.StatusNotImplemented, w.Code)
-	assert.Equal(t, `{"message":"never!"}`+"\n", w.Body.String())
+
+	wantStr := fmt.Sprintf("{%q:%q}", "message", aghos.Unsupported("dhcp"))
+	assert.JSONEq(t, wantStr, w.Body.String())
 }

internal/dhcpd/options_unix.go

@@ -196,10 +196,10 @@ func parseDHCPOption(s string) (code dhcpv4.OptionCode, val dhcpv4.OptionValue,
 
 // prepareOptions builds the set of DHCP options according to host requirements
 // document and values from conf.
-func prepareOptions(conf V4ServerConf) (implicit, explicit dhcpv4.Options) {
+func (s *v4Server) prepareOptions() {
 	// Set default values of host configuration parameters listed in Appendix A
 	// of RFC-2131.
-	implicit = dhcpv4.OptionsFromList(
+	s.implicitOpts = dhcpv4.OptionsFromList(
 		// IP-Layer Per Host
 
 		// An Internet host that includes embedded gateway code MUST have a
@@ -375,14 +375,14 @@ func prepareOptions(conf V4ServerConf) (implicit, explicit dhcpv4.Options) {
 
 		// Set the Router Option to working subnet's IP since it's initialized
 		// with the address of the gateway.
-		dhcpv4.OptRouter(conf.subnet.IP),
+		dhcpv4.OptRouter(s.conf.subnet.IP),
 
-		dhcpv4.OptSubnetMask(conf.subnet.Mask),
+		dhcpv4.OptSubnetMask(s.conf.subnet.Mask),
 	)
 
 	// Set values for explicitly configured options.
-	explicit = dhcpv4.Options{}
+	s.explicitOpts = dhcpv4.Options{}
-	for i, o := range conf.Options {
+	for i, o := range s.conf.Options {
 		code, val, err := parseDHCPOption(o)
 		if err != nil {
 			log.Error("dhcpv4: bad option string at index %d: %s", i, err)
@@ -390,17 +390,15 @@ func prepareOptions(conf V4ServerConf) (implicit, explicit dhcpv4.Options) {
 			continue
 		}
 
-		explicit.Update(dhcpv4.Option{Code: code, Value: val})
+		s.explicitOpts.Update(dhcpv4.Option{Code: code, Value: val})
 		// Remove those from the implicit options.
-		delete(implicit, code.Code())
+		delete(s.implicitOpts, code.Code())
 	}
 
-	log.Debug("dhcpv4: implicit options:\n%s", implicit.Summary(nil))
+	log.Debug("dhcpv4: implicit options:\n%s", s.implicitOpts.Summary(nil))
-	log.Debug("dhcpv4: explicit options:\n%s", explicit.Summary(nil))
+	log.Debug("dhcpv4: explicit options:\n%s", s.explicitOpts.Summary(nil))
 
-	if len(explicit) == 0 {
+	if len(s.explicitOpts) == 0 {
-		explicit = nil
+		s.explicitOpts = nil
 	}
-
-	return implicit, explicit
 }

internal/dhcpd/options_unix_test.go

@@ -249,17 +249,21 @@ func TestPrepareOptions(t *testing.T) {
 	}}
 
 	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
+		s := &v4Server{
-			implicit, explicit := prepareOptions(V4ServerConf{
+			conf: &V4ServerConf{
 				// Just to avoid nil pointer dereference.
 				subnet:  &net.IPNet{},
 				Options: tc.opts,
-			})
+			},
+		}
 
-			assert.Equal(t, tc.wantExplicit, explicit)
+		t.Run(tc.name, func(t *testing.T) {
+			s.prepareOptions()
 
-			for c := range explicit {
+			assert.Equal(t, tc.wantExplicit, s.explicitOpts)
-				assert.NotContains(t, implicit, c)
+
+			for c := range s.explicitOpts {
+				assert.NotContains(t, s.implicitOpts, c)
 			}
 		})
 	}

internal/dhcpd/v46.go

@@ -1,12 +0,0 @@
-package dhcpd
-
-import (
-	"time"
-)
-
-// Currently used defaults for ifaceDNSAddrs.
-const (
-	defaultMaxAttempts int = 10
-
-	defaultBackoff time.Duration = 500 * time.Millisecond
-)

internal/dhcpd/v46_windows.go

@@ -8,15 +8,19 @@ import "net"
 
 type winServer struct{}
 
-func (s *winServer) ResetLeases(_ []*Lease) (err error)           { return nil }
+// type check
-func (s *winServer) GetLeases(_ GetLeasesFlags) (leases []*Lease) { return nil }
+var _ DHCPServer = winServer{}
-func (s *winServer) getLeasesRef() []*Lease                       { return nil }
+
-func (s *winServer) AddStaticLease(_ *Lease) (err error)          { return nil }
+func (winServer) ResetLeases(_ []*Lease) (err error)           { return nil }
-func (s *winServer) RemoveStaticLease(_ *Lease) (err error)       { return nil }
+func (winServer) GetLeases(_ GetLeasesFlags) (leases []*Lease) { return nil }
-func (s *winServer) FindMACbyIP(ip net.IP) (mac net.HardwareAddr) { return nil }
+func (winServer) getLeasesRef() []*Lease                       { return nil }
-func (s *winServer) WriteDiskConfig4(c *V4ServerConf)             {}
+func (winServer) AddStaticLease(_ *Lease) (err error)          { return nil }
-func (s *winServer) WriteDiskConfig6(c *V6ServerConf)             {}
+func (winServer) RemoveStaticLease(_ *Lease) (err error)       { return nil }
-func (s *winServer) Start() (err error)                           { return nil }
+func (winServer) FindMACbyIP(_ net.IP) (mac net.HardwareAddr)  { return nil }
-func (s *winServer) Stop() (err error)                            { return nil }
+func (winServer) WriteDiskConfig4(_ *V4ServerConf)             {}
-func v4Create(conf V4ServerConf) (DHCPServer, error)              { return &winServer{}, nil }
+func (winServer) WriteDiskConfig6(_ *V6ServerConf)             {}
-func v6Create(conf V6ServerConf) (DHCPServer, error)              { return &winServer{}, nil }
+func (winServer) Start() (err error)                           { return nil }
+func (winServer) Stop() (err error)                            { return nil }
+
+func v4Create(_ *V4ServerConf) (s DHCPServer, err error) { return winServer{}, nil }
+func v6Create(_ V6ServerConf) (s DHCPServer, err error)  { return winServer{}, nil }

internal/dhcpd/v4_unix.go

@@ -1,5 +1,4 @@
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+//go:build darwin || freebsd || linux || openbsd
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
 
 package dhcpd
 
@@ -30,8 +29,9 @@ import (
 //
 // TODO(a.garipov): Think about unifying this and v6Server.
 type v4Server struct {
-	conf V4ServerConf
+	conf *V4ServerConf
-	srv  *server4.Server
+
+	srv *server4.Server
 
 	// implicitOpts are the options listed in Appendix A of RFC 2131 initialized
 	// with default values.  It must not have intersections with [explicitOpts].
@@ -55,9 +55,15 @@ type v4Server struct {
 	leases []*Lease
 }
 
+func (s *v4Server) enabled() (ok bool) {
+	return s.conf != nil && s.conf.Enabled
+}
+
 // WriteDiskConfig4 - write configuration
 func (s *v4Server) WriteDiskConfig4(c *V4ServerConf) {
-	*c = s.conf
+	if s.conf != nil {
+		*c = *s.conf
+	}
 }
 
 // WriteDiskConfig6 - write configuration
@@ -114,8 +120,8 @@ func (s *v4Server) validHostnameForClient(cliHostname string, ip net.IP) (hostna
 func (s *v4Server) ResetLeases(leases []*Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "dhcpv4: %w") }()
 
-	if !s.conf.Enabled {
+	if s.conf == nil {
-		return
+		return nil
 	}
 
 	s.leasedOffsets = newBitSet()
@@ -129,12 +135,7 @@ func (s *v4Server) ResetLeases(leases []*Lease) (err error) {
 		err = s.addLease(l)
 		if err != nil {
 			// TODO(a.garipov): Wrap and bubble up the error.
-			log.Error(
+			log.Error("dhcpv4: reset: re-adding a lease for %s (%s): %s", l.IP, l.HWAddr, err)
-				"dhcpv4: reset: re-adding a lease for %s (%s): %s",
-				l.IP,
-				l.HWAddr,
-				err,
-			)
 
 			continue
 		}
@@ -336,11 +337,19 @@ func (s *v4Server) rmLease(lease *Lease) (err error) {
 	return errors.Error("lease not found")
 }
 
+// ErrUnconfigured is returned from the server's method when it requires the
+// server to be configured and it's not.
+const ErrUnconfigured errors.Error = "server is unconfigured"
+
 // AddStaticLease implements the DHCPServer interface for *v4Server.  It is safe
 // for concurrent use.
 func (s *v4Server) AddStaticLease(l *Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "dhcpv4: adding static lease: %w") }()
 
+	if s.conf == nil {
+		return ErrUnconfigured
+	}
+
 	ip := l.IP.To4()
 	if ip == nil {
 		return fmt.Errorf("invalid ip %q, only ipv4 is supported", l.IP)
@@ -414,6 +423,10 @@ func (s *v4Server) AddStaticLease(l *Lease) (err error) {
 func (s *v4Server) RemoveStaticLease(l *Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "dhcpv4: %w") }()
 
+	if s.conf == nil {
+		return ErrUnconfigured
+	}
+
 	if len(l.IP) != 4 {
 		return fmt.Errorf("invalid IP")
 	}
@@ -1140,7 +1153,7 @@ func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DH
 func (s *v4Server) Start() (err error) {
 	defer func() { err = errors.Annotate(err, "dhcpv4: %w") }()
 
-	if !s.conf.Enabled {
+	if !s.enabled() {
 		return nil
 	}
 
@@ -1232,62 +1245,20 @@ func (s *v4Server) Stop() (err error) {
 }
 
 // Create DHCPv4 server
-func v4Create(conf V4ServerConf) (srv DHCPServer, err error) {
+func v4Create(conf *V4ServerConf) (srv *v4Server, err error) {
-	s := &v4Server{}
+	s := &v4Server{
-	s.conf = conf
+		leaseHosts: stringutil.NewSet(),
-	s.leaseHosts = stringutil.NewSet()
-
-	// TODO(a.garipov): Don't use a disabled server in other places or just
-	// use an interface.
-	if !conf.Enabled {
-		return s, nil
 	}
 
-	var routerIP net.IP
+	err = conf.Validate()
-	routerIP, err = tryTo4(s.conf.GatewayIP)
 	if err != nil {
-		return s, fmt.Errorf("dhcpv4: %w", err)
+		// TODO(a.garipov): Don't use a disabled server in other places or just
+		// use an interface.
+		return s, err
 	}
 
-	if s.conf.SubnetMask == nil {
+	s.conf = &V4ServerConf{}
-		return s, fmt.Errorf("dhcpv4: invalid subnet mask: %v", s.conf.SubnetMask)
+	*s.conf = *conf
-	}
-
-	subnetMask := make([]byte, 4)
-	copy(subnetMask, s.conf.SubnetMask.To4())
-
-	s.conf.subnet = &net.IPNet{
-		IP:   routerIP,
-		Mask: subnetMask,
-	}
-	s.conf.broadcastIP = aghnet.BroadcastFromIPNet(s.conf.subnet)
-
-	s.conf.ipRange, err = newIPRange(conf.RangeStart, conf.RangeEnd)
-	if err != nil {
-		return s, fmt.Errorf("dhcpv4: %w", err)
-	}
-
-	if s.conf.ipRange.contains(routerIP) {
-		return s, fmt.Errorf("dhcpv4: gateway ip %v in the ip range: %v-%v",
-			routerIP,
-			conf.RangeStart,
-			conf.RangeEnd,
-		)
-	}
-
-	if !s.conf.subnet.Contains(conf.RangeStart) {
-		return s, fmt.Errorf("dhcpv4: range start %v is outside network %v",
-			conf.RangeStart,
-			s.conf.subnet,
-		)
-	}
-
-	if !s.conf.subnet.Contains(conf.RangeEnd) {
-		return s, fmt.Errorf("dhcpv4: range end %v is outside network %v",
-			conf.RangeEnd,
-			s.conf.subnet,
-		)
-	}
 
 	// TODO(a.garipov, d.seregin): Check that every lease is inside the IPRange.
 	s.leasedOffsets = newBitSet()
@@ -1299,7 +1270,7 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) {
 		s.conf.leaseTime = time.Second * time.Duration(conf.LeaseDuration)
 	}
 
-	s.implicitOpts, s.explicitOpts = prepareOptions(s.conf)
+	s.prepareOptions()
 
 	return s, nil
 }

internal/dhcpd/v4_unix_test.go

@@ -1,5 +1,4 @@
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+//go:build darwin || freebsd || linux || openbsd
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
 
 package dhcpd
 
@@ -30,19 +29,16 @@ var (
 	DefaultSubnetMask = net.IP{255, 255, 255, 0}
 )
 
-func notify4(flags uint32) {
-}
-
 // defaultV4ServerConf returns the default configuration for *v4Server to use in
 // tests.
-func defaultV4ServerConf() (conf V4ServerConf) {
+func defaultV4ServerConf() (conf *V4ServerConf) {
-	return V4ServerConf{
+	return &V4ServerConf{
 		Enabled:    true,
 		RangeStart: DefaultRangeStart,
 		RangeEnd:   DefaultRangeEnd,
 		GatewayIP:  DefaultGatewayIP,
 		SubnetMask: DefaultSubnetMask,
-		notify:     notify4,
+		notify:     testNotify,
 		dnsIPAddrs: []net.IP{DefaultSelfIP},
 	}
 }
@@ -350,13 +346,10 @@ func TestV4Server_handle_optionsPriority(t *testing.T) {
 			defer func() { s.implicitOpts.Update(dhcpv4.OptDNS(defaultIP)) }()
 		}
 
-		ss, err := v4Create(conf)
+		var err error
+		s, err = v4Create(conf)
 		require.NoError(t, err)
 
-		var ok bool
-		s, ok = ss.(*v4Server)
-		require.True(t, ok)
-
 		s.conf.dnsIPAddrs = []net.IP{defaultIP}
 
 		return s
@@ -490,10 +483,9 @@ func TestV4Server_updateOptions(t *testing.T) {
 		require.NoError(t, err)
 
 		require.IsType(t, (*v4Server)(nil), s)
-		s4, _ := s.(*v4Server)
 
 		t.Run(tc.name, func(t *testing.T) {
-			s4.updateOptions(req, resp)
+			s.updateOptions(req, resp)
 
 			for c, v := range tc.wantOpts {
 				if v == nil {
@@ -596,13 +588,9 @@ func TestV4DynamicLease_Get(t *testing.T) {
 		"82 ip 1.2.3.4",
 	}
 
-	var err error
+	s, err := v4Create(conf)
-	sIface, err := v4Create(conf)
 	require.NoError(t, err)
 
-	s, ok := sIface.(*v4Server)
-	require.True(t, ok)
-
 	s.conf.dnsIPAddrs = []net.IP{{192, 168, 10, 1}}
 	s.implicitOpts.Update(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 

internal/dhcpd/v6_unix.go

@@ -1,5 +1,4 @@
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+//go:build darwin || freebsd || linux || openbsd
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
 
 package dhcpd
 
@@ -173,7 +172,7 @@ func (s *v6Server) rmDynamicLease(lease *Lease) (err error) {
 func (s *v6Server) AddStaticLease(l *Lease) (err error) {
 	defer func() { err = errors.Annotate(err, "dhcpv6: %w") }()
 
-	if len(l.IP) != 16 {
+	if len(l.IP) != net.IPv6len {
 		return fmt.Errorf("invalid IP")
 	}
 

internal/dhcpd/v6_unix_test.go

@@ -1,5 +1,4 @@
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+//go:build darwin || freebsd || linux || openbsd
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
 
 package dhcpd
 

internal/dnsforward/dns_test.go

@@ -267,7 +267,7 @@ func TestServer_ProcessDHCPHosts_localRestriction(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			s := &Server{
-				dhcpServer:        &testDHCP{},
+				dhcpServer:        testDHCP,
 				localDomainSuffix: defaultLocalDomainSuffix,
 				tableHostToIP: hostToIPTable{
 					"example." + defaultLocalDomainSuffix: knownIP,
@@ -378,7 +378,7 @@ func TestServer_ProcessDHCPHosts(t *testing.T) {
 
 	for _, tc := range testCases {
 		s := &Server{
-			dhcpServer:        &testDHCP{},
+			dhcpServer:        testDHCP,
 			localDomainSuffix: tc.suffix,
 			tableHostToIP: hostToIPTable{
 				"example." + tc.suffix: knownIP,

internal/dnsforward/dnsforward.go

@@ -58,10 +58,10 @@ type hostToIPTable = map[string]net.IP
 //
 // The zero Server is empty and ready for use.
 type Server struct {
-	dnsProxy   *proxy.Proxy          // DNS proxy instance
+	dnsProxy   *proxy.Proxy         // DNS proxy instance
-	dnsFilter  *filtering.DNSFilter  // DNS filter instance
+	dnsFilter  *filtering.DNSFilter // DNS filter instance
-	dhcpServer dhcpd.ServerInterface // DHCP server instance (optional)
+	dhcpServer dhcpd.Interface      // DHCP server instance (optional)
-	queryLog   querylog.QueryLog     // Query log instance
+	queryLog   querylog.QueryLog    // Query log instance
 	stats      stats.Interface
 	access     *accessCtx
 
@@ -110,7 +110,7 @@ type DNSCreateParams struct {
 	DNSFilter   *filtering.DNSFilter
 	Stats       stats.Interface
 	QueryLog    querylog.QueryLog
-	DHCPServer  dhcpd.ServerInterface
+	DHCPServer  dhcpd.Interface
 	PrivateNets netutil.SubnetSet
 	Anonymizer  *aghnet.IPMut
 	LocalDomain string

internal/dnsforward/dnsforward_test.go

@@ -72,7 +72,7 @@ func createTestServer(
 
 	var err error
 	s, err = NewServer(DNSCreateParams{
-		DHCPServer:  &testDHCP{},
+		DHCPServer:  testDHCP,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
 	})
@@ -776,7 +776,7 @@ func TestBlockedCustomIP(t *testing.T) {
 
 	f := filtering.New(&filtering.Config{}, filters)
 	s, err := NewServer(DNSCreateParams{
-		DHCPServer:  &testDHCP{},
+		DHCPServer:  testDHCP,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
 	})
@@ -910,7 +910,7 @@ func TestRewrite(t *testing.T) {
 	f.SetEnabled(true)
 
 	s, err := NewServer(DNSCreateParams{
-		DHCPServer:  &testDHCP{},
+		DHCPServer:  testDHCP,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
 	})
@@ -1005,26 +1005,36 @@ func publicKey(priv any) any {
 	}
 }
 
-type testDHCP struct{}
+var testDHCP = &dhcpd.MockInterface{
-
+	OnStart:   func() (err error) { panic("not implemented") },
-func (d *testDHCP) Enabled() (ok bool) { return true }
+	OnStop:    func() (err error) { panic("not implemented") },
-
+	OnEnabled: func() (ok bool) { return true },
-func (d *testDHCP) Leases(flags dhcpd.GetLeasesFlags) (leases []*dhcpd.Lease) {
+	OnLeases: func(flags dhcpd.GetLeasesFlags) (leases []*dhcpd.Lease) {
-	return []*dhcpd.Lease{{
+		return []*dhcpd.Lease{{
-		IP:       net.IP{192, 168, 12, 34},
+			IP:       net.IP{192, 168, 12, 34},
-		HWAddr:   net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
+			HWAddr:   net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
-		Hostname: "myhost",
+			Hostname: "myhost",
-	}}
+		}}
+	},
+	OnSetOnLeaseChanged: func(olct dhcpd.OnLeaseChangedT) {},
+	OnFindMACbyIP:       func(ip net.IP) (mac net.HardwareAddr) { panic("not implemented") },
+	OnWriteDiskConfig:   func(c *dhcpd.ServerConfig) { panic("not implemented") },
 }
 
-func (d *testDHCP) SetOnLeaseChanged(onLeaseChanged dhcpd.OnLeaseChangedT) {}
+// func (*testDHCP) Leases(flags dhcpd.GetLeasesFlags) (leases []*dhcpd.Lease) {
+// 	return []*dhcpd.Lease{{
+// 		IP:       net.IP{192, 168, 12, 34},
+// 		HWAddr:   net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
+// 		Hostname: "myhost",
+// 	}}
+// }
 
 func TestPTRResponseFromDHCPLeases(t *testing.T) {
 	const localDomain = "lan"
 
 	s, err := NewServer(DNSCreateParams{
 		DNSFilter:   filtering.New(&filtering.Config{}, nil),
-		DHCPServer:  &testDHCP{},
+		DHCPServer:  testDHCP,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
 		LocalDomain: localDomain,
 	})
@@ -1097,7 +1107,7 @@ func TestPTRResponseFromHosts(t *testing.T) {
 
 	var s *Server
 	s, err = NewServer(DNSCreateParams{
-		DHCPServer:  &testDHCP{},
+		DHCPServer:  testDHCP,
 		DNSFilter:   flt,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
 	})

internal/dnsforward/filter_test.go

@@ -39,7 +39,7 @@ func TestHandleDNSRequest_filterDNSResponse(t *testing.T) {
 	f.SetEnabled(true)
 
 	s, err := NewServer(DNSCreateParams{
-		DHCPServer:  &testDHCP{},
+		DHCPServer:  testDHCP,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
 	})

internal/home/clients.go

@@ -126,7 +126,7 @@ type clientsContainer struct {
 	allTags *stringutil.Set
 
 	// dhcpServer is used for looking up clients IP addresses by MAC addresses
-	dhcpServer *dhcpd.Server
+	dhcpServer dhcpd.Interface
 
 	// dnsServer is used for checking clients IP status access list status
 	dnsServer *dnsforward.Server
@@ -146,7 +146,7 @@ type clientsContainer struct {
 // Note: this function must be called only once
 func (clients *clientsContainer) Init(
 	objects []*clientObject,
-	dhcpServer *dhcpd.Server,
+	dhcpServer dhcpd.Interface,
 	etcHosts *aghnet.HostsContainer,
 	arpdb aghnet.ARPDB,
 ) {

internal/home/clients_test.go

@@ -279,8 +279,6 @@ func TestClientsAddExisting(t *testing.T) {
 			t.Skip("skipping dhcp test on windows")
 		}
 
-		var err error
-
 		ip := net.IP{1, 2, 3, 4}
 
 		// First, init a DHCP server with a single static lease.
@@ -296,13 +294,15 @@ func TestClientsAddExisting(t *testing.T) {
 			},
 		}
 
-		clients.dhcpServer, err = dhcpd.Create(config)
+		dhcpServer, err := dhcpd.Create(config)
 		require.NoError(t, err)
 		testutil.CleanupAndRequireSuccess(t, func() (err error) {
 			return os.Remove("leases.db")
 		})
 
-		err = clients.dhcpServer.AddStaticLease(&dhcpd.Lease{
+		clients.dhcpServer = dhcpServer
+
+		err = dhcpServer.AddStaticLease(&dhcpd.Lease{
 			HWAddr:   net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA},
 			IP:       ip,
 			Hostname: "testhost",

internal/home/config.go

@@ -433,9 +433,7 @@ func (c *configuration) write() (err error) {
 	}
 
 	if Context.dhcpServer != nil {
-		c := &dhcpd.ServerConfig{}
+		Context.dhcpServer.WriteDiskConfig(config.DHCP)
-		Context.dhcpServer.WriteDiskConfig(c)
-		config.DHCP = c
 	}
 
 	config.Clients.Persistent = Context.clients.forConfig()

internal/home/home.go

@@ -53,7 +53,7 @@ type homeContext struct {
 	rdns       *RDNS                // rDNS module
 	whois      *WHOIS               // WHOIS module
 	dnsFilter  *filtering.DNSFilter // DNS filtering module
-	dhcpServer *dhcpd.Server        // DHCP module
+	dhcpServer dhcpd.Interface      // DHCP module
 	auth       *Auth                // HTTP authentication module
 	filters    Filtering            // DNS filtering module
 	web        *Web                 // Web (HTTP, HTTPS) module
@@ -641,14 +641,9 @@ func configureLogger(args options) {
 			log.Fatalf("cannot initialize syslog: %s", err)
 		}
 	} else {
-		logFilePath := filepath.Join(Context.workDir, ls.File)
+		logFilePath := ls.File
-		if filepath.IsAbs(ls.File) {
+		if !filepath.IsAbs(logFilePath) {
-			logFilePath = ls.File
+			logFilePath = filepath.Join(Context.workDir, logFilePath)
-		}
-
-		_, err := os.OpenFile(logFilePath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644)
-		if err != nil {
-			log.Fatalf("cannot create a log file: %s", err)
 		}
 
 		log.SetOutput(&lumberjack.Logger{