Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICMPv6 checksum not recalculated correctly if fragment header is present #271

Closed
KarlFK opened this issue Sep 5, 2018 · 2 comments

Comments

@KarlFK
Copy link

commented Sep 5, 2018

It appears that the fragment header (which in this case indicates that there are no more fragments) causes the checksum recalculation to go badly. Packet capture attached.
ICMPv6_checksum_error.zip

@pstavirs

This comment has been minimized.

Copy link
Owner

commented Sep 10, 2018

I am able to confirm that the presence of IPv6 extension header(s) causes the checksum to be wrong.

As per RFC 8200 Section 8.1 -

      o  If the IPv6 packet contains a Routing header, the Destination
         Address used in the pseudo-header is that of the final
         destination.  At the originating node, that address will be in
         the last element of the Routing header; at the recipient(s),
         that address will be in the Destination Address field of the
         IPv6 header.
      o  The Next Header value in the pseudo-header identifies the
         upper-layer protocol (e.g., 6 for TCP or 17 for UDP).  It will
         differ from the Next Header value in the IPv6 header if there
         are extension headers between the IPv6 header and the upper-
         layer header.
      o  The Upper-Layer Packet Length in the pseudo-header is the
         length of the upper-layer header and data (e.g., TCP header
         plus TCP data).  Some upper-layer protocols carry their own
         length information (e.g., the Length field in the UDP header);
         for such protocols, that is the length used in the pseudo-
         header.  Other protocols (such as TCP) do not carry their own
         length information, in which case the length used in the
         pseudo-header is the Payload Length from the IPv6 header, minus
         the length of any extension headers present between the IPv6
         header and the upper-layer header.
. . .

   The IPv6 version of ICMP [RFC4443] includes the above pseudo-header
   in its checksum computation; this is a change from the IPv4 version
   of ICMP, which does not include a pseudo-header in its checksum.  The
   reason for the change is to protect ICMP from misdelivery or
   corruption of those fields of the IPv6 header on which it depends,
   which, unlike IPv4, are not covered by an internet-layer checksum.
   The Next Header field in the pseudo-header for ICMP contains the
   value 58, which identifies the IPv6 version of ICMP.

I have a fix for this. But I need to be careful that the fix doesn't cause regressions, so I'm doing some extra testing on the same before committing.

@pstavirs pstavirs closed this in 488a2ea Sep 13, 2018

@pstavirs

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2018

Test results after the fix:

pytest -rapP --tb=no cksumtest.py
============================= test session starts =============================
platform win32 -- Python 2.7.13, pytest-3.0.4, py-1.5.2, pluggy-0.4.0
plugins: repeat-0.4.1
collected 72 items

