Download raw body.
dhcp daemon checksum fix
On Tue, Feb 04, 2025 at 10:32:12PM GMT, Alexander Bluhm wrote:
> 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
I reviewed and tested the dhcpd part. Looks correct and works in
offloaded and non-offloaded cases.
ok jan@
> 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));
dhcp daemon checksum fix