Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
dhcp daemon checksum fix
To:
tech@openbsd.org
Cc:
Jan Klemkow <jan@openbsd.org>, Markus Friedl <markus@openbsd.org>
Date:
Tue, 4 Feb 2025 22:32:12 +0100

Download raw body.

Thread
Hi,

When sending traffic via vio(4) between guests living within a
single KVM host, the packet checksum is never calculated.  A flag
is set to assume the checksum is correct.  Our IP stack handles
that correctly but when using bpf(4) userland has to care.

jan@ has fixed it for dhcpleased a while ago.

----------------------------
revision 1.42
date: 2024/01/26 21:14:08;  author: jan;  state: Exp;  lines: +17 -12;  commitid: nvgEHwLOyfqqyGwh;
Put checksum flags in bpf_hdr to use them in userland dhcpleased.

Thus, dhcpleased accept non-calculated checksums which were verified by
hardware/hypervisor.

With tweaks from dlg@

ok bluhm@
mkay tobhe@
----------------------------

But our other dhcp daemons suffer from the same problem.  Below is
the diff for dhcpd(8), dhcrelay(8), and dhcrelay6(8).

While there, I discovered that checksum calculation is suboptimal.
To verify, calculate the checksum of the received packet and it has
to be 0.  That is what RFC 1071 says and our kernel does.

I did run dhcpleased on a KVM guest and markus@ tested dhcpd.

If you run any of these daemons on KVM guest, please test.

ok?

bluhm