cksumtest.py ........ssss....................ssss........ssss........................
=========================== short test summary info ===========================
SKIP [2] cksumtest.py:173: invalid combo - mld/ip4o4
SKIP [2] cksumtest.py:173: invalid combo - icmpv6/ip4o4
SKIP [2] cksumtest.py:173: invalid combo - icmpv6/ip
SKIP [2] cksumtest.py:173: invalid combo - mld/ip4o6
SKIP [2] cksumtest.py:173: invalid combo - icmpv6/ip4o6
SKIP [2] cksumtest.py:173: invalid combo - mld/ip
PASSED cksumtest.py::test_cksum[ip-icmp-False]
PASSED cksumtest.py::test_cksum[ip-icmp-True]
PASSED cksumtest.py::test_cksum[ip-igmp-False]
PASSED cksumtest.py::test_cksum[ip-igmp-True]
PASSED cksumtest.py::test_cksum[ip-tcp-False]
PASSED cksumtest.py::test_cksum[ip-tcp-True]
PASSED cksumtest.py::test_cksum[ip-udp-False]
PASSED cksumtest.py::test_cksum[ip-udp-True]
PASSED cksumtest.py::test_cksum[ipv6-icmp-False]
PASSED cksumtest.py::test_cksum[ipv6-icmp-True]
PASSED cksumtest.py::test_cksum[ipv6-igmp-False]
PASSED cksumtest.py::test_cksum[ipv6-igmp-True]
PASSED cksumtest.py::test_cksum[ipv6-tcp-False]
PASSED cksumtest.py::test_cksum[ipv6-tcp-True]
PASSED cksumtest.py::test_cksum[ipv6-udp-False]
PASSED cksumtest.py::test_cksum[ipv6-udp-True]
PASSED cksumtest.py::test_cksum[ipv6-icmpv6-False]
PASSED cksumtest.py::test_cksum[ipv6-icmpv6-True]
PASSED cksumtest.py::test_cksum[ipv6-mld-False]
PASSED cksumtest.py::test_cksum[ipv6-mld-True]
PASSED cksumtest.py::test_cksum[ip4o4-icmp-False]
PASSED cksumtest.py::test_cksum[ip4o4-icmp-True]
PASSED cksumtest.py::test_cksum[ip4o4-igmp-False]
PASSED cksumtest.py::test_cksum[ip4o4-igmp-True]
PASSED cksumtest.py::test_cksum[ip4o4-tcp-False]
PASSED cksumtest.py::test_cksum[ip4o4-tcp-True]
PASSED cksumtest.py::test_cksum[ip4o4-udp-False]
PASSED cksumtest.py::test_cksum[ip4o4-udp-True]
PASSED cksumtest.py::test_cksum[ip4o6-icmp-False]
PASSED cksumtest.py::test_cksum[ip4o6-icmp-True]
PASSED cksumtest.py::test_cksum[ip4o6-igmp-False]
PASSED cksumtest.py::test_cksum[ip4o6-igmp-True]
PASSED cksumtest.py::test_cksum[ip4o6-tcp-False]
PASSED cksumtest.py::test_cksum[ip4o6-tcp-True]
PASSED cksumtest.py::test_cksum[ip4o6-udp-False]
PASSED cksumtest.py::test_cksum[ip4o6-udp-True]
PASSED cksumtest.py::test_cksum[ip6o4-icmp-False]
PASSED cksumtest.py::test_cksum[ip6o4-icmp-True]
PASSED cksumtest.py::test_cksum[ip6o4-igmp-False]
PASSED cksumtest.py::test_cksum[ip6o4-igmp-True]
PASSED cksumtest.py::test_cksum[ip6o4-tcp-False]
PASSED cksumtest.py::test_cksum[ip6o4-tcp-True]
PASSED cksumtest.py::test_cksum[ip6o4-udp-False]
PASSED cksumtest.py::test_cksum[ip6o4-udp-True]
PASSED cksumtest.py::test_cksum[ip6o4-icmpv6-False]
PASSED cksumtest.py::test_cksum[ip6o4-icmpv6-True]
PASSED cksumtest.py::test_cksum[ip6o4-mld-False]
PASSED cksumtest.py::test_cksum[ip6o4-mld-True]
PASSED cksumtest.py::test_cksum[ip6o6-icmp-False]
PASSED cksumtest.py::test_cksum[ip6o6-icmp-True]
PASSED cksumtest.py::test_cksum[ip6o6-igmp-False]
PASSED cksumtest.py::test_cksum[ip6o6-igmp-True]
PASSED cksumtest.py::test_cksum[ip6o6-tcp-False]
PASSED cksumtest.py::test_cksum[ip6o6-tcp-True]
PASSED cksumtest.py::test_cksum[ip6o6-udp-False]
PASSED cksumtest.py::test_cksum[ip6o6-udp-True]
PASSED cksumtest.py::test_cksum[ip6o6-icmpv6-False]
PASSED cksumtest.py::test_cksum[ip6o6-icmpv6-True]
PASSED cksumtest.py::test_cksum[ip6o6-mld-False]
PASSED cksumtest.py::test_cksum[ip6o6-mld-True]

=================== 60 passed, 12 skipped in 208.45 seconds ===================

Test results with Ostinato 0.9:

pytest -rapP --tb=no cksumtest.py
============================= test session starts =============================
platform win32 -- Python 2.7.13, pytest-3.0.4, py-1.5.2, pluggy-0.4.0
plugins: repeat-0.4.1
collected 72 items

