From 6e334e3e77bb156a9317a37500077a218a04f7a3 Mon Sep 17 00:00:00 2001 From: Benoît Ganne Date: Mon, 31 Aug 2020 18:59:34 +0200 Subject: ip: fix ip zero checksum verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In one's complement, there are two representations of zero: the all zero and the all one bit values, often referred to as +0 and -0. See RFC 1624 section 3 for more details. This used to be taken care of in ip4_header_checksum(), but it is no longer the case. The check ip->checksum == ip4_header_checksum (ip) is no longer correct in the -0 case. Always use ip4_header_checksum_is_valid() instead (which behaves correctly since 9a79a1ab931c3b5a7ae07d6f0fcfef7c4368a2c4). Type: fix Fixes: e5f0050c7a5d411f96af6401797529d58825e2af Change-Id: Iacc6b60645a834287b085aecb9e3fdb4554cf0cf Signed-off-by: Benoît Ganne --- src/plugins/flowprobe/node.c | 2 +- src/plugins/ioam/udp-ping/udp_ping_export.c | 4 ++-- src/plugins/map/ip4_map.c | 2 +- src/plugins/nat/nat_ipfix_logging.c | 2 +- src/plugins/ping/ping.c | 6 +++--- src/vnet/ip/ip4_format.c | 7 ++++--- src/vnet/ip/ip4_forward.c | 4 ++-- src/vnet/ip/ip4_pg.c | 6 +++--- src/vnet/ipfix-export/flow_report_classify.c | 4 ++-- src/vnet/ipfix-export/ipfix_doc.md | 2 +- 10 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/plugins/flowprobe/node.c b/src/plugins/flowprobe/node.c index 2dd49b3f48f..a81f7a6c45b 100644 --- a/src/plugins/flowprobe/node.c +++ b/src/plugins/flowprobe/node.c @@ -598,7 +598,7 @@ flowprobe_export_send (vlib_main_t * vm, vlib_buffer_t * b0, udp->checksum = 0xffff; } - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); /* Find or allocate a frame */ f = fm->context[which].frames_per_worker[my_cpu_number]; diff --git a/src/plugins/ioam/udp-ping/udp_ping_export.c b/src/plugins/ioam/udp-ping/udp_ping_export.c index d25eb1041dd..3c632c86900 100644 --- a/src/plugins/ioam/udp-ping/udp_ping_export.c +++ b/src/plugins/ioam/udp-ping/udp_ping_export.c @@ -152,7 +152,7 @@ udp_ping_send_flows (flow_report_main_t * frm, flow_report_t * fr, if (udp->checksum == 0) udp->checksum = 0xffff; - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); to_next[0] = bi0; f->n_vectors++; @@ -203,7 +203,7 @@ udp_ping_send_flows (flow_report_main_t * frm, flow_report_t * fr, if (udp->checksum == 0) udp->checksum = 0xffff; - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); to_next[0] = bi0; f->n_vectors++; diff --git a/src/plugins/map/ip4_map.c b/src/plugins/map/ip4_map.c index ea63901bf89..a4889627f0b 100644 --- a/src/plugins/map/ip4_map.c +++ b/src/plugins/map/ip4_map.c @@ -97,7 +97,7 @@ ip4_map_decrement_ttl (ip4_header_t * ip, u8 * error) *error = ttl <= 0 ? IP4_ERROR_TIME_EXPIRED : *error; /* Verify checksum. */ - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); } static u32 diff --git a/src/plugins/nat/nat_ipfix_logging.c b/src/plugins/nat/nat_ipfix_logging.c index 3b75260148f..42252b2eb0c 100644 --- a/src/plugins/nat/nat_ipfix_logging.c +++ b/src/plugins/nat/nat_ipfix_logging.c @@ -567,7 +567,7 @@ snat_ipfix_send (u32 thread_index, flow_report_main_t * frm, udp->checksum = 0xffff; } - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); vlib_put_frame_to_node (vm, ip4_lookup_node.index, f); } diff --git a/src/plugins/ping/ping.c b/src/plugins/ping/ping.c index f56f44ffb26..0ce4f9698f0 100644 --- a/src/plugins/ping/ping.c +++ b/src/plugins/ping/ping.c @@ -474,8 +474,8 @@ ip4_icmp_echo_request (vlib_main_t * vm, ip0->checksum = ip_csum_fold (sum0); ip1->checksum = ip_csum_fold (sum1); - ASSERT (ip0->checksum == ip4_header_checksum (ip0)); - ASSERT (ip1->checksum == ip4_header_checksum (ip1)); + ASSERT (ip4_header_checksum_is_valid (ip0)); + ASSERT (ip4_header_checksum_is_valid (ip1)); p0->flags |= VNET_BUFFER_F_LOCALLY_ORIGINATED; p1->flags |= VNET_BUFFER_F_LOCALLY_ORIGINATED; @@ -531,7 +531,7 @@ ip4_icmp_echo_request (vlib_main_t * vm, ip0->checksum = ip_csum_fold (sum0); - ASSERT (ip0->checksum == ip4_header_checksum (ip0)); + ASSERT (ip4_header_checksum_is_valid (ip0)); p0->flags |= VNET_BUFFER_F_LOCALLY_ORIGINATED; } diff --git a/src/vnet/ip/ip4_format.c b/src/vnet/ip/ip4_format.c index 786a01d396b..c6639b20716 100644 --- a/src/vnet/ip/ip4_format.c +++ b/src/vnet/ip/ip4_format.c @@ -150,9 +150,10 @@ format_ip4_header (u8 * s, va_list * args) /* Check and report invalid checksums. */ { - u16 c = ip4_header_checksum (ip); - if (c != ip->checksum) - s = format (s, " (should be 0x%04x)", clib_net_to_host_u16 (c)); + if (!ip4_header_checksum_is_valid (ip)) + s = + format (s, " (should be 0x%04x)", + clib_net_to_host_u16 (ip4_header_checksum (ip))); } s = format (s, " dscp %U ecn %U", diff --git a/src/vnet/ip/ip4_forward.c b/src/vnet/ip/ip4_forward.c index 595a0a1913c..3bf305303d4 100644 --- a/src/vnet/ip/ip4_forward.c +++ b/src/vnet/ip/ip4_forward.c @@ -2063,7 +2063,7 @@ ip4_ttl_inc (vlib_buffer_t * b, ip4_header_t * ip) ttl += 1; ip->ttl = ttl; - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); } /* Decrement TTL & update checksum. @@ -2104,7 +2104,7 @@ ip4_ttl_and_checksum_check (vlib_buffer_t * b, ip4_header_t * ip, u16 * next, } /* Verify checksum. */ - ASSERT ((ip->checksum == ip4_header_checksum (ip)) || + ASSERT (ip4_header_checksum_is_valid (ip) || (b->flags & VNET_BUFFER_F_OFFLOAD_IP_CKSUM)); } diff --git a/src/vnet/ip/ip4_pg.c b/src/vnet/ip/ip4_pg.c index d89424496f0..2ccd2b45a6f 100644 --- a/src/vnet/ip/ip4_pg.c +++ b/src/vnet/ip/ip4_pg.c @@ -90,8 +90,8 @@ compute_length_and_or_checksum (vlib_main_t * vm, ip0->checksum = ~ip_csum_fold (sum0); ip1->checksum = ~ip_csum_fold (sum1); - ASSERT (ip0->checksum == ip4_header_checksum (ip0)); - ASSERT (ip1->checksum == ip4_header_checksum (ip1)); + ASSERT (ip4_header_checksum_is_valid (ip0)); + ASSERT (ip4_header_checksum_is_valid (ip1)); } } @@ -123,7 +123,7 @@ compute_length_and_or_checksum (vlib_main_t * vm, ip4_partial_header_checksum_x1 (ip0, sum0); ip0->checksum = ~ip_csum_fold (sum0); - ASSERT (ip0->checksum == ip4_header_checksum (ip0)); + ASSERT (ip4_header_checksum_is_valid (ip0)); } } } diff --git a/src/vnet/ipfix-export/flow_report_classify.c b/src/vnet/ipfix-export/flow_report_classify.c index f004e9a0806..58f623db145 100644 --- a/src/vnet/ipfix-export/flow_report_classify.c +++ b/src/vnet/ipfix-export/flow_report_classify.c @@ -321,7 +321,7 @@ ipfix_classify_send_flows (flow_report_main_t * frm, udp->checksum = 0xffff; } - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); to_next[0] = bi0; f->n_vectors++; @@ -376,7 +376,7 @@ flush: udp->checksum = 0xffff; } - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); to_next[0] = bi0; f->n_vectors++; diff --git a/src/vnet/ipfix-export/ipfix_doc.md b/src/vnet/ipfix-export/ipfix_doc.md index 59118f68e11..1c7aad7750d 100644 --- a/src/vnet/ipfix-export/ipfix_doc.md +++ b/src/vnet/ipfix-export/ipfix_doc.md @@ -259,7 +259,7 @@ This function creates the packet header for an ipfix data packet udp->checksum = 0xffff; } - ASSERT (ip->checksum == ip4_header_checksum (ip)); + ASSERT (ip4_header_checksum_is_valid (ip)); vlib_put_frame_to_node (vm, ip4_lookup_node.index, f); } -- cgit 1.2.3-korg