From: Jan Klemkow Subject: Re: dhcp daemon checksum fix To: Alexander Bluhm Cc: tech@openbsd.org, Markus Friedl Date: Fri, 7 Feb 2025 22:53:59 +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 dhcrelay part. Looks correct and works in my test setup. ok jan@ > 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 > #include > +#include > > #include > > @@ -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;