cksumtest.py ........ssss.....F.F.F.F........ssss........ssss.....F.F.F.F.....F.F.F.F
=========================== short test summary info ===========================
FAIL cksumtest.py::test_cksum[ipv6-tcp-True]
FAIL cksumtest.py::test_cksum[ipv6-udp-True]
FAIL cksumtest.py::test_cksum[ipv6-icmpv6-True]
FAIL cksumtest.py::test_cksum[ipv6-mld-True]
FAIL cksumtest.py::test_cksum[ip6o4-tcp-True]
FAIL cksumtest.py::test_cksum[ip6o4-udp-True]
FAIL cksumtest.py::test_cksum[ip6o4-icmpv6-True]
FAIL cksumtest.py::test_cksum[ip6o4-mld-True]
FAIL cksumtest.py::test_cksum[ip6o6-tcp-True]
FAIL cksumtest.py::test_cksum[ip6o6-udp-True]
FAIL cksumtest.py::test_cksum[ip6o6-icmpv6-True]
FAIL cksumtest.py::test_cksum[ip6o6-mld-True]
SKIP [2] cksumtest.py:173: invalid combo - mld/ip4o4
SKIP [2] cksumtest.py:173: invalid combo - icmpv6/ip4o4
SKIP [2] cksumtest.py:173: invalid combo - icmpv6/ip
SKIP [2] cksumtest.py:173: invalid combo - mld/ip4o6
SKIP [2] cksumtest.py:173: invalid combo - icmpv6/ip4o6
SKIP [2] cksumtest.py:173: invalid combo - mld/ip
PASSED cksumtest.py::test_cksum[ip-icmp-False]
PASSED cksumtest.py::test_cksum[ip-icmp-True]
PASSED cksumtest.py::test_cksum[ip-igmp-False]
PASSED cksumtest.py::test_cksum[ip-igmp-True]
PASSED cksumtest.py::test_cksum[ip-tcp-False]
PASSED cksumtest.py::test_cksum[ip-tcp-True]
PASSED cksumtest.py::test_cksum[ip-udp-False]
PASSED cksumtest.py::test_cksum[ip-udp-True]
PASSED cksumtest.py::test_cksum[ipv6-icmp-False]
PASSED cksumtest.py::test_cksum[ipv6-icmp-True]
PASSED cksumtest.py::test_cksum[ipv6-igmp-False]
PASSED cksumtest.py::test_cksum[ipv6-igmp-True]
PASSED cksumtest.py::test_cksum[ipv6-tcp-False]
PASSED cksumtest.py::test_cksum[ipv6-udp-False]
PASSED cksumtest.py::test_cksum[ipv6-icmpv6-False]
PASSED cksumtest.py::test_cksum[ipv6-mld-False]
PASSED cksumtest.py::test_cksum[ip4o4-icmp-False]
PASSED cksumtest.py::test_cksum[ip4o4-icmp-True]
PASSED cksumtest.py::test_cksum[ip4o4-igmp-False]
PASSED cksumtest.py::test_cksum[ip4o4-igmp-True]
PASSED cksumtest.py::test_cksum[ip4o4-tcp-False]
PASSED cksumtest.py::test_cksum[ip4o4-tcp-True]
PASSED cksumtest.py::test_cksum[ip4o4-udp-False]
PASSED cksumtest.py::test_cksum[ip4o4-udp-True]
PASSED cksumtest.py::test_cksum[ip4o6-icmp-False]
PASSED cksumtest.py::test_cksum[ip4o6-icmp-True]
PASSED cksumtest.py::test_cksum[ip4o6-igmp-False]
PASSED cksumtest.py::test_cksum[ip4o6-igmp-True]
PASSED cksumtest.py::test_cksum[ip4o6-tcp-False]
PASSED cksumtest.py::test_cksum[ip4o6-tcp-True]
PASSED cksumtest.py::test_cksum[ip4o6-udp-False]
PASSED cksumtest.py::test_cksum[ip4o6-udp-True]
PASSED cksumtest.py::test_cksum[ip6o4-icmp-False]
PASSED cksumtest.py::test_cksum[ip6o4-icmp-True]
PASSED cksumtest.py::test_cksum[ip6o4-igmp-False]
PASSED cksumtest.py::test_cksum[ip6o4-igmp-True]
PASSED cksumtest.py::test_cksum[ip6o4-tcp-False]
PASSED cksumtest.py::test_cksum[ip6o4-udp-False]
PASSED cksumtest.py::test_cksum[ip6o4-icmpv6-False]
PASSED cksumtest.py::test_cksum[ip6o4-mld-False]
PASSED cksumtest.py::test_cksum[ip6o6-icmp-False]
PASSED cksumtest.py::test_cksum[ip6o6-icmp-True]
PASSED cksumtest.py::test_cksum[ip6o6-igmp-False]
PASSED cksumtest.py::test_cksum[ip6o6-igmp-True]
PASSED cksumtest.py::test_cksum[ip6o6-tcp-False]
PASSED cksumtest.py::test_cksum[ip6o6-udp-False]
PASSED cksumtest.py::test_cksum[ip6o6-icmpv6-False]
PASSED cksumtest.py::test_cksum[ip6o6-mld-False]

============= 12 failed, 48 passed, 12 skipped in 162.18 seconds ==============

Legend: True/False in the combination indicates the presence of IPv4 options or IPv6 Ext Headers

pstavirs added a commit that referenced this issue Sep 17, 2018
Use streamIndex in PseudoIpCksum calculation
This bug was introduced while fixing #271
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.