From da86620288256df7b0c51b0b413ded2a09aae940 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Thu, 30 Sep 2021 18:28:19 +0300 Subject: [PATCH] Pull request: 3443 dhcp broadcast vol.2 Merge in DNS/adguard-home from 3443-dhcp-broadcast-vol.2 to master Closes #3443. Squashed commit of the following: commit a85af89cb43f2489126fe3c12366fc034e89f59d Merge: 72eb3a88 a4e07827 Author: Eugene Burkov Date: Thu Sep 30 18:08:19 2021 +0300 Merge branch 'master' into 3443-dhcp-broadcast-vol.2 commit 72eb3a8853540b06ee1096decf50e836b539fe45 Author: Eugene Burkov Date: Thu Sep 30 18:03:19 2021 +0300 dhcpd: imp code readability commit 2d1fbc40d04a4125855d6be9f02e09d15430150d Author: Eugene Burkov Date: Thu Sep 30 14:16:59 2021 +0300 dhcpd: imp tests commit 889fad3084ad2b81edfc12100e2ce29d323227ba Author: Eugene Burkov Date: Wed Sep 29 20:09:25 2021 +0300 dhcpd: imp code, docs commit 1fd6b2346ff66e033bceaa169aed751be5822ca8 Author: Eugene Burkov Date: Thu Sep 23 16:08:18 2021 +0300 dhcpd: unicast to mac address --- go.mod | 4 +- go.sum | 4 + internal/dhcpd/broadcast_bsd.go | 30 +-- internal/dhcpd/broadcast_bsd_test.go | 62 ++---- internal/dhcpd/broadcast_others.go | 30 +-- internal/dhcpd/broadcast_others_test.go | 72 ++----- internal/dhcpd/conn_unix.go | 244 ++++++++++++++++++++++++ internal/dhcpd/conn_unix_test.go | 114 +++++++++++ internal/dhcpd/dhcpd_test.go | 2 +- internal/dhcpd/options_unix.go | 4 +- internal/dhcpd/v4.go | 59 +++--- internal/dhcpd/v4_test.go | 137 ++++++++----- 12 files changed, 531 insertions(+), 231 deletions(-) create mode 100644 internal/dhcpd/conn_unix.go create mode 100644 internal/dhcpd/conn_unix_test.go diff --git a/go.mod b/go.mod index 76191c8a..13e3e74a 100644 --- a/go.mod +++ b/go.mod @@ -12,12 +12,14 @@ require ( github.com/fsnotify/fsnotify v1.4.9 github.com/go-ping/ping v0.0.0-20210506233800-ff8be3320020 github.com/google/go-cmp v0.5.5 + github.com/google/gopacket v1.1.19 github.com/google/renameio v1.0.1 github.com/insomniacslk/dhcp v0.0.0-20210310193751-cfd4d47082c2 github.com/kardianos/service v1.2.0 github.com/lucas-clemente/quic-go v0.21.1 + github.com/mdlayher/ethernet v0.0.0-20190606142754-0394541c37b7 github.com/mdlayher/netlink v1.4.0 - github.com/mdlayher/raw v0.0.0-20210412142147-51b895745faf // indirect + github.com/mdlayher/raw v0.0.0-20210412142147-51b895745faf github.com/miekg/dns v1.1.43 github.com/satori/go.uuid v1.2.0 github.com/stretchr/objx v0.1.1 // indirect diff --git a/go.sum b/go.sum index 017d6477..b3dcd9f3 100644 --- a/go.sum +++ b/go.sum @@ -93,6 +93,8 @@ github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= +github.com/google/gopacket v1.1.19 h1:ves8RnFZPGiFnTS0uPQStjwru6uO6h+nlr9j6fL7kF8= +github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo/Vo+TKTo= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= github.com/google/renameio v1.0.1 h1:Lh/jXZmvZxb0BBeSY5VKEfidcbcbenKjZFzM/q0fSeU= @@ -268,6 +270,7 @@ golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL golang.org/x/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= +golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -377,6 +380,7 @@ golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3 golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191216052735-49a3e744a425/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/dhcpd/broadcast_bsd.go b/internal/dhcpd/broadcast_bsd.go index b2d41f75..b55a903d 100644 --- a/internal/dhcpd/broadcast_bsd.go +++ b/internal/dhcpd/broadcast_bsd.go @@ -5,33 +5,17 @@ package dhcpd import ( "net" - - "github.com/AdguardTeam/golibs/log" - "github.com/insomniacslk/dhcp/dhcpv4" ) // broadcast sends resp to the broadcast address specific for network interface. -func (s *v4Server) broadcast(peer net.Addr, conn net.PacketConn, resp *dhcpv4.DHCPv4) { - // peer is expected to be of type *net.UDPConn as the server4.NewServer - // initializes it. - udpPeer, ok := peer.(*net.UDPAddr) - if !ok { - log.Error("dhcpv4: peer is of unexpected type %T", peer) - - return - } - +func (c *dhcpConn) broadcast(respData []byte, peer *net.UDPAddr) (n int, err error) { // Despite the fact that server4.NewIPv4UDPConn explicitly sets socket // options to allow broadcasting, it also binds the connection to a - // specific interface. On FreeBSD and OpenBSD conn.WriteTo causes - // errors while writing to the addresses that belong to another - // interface. So, use the broadcast address specific for the binded - // interface. - udpPeer.IP = s.conf.broadcastIP + // specific interface. On FreeBSD and OpenBSD net.UDPConn.WriteTo + // causes errors while writing to the addresses that belong to another + // interface. So, use the broadcast address specific for the interface + // bound. + peer.IP = c.bcastIP - log.Debug("dhcpv4: sending to %s: %s", peer, resp.Summary()) - - if _, err := conn.WriteTo(resp.ToBytes(), peer); err != nil { - log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) - } + return c.udpConn.WriteTo(respData, peer) } diff --git a/internal/dhcpd/broadcast_bsd_test.go b/internal/dhcpd/broadcast_bsd_test.go index 49221b04..ed81d272 100644 --- a/internal/dhcpd/broadcast_bsd_test.go +++ b/internal/dhcpd/broadcast_bsd_test.go @@ -8,17 +8,16 @@ import ( "net" "testing" - "github.com/AdguardTeam/golibs/netutil" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestV4Server_Send_broadcast(t *testing.T) { +func TestDHCPConn_Broadcast(t *testing.T) { b := &bytes.Buffer{} var peer *net.UDPAddr - conn := &fakePacketConn{ + udpConn := &fakePacketConn{ writeTo: func(p []byte, addr net.Addr) (n int, err error) { udpPeer, ok := addr.(*net.UDPAddr) require.True(t, ok) @@ -31,57 +30,22 @@ func TestV4Server_Send_broadcast(t *testing.T) { return n, nil }, } - + conn := &dhcpConn{ + udpConn: udpConn, + bcastIP: net.IP{1, 2, 3, 255}, + } defaultPeer := &net.UDPAddr{ IP: net.IP{1, 2, 3, 4}, // Use neither client nor server port. Port: 1234, } - s := &v4Server{ - conf: V4ServerConf{ - broadcastIP: net.IP{1, 2, 3, 255}, - }, - } + respData := (&dhcpv4.DHCPv4{}).ToBytes() - testCases := []struct { - name string - req *dhcpv4.DHCPv4 - resp *dhcpv4.DHCPv4 - }{{ - name: "nak", - req: &dhcpv4.DHCPv4{ - GatewayIPAddr: netutil.IPv4Zero(), - }, - resp: &dhcpv4.DHCPv4{ - Options: dhcpv4.OptionsFromList( - dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), - ), - }, - }, { - name: "fully_unspecified", - req: &dhcpv4.DHCPv4{ - GatewayIPAddr: netutil.IPv4Zero(), - ClientIPAddr: netutil.IPv4Zero(), - }, - resp: &dhcpv4.DHCPv4{ - Options: dhcpv4.OptionsFromList( - dhcpv4.OptMessageType(dhcpv4.MessageTypeOffer), - ), - }, - }} + _, _ = conn.broadcast(respData, cloneUDPAddr(defaultPeer)) - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - s.send(cloneUDPAddr(defaultPeer), conn, tc.req, tc.resp) - assert.EqualValues(t, tc.resp.ToBytes(), b.Bytes()) - assert.Equal(t, &net.UDPAddr{ - IP: s.conf.broadcastIP, - Port: defaultPeer.Port, - Zone: defaultPeer.Zone, - }, peer) - }) - - b.Reset() - peer = nil - } + assert.EqualValues(t, respData, b.Bytes()) + assert.Equal(t, &net.UDPAddr{ + IP: conn.bcastIP, + Port: defaultPeer.Port, + }, peer) } diff --git a/internal/dhcpd/broadcast_others.go b/internal/dhcpd/broadcast_others.go index f801497e..a6e1307a 100644 --- a/internal/dhcpd/broadcast_others.go +++ b/internal/dhcpd/broadcast_others.go @@ -5,17 +5,10 @@ package dhcpd import ( "net" - - "github.com/AdguardTeam/golibs/log" - "github.com/insomniacslk/dhcp/dhcpv4" ) // broadcast sends resp to the broadcast address specific for network interface. -func (s *v4Server) broadcast(peer net.Addr, conn net.PacketConn, resp *dhcpv4.DHCPv4) { - respData := resp.ToBytes() - - log.Debug("dhcpv4: sending to %s: %s", peer, resp.Summary()) - +func (c *dhcpConn) broadcast(respData []byte, peer *net.UDPAddr) (n int, err error) { // This write to 0xffffffff reverts some behavior changes made in // https://github.com/AdguardTeam/AdGuardHome/issues/3289. The DHCP // server should broadcast the message to 0xffffffff but it's @@ -26,26 +19,13 @@ func (s *v4Server) broadcast(peer net.Addr, conn net.PacketConn, resp *dhcpv4.DH // https://github.com/AdguardTeam/AdGuardHome/issues/3366. // // See also https://github.com/AdguardTeam/AdGuardHome/issues/3539. - if _, err := conn.WriteTo(respData, peer); err != nil { - log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) - } - - // peer is expected to be of type *net.UDPConn as the server4.NewServer - // initializes it. - udpPeer, ok := peer.(*net.UDPAddr) - if !ok { - log.Error("dhcpv4: peer is of unexpected type %T", peer) - - return + if n, err = c.udpConn.WriteTo(respData, peer); err != nil { + return n, err } // Broadcast the message one more time using the interface-specific // broadcast address. - udpPeer.IP = s.conf.broadcastIP + peer.IP = c.bcastIP - log.Debug("dhcpv4: sending to %s: %s", peer, resp.Summary()) - - if _, err := conn.WriteTo(respData, peer); err != nil { - log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) - } + return c.udpConn.WriteTo(respData, peer) } diff --git a/internal/dhcpd/broadcast_others_test.go b/internal/dhcpd/broadcast_others_test.go index 246be382..cb25ac28 100644 --- a/internal/dhcpd/broadcast_others_test.go +++ b/internal/dhcpd/broadcast_others_test.go @@ -8,17 +8,16 @@ import ( "net" "testing" - "github.com/AdguardTeam/golibs/netutil" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestV4Server_Send_broadcast(t *testing.T) { +func TestDHCPConn_Broadcast(t *testing.T) { b := &bytes.Buffer{} var peers []*net.UDPAddr - conn := &fakePacketConn{ + udpConn := &fakePacketConn{ writeTo: func(p []byte, addr net.Addr) (n int, err error) { udpPeer, ok := addr.(*net.UDPAddr) require.True(t, ok) @@ -31,66 +30,27 @@ func TestV4Server_Send_broadcast(t *testing.T) { return n, nil }, } - + conn := &dhcpConn{ + udpConn: udpConn, + bcastIP: net.IP{1, 2, 3, 255}, + } defaultPeer := &net.UDPAddr{ IP: net.IP{1, 2, 3, 4}, // Use neither client nor server port. Port: 1234, } - s := &v4Server{ - conf: V4ServerConf{ - broadcastIP: net.IP{1, 2, 3, 255}, - }, - } + respData := (&dhcpv4.DHCPv4{}).ToBytes() - testCases := []struct { - name string - req *dhcpv4.DHCPv4 - resp *dhcpv4.DHCPv4 - }{{ - name: "nak", - req: &dhcpv4.DHCPv4{ - GatewayIPAddr: netutil.IPv4Zero(), - }, - resp: &dhcpv4.DHCPv4{ - Options: dhcpv4.OptionsFromList( - dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), - ), - }, - }, { - name: "fully_unspecified", - req: &dhcpv4.DHCPv4{ - GatewayIPAddr: netutil.IPv4Zero(), - ClientIPAddr: netutil.IPv4Zero(), - }, - resp: &dhcpv4.DHCPv4{ - Options: dhcpv4.OptionsFromList( - dhcpv4.OptMessageType(dhcpv4.MessageTypeOffer), - ), - }, - }} + _, _ = conn.broadcast(respData, cloneUDPAddr(defaultPeer)) - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - s.send(cloneUDPAddr(defaultPeer), conn, tc.req, tc.resp) + // The same response is written twice but for different peers. + assert.EqualValues(t, append(respData, respData...), b.Bytes()) - // The same response is written twice. - respData := tc.resp.ToBytes() - assert.EqualValues(t, append(respData, respData...), b.Bytes()) + require.Len(t, peers, 2) - require.Len(t, peers, 2) - - assert.Equal(t, &net.UDPAddr{ - IP: defaultPeer.IP, - Port: defaultPeer.Port, - }, peers[0]) - assert.Equal(t, &net.UDPAddr{ - IP: s.conf.broadcastIP, - Port: defaultPeer.Port, - }, peers[1]) - }) - - b.Reset() - peers = nil - } + assert.Equal(t, cloneUDPAddr(defaultPeer), peers[0]) + assert.Equal(t, &net.UDPAddr{ + IP: conn.bcastIP, + Port: defaultPeer.Port, + }, peers[1]) } diff --git a/internal/dhcpd/conn_unix.go b/internal/dhcpd/conn_unix.go new file mode 100644 index 00000000..8f8e7ed5 --- /dev/null +++ b/internal/dhcpd/conn_unix.go @@ -0,0 +1,244 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris +// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris + +package dhcpd + +import ( + "fmt" + "net" + "os" + "time" + + "github.com/AdguardTeam/golibs/errors" + "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" + "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(ifi *net.Interface) (c net.PacketConn, err error) { + // Create the raw connection. + var ucast net.PacketConn + if ucast, err = raw.ListenPacket(ifi, 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(ifi.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, + rawConn: ucast, + srcMAC: ifi.HardwareAddr, + srcIP: s.conf.dnsIPAddrs[0], + }, nil +} + +// wrapErrs is a helper to wrap the errors from two independent underlying +// connections. +func (c *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("peer is of 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 as recommended by +// RFC-1700 (https://datatracker.ietf.org/doc/html/rfc1700) in seconds. +const ipv4DefaultTTL = 64 + +// errInvalidPktDHCP is returned when the provided payload is not a valid DHCP +// packet. +const errInvalidPktDHCP errors.Error = "packet is not a valid dhcp packet" + +// buildEtherPkt wraps the payload with IPv4, UDP and Ethernet frames. The +// payload is expected to be an encoded DHCP packet. +func (c *dhcpConn) buildEtherPkt(payload []byte, peer *dhcpUnicastAddr) (pkt []byte, err error) { + dhcpLayer := gopacket.NewPacket(payload, layers.LayerTypeDHCPv4, gopacket.DecodeOptions{ + NoCopy: true, + }).Layer(layers.LayerTypeDHCPv4) + + // Check if the decoding succeeded and the resulting layer doesn't + // contain any errors. It should guarantee panic-safe converting of the + // layer into gopacket.SerializableLayer. + if dhcpLayer == nil || dhcpLayer.LayerType() != layers.LayerTypeDHCPv4 { + return nil, errInvalidPktDHCP + } + + 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() + err = gopacket.SerializeLayers(buf, gopacket.SerializeOptions{ + FixLengths: true, + ComputeChecksums: true, + }, ethLayer, ipv4Layer, udpLayer, dhcpLayer.(gopacket.SerializableLayer)) + if err != nil { + return nil, fmt.Errorf("serializing layers: %w", err) + } + + return buf.Bytes(), nil +} diff --git a/internal/dhcpd/conn_unix_test.go b/internal/dhcpd/conn_unix_test.go new file mode 100644 index 00000000..f3c20805 --- /dev/null +++ b/internal/dhcpd/conn_unix_test.go @@ -0,0 +1,114 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris +// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris + +package dhcpd + +import ( + "net" + "strings" + "testing" + + "github.com/google/gopacket" + "github.com/google/gopacket/layers" + "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/mdlayher/raw" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +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) + + assert.True(t, strings.Contains(err.Error(), "peer is of unexpected type")) + 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("non-serializable", func(t *testing.T) { + // Create an invalid DHCP packet. + invalidPayload := []byte{1, 2, 3, 4} + pkt, err := conn.buildEtherPkt(invalidPayload, nil) + require.Error(t, err) + + assert.ErrorIs(t, err, errInvalidPktDHCP) + assert.Empty(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) + }) +} diff --git a/internal/dhcpd/dhcpd_test.go b/internal/dhcpd/dhcpd_test.go index 71878369..21f63fd1 100644 --- a/internal/dhcpd/dhcpd_test.go +++ b/internal/dhcpd/dhcpd_test.go @@ -139,7 +139,7 @@ func TestNormalizeLeases(t *testing.T) { } // cloneUDPAddr returns a deep copy of a. -func cloneUDPAddr(a *net.UDPAddr) (copy *net.UDPAddr) { +func cloneUDPAddr(a *net.UDPAddr) (clone *net.UDPAddr) { return &net.UDPAddr{ IP: netutil.CloneIP(a.IP), Port: a.Port, diff --git a/internal/dhcpd/options_unix.go b/internal/dhcpd/options_unix.go index 820eabd1..07375eaf 100644 --- a/internal/dhcpd/options_unix.go +++ b/internal/dhcpd/options_unix.go @@ -157,8 +157,8 @@ func prepareOptions(conf V4ServerConf) (opts dhcpv4.Options) { dhcpv4.OptionPerformRouterDiscovery.Code(): []byte{1}, // The all-routers address is preferred wherever possible, see // https://datatracker.ietf.org/doc/html/rfc1256#section-5.1. - dhcpv4.OptionRouterSolicitationAddress.Code(): net.IPv4allrouter.To4(), - dhcpv4.OptionBroadcastAddress.Code(): net.IPv4bcast.To4(), + dhcpv4.OptionRouterSolicitationAddress.Code(): netutil.IPv4allrouter(), + dhcpv4.OptionBroadcastAddress.Code(): netutil.IPv4bcast(), // Link-Layer Per Interface diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go index b13be453..6c2d4b5e 100644 --- a/internal/dhcpd/v4.go +++ b/internal/dhcpd/v4.go @@ -19,6 +19,7 @@ import ( "github.com/go-ping/ping" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv4/server4" + "github.com/mdlayher/raw" ) // v4Server is a DHCPv4 server. @@ -955,43 +956,44 @@ func (s *v4Server) packetHandler(conn net.PacketConn, peer net.Addr, req *dhcpv4 // // 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) { - var unicast bool - if giaddr := req.GatewayIPAddr; giaddr != nil && !giaddr.IsUnspecified() { + 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, } - unicast = true - } else if mtype := resp.MessageType(); mtype == dhcpv4.MessageTypeNak { + if mtype == dhcpv4.MessageTypeNak { + // Set the broadcast bit in the DHCPNAK, so that the + // relay agent broadcasted 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. - } else if ciaddr := req.ClientIPAddr; ciaddr != nil && !ciaddr.IsUnspecified() { + case ciaddr != nil && !ciaddr.IsUnspecified(): // Unicast DHCPOFFER and DHCPACK messages to the address in // ciaddr. peer = &net.UDPAddr{ IP: ciaddr, Port: dhcpv4.ClientPort, } - unicast = true - } - - // TODO(e.burkov): Unicast the message to the actual link-layer address - // of the client if broadcast bit is not set. Perhaps, use custom - // connection when creating the server. - // - // See https://github.com/AdguardTeam/AdGuardHome/issues/3443. - - if !unicast { - s.broadcast(peer, conn, resp) - - return + 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. } log.Debug("dhcpv4: sending to %s: %s", peer, resp.Summary()) - - _, err := conn.WriteTo(resp.ToBytes(), peer) - if err != nil { + if _, err := conn.WriteTo(resp.ToBytes(), peer); err != nil { log.Error("dhcpv4: conn.Write to %s failed: %s", peer, err) } } @@ -1029,11 +1031,18 @@ func (s *v4Server) Start() (err error) { s.conf.dnsIPAddrs = dnsIPAddrs - laddr := &net.UDPAddr{ - IP: net.IP{0, 0, 0, 0}, - Port: dhcpv4.ServerPort, + var c net.PacketConn + if c, err = s.newDHCPConn(iface); err != nil { + return err } - s.srv, err = server4.NewServer(iface.Name, laddr, s.packetHandler, server4.WithDebugLogger()) + + s.srv, err = server4.NewServer( + iface.Name, + nil, + s.packetHandler, + server4.WithConn(c), + server4.WithDebugLogger(), + ) if err != nil { return err } diff --git a/internal/dhcpd/v4_test.go b/internal/dhcpd/v4_test.go index 51cd82a3..d20e715c 100644 --- a/internal/dhcpd/v4_test.go +++ b/internal/dhcpd/v4_test.go @@ -4,12 +4,11 @@ package dhcpd import ( - "bytes" "net" "testing" - "github.com/AdguardTeam/golibs/netutil" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/mdlayher/raw" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -390,67 +389,107 @@ func (fc *fakePacketConn) WriteTo(p []byte, addr net.Addr) (n int, err error) { return fc.writeTo(p, addr) } -func TestV4Server_Send_unicast(t *testing.T) { - b := &bytes.Buffer{} - var peer *net.UDPAddr - - conn := &fakePacketConn{ - writeTo: func(p []byte, addr net.Addr) (n int, err error) { - udpPeer, ok := addr.(*net.UDPAddr) - require.True(t, ok) - - peer = cloneUDPAddr(udpPeer) - - n, err = b.Write(p) - require.NoError(t, err) - - return n, nil - }, - } - - defaultPeer := &net.UDPAddr{ - IP: net.IP{1, 2, 3, 4}, - // Use neither client nor server port. - Port: 1234, - } - defaultResp := &dhcpv4.DHCPv4{ - OpCode: dhcpv4.OpcodeBootReply, - } +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 { - name string - req *dhcpv4.DHCPv4 - wantPeer net.Addr + want net.Addr + req *dhcpv4.DHCPv4 + resp *dhcpv4.DHCPv4 + name string }{{ - name: "relay_agent", - req: &dhcpv4.DHCPv4{ - GatewayIPAddr: defaultPeer.IP, - }, - wantPeer: &net.UDPAddr{ - IP: defaultPeer.IP, + name: "giaddr", + req: &dhcpv4.DHCPv4{GatewayIPAddr: knownIP}, + resp: defaultResp, + want: &net.UDPAddr{ + IP: knownIP, Port: dhcpv4.ServerPort, }, }, { - name: "known_client", - req: &dhcpv4.DHCPv4{ - GatewayIPAddr: netutil.IPv4Zero(), - ClientIPAddr: net.IP{2, 3, 4, 5}, + name: "nak", + req: &dhcpv4.DHCPv4{}, + resp: &dhcpv4.DHCPv4{ + Options: dhcpv4.OptionsFromList( + dhcpv4.OptMessageType(dhcpv4.MessageTypeNak), + ), }, - wantPeer: &net.UDPAddr{ - IP: net.IP{2, 3, 4, 5}, + 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) { - s.send(defaultPeer, conn, tc.req, defaultResp) - assert.EqualValues(t, defaultResp.ToBytes(), b.Bytes()) - assert.Equal(t, tc.wantPeer, peer) - }) + conn := &fakePacketConn{ + writeTo: func(_ []byte, addr net.Addr) (_ int, _ error) { + assert.Equal(t, tc.want, addr) - b.Reset() - peer = nil + 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()) + }) }