diff --git a/internal/aghnet/dhcp_unix.go b/internal/aghnet/dhcp_unix.go index b75f40c4e..6f4e11792 100644 --- a/internal/aghnet/dhcp_unix.go +++ b/internal/aghnet/dhcp_unix.go @@ -39,6 +39,7 @@ func checkOtherDHCP(ifaceName string) (ok4, ok6 bool, err4, err6 error) { } // ifaceIPv4Subnet returns the first suitable IPv4 subnetwork iface has. +// iface must not be nil. func ifaceIPv4Subnet(iface *net.Interface) (subnet netip.Prefix, err error) { var addrs []net.Addr if addrs, err = iface.Addrs(); err != nil { @@ -90,6 +91,8 @@ func checkOtherDHCPv4(iface *net.Interface) (ok bool, err error) { return discover4(iface, dstAddr, hostname) } +// discover4 sends a DHCPv4 discovery to the specified network interface and +// waits for response. iface and dstAddr must not be nil. func discover4(iface *net.Interface, dstAddr *net.UDPAddr, hostname string) (ok bool, err error) { var req *dhcpv4.DHCPv4 if req, err = dhcpv4.NewDiscovery(iface.HardwareAddr); err != nil { @@ -120,17 +123,9 @@ func discover4(iface *net.Interface, dstAddr *net.UDPAddr, hostname string) (ok } for { - if err = c.SetDeadline(time.Now().Add(defaultDiscoverTime)); err != nil { - return false, fmt.Errorf("setting deadline: %w", err) - } - var next bool ok, next, err = tryConn4(req, c, iface) if next { - if err != nil { - log.Debug("dhcpv4: trying a connection: %s", err) - } - continue } @@ -142,9 +137,20 @@ func discover4(iface *net.Interface, dstAddr *net.UDPAddr, hostname string) (ok } } +// tryConn4 reads and validates DHCPv4 response packet if it matches +// the original request. req and c must not be nil. +// // TODO(a.garipov): Refactor further. Inspect error handling, remove parameter // next, address the TODO, merge with tryConn6, etc. -func tryConn4(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, next bool, err error) { +func tryConn4( + req *dhcpv4.DHCPv4, + c net.PacketConn, + iface *net.Interface, +) (ok, next bool, err error) { + if err = c.SetDeadline(time.Now().Add(defaultDiscoverTime)); err != nil { + return false, false, fmt.Errorf("dhcpv4: setting deadline: %w", err) + } + // TODO: replicate dhclient's behavior of retrying several times with // progressively longer timeouts. log.Tracef("dhcpv4: waiting %v for an answer", defaultDiscoverTime) @@ -221,6 +227,8 @@ func checkOtherDHCPv6(iface *net.Interface) (ok bool, err error) { return discover6(iface, udpAddr, dstAddr) } +// discover6 sends a DHCPv6 discovery to the specified network interface and +// waits for response. iface, updAddr and dstAddr must not be nil. func discover6(iface *net.Interface, udpAddr, dstAddr *net.UDPAddr) (ok bool, err error) { req, err := dhcpv6.NewSolicit(iface.HardwareAddr) if err != nil { @@ -243,10 +251,6 @@ func discover6(iface *net.Interface, udpAddr, dstAddr *net.UDPAddr) (ok bool, er var next bool ok, next, err = tryConn6(req, c) if next { - if err != nil { - log.Debug("dhcpv6: trying a connection: %s", err) - } - continue } @@ -258,6 +262,9 @@ func discover6(iface *net.Interface, udpAddr, dstAddr *net.UDPAddr) (ok bool, er } } +// tryConn6 reads and validates DHCPv6 response packet if it matches +// the original request. req and c must not be nil. +// // TODO(a.garipov): See the comment on tryConn4. Sigh… func tryConn6(req *dhcpv6.Message, c net.PacketConn) (ok, next bool, err error) { // TODO: replicate dhclient's behavior of retrying several times with diff --git a/internal/aghnet/interfaces.go b/internal/aghnet/interfaces.go index a667a1f3a..e3112a033 100644 --- a/internal/aghnet/interfaces.go +++ b/internal/aghnet/interfaces.go @@ -23,7 +23,7 @@ type NetIface interface { Addrs() ([]net.Addr, error) } -// IfaceIPAddrs returns the interface's IP addresses. +// IfaceIPAddrs returns the interface's IP addresses. iface must not be nil. func IfaceIPAddrs(iface NetIface, ipv IPVersion) (ips []net.IP, err error) { switch ipv { case IPVersion4, IPVersion6: @@ -38,25 +38,7 @@ func IfaceIPAddrs(iface NetIface, ipv IPVersion) (ips []net.IP, err error) { } for _, a := range addrs { - var ip net.IP - switch a := a.(type) { - case *net.IPAddr: - ip = a.IP - case *net.IPNet: - ip = a.IP - default: - continue - } - - // Assume that net.(*Interface).Addrs can only return valid IPv4 and - // IPv6 addresses. Thus, if it isn't an IPv4 address, it must be an - // IPv6 one. - ip4 := ip.To4() - if ipv == IPVersion4 { - if ip4 != nil { - ips = append(ips, ip4) - } - } else if ip4 == nil { + if ip := ipFromAddr(a, ipv); ip != nil { ips = append(ips, ip) } } @@ -64,6 +46,29 @@ func IfaceIPAddrs(iface NetIface, ipv IPVersion) (ips []net.IP, err error) { return ips, nil } +// ipFromAddr converts addr to IP. addr must not be nil. +func ipFromAddr(addr net.Addr, ipv IPVersion) (ip net.IP) { + switch addr := addr.(type) { + case *net.IPAddr: + ip = addr.IP + case *net.IPNet: + ip = addr.IP + default: + return nil + } + + // Assume that net.Addr can only be valid IPv4 or IPv6. Thus, + // if it isn't an IPv4 address, it must be an IPv6 one. + ip4 := ip.To4() + if ipv == IPVersion4 { + return ip4 + } else if ip4 == nil { + return ip + } + + return nil +} + // IfaceDNSIPAddrs returns IP addresses of the interface suitable to send to // clients as DNS addresses. If err is nil, addrs contains either no addresses // or at least two. diff --git a/internal/aghnet/net.go b/internal/aghnet/net.go index 5bfd0c893..89a20c52b 100644 --- a/internal/aghnet/net.go +++ b/internal/aghnet/net.go @@ -148,44 +148,72 @@ func NetInterfaceFrom(iface *net.Interface) (niface *NetInterface, err error) { addrs, err := iface.Addrs() if err != nil { - return nil, fmt.Errorf("failed to get addresses for interface %s: %w", iface.Name, err) + return nil, fmt.Errorf("getting addresses for interface %q: %w", iface.Name, err) } - // Collect network interface addresses. - for _, addr := range addrs { - n, ok := addr.(*net.IPNet) - if !ok { - // Should be *net.IPNet, this is weird. - return nil, fmt.Errorf("expected %[2]s to be %[1]T, got %[2]T", n, addr) - } else if ip4 := n.IP.To4(); ip4 != nil { - n.IP = ip4 + for i, addr := range addrs { + if err = populateAddrs(addr, niface); err != nil { + return nil, fmt.Errorf("populating at index %d: %w", i, err) } - - ip, ok := netip.AddrFromSlice(n.IP) - if !ok { - return nil, fmt.Errorf("bad address %s", n.IP) - } - - ip = ip.Unmap() - if ip.IsLinkLocalUnicast() { - // Ignore link-local IPv4. - if ip.Is4() { - continue - } - - ip = ip.WithZone(iface.Name) - } - - ones, _ := n.Mask.Size() - p := netip.PrefixFrom(ip, ones) - - niface.Addresses = append(niface.Addresses, ip) - niface.Subnets = append(niface.Subnets, p) } return niface, nil } +// populateAddrs fills *NetInterface IP addresses and subnets. addr and niface +// must not be nil. +func populateAddrs(addr net.Addr, niface *NetInterface) (err error) { + n, err := ipNetFromAddr(addr) + if err != nil { + return err + } + + ip, ok := netip.AddrFromSlice(n.IP) + if !ok { + return fmt.Errorf("bad address %s", n.IP) + } + + ip = ip.Unmap() + + // Skip link-local IPv4 addresses + if isLinkLocalV4(ip) { + return nil + } + + if ip.IsLinkLocalUnicast() { + ip = ip.WithZone(niface.Name) + } + + ones, _ := n.Mask.Size() + p := netip.PrefixFrom(ip, ones) + + niface.Addresses = append(niface.Addresses, ip) + niface.Subnets = append(niface.Subnets, p) + + return nil +} + +// ipNetFromAddr converts net.Addr to *net.IPNet and its IP to v4 if necessary. +func ipNetFromAddr(addr net.Addr) (ip *net.IPNet, err error) { + ipNet, ok := addr.(*net.IPNet) + if !ok { + // Should be *net.IPNet, this is weird. + return nil, fmt.Errorf("bad type for interface net.Addr %T(%[1]v)", ipNet) + } + + // TODO(f.setrakov): Explore whether this logic can be safely removed. + if ip4 := ipNet.IP.To4(); ip4 != nil { + ipNet.IP = ip4 + } + + return ipNet, nil +} + +// isLinkLocalV4 checks if ip is link-local unicast IPv4 address. +func isLinkLocalV4(ip netip.Addr) (ok bool) { + return ip.Is4() && ip.IsLinkLocalUnicast() +} + // GetValidNetInterfacesForWeb returns interfaces that are eligible for DNS and // WEB only we do not return link-local addresses here. // diff --git a/scripts/install.sh b/scripts/install.sh index be7575d5a..fa5a323b1 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -301,7 +301,7 @@ fix_darwin() { # Function fix_freebsd performs some fixes to make it work on FreeBSD. fix_freebsd() { - if ! [ "$os" = 'freebsd' ]; then + if [ "$os" != 'freebsd' ]; then return 0 fi diff --git a/scripts/make/go-lint.sh b/scripts/make/go-lint.sh index 2e2448ac4..400c74493 100644 --- a/scripts/make/go-lint.sh +++ b/scripts/make/go-lint.sh @@ -180,13 +180,10 @@ run_linter gocognit --over='14' \ ./internal/dhcpd \ ; -run_linter gocognit --over='13' \ - ./internal/aghnet/ \ - ; - run_linter gocognit --over='10' \ ./internal/aghalg/ \ ./internal/aghhttp/ \ + ./internal/aghnet/ \ ./internal/aghos/ \ ./internal/aghrenameio/ \ ./internal/aghtest/ \