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: Fri, 7 Feb 2025 22:40:05 +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 reviewd the dhcpleased part.  Works and looks correct.

ok jan@

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