From 486c8612a71ba5e1f251fbbbf08a91e3d8f77bfa Mon Sep 17 00:00:00 2001 From: Anatole Denis Date: Tue, 15 Oct 2019 10:16:05 +0200 Subject: server4: Respect listen address NewIPv4UDPConn doesn't support listening on a specific address, only on the wildcard address. This extends it to allow listening on an address, and at the same time homogenizes the function signature with the NewIPv6UDPConn server6 equivalent. It modifies NewServer() to pass the full address given to it instead of just the port as well Note that listening on a non-wildcard interface is seldom useful as the socket won't receive broadcasts, so it is useless in a direct-attached server. It can be useful in a server only used behind relays This breaks API compatibility for NewIPv4UDPConn, which as far as I know nobody uses (yet) Signed-off-by: Anatole Denis --- dhcpv4/server4/conn.go | 12 +++++++++--- dhcpv4/server4/server.go | 2 +- dhcpv4/server4/server_test.go | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) (limited to 'dhcpv4/server4') diff --git a/dhcpv4/server4/conn.go b/dhcpv4/server4/conn.go index 0a4c73a..d62a5ac 100644 --- a/dhcpv4/server4/conn.go +++ b/dhcpv4/server4/conn.go @@ -14,7 +14,7 @@ import ( // given based on a IPv4 DGRAM socket. The UDP connection allows broadcasting. // // The interface must already be configured. -func NewIPv4UDPConn(iface string, port int) (*net.UDPConn, error) { +func NewIPv4UDPConn(iface string, addr *net.UDPAddr) (*net.UDPConn, error) { fd, err := unix.Socket(unix.AF_INET, unix.SOCK_DGRAM, unix.IPPROTO_UDP) if err != nil { return nil, fmt.Errorf("cannot get a UDP socket: %v", err) @@ -37,9 +37,15 @@ func NewIPv4UDPConn(iface string, port int) (*net.UDPConn, error) { return nil, fmt.Errorf("cannot bind to interface %s: %v", iface, err) } } + + if addr == nil { + addr = &net.UDPAddr{Port: dhcpv4.ServerPort} + } // Bind to the port. - if err := unix.Bind(fd, &unix.SockaddrInet4{Port: port}); err != nil { - return nil, fmt.Errorf("cannot bind to port %d: %v", port, err) + saddr := unix.SockaddrInet4{Port: addr.Port} + copy(saddr.Addr[:], addr.IP.To4()) + if err := unix.Bind(fd, &saddr); err != nil { + return nil, fmt.Errorf("cannot bind to port %d: %v", addr.Port, err) } conn, err := net.FilePacketConn(f) diff --git a/dhcpv4/server4/server.go b/dhcpv4/server4/server.go index fe4ef09..8bf7924 100644 --- a/dhcpv4/server4/server.go +++ b/dhcpv4/server4/server.go @@ -133,7 +133,7 @@ func NewServer(ifname string, addr *net.UDPAddr, handler Handler, opt ...ServerO } if s.conn == nil { var err error - conn, err := NewIPv4UDPConn(ifname, addr.Port) + conn, err := NewIPv4UDPConn(ifname, addr) if err != nil { return nil, err } diff --git a/dhcpv4/server4/server_test.go b/dhcpv4/server4/server_test.go index a596d04..e64be09 100644 --- a/dhcpv4/server4/server_test.go +++ b/dhcpv4/server4/server_test.go @@ -81,7 +81,7 @@ func setUpClientAndServer(t *testing.T, iface net.Interface, handler Handler) (* _ = s.Serve() }() - clientConn, err := NewIPv4UDPConn("", caddr.Port) + clientConn, err := NewIPv4UDPConn("", &caddr) if err != nil { t.Fatal(err) } -- cgit v1.2.3 From 62a3f6317a49b19232e67faa31d90f94b522eb82 Mon Sep 17 00:00:00 2001 From: Anatole Denis Date: Tue, 15 Oct 2019 10:27:38 +0200 Subject: server4: Use port 0 in tests Tests use a randPort() workaround for not using port 0, as port 0 is not usable with raw sockets. We don't actually use raw sockets, so port 0 is fine, this makes use of it which ensures we avoid port collisions. Signed-off-by: Anatole Denis --- dhcpv4/server4/server_test.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'dhcpv4/server4') diff --git a/dhcpv4/server4/server_test.go b/dhcpv4/server4/server_test.go index e64be09..da2b199 100644 --- a/dhcpv4/server4/server_test.go +++ b/dhcpv4/server4/server_test.go @@ -22,13 +22,6 @@ func init() { rand.Seed(time.Now().UTC().UnixNano()) } -func randPort() int { - // can't use port 0 with raw sockets, so until we implement - // a non-raw-sockets client for non-static ports, we have to - // deal with this "randomness" - return 32*1024 + rand.Intn(65536-32*1024) -} - // DORAHandler is a server handler suitable for DORA transactions func DORAHandler(conn net.PacketConn, peer net.Addr, m *dhcpv4.DHCPv4) { if m == nil { @@ -65,27 +58,28 @@ func DORAHandler(conn net.PacketConn, peer net.Addr, m *dhcpv4.DHCPv4) { func setUpClientAndServer(t *testing.T, iface net.Interface, handler Handler) (*nclient4.Client, *Server) { // strong assumption, I know loAddr := net.ParseIP("127.0.0.1") - saddr := net.UDPAddr{ + saddr := &net.UDPAddr{ IP: loAddr, - Port: randPort(), + Port: 0, } caddr := net.UDPAddr{ IP: loAddr, - Port: randPort(), + Port: 0, } - s, err := NewServer("", &saddr, handler) + s, err := NewServer("", saddr, handler) if err != nil { t.Fatal(err) } go func() { _ = s.Serve() }() + saddr = s.conn.LocalAddr().(*net.UDPAddr) clientConn, err := NewIPv4UDPConn("", &caddr) if err != nil { t.Fatal(err) } - c, err := nclient4.NewWithConn(clientConn, iface.HardwareAddr, nclient4.WithServerAddr(&saddr)) + c, err := nclient4.NewWithConn(clientConn, iface.HardwareAddr, nclient4.WithServerAddr(saddr)) if err != nil { t.Fatal(err) } -- cgit v1.2.3 From 4a980462f24f67f989ac2860fcf9879aef3e3fa0 Mon Sep 17 00:00:00 2001 From: Anatole Denis Date: Tue, 15 Oct 2019 11:16:29 +0200 Subject: server4: Only allow IPv4 addresses IPv6 addresses would not cause a crash, but would silently listen on the wildcard address instead of the passed address, which is surprising at best. Instead check for the address family and reject non-v4 addresses Signed-off-by: Anatole Denis --- dhcpv4/server4/conn.go | 3 +++ dhcpv4/server4/server_test.go | 11 +++++++++++ 2 files changed, 14 insertions(+) (limited to 'dhcpv4/server4') diff --git a/dhcpv4/server4/conn.go b/dhcpv4/server4/conn.go index d62a5ac..3e49669 100644 --- a/dhcpv4/server4/conn.go +++ b/dhcpv4/server4/conn.go @@ -43,6 +43,9 @@ func NewIPv4UDPConn(iface string, addr *net.UDPAddr) (*net.UDPConn, error) { } // Bind to the port. saddr := unix.SockaddrInet4{Port: addr.Port} + if addr.IP != nil && addr.IP.To4() == nil { + return nil, fmt.Errorf("wrong address family (expected v4) for %s", addr.IP) + } copy(saddr.Addr[:], addr.IP.To4()) if err := unix.Bind(fd, &saddr); err != nil { return nil, fmt.Errorf("cannot bind to port %d: %v", addr.Port, err) diff --git a/dhcpv4/server4/server_test.go b/dhcpv4/server4/server_test.go index da2b199..43314ad 100644 --- a/dhcpv4/server4/server_test.go +++ b/dhcpv4/server4/server_test.go @@ -116,3 +116,14 @@ func TestServer(t *testing.T) { require.Equal(t, ifaces[0].HardwareAddr, p.ClientHWAddr) } } + +func TestBadAddrFamily(t *testing.T) { + saddr := &net.UDPAddr{ + IP: net.IPv6loopback, + Port: 0, + } + _, err := NewServer("", saddr, DORAHandler) + if err == nil { + t.Fatal("Expected server4.NewServer to fail with an IPv6 address") + } +} -- cgit v1.2.3