Index: sbin/dhcpleased/engine.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sbin/dhcpleased/engine.c,v
diff -u -p -r1.55 engine.c
--- sbin/dhcpleased/engine.c	21 Nov 2024 13:35:20 -0000	1.55
+++ sbin/dhcpleased/engine.c	16 Jan 2025 20:14:10 -0000
@@ -744,7 +744,7 @@ parse_dhcp(struct dhcpleased_iface *ifac
 	struct in_addr		 nameservers[MAX_RDNS_COUNT];
 	struct dhcp_route	 routes[MAX_DHCP_ROUTES];
 	size_t			 rem, i;
-	uint32_t		 sum, usum, lease_time = 0, renewal_time = 0;
+	uint32_t		 lease_time = 0, renewal_time = 0;
 	uint32_t		 rebinding_time = 0;
 	uint32_t		 ipv6_only_time = 0;
 	uint8_t			*p, dho = DHO_PAD, dho_len, slen;
@@ -850,16 +850,14 @@ parse_dhcp(struct dhcpleased_iface *ifac
 	p += sizeof(*udp);
 	rem -= sizeof(*udp);
 
-	if ((dhcp->csumflags & M_UDP_CSUM_IN_OK) == 0) {
-		usum = udp->uh_sum;
-		udp->uh_sum = 0;
-
-		sum = wrapsum(checksum((uint8_t *)udp, sizeof(*udp),
+	if ((dhcp->csumflags & M_UDP_CSUM_IN_OK) == 0 &&
+	    udp->uh_sum != 0) {
+		udp->uh_sum = wrapsum(checksum((uint8_t *)udp, sizeof(*udp),
 		    checksum(p, rem,
 		    checksum((uint8_t *)&ip->ip_src, 2 * sizeof(ip->ip_src),
 		    IPPROTO_UDP + ntohs(udp->uh_ulen)))));
 
-		if (usum != 0 && usum != sum) {
+		if (udp->uh_sum != 0) {
 			log_warnx("%s: bad UDP checksum", __func__);
 			return;
 		}
Index: usr.sbin/dhcpd/bpf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcpd/bpf.c,v
diff -u -p -r1.20 bpf.c
--- usr.sbin/dhcpd/bpf.c	18 Mar 2019 00:00:59 -0000	1.20
+++ usr.sbin/dhcpd/bpf.c	16 Jan 2025 20:10:42 -0000
@@ -347,7 +347,8 @@ receive_packet(struct interface_info *in
 
 		/* Decode the IP and UDP headers... */
 		offset = decode_udp_ip_header(interface->rbuf +
-		    interface->rbuf_offset, hdr.bh_caplen, from);
+		    interface->rbuf_offset, hdr.bh_caplen, from,
+		    hdr.bh_csumflags);
 
 		/* If the IP or UDP checksum was bad, skip the packet... */
 		if (offset < 0) {
Index: usr.sbin/dhcpd/dhcpd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcpd/dhcpd.h,v
diff -u -p -r1.69 dhcpd.h
--- usr.sbin/dhcpd/dhcpd.h	21 May 2024 05:00:48 -0000	1.69
+++ usr.sbin/dhcpd/dhcpd.h	16 Jan 2025 20:11:40 -0000
@@ -494,7 +494,8 @@ void assemble_hw_header(struct interface
 void assemble_udp_ip_header(struct interface_info *, unsigned char *,
     int *, u_int32_t, u_int32_t, unsigned int, unsigned char *, int);
 ssize_t decode_hw_header(unsigned char *, u_int32_t, struct hardware *);
-ssize_t decode_udp_ip_header(unsigned char *, u_int32_t, struct sockaddr_in *);
+ssize_t decode_udp_ip_header(unsigned char *, u_int32_t, struct sockaddr_in *,
+    u_int16_t);
 u_int32_t	checksum(unsigned char *, u_int32_t, u_int32_t);
 u_int32_t	wrapsum(u_int32_t);
 
Index: usr.sbin/dhcpd/packet.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcpd/packet.c,v
diff -u -p -r1.14 packet.c
--- usr.sbin/dhcpd/packet.c	18 Apr 2017 13:59:09 -0000	1.14
+++ usr.sbin/dhcpd/packet.c	16 Jan 2025 20:14:23 -0000
@@ -42,6 +42,7 @@
 
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/mbuf.h>
 
 #include <net/if.h>
 
@@ -168,13 +169,12 @@ decode_hw_header(unsigned char *buf, u_i
 
 ssize_t
 decode_udp_ip_header(unsigned char *buf, u_int32_t buflen,
-    struct sockaddr_in *from)
+    struct sockaddr_in *from, u_int16_t csumflags)
 {
 	struct ip *ip;
 	struct udphdr *udp;
 	unsigned char *data;
 	u_int32_t ip_len;
-	u_int32_t sum, usum;
 	static unsigned int ip_packets_seen;
 	static unsigned int ip_packets_bad_checksum;
 	static unsigned int udp_packets_seen;
@@ -194,7 +194,8 @@ decode_udp_ip_header(unsigned char *buf,
 	ip_packets_seen++;
 
 	/* Check the IP header checksum - it should be zero. */
-	if (wrapsum(checksum((unsigned char *)ip, ip_len, 0)) != 0) {
+	if ((csumflags & M_IPV4_CSUM_IN_OK) == 0 &&
+	    wrapsum(checksum((unsigned char *)ip, ip_len, 0)) != 0) {
 		ip_packets_bad_checksum++;
 		if (ip_packets_seen > 4 && ip_packets_bad_checksum != 0 &&
 		    (ip_packets_seen / ip_packets_bad_checksum) < 2) {
@@ -253,24 +254,26 @@ decode_udp_ip_header(unsigned char *buf,
 	if (len + data != buf + buflen)
 		log_debug("accepting packet with data after udp payload.");
 
-	usum = udp->uh_sum;
-	udp->uh_sum = 0;
-
-	sum = wrapsum(checksum((unsigned char *)udp, sizeof(*udp),
-	    checksum(data, len, checksum((unsigned char *)&ip->ip_src,
-	    2 * sizeof(ip->ip_src),
-	    IPPROTO_UDP + (u_int32_t)ntohs(udp->uh_ulen)))));
-
 	udp_packets_seen++;
-	if (usum && usum != sum) {
-		udp_packets_bad_checksum++;
-		if (udp_packets_seen > 4 && udp_packets_bad_checksum != 0 &&
-		    (udp_packets_seen / udp_packets_bad_checksum) < 2) {
-			log_info("%u bad udp checksums in %u packets",
-			    udp_packets_bad_checksum, udp_packets_seen);
-			udp_packets_seen = udp_packets_bad_checksum = 0;
+
+	if ((csumflags & M_UDP_CSUM_IN_OK) == 0 &&
+	    udp->uh_sum != 0) {
+		udp->uh_sum = wrapsum(checksum((uint8_t *)udp, sizeof(*udp),
+		    checksum(data, len,
+		    checksum((uint8_t *)&ip->ip_src, 2 * sizeof(ip->ip_src),
+		    IPPROTO_UDP + ntohs(udp->uh_ulen)))));
+
+		if (udp->uh_sum != 0) {
+			udp_packets_bad_checksum++;
+			if (udp_packets_seen > 4 &&
+			    udp_packets_bad_checksum != 0 &&
+			    (udp_packets_seen / udp_packets_bad_checksum) < 2) {
+				log_info("%u bad udp checksums in %u packets",
+				    udp_packets_bad_checksum, udp_packets_seen);
+				udp_packets_seen = udp_packets_bad_checksum = 0;
+			}
+			return (-1);
 		}
-		return (-1);
 	}
 
 	memcpy(&from->sin_port, &udp->uh_sport, sizeof(udp->uh_sport));
Index: usr.sbin/dhcrelay/bpf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcrelay/bpf.c,v
diff -u -p -r1.19 bpf.c
--- usr.sbin/dhcrelay/bpf.c	19 Apr 2017 05:36:12 -0000	1.19
+++ usr.sbin/dhcrelay/bpf.c	16 Jan 2025 22:03:10 -0000
@@ -453,7 +453,7 @@ receive_packet(struct interface_info *in
 
 		/* Decode the IP and UDP headers... */
 		offset = decode_udp_ip_header(interface->rbuf,
-		    interface->rbuf_len, offset, pc);
+		    interface->rbuf_len, offset, pc, hdr.bh_csumflags);
 
 		/* If the IP or UDP checksum was bad, skip the packet... */
 		if (offset < 0) {
Index: usr.sbin/dhcrelay/dhcpd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcrelay/dhcpd.h,v
diff -u -p -r1.24 dhcpd.h
--- usr.sbin/dhcrelay/dhcpd.h	21 May 2024 05:00:48 -0000	1.24
+++ usr.sbin/dhcrelay/dhcpd.h	16 Jan 2025 22:01:55 -0000
@@ -169,7 +169,7 @@ ssize_t assemble_udp_ip_header(unsigned 
 ssize_t decode_hw_header(unsigned char *, size_t, size_t, struct packet_ctx *,
     unsigned int);
 ssize_t decode_udp_ip_header(unsigned char *, size_t, size_t,
-    struct packet_ctx *);
+    struct packet_ctx *, u_int16_t);
 
 /* dhcrelay.c */
 extern int server_fd;
Index: usr.sbin/dhcrelay/packet.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcrelay/packet.c,v
diff -u -p -r1.14 packet.c
--- usr.sbin/dhcrelay/packet.c	5 Apr 2017 14:40:56 -0000	1.14
+++ usr.sbin/dhcrelay/packet.c	16 Jan 2025 22:03:52 -0000
@@ -42,6 +42,7 @@
 
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/mbuf.h>
 
 #include <arpa/inet.h>
 
@@ -214,13 +215,12 @@ decode_hw_header(unsigned char *buf, siz
 
 ssize_t
 decode_udp_ip_header(unsigned char *buf, size_t buflen,
-    size_t offset, struct packet_ctx *pc)
+    size_t offset, struct packet_ctx *pc, u_int16_t csumflags)
 {
 	struct ip *ip;
 	struct udphdr *udp;
 	unsigned char *data;
 	u_int32_t ip_len;
-	u_int32_t sum, usum;
 	static unsigned int ip_packets_seen;
 	static unsigned int ip_packets_bad_checksum;
 	static unsigned int udp_packets_seen;
@@ -240,7 +240,8 @@ decode_udp_ip_header(unsigned char *buf,
 	ip_packets_seen++;
 
 	/* Check the IP header checksum - it should be zero. */
-	if (wrapsum(checksum(buf + offset, ip_len, 0)) != 0) {
+	if ((csumflags & M_IPV4_CSUM_IN_OK) == 0 &&
+	    wrapsum(checksum(buf + offset, ip_len, 0)) != 0) {
 		ip_packets_bad_checksum++;
 		if (ip_packets_seen > 4 && ip_packets_bad_checksum != 0 &&
 		    (ip_packets_seen / ip_packets_bad_checksum) < 2) {
@@ -306,24 +307,26 @@ decode_udp_ip_header(unsigned char *buf,
 	if (len + data != buf + buflen)
 		log_debug("accepting packet with data after udp payload.");
 
-	usum = udp->uh_sum;
-	udp->uh_sum = 0;
-
-	sum = wrapsum(checksum((unsigned char *)udp, sizeof(*udp),
-	    checksum(data, len, checksum((unsigned char *)&ip->ip_src,
-	    2 * sizeof(ip->ip_src),
-	    IPPROTO_UDP + (u_int32_t)ntohs(udp->uh_ulen)))));
-
 	udp_packets_seen++;
-	if (usum && usum != sum) {
-		udp_packets_bad_checksum++;
-		if (udp_packets_seen > 4 && udp_packets_bad_checksum != 0 &&
-		    (udp_packets_seen / udp_packets_bad_checksum) < 2) {
-			log_info("%u bad udp checksums in %u packets",
-			    udp_packets_bad_checksum, udp_packets_seen);
-			udp_packets_seen = udp_packets_bad_checksum = 0;
+
+	if ((csumflags & M_UDP_CSUM_IN_OK) == 0 &&
+	    udp->uh_sum != 0) {
+		udp->uh_sum = wrapsum(checksum((uint8_t *)udp, sizeof(*udp),
+		    checksum(data, len,
+		    checksum((uint8_t *)&ip->ip_src, 2 * sizeof(ip->ip_src),
+		    IPPROTO_UDP + ntohs(udp->uh_ulen)))));
+
+		if (udp->uh_sum != 0) {
+			udp_packets_bad_checksum++;
+			if (udp_packets_seen > 4 &&
+			    udp_packets_bad_checksum != 0 &&
+			    (udp_packets_seen / udp_packets_bad_checksum) < 2) {
+				log_info("%u bad udp checksums in %u packets",
+				    udp_packets_bad_checksum, udp_packets_seen);
+				udp_packets_seen = udp_packets_bad_checksum = 0;
+			}
+			return (-1);
 		}
-		return (-1);
 	}
 
 	ss2sin(&pc->pc_src)->sin_port = udp->uh_sport;
Index: usr.sbin/dhcrelay6/bpf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcrelay6/bpf.c,v
diff -u -p -r1.3 bpf.c
--- usr.sbin/dhcrelay6/bpf.c	18 Mar 2019 00:00:59 -0000	1.3
+++ usr.sbin/dhcrelay6/bpf.c	16 Jan 2025 22:10:31 -0000
@@ -381,7 +381,8 @@ receive_packet(struct interface_info *in
 
 		/* Decode the IP and UDP headers... */
 		offset = decode_udp_ip6_header(interface->rbuf,
-		    interface->rbuf_offset, pc, hdr.bh_caplen);
+		    interface->rbuf_offset, pc, hdr.bh_caplen,
+		    hdr.bh_csumflags);
 
 		/* If the IP or UDP checksum was bad, skip the packet... */
 		if (offset < 0) {
Index: usr.sbin/dhcrelay6/dhcpd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcrelay6/dhcpd.h,v
diff -u -p -r1.2 dhcpd.h
--- usr.sbin/dhcrelay6/dhcpd.h	21 May 2024 05:00:48 -0000	1.2
+++ usr.sbin/dhcrelay6/dhcpd.h	16 Jan 2025 22:08:06 -0000
@@ -164,7 +164,7 @@ void assemble_udp_ip6_header(unsigned ch
     unsigned char *, int);
 ssize_t decode_hw_header(unsigned char *, int, struct packet_ctx *);
 ssize_t decode_udp_ip6_header(unsigned char *, int, struct packet_ctx *,
-    size_t);
+    size_t, u_int16_t);
 
 /* dhcrelay6.c */
 const char *v6addr2str(struct in6_addr *);
Index: usr.sbin/dhcrelay6/packet.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/dhcrelay6/packet.c,v
diff -u -p -r1.1 packet.c
--- usr.sbin/dhcrelay6/packet.c	17 Mar 2017 14:45:16 -0000	1.1
+++ usr.sbin/dhcrelay6/packet.c	16 Jan 2025 22:09:30 -0000
@@ -42,6 +42,7 @@
 
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/mbuf.h>
 
 #include <arpa/inet.h>
 
@@ -160,13 +161,12 @@ decode_hw_header(unsigned char *buf, int
 
 ssize_t
 decode_udp_ip6_header(unsigned char *p, int off, struct packet_ctx *pc,
-   size_t plen)
+   size_t plen, u_int16_t csumflags)
 {
 	struct ip6_hdr		*ip6;
 	struct udphdr		*uh;
 	struct in6_addr		*asrc, *adst;
 	size_t			 ptotal, poff = 0;
-	uint16_t		 ocksum, cksum;
 
 	/* Check the IPv6 header. */
 	if (plen < sizeof(*ip6)) {
@@ -208,23 +208,21 @@ decode_udp_ip6_header(unsigned char *p, 
 	uh = (struct udphdr *)((uint8_t *)ip6 + sizeof(*ip6));
 	ss2sin6(&pc->pc_src)->sin6_port = uh->uh_sport;
 	ss2sin6(&pc->pc_dst)->sin6_port = uh->uh_dport;
-	ocksum = uh->uh_sum;
-	uh->uh_sum = 0;
 	poff += sizeof(*uh);
 
 	/* Validate the packet. */
-	cksum = wrapsum(
-	    checksum((unsigned char *)asrc, sizeof(*asrc),
-	    checksum((unsigned char *)adst, sizeof(*adst),
-	    checksum((unsigned char *)uh, sizeof(*uh),
-	    checksum(p + off + poff, ptotal - sizeof(*uh),
-	    IPPROTO_UDP + ntohs(uh->uh_ulen)))))
-	);
-
-	if (ocksum != cksum) {
-		log_debug("checksum invalid (%#04x != %#04x)",
-		    ocksum, cksum);
-		return -1;
+	if ((csumflags & M_UDP_CSUM_IN_OK) == 0) {
+		uh->uh_sum = wrapsum(
+		    checksum((unsigned char *)asrc, sizeof(*asrc),
+		    checksum((unsigned char *)adst, sizeof(*adst),
+		    checksum((unsigned char *)uh, sizeof(*uh),
+		    checksum(p + off + poff, ptotal - sizeof(*uh),
+		    IPPROTO_UDP + ntohs(uh->uh_ulen))))));
+
+		if (uh->uh_sum != 0) {
+			log_debug("checksum invalid");
+			return -1;
+		}
 	}
 
 	return poff;