diff --git a/internal/dhcpd/README.md b/internal/dhcpd/README.md index fb2bdc8d..5c692e04 100644 --- a/internal/dhcpd/README.md +++ b/internal/dhcpd/README.md @@ -1,46 +1,60 @@ -# DHCP server + # Testing DHCP Server Contents: -* [Test setup with Virtual Box](#vbox) + * [Test setup with Virtual Box](#vbox) + * [Quick test with DHCPTest](#dhcptest) - -## Test setup with Virtual Box +## Test setup with Virtual Box -To set up a test environment for DHCP server you need: + ### Prerequisites -* Linux host machine -* Virtual Box -* Virtual machine (guest OS doesn't matter) +To set up a test environment for DHCP server you will need: -### Configure client + * Linux AG Home host machine (Virtual). + * Virtual Box. + * Virtual machine (guest OS doesn't matter). -1. Install Virtual Box and run the following command to create a Host-Only network: + ### Configure Virtual Box - $ VBoxManage hostonlyif create + 1. Install Virtual Box and run the following command to create a Host-Only + network: - You can check its status by `ip a` command. + ```sh + $ VBoxManage hostonlyif create + ``` + + You can check its status by `ip a` command. - You can also set up Host-Only network using Virtual Box menu: + You can also set up Host-Only network using Virtual Box menu: + + ``` + File -> Host Network Manager... + ``` - File -> Host Network Manager... + 2. Create your virtual machine and set up its network: -2. Create your virtual machine and set up its network: + ``` + VM Settings -> Network -> Host-only Adapter + ``` - VM Settings -> Network -> Host-only Adapter + 3. Start your VM, install an OS. Configure your network interface to use + DHCP and the OS should ask for a IP address from our DHCP server. -3. Start your VM, install an OS. Configure your network interface to use DHCP and the OS should ask for a IP address from our DHCP server. + 4. To see the current IP addresses on client OS you can use `ip a` command on + Linux or `ipconfig` on Windows. -4. To see the current IP address on client OS you can use `ip a` command on Linux or `ipconfig` on Windows. + 5. To force the client OS to request an IP from DHCP server again, you can + use `dhclient` on Linux or `ipconfig /release` on Windows. -5. To force the client OS to request an IP from DHCP server again, you can use `dhclient` on Linux or `ipconfig /release` on Windows. + ### Configure server -### Configure server + 1. Edit server configuration file `AdGuardHome.yaml`, for example: -1. Edit server configuration file 'AdGuardHome.yaml', for example: - - dhcp: + ```yaml + dhcp: enabled: true interface_name: vboxnet0 + local_domain_name: lan dhcpv4: gateway_ip: 192.168.56.1 subnet_mask: 255.255.255.0 @@ -54,11 +68,29 @@ To set up a test environment for DHCP server you need: lease_duration: 86400 ra_slaac_only: false ra_allow_slaac: false + ``` -2. Start the server + 2. Start the server - ./AdGuardHome + ```sh + ./AdGuardHome -v + ``` - There should be a message in log which shows that DHCP server is ready: + There should be a message in log which shows that DHCP server is ready: - [info] DHCP: listening on 0.0.0.0:67 + ``` + [info] DHCP: listening on 0.0.0.0:67 + ``` + +## Quick test with DHCPTest utility + + ### Prerequisites + + * [DHCP test utility][dhcptest-gh]. + + ### Quick test + +The DHCP server could be tested for DISCOVER-OFFER packets with in +interactive mode. + +[dhcptest-gh]: https://github.com/CyberShadow/dhcptest diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index 69082c0c..5a3656d1 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -239,36 +239,16 @@ func Create(conf *ServerConfig) (s *server, err error) { // [aghhttp.RegisterFunc]. s.registerHandlers() - v4conf := conf.Conf4 - v4conf.InterfaceName = s.conf.InterfaceName - v4conf.notify = s.onNotify - v4conf.Enabled = s.conf.Enabled && v4conf.RangeStart.IsValid() - - s.srv4, err = v4Create(&v4conf) + v4Enabled, v6Enabled, err := s.setServers(conf) if err != nil { - if v4conf.Enabled { - return nil, fmt.Errorf("creating dhcpv4 srv: %w", err) - } - - log.Debug("dhcpd: warning: creating dhcpv4 srv: %s", err) - } - - v6conf := conf.Conf6 - v6conf.Enabled = s.conf.Enabled - if len(v6conf.RangeStart) == 0 { - v6conf.Enabled = false - } - v6conf.InterfaceName = s.conf.InterfaceName - v6conf.notify = s.onNotify - s.srv6, err = v6Create(v6conf) - if err != nil { - return nil, fmt.Errorf("creating dhcpv6 srv: %w", err) + // Don't wrap the error, because it's informative enough as is. + return nil, err } s.conf.Conf4 = conf.Conf4 s.conf.Conf6 = conf.Conf6 - if s.conf.Enabled && !v4conf.Enabled && !v6conf.Enabled { + if s.conf.Enabled && !v4Enabled && !v6Enabled { return nil, fmt.Errorf("neither dhcpv4 nor dhcpv6 srv is configured") } @@ -289,6 +269,39 @@ func Create(conf *ServerConfig) (s *server, err error) { return s, nil } +// setServers updates DHCPv4 and DHCPv6 servers created from the provided +// configuration conf. +func (s *server) setServers(conf *ServerConfig) (v4Enabled, v6Enabled bool, err error) { + v4conf := conf.Conf4 + v4conf.InterfaceName = s.conf.InterfaceName + v4conf.notify = s.onNotify + v4conf.Enabled = s.conf.Enabled && v4conf.RangeStart.IsValid() + + s.srv4, err = v4Create(&v4conf) + if err != nil { + if v4conf.Enabled { + return true, false, fmt.Errorf("creating dhcpv4 srv: %w", err) + } + + log.Debug("dhcpd: warning: creating dhcpv4 srv: %s", err) + } + + v6conf := conf.Conf6 + v6conf.InterfaceName = s.conf.InterfaceName + v6conf.notify = s.onNotify + v6conf.Enabled = s.conf.Enabled + if len(v6conf.RangeStart) == 0 { + v6conf.Enabled = false + } + + s.srv6, err = v6Create(v6conf) + if err != nil { + return v4conf.Enabled, v6conf.Enabled, fmt.Errorf("creating dhcpv6 srv: %w", err) + } + + return v4conf.Enabled, v6conf.Enabled, nil +} + // Enabled returns true when the server is enabled. func (s *server) Enabled() (ok bool) { return s.conf.Enabled diff --git a/internal/dhcpd/http_unix.go b/internal/dhcpd/http_unix.go index 23e7fe81..b07f9543 100644 --- a/internal/dhcpd/http_unix.go +++ b/internal/dhcpd/http_unix.go @@ -16,6 +16,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/netutil" ) type v4ServerConfJSON struct { @@ -263,6 +264,28 @@ func (s *server) handleDHCPSetConfigV6( return srv6, enabled, err } +// createServers returns DHCPv4 and DHCPv6 servers created from the provided +// configuration conf. +func (s *server) createServers(conf *dhcpServerConfigJSON) (srv4, srv6 DHCPServer, err error) { + srv4, v4Enabled, err := s.handleDHCPSetConfigV4(conf) + if err != nil { + return nil, nil, fmt.Errorf("bad dhcpv4 configuration: %s", err) + } + + srv6, v6Enabled, err := s.handleDHCPSetConfigV6(conf) + if err != nil { + return nil, nil, fmt.Errorf("bad dhcpv6 configuration: %s", err) + } + + if conf.Enabled == aghalg.NBTrue && !v4Enabled && !v6Enabled { + return nil, nil, fmt.Errorf("dhcpv4 or dhcpv6 configuration must be complete") + } + + return srv4, srv6, nil +} + +// handleDHCPSetConfig is the handler for the POST /control/dhcp/set_config +// HTTP API. func (s *server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) { conf := &dhcpServerConfigJSON{} conf.Enabled = aghalg.BoolToNullBool(s.conf.Enabled) @@ -275,22 +298,9 @@ func (s *server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) { return } - srv4, v4Enabled, err := s.handleDHCPSetConfigV4(conf) + srv4, srv6, err := s.createServers(conf) if err != nil { - aghhttp.Error(r, w, http.StatusBadRequest, "bad dhcpv4 configuration: %s", err) - - return - } - - srv6, v6Enabled, err := s.handleDHCPSetConfigV6(conf) - if err != nil { - aghhttp.Error(r, w, http.StatusBadRequest, "bad dhcpv6 configuration: %s", err) - - return - } - - if conf.Enabled == aghalg.NBTrue && !v4Enabled && !v6Enabled { - aghhttp.Error(r, w, http.StatusBadRequest, "dhcpv4 or dhcpv6 configuration must be complete") + aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) return } @@ -350,10 +360,10 @@ type netInterfaceJSON struct { Addrs6 []netip.Addr `json:"ipv6_addresses"` } -// handleDHCPInterfaces is the handler for the GET /control/dhcp/interfaces HTTP -// API. +// handleDHCPInterfaces is the handler for the GET /control/dhcp/interfaces +// HTTP API. func (s *server) handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) { - resp := map[string]netInterfaceJSON{} + resp := map[string]*netInterfaceJSON{} ifaces, err := net.Interfaces() if err != nil { @@ -364,68 +374,23 @@ func (s *server) handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) { for _, iface := range ifaces { if iface.Flags&net.FlagLoopback != 0 { - // it's a loopback, skip it - continue - } - if iface.Flags&net.FlagBroadcast == 0 { - // this interface doesn't support broadcast, skip it + // It's a loopback, skip it. continue } - var addrs []net.Addr - addrs, err = iface.Addrs() - if err != nil { - aghhttp.Error( - r, - w, - http.StatusInternalServerError, - "Failed to get addresses for interface %s: %s", - iface.Name, - err, - ) + if iface.Flags&net.FlagBroadcast == 0 { + // This interface doesn't support broadcast, skip it. + continue + } + + jsonIface, iErr := newNetInterfaceJSON(iface) + if iErr != nil { + aghhttp.Error(r, w, http.StatusInternalServerError, "%s", iErr) return } - jsonIface := netInterfaceJSON{ - Name: iface.Name, - HardwareAddr: iface.HardwareAddr.String(), - } - - if iface.Flags != 0 { - jsonIface.Flags = iface.Flags.String() - } - // we don't want link-local addresses in json, so skip them - for _, addr := range addrs { - ipnet, ok := addr.(*net.IPNet) - if !ok { - // not an IPNet, should not happen - aghhttp.Error( - r, - w, - http.StatusInternalServerError, - "got iface.Addrs() element %[1]s that is not net.IPNet, it is %[1]T", - addr) - - return - } - // ignore link-local - // - // TODO(e.burkov): Try to listen DHCP on LLA as well. - if ipnet.IP.IsLinkLocalUnicast() { - continue - } - - if ip4 := ipnet.IP.To4(); ip4 != nil { - addr := netip.AddrFrom4(*(*[4]byte)(ip4)) - jsonIface.Addrs4 = append(jsonIface.Addrs4, addr) - } else { - addr := netip.AddrFrom16(*(*[16]byte)(ipnet.IP)) - jsonIface.Addrs6 = append(jsonIface.Addrs6, addr) - } - } - if len(jsonIface.Addrs4)+len(jsonIface.Addrs6) != 0 { - jsonIface.GatewayIP = aghnet.GatewayIP(iface.Name) + if jsonIface != nil { resp[iface.Name] = jsonIface } } @@ -433,6 +398,64 @@ func (s *server) handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) { _ = aghhttp.WriteJSONResponse(w, r, resp) } +// newNetInterfaceJSON creates a JSON object from a [net.Interface] iface. +func newNetInterfaceJSON(iface net.Interface) (out *netInterfaceJSON, err error) { + addrs, err := iface.Addrs() + if err != nil { + return nil, fmt.Errorf( + "failed to get addresses for interface %s: %s", + iface.Name, + err, + ) + } + + out = &netInterfaceJSON{ + Name: iface.Name, + HardwareAddr: iface.HardwareAddr.String(), + } + + if iface.Flags != 0 { + out.Flags = iface.Flags.String() + } + + // We don't want link-local addresses in JSON, so skip them. + for _, addr := range addrs { + ipNet, ok := addr.(*net.IPNet) + if !ok { + // Not an IPNet, should not happen. + return nil, fmt.Errorf("got iface.Addrs() element %[1]s that is not"+ + " net.IPNet, it is %[1]T", addr) + } + + // Ignore link-local. + // + // TODO(e.burkov): Try to listen DHCP on LLA as well. + if ipNet.IP.IsLinkLocalUnicast() { + continue + } + + vAddr, iErr := netutil.IPToAddrNoMapped(ipNet.IP) + if iErr != nil { + // Not an IPNet, should not happen. + return nil, fmt.Errorf("failed to convert IP address %[1]s: %w", addr, iErr) + } + + if vAddr.Is4() { + out.Addrs4 = append(out.Addrs4, vAddr) + } else { + out.Addrs6 = append(out.Addrs6, vAddr) + } + } + + if len(out.Addrs4)+len(out.Addrs6) == 0 { + return nil, nil + } + + out.GatewayIP = aghnet.GatewayIP(iface.Name) + + return out, nil +} + // dhcpSearchOtherResult contains information about other DHCP server for // specific network interface. type dhcpSearchOtherResult struct { diff --git a/internal/dhcpd/routeradv.go b/internal/dhcpd/routeradv.go index 9c87ca9f..a826df91 100644 --- a/internal/dhcpd/routeradv.go +++ b/internal/dhcpd/routeradv.go @@ -7,6 +7,7 @@ import ( "sync/atomic" "time" + "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" "golang.org/x/net/icmp" @@ -195,7 +196,7 @@ func createICMPv6RAPacket(params icmpv6RA) (data []byte, err error) { return data, nil } -// Init - initialize RA module +// Init initializes RA module. func (ra *raCtx) Init() (err error) { ra.stop.Store(0) ra.conn = nil @@ -203,8 +204,7 @@ func (ra *raCtx) Init() (err error) { return nil } - log.Debug("dhcpv6 ra: source IP address: %s DNS IP address: %s", - ra.ipAddr, ra.dnsIPAddr) + log.Debug("dhcpv6 ra: source IP address: %s DNS IP address: %s", ra.ipAddr, ra.dnsIPAddr) params := icmpv6RA{ managedAddressConfiguration: !ra.raSLAACOnly, @@ -223,18 +223,15 @@ func (ra *raCtx) Init() (err error) { return fmt.Errorf("creating packet: %w", err) } - success := false ipAndScope := ra.ipAddr.String() + "%" + ra.ifaceName ra.conn, err = icmp.ListenPacket("ip6:ipv6-icmp", ipAndScope) if err != nil { return fmt.Errorf("dhcpv6 ra: icmp.ListenPacket: %w", err) } + defer func() { - if !success { - derr := ra.Close() - if derr != nil { - log.Error("closing context: %s", derr) - } + if err != nil { + err = errors.WithDeferred(err, ra.Close()) } }() @@ -269,7 +266,6 @@ func (ra *raCtx) Init() (err error) { log.Debug("dhcpv6 ra: loop exit") }() - success = true return nil } diff --git a/internal/dhcpd/v4_unix.go b/internal/dhcpd/v4_unix.go index 20b2c96e..34f96210 100644 --- a/internal/dhcpd/v4_unix.go +++ b/internal/dhcpd/v4_unix.go @@ -342,8 +342,8 @@ func (s *v4Server) rmLease(lease *Lease) (err error) { // 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. +// 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") }() @@ -354,21 +354,23 @@ func (s *v4Server) AddStaticLease(l *Lease) (err error) { l.IP = l.IP.Unmap() if !l.IP.Is4() { - return fmt.Errorf("invalid ip %q, only ipv4 is supported", l.IP) + return fmt.Errorf("invalid IP %q: only IPv4 is supported", l.IP) } else if gwIP := s.conf.GatewayIP; gwIP == l.IP { - return fmt.Errorf("can't assign the gateway IP %s to the lease", gwIP) + return fmt.Errorf("can't assign the gateway IP %q to the lease", gwIP) } l.IsStatic = true err = netutil.ValidateMAC(l.HWAddr) if err != nil { + // Don't wrap the error, because it's informative enough as is. return err } if hostname := l.Hostname; hostname != "" { hostname, err = normalizeHostname(hostname) if err != nil { + // Don't wrap the error, because it's informative enough as is. return err } @@ -386,32 +388,9 @@ func (s *v4Server) AddStaticLease(l *Lease) (err error) { l.Hostname = hostname } - // Perform the following actions in an anonymous function to make sure - // that the lock gets unlocked before the notification step. - func() { - s.leasesLock.Lock() - defer s.leasesLock.Unlock() - - err = s.rmDynamicLease(l) - if err != nil { - err = fmt.Errorf( - "removing dynamic leases for %s (%s): %w", - l.IP, - l.HWAddr, - err, - ) - - return - } - - err = s.addLease(l) - if err != nil { - err = fmt.Errorf("adding static lease for %s (%s): %w", l.IP, l.HWAddr, err) - - return - } - }() + err = s.updateStaticLease(l) if err != nil { + // Don't wrap the error, because it's informative enough as is. return err } @@ -421,6 +400,25 @@ func (s *v4Server) AddStaticLease(l *Lease) (err error) { return nil } +// updateStaticLease safe removes dynamic lease with the same properties and +// then adds a static lease l. +func (s *v4Server) updateStaticLease(l *Lease) (err error) { + s.leasesLock.Lock() + defer s.leasesLock.Unlock() + + err = s.rmDynamicLease(l) + if err != nil { + return fmt.Errorf("removing dynamic leases for %s (%s): %w", l.IP, l.HWAddr, err) + } + + err = s.addLease(l) + if err != nil { + return fmt.Errorf("adding static lease for %s (%s): %w", l.IP, l.HWAddr, err) + } + + return nil +} + // RemoveStaticLease removes a static lease. It is safe for concurrent use. func (s *v4Server) RemoveStaticLease(l *Lease) (err error) { defer func() { err = errors.Annotate(err, "dhcpv4: %w") }() @@ -894,24 +892,9 @@ func (s *v4Server) handleDecline(req, resp *dhcpv4.DHCPv4) (err error) { reqIP = req.ClientIPAddr } - netIP, ok := netip.AddrFromSlice(reqIP) - if !ok { - log.Info("dhcpv4: invalid IP: %s", reqIP) - - return nil - } - - var oldLease *Lease - for _, l := range s.leases { - if bytes.Equal(l.HWAddr, mac) && l.IP == netIP { - oldLease = l - - break - } - } - + oldLease := s.findLeaseForIP(reqIP, mac) if oldLease == nil { - log.Info("dhcpv4: lease with ip %s for %s not found", reqIP, mac) + log.Info("dhcpv4: lease with IP %s for %s not found", reqIP, mac) return nil } @@ -925,7 +908,7 @@ func (s *v4Server) handleDecline(req, resp *dhcpv4.DHCPv4) (err error) { if err != nil { return fmt.Errorf("allocating new lease for %s: %w", mac, err) } else if newLease == nil { - log.Info("dhcpv4: allocating new lease for %s: no more ip addresses", mac) + log.Info("dhcpv4: allocating new lease for %s: no more IP addresses", mac) resp.YourIPAddr = make([]byte, 4) resp.UpdateOption(dhcpv4.OptMessageType(dhcpv4.MessageTypeAck)) @@ -941,15 +924,32 @@ func (s *v4Server) handleDecline(req, resp *dhcpv4.DHCPv4) (err error) { return fmt.Errorf("adding new lease for %s: %w", mac, err) } - log.Info("dhcpv4: changed ip from %s to %s for %s", reqIP, newLease.IP, mac) - - resp.YourIPAddr = net.IP(newLease.IP.AsSlice()) + log.Info("dhcpv4: changed IP from %s to %s for %s", reqIP, newLease.IP, mac) + resp.YourIPAddr = newLease.IP.AsSlice() resp.UpdateOption(dhcpv4.OptMessageType(dhcpv4.MessageTypeAck)) return nil } +// findLeaseForIP returns a lease for provided ip and mac. +func (s *v4Server) findLeaseForIP(ip net.IP, mac net.HardwareAddr) (l *Lease) { + netIP, ok := netip.AddrFromSlice(ip) + if !ok { + log.Info("dhcpv4: invalid IP: %s", ip) + + return nil + } + + for _, il := range s.leases { + if bytes.Equal(il.HWAddr, mac) && il.IP == netIP { + return il + } + } + + return nil +} + // handleRelease is the handler for the DHCP Release request. func (s *v4Server) handleRelease(req, resp *dhcpv4.DHCPv4) (err error) { mac := req.ClientHWAddr @@ -995,11 +995,80 @@ func (s *v4Server) handleRelease(req, resp *dhcpv4.DHCPv4) (err error) { return nil } -// Find a lease associated with MAC and prepare response -// Return 1: OK -// Return 0: error; reply with Nak -// Return -1: error; don't reply -func (s *v4Server) handle(req, resp *dhcpv4.DHCPv4) int { +// messageHandler describes a DHCPv4 message handler function. +type messageHandler func(s *v4Server, req, resp *dhcpv4.DHCPv4) (rCode int, l *Lease, err error) + +// messageHandlers is a map of handlers for various messages with message types +// keys. +var messageHandlers = map[dhcpv4.MessageType]messageHandler{ + dhcpv4.MessageTypeDiscover: func( + s *v4Server, + req *dhcpv4.DHCPv4, + resp *dhcpv4.DHCPv4, + ) (rCode int, l *Lease, err error) { + l, err = s.handleDiscover(req, resp) + if err != nil { + return 0, nil, fmt.Errorf("handling discover: %s", err) + } + + if l == nil { + return 0, nil, nil + } + + return 1, l, nil + }, + dhcpv4.MessageTypeRequest: func( + s *v4Server, + req *dhcpv4.DHCPv4, + resp *dhcpv4.DHCPv4, + ) (rCode int, l *Lease, err error) { + var toReply bool + l, toReply = s.handleRequest(req, resp) + if l == nil { + if toReply { + return 0, nil, nil + } + + // Drop the packet. + return -1, nil, nil + } + + return 1, l, nil + }, + dhcpv4.MessageTypeDecline: func( + s *v4Server, + req *dhcpv4.DHCPv4, + resp *dhcpv4.DHCPv4, + ) (rCode int, l *Lease, err error) { + err = s.handleDecline(req, resp) + if err != nil { + return 0, nil, fmt.Errorf("handling decline: %s", err) + } + + return 1, nil, nil + }, + dhcpv4.MessageTypeRelease: func( + s *v4Server, + req *dhcpv4.DHCPv4, + resp *dhcpv4.DHCPv4, + ) (rCode int, l *Lease, err error) { + err = s.handleRelease(req, resp) + if err != nil { + return 0, nil, fmt.Errorf("handling release: %s", err) + } + + return 1, nil, nil + }, +} + +// handle processes request, it finds a lease associated with MAC address and +// prepares response. +// +// Possible return values are: +// - "1": OK, +// - "0": error, reply with Nak, +// - "-1": error, don't reply. +func (s *v4Server) handle(req, resp *dhcpv4.DHCPv4) (rCode int) { var err error // Include server's identifier option since any reply should contain it. @@ -1007,47 +1076,26 @@ func (s *v4Server) handle(req, resp *dhcpv4.DHCPv4) int { // See https://datatracker.ietf.org/doc/html/rfc2131#page-29. resp.UpdateOption(dhcpv4.OptServerIdentifier(s.conf.dnsIPAddrs[0].AsSlice())) - // TODO(a.garipov): Refactor this into handlers. - var l *Lease - switch mt := req.MessageType(); mt { - case dhcpv4.MessageTypeDiscover: - l, err = s.handleDiscover(req, resp) - if err != nil { - log.Error("dhcpv4: handling discover: %s", err) + handler := messageHandlers[req.MessageType()] + if handler == nil { + s.updateOptions(req, resp) - return 0 - } + return 1 + } - if l == nil { - return 0 - } - case dhcpv4.MessageTypeRequest: - var toReply bool - l, toReply = s.handleRequest(req, resp) - if l == nil { - if toReply { - return 0 - } - return -1 // drop packet - } - case dhcpv4.MessageTypeDecline: - err = s.handleDecline(req, resp) - if err != nil { - log.Error("dhcpv4: handling decline: %s", err) + rCode, l, err := handler(s, req, resp) + if err != nil { + log.Error("dhcpv4: %s", err) - return 0 - } - case dhcpv4.MessageTypeRelease: - err = s.handleRelease(req, resp) - if err != nil { - log.Error("dhcpv4: handling release: %s", err) + return 0 + } - return 0 - } + if rCode != 1 { + return rCode } if l != nil { - resp.YourIPAddr = net.IP(l.IP.AsSlice()) + resp.YourIPAddr = l.IP.AsSlice() } s.updateOptions(req, resp) @@ -1162,23 +1210,8 @@ func (s *v4Server) Start() (err error) { // No available IP addresses which may appear later. return nil } - // Update the value of Domain Name Server option separately from others if - // not assigned yet since its value is available only at server's start. - // - // TODO(e.burkov): Initialize as implicit option with the rest of default - // options when it will be possible to do before the call to Start. - if !s.explicitOpts.Has(dhcpv4.OptionDomainNameServer) { - s.implicitOpts.Update(dhcpv4.OptDNS(dnsIPAddrs...)) - } - for _, ip := range dnsIPAddrs { - ip = ip.To4() - if ip == nil { - continue - } - - s.conf.dnsIPAddrs = append(s.conf.dnsIPAddrs, netip.AddrFrom4(*(*[4]byte)(ip))) - } + s.configureDNSIPAddrs(dnsIPAddrs) var c net.PacketConn if c, err = s.newDHCPConn(iface); err != nil { @@ -1199,10 +1232,10 @@ func (s *v4Server) Start() (err error) { log.Info("dhcpv4: listening") go func() { - if serr := s.srv.Serve(); errors.Is(serr, net.ErrClosed) { + if sErr := s.srv.Serve(); errors.Is(sErr, net.ErrClosed) { log.Info("dhcpv4: server is closed") - } else if serr != nil { - log.Error("dhcpv4: srv.Serve: %s", serr) + } else if sErr != nil { + log.Error("dhcpv4: srv.Serve: %s", sErr) } }() @@ -1213,6 +1246,28 @@ func (s *v4Server) Start() (err error) { return nil } +// configureDNSIPAddrs updates v4Server configuration with provided slice of +// dns IP addresses. +func (s *v4Server) configureDNSIPAddrs(dnsIPAddrs []net.IP) { + // Update the value of Domain Name Server option separately from others if + // not assigned yet since its value is available only at server's start. + // + // TODO(e.burkov): Initialize as implicit option with the rest of default + // options when it will be possible to do before the call to Start. + if !s.explicitOpts.Has(dhcpv4.OptionDomainNameServer) { + s.implicitOpts.Update(dhcpv4.OptDNS(dnsIPAddrs...)) + } + + for _, ip := range dnsIPAddrs { + vAddr, err := netutil.IPToAddr(ip, netutil.AddrFamilyIPv4) + if err != nil { + continue + } + + s.conf.dnsIPAddrs = append(s.conf.dnsIPAddrs, vAddr) + } +} + // Stop - stop server func (s *v4Server) Stop() (err error) { if s.srv == nil { diff --git a/internal/dhcpd/v4_unix_test.go b/internal/dhcpd/v4_unix_test.go index a5ce5e0e..162b5b88 100644 --- a/internal/dhcpd/v4_unix_test.go +++ b/internal/dhcpd/v4_unix_test.go @@ -227,7 +227,7 @@ func TestV4Server_AddRemove_static(t *testing.T) { }, name: "with_gateway_ip", wantErrMsg: "dhcpv4: adding static lease: " + - "can't assign the gateway IP 192.168.10.1 to the lease", + `can't assign the gateway IP "192.168.10.1" to the lease`, }, { lease: &Lease{ Hostname: "ip6.local", @@ -236,7 +236,7 @@ func TestV4Server_AddRemove_static(t *testing.T) { }, name: "ipv6", wantErrMsg: `dhcpv4: adding static lease: ` + - `invalid ip "ffff::1", only ipv4 is supported`, + `invalid IP "ffff::1": only IPv4 is supported`, }, { lease: &Lease{ Hostname: "bad-mac.local", diff --git a/internal/dhcpd/v6_unix.go b/internal/dhcpd/v6_unix.go index e192e58a..fa3640f9 100644 --- a/internal/dhcpd/v6_unix.go +++ b/internal/dhcpd/v6_unix.go @@ -586,9 +586,31 @@ func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6. } } -// initialize RA module -func (s *v6Server) initRA(iface *net.Interface) error { - // choose the source IP address - should be link-local-unicast +// configureDNSIPAddrs updates v6Server configuration with the slice of DNS IP +// addresses of provided interface iface. Initializes RA module. +func (s *v6Server) configureDNSIPAddrs(iface *net.Interface) (ok bool, err error) { + dnsIPAddrs, err := aghnet.IfaceDNSIPAddrs( + iface, + aghnet.IPVersion6, + defaultMaxAttempts, + defaultBackoff, + ) + if err != nil { + return false, fmt.Errorf("interface %s: %w", iface.Name, err) + } + + if len(dnsIPAddrs) == 0 { + return false, nil + } + + s.conf.dnsIPAddrs = dnsIPAddrs + + return true, s.initRA(iface) +} + +// initRA initializes RA module. +func (s *v6Server) initRA(iface *net.Interface) (err error) { + // Choose the source IP address - should be link-local-unicast. s.ra.ipAddr = s.conf.dnsIPAddrs[0] for _, ip := range s.conf.dnsIPAddrs { if ip.IsLinkLocalUnicast() { @@ -604,6 +626,7 @@ func (s *v6Server) initRA(iface *net.Interface) error { s.ra.ifaceName = s.conf.InterfaceName s.ra.iface = iface s.ra.packetSendPeriod = 1 * time.Second + return s.ra.Init() } @@ -623,37 +646,24 @@ func (s *v6Server) Start() (err error) { log.Debug("dhcpv6: starting...") - dnsIPAddrs, err := aghnet.IfaceDNSIPAddrs( - iface, - aghnet.IPVersion6, - defaultMaxAttempts, - defaultBackoff, - ) + ok, err := s.configureDNSIPAddrs(iface) if err != nil { - return fmt.Errorf("interface %s: %w", ifaceName, err) + // Don't wrap the error, because it's informative enough as is. + return err } - if len(dnsIPAddrs) == 0 { + if !ok { // No available IP addresses which may appear later. return nil } - s.conf.dnsIPAddrs = dnsIPAddrs - - err = s.initRA(iface) - if err != nil { - return err - } - - // don't initialize DHCPv6 server if we must force the clients to use SLAAC + // Don't initialize DHCPv6 server if we must force the clients to use SLAAC. if s.conf.RASLAACOnly { log.Debug("not starting dhcpv6 server due to ra_slaac_only=true") return nil } - log.Debug("dhcpv6: listening...") - err = netutil.ValidateMAC(iface.HardwareAddr) if err != nil { return fmt.Errorf("validating interface %s: %w", iface.Name, err) @@ -665,20 +675,18 @@ func (s *v6Server) Start() (err error) { Time: dhcpv6.GetTime(), } - laddr := &net.UDPAddr{ - IP: net.ParseIP("::"), - Port: dhcpv6.DefaultServerPort, - } - s.srv, err = server6.NewServer(iface.Name, laddr, s.packetHandler, server6.WithDebugLogger()) + s.srv, err = server6.NewServer(iface.Name, nil, s.packetHandler, server6.WithDebugLogger()) if err != nil { return err } + log.Debug("dhcpv6: listening...") + go func() { - if serr := s.srv.Serve(); errors.Is(serr, net.ErrClosed) { + if sErr := s.srv.Serve(); errors.Is(sErr, net.ErrClosed) { log.Info("dhcpv6: server is closed") - } else if serr != nil { - log.Error("dhcpv6: srv.Serve: %s", serr) + } else if sErr != nil { + log.Error("dhcpv6: srv.Serve: %s", sErr) } }() diff --git a/scripts/make/go-lint.sh b/scripts/make/go-lint.sh index acaca705..991fdb23 100644 --- a/scripts/make/go-lint.sh +++ b/scripts/make/go-lint.sh @@ -160,29 +160,7 @@ run_linter "$GO" vet ./... run_linter govulncheck ./... -# Apply more lax standards to the code we haven't properly refactored yet. -run_linter gocyclo --over 12 ./internal/dhcpd - -# Apply the normal standards to new or somewhat refactored code. -run_linter gocyclo --over 10\ - ./internal/aghio/\ - ./internal/aghnet/\ - ./internal/aghos/\ - ./internal/aghtest/\ - ./internal/dnsforward/\ - ./internal/filtering/\ - ./internal/home/\ - ./internal/next/\ - ./internal/querylog/\ - ./internal/stats/\ - ./internal/tools/\ - ./internal/updater/\ - ./internal/version/\ - ./scripts/blocked-services/\ - ./scripts/vetted-filters/\ - ./scripts/translations/\ - ./main.go\ - ; +run_linter gocyclo --over 10 . run_linter ineffassign ./...