From: Jan Klemkow <jan@openbsd.org>
Subject: Re: dhcp daemon checksum fix
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org, Markus Friedl <markus@openbsd.org>
Date: Sat, 8 Feb 2025 00:07:38 +0100

On Tue, Feb 04, 2025 at 10:32:12PM GMT, Alexander Bluhm wrote:
> 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?

I tested and reviewed the dhcrelay6 part.  Looks correct and works in my
test setup.

ok jan@

> 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;
>