Merge pull request #4343 from kb2ma/misc/fix-checksum

Fix UDP/ICMPv6 checksum for a sliced/accumulated payload.

sys/include/net/inet_csum.h

@@ -21,13 +21,15 @@
 #define INET_CSUM_H_
 
 #include <inttypes.h>
+#include <stddef.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /**
- * @brief   Calculates the unnormalized Internet Checksum of @p buf.
+ * @brief   Calculates the unnormalized Internet Checksum of @p buf, where the
+ *          buffer provides a slice of the full checksum domain, calculated in order.
  *
  * @see <a href="https://tools.ietf.org/html/rfc1071">
  *          RFC 1071
@@ -35,6 +37,31 @@ extern "C" {
  *
  * @details The Internet Checksum is not normalized (i. e. its 1's complement
  *          was not taken of the result) to use it for further calculation.
+ *          This function handles padding an odd number of bytes across the full domain.
+ *
+ * @param[in] sum       An initial value for the checksum.
+ * @param[in] buf       A buffer.
+ * @param[in] len       Length of @p buf in byte.
+ * @param[in] accum_len Accumulated length of checksum domain that has already
+ *                      been checksummed.
+ *
+ * @return  The unnormalized Internet Checksum of @p buf.
+ */
+uint16_t inet_csum_slice(uint16_t sum, const uint8_t *buf, uint16_t len, size_t accum_len);
+
+/**
+ * @brief   Calculates the unnormalized Internet Checksum of @p buf, where the
+ *          buffer provides a standalone domain for the checksum.
+ *
+ * @see <a href="https://tools.ietf.org/html/rfc1071">
+ *          RFC 1071
+ *      </a>
+ *
+ * @details The Internet Checksum is not normalized (i. e. its 1's complement
+ *          was not taken of the result) to use it for further calculation.
+ *          This function, rather than inet_csum_slice(), has been used historically
+ *          when we are not concerned with padding for an odd number of bytes.
+ *
  *
  * @param[in] sum       An initial value for the checksum.
  * @param[in] buf       A buffer.
@@ -42,7 +69,9 @@ extern "C" {
  *
  * @return  The unnormalized Internet Checksum of @p buf.
  */
-uint16_t inet_csum(uint16_t sum, const uint8_t *buf, uint16_t len);
+static inline uint16_t inet_csum(uint16_t sum, const uint8_t *buf, uint16_t len) {
+    return inet_csum_slice(sum, buf, len, 0);
+}
 
 #ifdef __cplusplus
 }

sys/net/crosslayer/inet_csum/inet_csum.c

@@ -20,7 +20,7 @@
 #define ENABLE_DEBUG    (0)
 #include "debug.h"
 
