Unicast to the giaddr address when non zero:

When the DHCP header `giaddr` is set, we must unicast responses to that address. DHCP RFC (https://www.ietf.org/rfc/rfc2131.txt) section 4.1, page 22: "If the 'giaddr' field in a DHCP message from a client is non-zero, the server sends any return messages to the 'DHCP server' port on the BOOTP relay agent whose address appears in 'giaddr'." Signed-off-by: Jacob Weinstock <[email protected]>
tinkerbell · Dec 15, 2023 · 2b59e7c · 2b59e7c
1 parent b3c8a31
commit 2b59e7c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 6 deletions.
diff --git a/handler/reservation/handler.go b/handler/reservation/handler.go
@@ -124,12 +124,15 @@ func (h *Handler) Handle(ctx context.Context, conn *ipv4.PacketConn, p data.Pack
 	if ns := reply.ServerIPAddr; ns != nil {
 		log = log.WithValues("nextServer", ns.String())
 	}
-	log = log.WithValues("ipAddress", reply.YourIPAddr.String(), "destination", p.Peer.String())
+
+	dst := replyDestination(p.Peer, p.Pkt.GatewayIPAddr)
+	log = log.WithValues("ipAddress", reply.YourIPAddr.String(), "destination", dst.String())
 	cm := &ipv4.ControlMessage{}
 	if p.Md != nil {
 		cm.IfIndex = p.Md.IfIndex
 	}
-	if _, err := conn.WriteTo(reply.ToBytes(), cm, p.Peer); err != nil {
+
+	if _, err := conn.WriteTo(reply.ToBytes(), cm, dst); err != nil {
 		log.Error(err, "failed to send DHCP")
 		span.SetStatus(codes.Error, err.Error())
 
@@ -141,6 +144,22 @@ func (h *Handler) Handle(ctx context.Context, conn *ipv4.PacketConn, p data.Pack
 	span.SetStatus(codes.Ok, "sent DHCP response")
 }
 
+// replyDestination determines the destination address for the DHCP reply.
+// If the giaddr is set, then the reply should be sent to the giaddr.
+// Otherwise, the reply should be sent to the direct peer.
+//
+// From page 22 of https://www.ietf.org/rfc/rfc2131.txt:
+// "If the 'giaddr' field in a DHCP message from a client is non-zero,
+// the server sends any return messages to the 'DHCP server' port on
+// the BOOTP relay agent whose address appears in 'giaddr'.".
+func replyDestination(directPeer net.Addr, giaddr net.IP) net.Addr {
+	if !giaddr.IsUnspecified() && giaddr != nil {
+		return &net.UDPAddr{IP: giaddr, Port: dhcpv4.ServerPort}
+	}
+
+	return directPeer
+}
+
 // readBackend encapsulates the backend read and opentelemetry handling.
 func (h *Handler) readBackend(ctx context.Context, mac net.HardwareAddr) (*data.DHCP, *data.Netboot, error) {
 	h.setDefaults()

diff --git a/handler/reservation/handler_test.go b/handler/reservation/handler_test.go
@@ -275,10 +275,6 @@ func TestHandle(t *testing.T) {
 			want:    nil,
 			wantErr: errBadBackend,
 		},
-		/*"nil incoming packet": {
-			want:    nil,
-			wantErr: errBadBackend,
-		},*/
 		"failure no hardware found discover": {
 			server: Handler{
 				Backend: &mockBackend{hardwareNotFound: true},