Skip to content

Commit

Permalink
Pull request: 3443 dhcp broadcast vol.2
Browse files Browse the repository at this point in the history 
Merge in DNS/adguard-home from 3443-dhcp-broadcast-vol.2 to master

Closes AdguardTeam#3443.

Squashed commit of the following:

commit a85af89
Merge: 72eb3a88 a4e0782
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 30 18:08:19 2021 +0300

    Merge branch 'master' into 3443-dhcp-broadcast-vol.2

commit 72eb3a8
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 30 18:03:19 2021 +0300

    dhcpd: imp code readability

commit 2d1fbc4
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 30 14:16:59 2021 +0300

    dhcpd: imp tests

commit 889fad3
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Sep 29 20:09:25 2021 +0300

    dhcpd: imp code, docs

commit 1fd6b23
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 23 16:08:18 2021 +0300

    dhcpd: unicast to mac address
  • Loading branch information
@EugeneOne1 @heyxkhoa
EugeneOne1 authored and heyxkhoa committed Mar 17, 2023
1 parent 0c1e0cc commit d630968
Show file tree
Hide file tree
Showing 12 changed files with 528 additions and 228 deletions.
4 changes: 3 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
30 changes: 7 additions & 23 deletions internal/dhcpd/broadcast_bsd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

log.Debug("dhcpv4: sending to %s: %s", peer, resp.Summary())
// 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

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)
}
62 changes: 13 additions & 49 deletions internal/dhcpd/broadcast_bsd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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},
},
}

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),
),
},
}}
respData := (&dhcpv4.DHCPv4{}).ToBytes()

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)
})
_, _ = conn.broadcast(respData, cloneUDPAddr(defaultPeer))

b.Reset()
peer = nil
}
assert.EqualValues(t, respData, b.Bytes())
assert.Equal(t, &net.UDPAddr{
IP: conn.bcastIP,
Port: defaultPeer.Port,
}, peer)
}
30 changes: 5 additions & 25 deletions internal/dhcpd/broadcast_others.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
72 changes: 16 additions & 56 deletions internal/dhcpd/broadcast_others_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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),
),
},
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
s.send(cloneUDPAddr(defaultPeer), conn, tc.req, tc.resp)
_, _ = conn.broadcast(respData, cloneUDPAddr(defaultPeer))

// The same response is written twice.
respData := tc.resp.ToBytes()
assert.EqualValues(t, append(respData, respData...), b.Bytes())
// The same response is written twice but for different peers.
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])
}
Loading

0 comments on commit d630968

Please sign in to comment.