From 8baefad65915defc493f28c6d8ac313b1152c858 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Thu, 10 Sep 2015 13:19:16 +0100 Subject: [PATCH] [tcpip] Avoid generating positive zero for transmitted UDP checksums TCP/IP checksum fields are one's complement values and therefore have two possible representations of zero: positive zero (0x0000) and negative zero (0xffff). In RFC768, UDP over IPv4 exploits this redundancy to repurpose the positive representation of zero (0x0000) to mean "no checksum calculated"; checksums are optional for UDP over IPv4. In RFC2460, checksums are made mandatory for UDP over IPv4. The wording of the RFC is such that the UDP header is mandated to use only the negative representation of zero (0xffff), rather than simply requiring the checksum to be correct but allowing for either representation of zero to be used. In RFC1071, an example algorithm is given for calculating the TCP/IP checksum. This algorithm happens to produce only the positive representation of zero (0x0000); this is an artifact of the way that unsigned arithmetic is used to calculate a signed one's complement sum (and its final negation). A common misconception has developed (exemplified in RFC1624) that this artifact is part of the specification. Many people have assumed that the checksum field should never contain the negative representation of zero (0xffff). A sensible receiver will calculate the checksum over the whole packet and verify that the result is zero (in whichever representation of zero happens to be generated by the receiver's algorithm). Such a receiver will not care which representation of zero happens to be used in the checksum field. However, there are receivers in existence which will verify the received checksum the hard way: by calculating the checksum over the remainder of the packet and comparing the result against the checksum field. If the representation of zero used by the receiver's algorithm does not match the representation of zero used by the transmitter (and so placed in the checksum field), and if the receiver does not explicitly allow for both representations to compare as equal, then the receiver may reject packets with a valid checksum. For UDP, the combined RFCs effectively mandate that we should generate only the negative representation of zero in the checksum field. For IP, TCP and ICMP, the RFCs do not mandate which representation of zero should be used, but the misconceptions which have grown up around RFC1071 and RFC1624 suggest that it would be least surprising to generate only the positive representation of zero in the checksum field. Fix by ensuring that all of our checksum algorithms generate only the positive representation of zero, and explicitly inverting this in the case of transmitted UDP packets. Reported-by: Wissam Shoukair Tested-by: Wissam Shoukair Signed-off-by: Michael Brown --- src/include/ipxe/tcpip.h | 38 ++++++++++++++++++++++++++++++++++++-- src/net/ipv4.c | 5 ++++- src/net/ipv6.c | 2 ++ src/net/udp.c | 1 + src/tests/tcpip_test.c | 19 ++++++++++++++++++- 5 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/include/ipxe/tcpip.h b/src/include/ipxe/tcpip.h index c3528c9c..be4ac838 100644 --- a/src/include/ipxe/tcpip.h +++ b/src/include/ipxe/tcpip.h @@ -19,11 +19,38 @@ struct io_buffer; struct net_device; struct ip_statistics; +/** Positive zero checksum value */ +#define TCPIP_POSITIVE_ZERO_CSUM 0x0000 + +/** Negative zero checksum value */ +#define TCPIP_NEGATIVE_ZERO_CSUM 0xffff + /** Empty checksum value * - * This is the TCP/IP checksum over a zero-length block of data. + * All of our TCP/IP checksum algorithms will return only the positive + * representation of zero (0x0000) for a zero checksum over non-zero + * input data. This property arises since the end-around carry used + * to mimic one's complement addition using unsigned arithmetic + * prevents the running total from ever returning to 0x0000. The + * running total will therefore use only the negative representation + * of zero (0xffff). Since the return value is the one's complement + * negation of the running total (calculated by simply bit-inverting + * the running total), the return value will therefore use only the + * positive representation of zero (0x0000). + * + * It is a very common misconception (found in many places such as + * RFC1624) that this is a property guaranteed by the underlying + * mathematics. It is not; the choice of which zero representation is + * used is merely an artifact of the software implementation of the + * checksum algorithm. + * + * For consistency, we choose to use the positive representation of + * zero (0x0000) for the checksum of a zero-length block of data. + * This ensures that all of our TCP/IP checksum algorithms will return + * only the positive representation of zero (0x0000) for a zero + * checksum (regardless of the input data). */ -#define TCPIP_EMPTY_CSUM 0xffff +#define TCPIP_EMPTY_CSUM TCPIP_POSITIVE_ZERO_CSUM /** TCP/IP address flags */ enum tcpip_st_flags { @@ -88,6 +115,13 @@ struct tcpip_protocol { int ( * rx ) ( struct io_buffer *iobuf, struct net_device *netdev, struct sockaddr_tcpip *st_src, struct sockaddr_tcpip *st_dest, uint16_t pshdr_csum ); + /** Preferred zero checksum value + * + * The checksum is a one's complement value: zero may be + * represented by either positive zero (0x0000) or negative + * zero (0xffff). + */ + uint16_t zero_csum; /** * Transport-layer protocol number * diff --git a/src/net/ipv4.c b/src/net/ipv4.c index 7959cf35..8eb04a65 100644 --- a/src/net/ipv4.c +++ b/src/net/ipv4.c @@ -358,8 +358,11 @@ static int ipv4_tx ( struct io_buffer *iobuf, ( ( netdev->rx_stats.good & 0xf ) << 0 ) ); /* Fix up checksums */ - if ( trans_csum ) + if ( trans_csum ) { *trans_csum = ipv4_pshdr_chksum ( iobuf, *trans_csum ); + if ( ! *trans_csum ) + *trans_csum = tcpip_protocol->zero_csum; + } iphdr->chksum = tcpip_chksum ( iphdr, sizeof ( *iphdr ) ); /* Print IP4 header for debugging */ diff --git a/src/net/ipv6.c b/src/net/ipv6.c index 012ba592..bbc00d33 100644 --- a/src/net/ipv6.c +++ b/src/net/ipv6.c @@ -522,6 +522,8 @@ static int ipv6_tx ( struct io_buffer *iobuf, *trans_csum = ipv6_pshdr_chksum ( iphdr, len, tcpip_protocol->tcpip_proto, *trans_csum ); + if ( ! *trans_csum ) + *trans_csum = tcpip_protocol->zero_csum; } /* Print IPv6 header for debugging */ diff --git a/src/net/udp.c b/src/net/udp.c index 0f7dfb24..1fbc12d4 100644 --- a/src/net/udp.c +++ b/src/net/udp.c @@ -328,6 +328,7 @@ static int udp_rx ( struct io_buffer *iobuf, struct tcpip_protocol udp_protocol __tcpip_protocol = { .name = "UDP", .rx = udp_rx, + .zero_csum = TCPIP_NEGATIVE_ZERO_CSUM, .tcpip_proto = IP_UDP, }; diff --git a/src/tests/tcpip_test.c b/src/tests/tcpip_test.c index 759f886b..fac0ec26 100644 --- a/src/tests/tcpip_test.c +++ b/src/tests/tcpip_test.c @@ -94,6 +94,12 @@ TCPIP_TEST ( one_byte, DATA ( 0xeb ) ); /** Double byte */ TCPIP_TEST ( two_bytes, DATA ( 0xba, 0xbe ) ); +/** Positive zero data */ +TCPIP_TEST ( positive_zero, DATA ( 0x00, 0x00 ) ); + +/** Negative zero data */ +TCPIP_TEST ( negative_zero, DATA ( 0xff, 0xff ) ); + /** Final wrap-around carry (big-endian) */ TCPIP_TEST ( final_carry_big, DATA ( 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 ) ); @@ -126,9 +132,17 @@ TCPIP_RANDOM_TEST ( partial, 0xcafebabe, 121, 5 ); * * This is a reference implementation taken from RFC1071 (and modified * to fix compilation without warnings under gcc). + * + * The initial value of the one's complement @c sum is changed from + * positive zero (0x0000) to negative zero (0xffff). This ensures + * that the return value will always use the positive representation + * of zero (0x0000). Without this change, the return value would use + * negative zero (0xffff) if the input data is zero length (or all + * zeros) but positive zero (0x0000) for any other data which sums to + * zero. */ static uint16_t rfc_tcpip_chksum ( const void *data, size_t len ) { - unsigned long sum = 0; + unsigned long sum = 0xffff; while ( len > 1 ) { sum += *( ( uint16_t * ) data ); @@ -142,6 +156,7 @@ static uint16_t rfc_tcpip_chksum ( const void *data, size_t len ) { while ( sum >> 16 ) sum = ( ( sum & 0xffff ) + ( sum >> 16 ) ); + assert ( sum != 0x0000 ); return ~sum; } @@ -227,6 +242,8 @@ static void tcpip_test_exec ( void ) { tcpip_ok ( &empty ); tcpip_ok ( &one_byte ); tcpip_ok ( &two_bytes ); + tcpip_ok ( &positive_zero ); + tcpip_ok ( &negative_zero ); tcpip_ok ( &final_carry_big ); tcpip_ok ( &final_carry_little ); tcpip_random_ok ( &random_aligned );