diff --git a/CHANGELOG.md b/CHANGELOG.md index f0812922..6731038c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Fixed +- The `github.com/mdlayher/raw` dependency has been temporarily returned to + support raw connections on Darwin ([#5712]). - Incorrect recording of blocked results as “Blocked by CNAME or IP” in the query log ([#5725]). - All Safe Search services being unchecked by default. diff --git a/go.mod b/go.mod index 56a53ab0..911624bf 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,9 @@ require ( github.com/mdlayher/ethernet v0.0.0-20220221185849-529eae5b6118 github.com/mdlayher/netlink v1.7.1 github.com/mdlayher/packet v1.1.1 + // TODO(a.garipov): This package is deprecated; find a new one or use our + // own code for that. Perhaps, use gopacket. + github.com/mdlayher/raw v0.1.0 github.com/miekg/dns v1.1.53 github.com/quic-go/quic-go v0.33.0 github.com/stretchr/testify v1.8.2 @@ -46,7 +49,6 @@ require ( github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect github.com/golang/mock v1.6.0 // indirect github.com/google/pprof v0.0.0-20230406165453-00490a63f317 // indirect - github.com/mdlayher/raw v0.1.0 // indirect github.com/mdlayher/socket v0.4.0 // indirect github.com/onsi/ginkgo/v2 v2.9.2 // indirect github.com/patrickmn/go-cache v2.1.0+incompatible // indirect diff --git a/internal/dhcpd/conn_darwin.go b/internal/dhcpd/conn_darwin.go new file mode 100644 index 00000000..a80ae482 --- /dev/null +++ b/internal/dhcpd/conn_darwin.go @@ -0,0 +1,293 @@ +//go:build darwin + +package dhcpd + +import ( + "fmt" + "net" + "os" + "time" + + "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/netutil" + "github.com/google/gopacket" + "github.com/google/gopacket/layers" + "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/insomniacslk/dhcp/dhcpv4/server4" + "github.com/mdlayher/ethernet" + + //lint:ignore SA1019 See the TODO in go.mod. + "github.com/mdlayher/raw" +) + +// dhcpUnicastAddr is the combination of MAC and IP addresses for responding to +// the unconfigured host. +type dhcpUnicastAddr struct { + // raw.Addr is embedded here to make *dhcpUcastAddr a net.Addr without + // actually implementing all methods. It also contains the client's + // hardware address. + raw.Addr + + // yiaddr is an IP address just allocated by server for the host. + yiaddr net.IP +} + +// dhcpConn is the net.PacketConn capable of handling both net.UDPAddr and +// net.HardwareAddr. +type dhcpConn struct { + // udpConn is the connection for UDP addresses. + udpConn net.PacketConn + // bcastIP is the broadcast address specific for the configured + // interface's subnet. + bcastIP net.IP + + // rawConn is the connection for MAC addresses. + rawConn net.PacketConn + // srcMAC is the hardware address of the configured network interface. + srcMAC net.HardwareAddr + // srcIP is the IP address of the configured network interface. + srcIP net.IP +} + +// newDHCPConn creates the special connection for DHCP server. +func (s *v4Server) newDHCPConn(iface *net.Interface) (c net.PacketConn, err error) { + var ucast net.PacketConn + if ucast, err = raw.ListenPacket(iface, uint16(ethernet.EtherTypeIPv4), nil); err != nil { + return nil, fmt.Errorf("creating raw udp connection: %w", err) + } + + // Create the UDP connection. + var bcast net.PacketConn + bcast, err = server4.NewIPv4UDPConn(iface.Name, &net.UDPAddr{ + // TODO(e.burkov): Listening on zeroes makes the server handle + // requests from all the interfaces. Inspect the ways to + // specify the interface-specific listening addresses. + // + // See https://github.com/AdguardTeam/AdGuardHome/issues/3539. + IP: net.IP{0, 0, 0, 0}, + Port: dhcpv4.ServerPort, + }) + if err != nil { + return nil, fmt.Errorf("creating ipv4 udp connection: %w", err) + } + + return &dhcpConn{ + udpConn: bcast, + bcastIP: s.conf.broadcastIP.AsSlice(), + rawConn: ucast, + srcMAC: iface.HardwareAddr, + srcIP: s.conf.dnsIPAddrs[0].AsSlice(), + }, nil +} + +// wrapErrs is a helper to wrap the errors from two independent underlying +// connections. +func (*dhcpConn) wrapErrs(action string, udpConnErr, rawConnErr error) (err error) { + switch { + case udpConnErr != nil && rawConnErr != nil: + return errors.List(fmt.Sprintf("%s both connections", action), udpConnErr, rawConnErr) + case udpConnErr != nil: + return fmt.Errorf("%s udp connection: %w", action, udpConnErr) + case rawConnErr != nil: + return fmt.Errorf("%s raw connection: %w", action, rawConnErr) + default: + return nil + } +} + +// WriteTo implements net.PacketConn for *dhcpConn. It selects the underlying +// connection to write to based on the type of addr. +func (c *dhcpConn) WriteTo(p []byte, addr net.Addr) (n int, err error) { + switch addr := addr.(type) { + case *dhcpUnicastAddr: + // Unicast the message to the client's MAC address. Use the raw + // connection. + // + // Note: unicasting is performed on the only network interface + // that is configured. For now it may be not what users expect + // so additionally broadcast the message via UDP connection. + // + // See https://github.com/AdguardTeam/AdGuardHome/issues/3539. + var rerr error + n, rerr = c.unicast(p, addr) + + _, uerr := c.broadcast(p, &net.UDPAddr{ + IP: netutil.IPv4bcast(), + Port: dhcpv4.ClientPort, + }) + + return n, c.wrapErrs("writing to", uerr, rerr) + case *net.UDPAddr: + if addr.IP.Equal(net.IPv4bcast) { + // Broadcast the message for the client which supports + // it. Use the UDP connection. + return c.broadcast(p, addr) + } + + // Unicast the message to the client's IP address. Use the UDP + // connection. + return c.udpConn.WriteTo(p, addr) + default: + return 0, fmt.Errorf("addr has an unexpected type %T", addr) + } +} + +// ReadFrom implements net.PacketConn for *dhcpConn. +func (c *dhcpConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) { + return c.udpConn.ReadFrom(p) +} + +// unicast wraps respData with required frames and writes it to the peer. +func (c *dhcpConn) unicast(respData []byte, peer *dhcpUnicastAddr) (n int, err error) { + var data []byte + data, err = c.buildEtherPkt(respData, peer) + if err != nil { + return 0, err + } + + return c.rawConn.WriteTo(data, &peer.Addr) +} + +// Close implements net.PacketConn for *dhcpConn. +func (c *dhcpConn) Close() (err error) { + rerr := c.rawConn.Close() + if errors.Is(rerr, os.ErrClosed) { + // Ignore the error since the actual file is closed already. + rerr = nil + } + + return c.wrapErrs("closing", c.udpConn.Close(), rerr) +} + +// LocalAddr implements net.PacketConn for *dhcpConn. +func (c *dhcpConn) LocalAddr() (a net.Addr) { + return c.udpConn.LocalAddr() +} + +// SetDeadline implements net.PacketConn for *dhcpConn. +func (c *dhcpConn) SetDeadline(t time.Time) (err error) { + return c.wrapErrs("setting deadline on", c.udpConn.SetDeadline(t), c.rawConn.SetDeadline(t)) +} + +// SetReadDeadline implements net.PacketConn for *dhcpConn. +func (c *dhcpConn) SetReadDeadline(t time.Time) error { + return c.wrapErrs( + "setting reading deadline on", + c.udpConn.SetReadDeadline(t), + c.rawConn.SetReadDeadline(t), + ) +} + +// SetWriteDeadline implements net.PacketConn for *dhcpConn. +func (c *dhcpConn) SetWriteDeadline(t time.Time) error { + return c.wrapErrs( + "setting writing deadline on", + c.udpConn.SetWriteDeadline(t), + c.rawConn.SetWriteDeadline(t), + ) +} + +// ipv4DefaultTTL is the default Time to Live value in seconds as recommended by +// RFC-1700. +// +// See https://datatracker.ietf.org/doc/html/rfc1700. +const ipv4DefaultTTL = 64 + +// buildEtherPkt wraps the payload with IPv4, UDP and Ethernet frames. +// Validation of the payload is a caller's responsibility. +func (c *dhcpConn) buildEtherPkt(payload []byte, peer *dhcpUnicastAddr) (pkt []byte, err error) { + udpLayer := &layers.UDP{ + SrcPort: dhcpv4.ServerPort, + DstPort: dhcpv4.ClientPort, + } + + ipv4Layer := &layers.IPv4{ + Version: uint8(layers.IPProtocolIPv4), + Flags: layers.IPv4DontFragment, + TTL: ipv4DefaultTTL, + Protocol: layers.IPProtocolUDP, + SrcIP: c.srcIP, + DstIP: peer.yiaddr, + } + + // Ignore the error since it's only returned for invalid network layer's + // type. + _ = udpLayer.SetNetworkLayerForChecksum(ipv4Layer) + + ethLayer := &layers.Ethernet{ + SrcMAC: c.srcMAC, + DstMAC: peer.HardwareAddr, + EthernetType: layers.EthernetTypeIPv4, + } + + buf := gopacket.NewSerializeBuffer() + setts := gopacket.SerializeOptions{ + FixLengths: true, + ComputeChecksums: true, + } + + err = gopacket.SerializeLayers( + buf, + setts, + ethLayer, + ipv4Layer, + udpLayer, + gopacket.Payload(payload), + ) + if err != nil { + return nil, fmt.Errorf("serializing layers: %w", err) + } + + return buf.Bytes(), nil +} + +// send writes resp for peer to conn considering the req's parameters according +// to RFC-2131. +// +// See https://datatracker.ietf.org/doc/html/rfc2131#section-4.1. +func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DHCPv4) { + switch giaddr, ciaddr, mtype := req.GatewayIPAddr, req.ClientIPAddr, resp.MessageType(); { + case giaddr != nil && !giaddr.IsUnspecified(): + // Send any return messages to the server port on the BOOTP + // relay agent whose address appears in giaddr. + peer = &net.UDPAddr{ + IP: giaddr, + Port: dhcpv4.ServerPort, + } + if mtype == dhcpv4.MessageTypeNak { + // Set the broadcast bit in the DHCPNAK, so that the relay agent + // broadcasts it to the client, because the client may not have + // a correct network address or subnet mask, and the client may not + // be answering ARP requests. + resp.SetBroadcast() + } + case mtype == dhcpv4.MessageTypeNak: + // Broadcast any DHCPNAK messages to 0xffffffff. + case ciaddr != nil && !ciaddr.IsUnspecified(): + // Unicast DHCPOFFER and DHCPACK messages to the address in + // ciaddr. + peer = &net.UDPAddr{ + IP: ciaddr, + Port: dhcpv4.ClientPort, + } + case !req.IsBroadcast() && req.ClientHWAddr != nil: + // Unicast DHCPOFFER and DHCPACK messages to the client's + // hardware address and yiaddr. + peer = &dhcpUnicastAddr{ + Addr: raw.Addr{HardwareAddr: req.ClientHWAddr}, + yiaddr: resp.YourIPAddr, + } + default: + // Go on since peer is already set to broadcast. + } + + pktData := resp.ToBytes() + + log.Debug("dhcpv4: sending %d bytes to %s: %s", len(pktData), peer, resp.Summary()) + + _, err := conn.WriteTo(pktData, peer) + if err != nil { + log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) + } +} diff --git a/internal/dhcpd/conn_darwin_internal_test.go b/internal/dhcpd/conn_darwin_internal_test.go new file mode 100644 index 00000000..e0522a0f --- /dev/null +++ b/internal/dhcpd/conn_darwin_internal_test.go @@ -0,0 +1,219 @@ +//go:build darwin + +package dhcpd + +import ( + "net" + "testing" + + "github.com/AdguardTeam/golibs/testutil" + "github.com/google/gopacket" + "github.com/google/gopacket/layers" + "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + //lint:ignore SA1019 See the TODO in go.mod. + "github.com/mdlayher/raw" +) + +func TestDHCPConn_WriteTo_common(t *testing.T) { + respData := (&dhcpv4.DHCPv4{}).ToBytes() + udpAddr := &net.UDPAddr{ + IP: net.IP{1, 2, 3, 4}, + Port: dhcpv4.ClientPort, + } + + t.Run("unicast_ip", func(t *testing.T) { + writeTo := func(_ []byte, addr net.Addr) (_ int, _ error) { + assert.Equal(t, udpAddr, addr) + + return 0, nil + } + + conn := &dhcpConn{udpConn: &fakePacketConn{writeTo: writeTo}} + + _, err := conn.WriteTo(respData, udpAddr) + assert.NoError(t, err) + }) + + t.Run("unexpected_addr_type", func(t *testing.T) { + type unexpectedAddrType struct { + net.Addr + } + + conn := &dhcpConn{} + n, err := conn.WriteTo(nil, &unexpectedAddrType{}) + require.Error(t, err) + + testutil.AssertErrorMsg(t, "addr has an unexpected type *dhcpd.unexpectedAddrType", err) + assert.Zero(t, n) + }) +} + +func TestBuildEtherPkt(t *testing.T) { + conn := &dhcpConn{ + srcMAC: net.HardwareAddr{1, 2, 3, 4, 5, 6}, + srcIP: net.IP{1, 2, 3, 4}, + } + peer := &dhcpUnicastAddr{ + Addr: raw.Addr{HardwareAddr: net.HardwareAddr{6, 5, 4, 3, 2, 1}}, + yiaddr: net.IP{4, 3, 2, 1}, + } + payload := (&dhcpv4.DHCPv4{}).ToBytes() + + t.Run("success", func(t *testing.T) { + pkt, err := conn.buildEtherPkt(payload, peer) + require.NoError(t, err) + + assert.NotEmpty(t, pkt) + + actualPkt := gopacket.NewPacket(pkt, layers.LayerTypeEthernet, gopacket.DecodeOptions{ + NoCopy: true, + }) + require.NotNil(t, actualPkt) + + wantTypes := []gopacket.LayerType{ + layers.LayerTypeEthernet, + layers.LayerTypeIPv4, + layers.LayerTypeUDP, + layers.LayerTypeDHCPv4, + } + actualLayers := actualPkt.Layers() + require.Len(t, actualLayers, len(wantTypes)) + + for i, wantType := range wantTypes { + layer := actualLayers[i] + require.NotNil(t, layer) + + assert.Equal(t, wantType, layer.LayerType()) + } + }) + + t.Run("bad_payload", func(t *testing.T) { + // Create an invalid DHCP packet. + invalidPayload := []byte{1, 2, 3, 4} + pkt, err := conn.buildEtherPkt(invalidPayload, peer) + require.NoError(t, err) + + assert.NotEmpty(t, pkt) + }) + + t.Run("serializing_error", func(t *testing.T) { + // Create a peer with invalid MAC. + badPeer := &dhcpUnicastAddr{ + Addr: raw.Addr{HardwareAddr: net.HardwareAddr{5, 4, 3, 2, 1}}, + yiaddr: net.IP{4, 3, 2, 1}, + } + + pkt, err := conn.buildEtherPkt(payload, badPeer) + require.Error(t, err) + + assert.Empty(t, pkt) + }) +} + +func TestV4Server_Send(t *testing.T) { + s := &v4Server{} + + var ( + defaultIP = net.IP{99, 99, 99, 99} + knownIP = net.IP{4, 2, 4, 2} + knownMAC = net.HardwareAddr{6, 5, 4, 3, 2, 1} + ) + + defaultPeer := &net.UDPAddr{ + IP: defaultIP, + // Use neither client nor server port to check it actually + // changed. + Port: dhcpv4.ClientPort + dhcpv4.ServerPort, + } + defaultResp := &dhcpv4.DHCPv4{} + + testCases := []struct { + want net.Addr + req *dhcpv4.DHCPv4 + resp *dhcpv4.DHCPv4 + name string + }{{ + name: "giaddr", + req: &dhcpv4.DHCPv4{GatewayIPAddr: knownIP}, + resp: defaultResp, + want: &net.UDPAddr{ + IP: knownIP, + Port: dhcpv4.ServerPort, + }, + }, { + name: "nak", + req: &dhcpv4.DHCPv4{}, + resp: &dhcpv4.DHCPv4{ + Options: dhcpv4.OptionsFromList( + dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), + ), + }, + want: defaultPeer, + }, { + name: "ciaddr", + req: &dhcpv4.DHCPv4{ClientIPAddr: knownIP}, + resp: &dhcpv4.DHCPv4{}, + want: &net.UDPAddr{ + IP: knownIP, + Port: dhcpv4.ClientPort, + }, + }, { + name: "chaddr", + req: &dhcpv4.DHCPv4{ClientHWAddr: knownMAC}, + resp: &dhcpv4.DHCPv4{YourIPAddr: knownIP}, + want: &dhcpUnicastAddr{ + Addr: raw.Addr{HardwareAddr: knownMAC}, + yiaddr: knownIP, + }, + }, { + name: "who_are_you", + req: &dhcpv4.DHCPv4{}, + resp: &dhcpv4.DHCPv4{}, + want: defaultPeer, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + conn := &fakePacketConn{ + writeTo: func(_ []byte, addr net.Addr) (_ int, _ error) { + assert.Equal(t, tc.want, addr) + + return 0, nil + }, + } + + s.send(cloneUDPAddr(defaultPeer), conn, tc.req, tc.resp) + }) + } + + t.Run("giaddr_nak", func(t *testing.T) { + req := &dhcpv4.DHCPv4{ + GatewayIPAddr: knownIP, + } + // Ensure the request is for unicast. + req.SetUnicast() + resp := &dhcpv4.DHCPv4{ + Options: dhcpv4.OptionsFromList( + dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), + ), + } + want := &net.UDPAddr{ + IP: req.GatewayIPAddr, + Port: dhcpv4.ServerPort, + } + + conn := &fakePacketConn{ + writeTo: func(_ []byte, addr net.Addr) (n int, err error) { + assert.Equal(t, want, addr) + + return 0, nil + }, + } + + s.send(cloneUDPAddr(defaultPeer), conn, req, resp) + assert.True(t, resp.IsBroadcast()) + }) +} diff --git a/internal/dhcpd/conn_unix.go b/internal/dhcpd/conn_unix.go index 37994e31..5602d126 100644 --- a/internal/dhcpd/conn_unix.go +++ b/internal/dhcpd/conn_unix.go @@ -1,4 +1,4 @@ -//go:build darwin || freebsd || linux || openbsd +//go:build freebsd || linux || openbsd package dhcpd @@ -9,6 +9,7 @@ import ( "time" "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" "github.com/google/gopacket" "github.com/google/gopacket/layers" @@ -238,3 +239,53 @@ func (c *dhcpConn) buildEtherPkt(payload []byte, peer *dhcpUnicastAddr) (pkt []b return buf.Bytes(), nil } + +// send writes resp for peer to conn considering the req's parameters according +// to RFC-2131. +// +// See https://datatracker.ietf.org/doc/html/rfc2131#section-4.1. +func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DHCPv4) { + switch giaddr, ciaddr, mtype := req.GatewayIPAddr, req.ClientIPAddr, resp.MessageType(); { + case giaddr != nil && !giaddr.IsUnspecified(): + // Send any return messages to the server port on the BOOTP + // relay agent whose address appears in giaddr. + peer = &net.UDPAddr{ + IP: giaddr, + Port: dhcpv4.ServerPort, + } + if mtype == dhcpv4.MessageTypeNak { + // Set the broadcast bit in the DHCPNAK, so that the relay agent + // broadcasts it to the client, because the client may not have + // a correct network address or subnet mask, and the client may not + // be answering ARP requests. + resp.SetBroadcast() + } + case mtype == dhcpv4.MessageTypeNak: + // Broadcast any DHCPNAK messages to 0xffffffff. + case ciaddr != nil && !ciaddr.IsUnspecified(): + // Unicast DHCPOFFER and DHCPACK messages to the address in + // ciaddr. + peer = &net.UDPAddr{ + IP: ciaddr, + Port: dhcpv4.ClientPort, + } + case !req.IsBroadcast() && req.ClientHWAddr != nil: + // Unicast DHCPOFFER and DHCPACK messages to the client's + // hardware address and yiaddr. + peer = &dhcpUnicastAddr{ + Addr: packet.Addr{HardwareAddr: req.ClientHWAddr}, + yiaddr: resp.YourIPAddr, + } + default: + // Go on since peer is already set to broadcast. + } + + pktData := resp.ToBytes() + + log.Debug("dhcpv4: sending %d bytes to %s: %s", len(pktData), peer, resp.Summary()) + + _, err := conn.WriteTo(pktData, peer) + if err != nil { + log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) + } +} diff --git a/internal/dhcpd/conn_unix_test.go b/internal/dhcpd/conn_unix_internal_test.go similarity index 53% rename from internal/dhcpd/conn_unix_test.go rename to internal/dhcpd/conn_unix_internal_test.go index 65e64682..ca68c1b9 100644 --- a/internal/dhcpd/conn_unix_test.go +++ b/internal/dhcpd/conn_unix_internal_test.go @@ -1,4 +1,4 @@ -//go:build darwin || freebsd || linux || openbsd +//go:build freebsd || linux || openbsd package dhcpd @@ -110,3 +110,108 @@ func TestBuildEtherPkt(t *testing.T) { assert.Empty(t, pkt) }) } + +func TestV4Server_Send(t *testing.T) { + s := &v4Server{} + + var ( + defaultIP = net.IP{99, 99, 99, 99} + knownIP = net.IP{4, 2, 4, 2} + knownMAC = net.HardwareAddr{6, 5, 4, 3, 2, 1} + ) + + defaultPeer := &net.UDPAddr{ + IP: defaultIP, + // Use neither client nor server port to check it actually + // changed. + Port: dhcpv4.ClientPort + dhcpv4.ServerPort, + } + defaultResp := &dhcpv4.DHCPv4{} + + testCases := []struct { + want net.Addr + req *dhcpv4.DHCPv4 + resp *dhcpv4.DHCPv4 + name string + }{{ + name: "giaddr", + req: &dhcpv4.DHCPv4{GatewayIPAddr: knownIP}, + resp: defaultResp, + want: &net.UDPAddr{ + IP: knownIP, + Port: dhcpv4.ServerPort, + }, + }, { + name: "nak", + req: &dhcpv4.DHCPv4{}, + resp: &dhcpv4.DHCPv4{ + Options: dhcpv4.OptionsFromList( + dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), + ), + }, + want: defaultPeer, + }, { + name: "ciaddr", + req: &dhcpv4.DHCPv4{ClientIPAddr: knownIP}, + resp: &dhcpv4.DHCPv4{}, + want: &net.UDPAddr{ + IP: knownIP, + Port: dhcpv4.ClientPort, + }, + }, { + name: "chaddr", + req: &dhcpv4.DHCPv4{ClientHWAddr: knownMAC}, + resp: &dhcpv4.DHCPv4{YourIPAddr: knownIP}, + want: &dhcpUnicastAddr{ + Addr: packet.Addr{HardwareAddr: knownMAC}, + yiaddr: knownIP, + }, + }, { + name: "who_are_you", + req: &dhcpv4.DHCPv4{}, + resp: &dhcpv4.DHCPv4{}, + want: defaultPeer, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + conn := &fakePacketConn{ + writeTo: func(_ []byte, addr net.Addr) (_ int, _ error) { + assert.Equal(t, tc.want, addr) + + return 0, nil + }, + } + + s.send(cloneUDPAddr(defaultPeer), conn, tc.req, tc.resp) + }) + } + + t.Run("giaddr_nak", func(t *testing.T) { + req := &dhcpv4.DHCPv4{ + GatewayIPAddr: knownIP, + } + // Ensure the request is for unicast. + req.SetUnicast() + resp := &dhcpv4.DHCPv4{ + Options: dhcpv4.OptionsFromList( + dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), + ), + } + want := &net.UDPAddr{ + IP: req.GatewayIPAddr, + Port: dhcpv4.ServerPort, + } + + conn := &fakePacketConn{ + writeTo: func(_ []byte, addr net.Addr) (n int, err error) { + assert.Equal(t, want, addr) + + return 0, nil + }, + } + + s.send(cloneUDPAddr(defaultPeer), conn, req, resp) + assert.True(t, resp.IsBroadcast()) + }) +} diff --git a/internal/dhcpd/v4_unix.go b/internal/dhcpd/v4_unix.go index 4ac46db6..bd5aba6e 100644 --- a/internal/dhcpd/v4_unix.go +++ b/internal/dhcpd/v4_unix.go @@ -20,7 +20,6 @@ import ( "github.com/go-ping/ping" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv4/server4" - "github.com/mdlayher/packet" "golang.org/x/exp/slices" ) @@ -1132,56 +1131,6 @@ func (s *v4Server) packetHandler(conn net.PacketConn, peer net.Addr, req *dhcpv4 s.send(peer, conn, req, resp) } -// send writes resp for peer to conn considering the req's parameters according -// to RFC-2131. -// -// See https://datatracker.ietf.org/doc/html/rfc2131#section-4.1. -func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DHCPv4) { - switch giaddr, ciaddr, mtype := req.GatewayIPAddr, req.ClientIPAddr, resp.MessageType(); { - case giaddr != nil && !giaddr.IsUnspecified(): - // Send any return messages to the server port on the BOOTP - // relay agent whose address appears in giaddr. - peer = &net.UDPAddr{ - IP: giaddr, - Port: dhcpv4.ServerPort, - } - if mtype == dhcpv4.MessageTypeNak { - // Set the broadcast bit in the DHCPNAK, so that the relay agent - // broadcasts it to the client, because the client may not have - // a correct network address or subnet mask, and the client may not - // be answering ARP requests. - resp.SetBroadcast() - } - case mtype == dhcpv4.MessageTypeNak: - // Broadcast any DHCPNAK messages to 0xffffffff. - case ciaddr != nil && !ciaddr.IsUnspecified(): - // Unicast DHCPOFFER and DHCPACK messages to the address in - // ciaddr. - peer = &net.UDPAddr{ - IP: ciaddr, - Port: dhcpv4.ClientPort, - } - case !req.IsBroadcast() && req.ClientHWAddr != nil: - // Unicast DHCPOFFER and DHCPACK messages to the client's - // hardware address and yiaddr. - peer = &dhcpUnicastAddr{ - Addr: packet.Addr{HardwareAddr: req.ClientHWAddr}, - yiaddr: resp.YourIPAddr, - } - default: - // Go on since peer is already set to broadcast. - } - - pktData := resp.ToBytes() - - log.Debug("dhcpv4: sending %d bytes to %s: %s", len(pktData), peer, resp.Summary()) - - _, err := conn.WriteTo(pktData, peer) - if err != nil { - log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) - } -} - // Start starts the IPv4 DHCP server. func (s *v4Server) Start() (err error) { defer func() { err = errors.Annotate(err, "dhcpv4: %w") }() diff --git a/internal/dhcpd/v4_unix_test.go b/internal/dhcpd/v4_unix_test.go index 7ce3cdc3..6c51cc5f 100644 --- a/internal/dhcpd/v4_unix_test.go +++ b/internal/dhcpd/v4_unix_test.go @@ -15,7 +15,6 @@ import ( "github.com/AdguardTeam/golibs/stringutil" "github.com/AdguardTeam/golibs/testutil" "github.com/insomniacslk/dhcp/dhcpv4" - "github.com/mdlayher/packet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -771,111 +770,6 @@ func (fc *fakePacketConn) WriteTo(p []byte, addr net.Addr) (n int, err error) { return fc.writeTo(p, addr) } -func TestV4Server_Send(t *testing.T) { - s := &v4Server{} - - var ( - defaultIP = net.IP{99, 99, 99, 99} - knownIP = net.IP{4, 2, 4, 2} - knownMAC = net.HardwareAddr{6, 5, 4, 3, 2, 1} - ) - - defaultPeer := &net.UDPAddr{ - IP: defaultIP, - // Use neither client nor server port to check it actually - // changed. - Port: dhcpv4.ClientPort + dhcpv4.ServerPort, - } - defaultResp := &dhcpv4.DHCPv4{} - - testCases := []struct { - want net.Addr - req *dhcpv4.DHCPv4 - resp *dhcpv4.DHCPv4 - name string - }{{ - name: "giaddr", - req: &dhcpv4.DHCPv4{GatewayIPAddr: knownIP}, - resp: defaultResp, - want: &net.UDPAddr{ - IP: knownIP, - Port: dhcpv4.ServerPort, - }, - }, { - name: "nak", - req: &dhcpv4.DHCPv4{}, - resp: &dhcpv4.DHCPv4{ - Options: dhcpv4.OptionsFromList( - dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), - ), - }, - want: defaultPeer, - }, { - name: "ciaddr", - req: &dhcpv4.DHCPv4{ClientIPAddr: knownIP}, - resp: &dhcpv4.DHCPv4{}, - want: &net.UDPAddr{ - IP: knownIP, - Port: dhcpv4.ClientPort, - }, - }, { - name: "chaddr", - req: &dhcpv4.DHCPv4{ClientHWAddr: knownMAC}, - resp: &dhcpv4.DHCPv4{YourIPAddr: knownIP}, - want: &dhcpUnicastAddr{ - Addr: packet.Addr{HardwareAddr: knownMAC}, - yiaddr: knownIP, - }, - }, { - name: "who_are_you", - req: &dhcpv4.DHCPv4{}, - resp: &dhcpv4.DHCPv4{}, - want: defaultPeer, - }} - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - conn := &fakePacketConn{ - writeTo: func(_ []byte, addr net.Addr) (_ int, _ error) { - assert.Equal(t, tc.want, addr) - - return 0, nil - }, - } - - s.send(cloneUDPAddr(defaultPeer), conn, tc.req, tc.resp) - }) - } - - t.Run("giaddr_nak", func(t *testing.T) { - req := &dhcpv4.DHCPv4{ - GatewayIPAddr: knownIP, - } - // Ensure the request is for unicast. - req.SetUnicast() - resp := &dhcpv4.DHCPv4{ - Options: dhcpv4.OptionsFromList( - dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), - ), - } - want := &net.UDPAddr{ - IP: req.GatewayIPAddr, - Port: dhcpv4.ServerPort, - } - - conn := &fakePacketConn{ - writeTo: func(_ []byte, addr net.Addr) (n int, err error) { - assert.Equal(t, want, addr) - - return 0, nil - }, - } - - s.send(cloneUDPAddr(defaultPeer), conn, req, resp) - assert.True(t, resp.IsBroadcast()) - }) -} - func TestV4Server_FindMACbyIP(t *testing.T) { const ( staticName = "static-client"