-uint16_t inet_csum(uint16_t sum, const uint8_t *buf, uint16_t len)
+uint16_t inet_csum_slice(uint16_t sum, const uint8_t *buf, uint16_t len, size_t accum_len)
 {
     uint32_t csum = sum;
 
@@ -34,14 +34,23 @@ uint16_t inet_csum(uint16_t sum, const uint8_t *buf, uint16_t len)
 #endif
 #endif
 
+    if (len == 0)
+        return csum;
+
+    if (accum_len & 1) {      /* if accumulated length is odd */
+        csum += *buf;         /* add first byte as bottom half of 16-byte word */
+        buf++;
+        len--;
+        accum_len++;
+    }
+
     for (int i = 0; i < (len >> 1); buf += 2, i++) {
         csum += (*buf << 8) + *(buf + 1);   /* group bytes by 16-byte words
                                              * and add them*/
     }
 
-    if (len & 1) {                      /* if len is odd */
+    if ((accum_len + len) & 1)      /* if accumulated length is odd */
         csum += (*buf << 8);        /* add last byte as top half of 16-byte word */
-    }
 
     while (csum >> 16) {
         uint16_t carry = csum >> 16;

sys/net/gnrc/network_layer/icmpv6/gnrc_icmpv6.c

@@ -42,7 +42,7 @@ static inline uint16_t _calc_csum(gnrc_pktsnip_t *hdr,
     uint16_t len = (uint16_t)hdr->size;
 
     while (payload && (payload != hdr)) {
-        csum = inet_csum(csum, payload->data, payload->size);
+        csum = inet_csum_slice(csum, payload->data, payload->size, len);
         len += (uint16_t)payload->size;
         payload = payload->next;
     }

sys/net/gnrc/transport_layer/udp/gnrc_udp.c

@@ -69,7 +69,7 @@ static uint16_t _calc_csum(gnrc_pktsnip_t *hdr, gnrc_pktsnip_t *pseudo_hdr,
 
     /* process the payload */
     while (payload && payload != hdr) {
-        csum = inet_csum(csum, (uint8_t *)(payload->data), payload->size);
+        csum = inet_csum_slice(csum, (uint8_t *)(payload->data), payload->size, len);
         len += (uint16_t)payload->size;
         payload = payload->next;
     }

tests/unittests/tests-inet_csum/tests-inet_csum.c

@@ -121,6 +121,50 @@ static void test_inet_csum__odd_len(void)
     TEST_ASSERT_EQUAL_INT(0xffff, inet_csum(17 + 39, data, sizeof(data)));
 }
 
+static void test_inet_csum__two_app_snips(void)
+{
+    /* CoAP header with Uri-Path and Content-Format options; odd length */
+    uint8_t data_hdr[] = {
+        0x50, 0x02, 0x00, 0x01, 0xb4, 0x74, 0x65, 0x73,
+        0x74, 0x10, 0xff,
+    };
+    /* Single character payload, 'a' */
+    uint8_t data_pyld[] = {
+        0x61,
+    };
+    uint16_t hdr_sum, pyld_sum, hdr_expected = 0xdcfc;
+
+    /* result unnormalized:
+     * initial sum (0) is arbitrary, and incoming length (0) must be even;
+     * we expect last byte is shifted left for this odd-sized header  */
+    hdr_sum = inet_csum_slice(0, data_hdr, sizeof(data_hdr), 0);
+    TEST_ASSERT_EQUAL_INT(hdr_expected, hdr_sum);
+
+    /* Since header was odd length, we expect the single byte in the payload
+     * snip is not shifted left */
+    pyld_sum = inet_csum_slice(hdr_expected, data_pyld, sizeof(data_pyld), sizeof(data_hdr));
+    TEST_ASSERT_EQUAL_INT(hdr_expected + 0x61, pyld_sum);
+}
+
+static void test_inet_csum__empty_app_buffer(void)
+{
+    /* CoAP header with Uri-Path and Content-Format options; odd length */
+    uint8_t data_hdr[] = {
+        0x50, 0x02, 0x00, 0x01, 0xb4, 0x74, 0x65, 0x73,
+        0x74, 0x10, 0xff,
+    };
+    uint16_t hdr_sum, pyld_sum, hdr_expected = 0xdcfc;
+
+    /* result unnormalized:
+     * explictly using an odd-sized header for the first slice, to setup corner case  */
+    hdr_sum = inet_csum_slice(0, data_hdr, sizeof(data_hdr), 0);
+    TEST_ASSERT_EQUAL_INT(hdr_expected, hdr_sum);
+
+    /* expect an empty buffer simply to reflect the incoming checksum */
+    pyld_sum = inet_csum_slice(hdr_expected, NULL, 0, sizeof(data_hdr));
+    TEST_ASSERT_EQUAL_INT(hdr_expected, pyld_sum);
+}
+
 Test *tests_inet_csum_tests(void)
 {
     EMB_UNIT_TESTFIXTURES(fixtures) {
@@ -130,6 +174,8 @@ Test *tests_inet_csum_tests(void)
         new_TestFixture(test_inet_csum__wraps_more_than_once),
         new_TestFixture(test_inet_csum__calculate_csum),
         new_TestFixture(test_inet_csum__odd_len),
+        new_TestFixture(test_inet_csum__two_app_snips),
+        new_TestFixture(test_inet_csum__empty_app_buffer),
     };
 
     EMB_UNIT_TESTCALLER(inet_csum_tests, NULL, NULL, fixtures);