icmpv6: fix checksum verification if fcs present

Calculate the length of the ICMPv6 packet from decoded information
instead of off the wire length. This will provide the correct
length if trailing data like an FCS is present.

Fixes issue:
https://redmine.openinfosecfoundation.org/issues/1849

diff --git a/src/decode-icmpv6.c b/src/decode-icmpv6.c
index 26a48b7439afc5ad2eea503369ba74d73c1ff551..75da935bfac89c8ca8a8136f1fab57f1b386ae5b 100644 (file)
--- a/src/decode-icmpv6.c
+++ b/src/decode-icmpv6.c
@@ -1603,6 +1603,61 @@ end:
     return retval;
 }
 
+/**
+ * \test Test for valid ICMPv6 checksum when the FCS is still attached.
+ *
+ * Tests that the packet is decoded with sufficient info to verify the
+ * checksum even if the packet has some trailing data like an ethernet
+ * FCS.
+ */
+static int ICMPV6CalculateValidChecksumWithFCS(void)
+{
+    /* IPV6/ICMPv6 packet with ethernet header.
+     * - IPv6 payload length: 36
+     */
+    uint8_t raw_ipv6[] = {
+        0x33, 0x33, 0x00, 0x00, 0x00, 0x16, 0x00, 0x50,
+        0x56, 0xa6, 0x6a, 0x7d, 0x86, 0xdd, 0x60, 0x00,
+        0x00, 0x00, 0x00, 0x24, 0x00, 0x01, 0xfe, 0x80,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf5, 0x09,
+        0xad, 0x44, 0x49, 0x38, 0x5f, 0xa9, 0xff, 0x02,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x16, 0x3a, 0x00,
+        0x05, 0x02, 0x00, 0x00, 0x01, 0x00, 0x8f, 0x00,
+        0x24, 0xe0, 0x00, 0x00, 0x00, 0x01, 0x03, 0x00, /* Checksum: 0x24e0. */
+        0x00, 0x00, 0xff, 0x02, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0xfb, 0x1f, 0x34, 0xf6, 0xa4
+    };
+    uint16_t csum = *(((uint16_t *)(raw_ipv6 + 64)));
+
+    Packet *p = SCMalloc(SIZE_OF_PACKET);
+    FAIL_IF_NULL(p);
+    IPV6Hdr ip6h;
+    ThreadVars tv;
+    DecodeThreadVars dtv;
+
+    memset(&tv, 0, sizeof(ThreadVars));
+    memset(p, 0, SIZE_OF_PACKET);
+    memset(&dtv, 0, sizeof(DecodeThreadVars));
+    memset(&ip6h, 0, sizeof(IPV6Hdr));
+
+    FlowInitConfig(FLOW_QUIET);
+    DecodeIPV6(&tv, &dtv, p, raw_ipv6 + 14, sizeof(raw_ipv6) - 14, NULL);
+    FAIL_IF_NULL(p->icmpv6h);
+
+    uint16_t icmpv6_len = IPV6_GET_RAW_PLEN(p->ip6h) -
+        ((uint8_t *)p->icmpv6h - (uint8_t *)p->ip6h - IPV6_HEADER_LEN);
+    FAIL_IF(icmpv6_len != 28);
+    FAIL_IF(ICMPV6CalculateChecksum(p->ip6h->s_ip6_addrs,
+            (uint16_t *)p->icmpv6h, icmpv6_len) != csum);
+
+    PACKET_RECYCLE(p);
+    FlowShutdown();
+    SCFree(p);
+    PASS;
+}
+
 #endif /* UNITTESTS */
 /**
  * \brief Registers ICMPV6 unit tests
@@ -1654,6 +1709,8 @@ void DecodeICMPV6RegisterTests(void)
     UtRegisterTest("ICMPV6RedirectTestKnownCode", ICMPV6RedirectTestKnownCode);
     UtRegisterTest("ICMPV6RedirectTestUnknownCode",
                    ICMPV6RedirectTestUnknownCode);
+    UtRegisterTest("ICMPV6CalculateValidChecksumWithFCS",
+                   ICMPV6CalculateValidChecksumWithFCS);
 #endif /* UNITTESTS */
 }
 /**

diff --git a/src/detect-csum.c b/src/detect-csum.c
index 37d2b22163cc2e123a5478d028d0426c41dbda1c..4a338f5409b79ae7249be2b8bb8c7cac6e8065be 100644 (file)
--- a/src/detect-csum.c
+++ b/src/detect-csum.c
@@ -814,10 +814,13 @@ int DetectICMPV6CsumMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx,
         return cd->valid;
     }
 
-    if (p->level4_comp_csum == -1)
+    if (p->level4_comp_csum == -1) {
+        uint16_t len = IPV6_GET_RAW_PLEN(p->ip6h) -
+            ((uint8_t *)p->icmpv6h - (uint8_t *)p->ip6h - IPV6_HEADER_LEN);
         p->level4_comp_csum = ICMPV6CalculateChecksum(p->ip6h->s_ip6_addrs,
                                                       (uint16_t *)p->icmpv6h,
-                                                      GET_PKT_LEN(p) - ((uint8_t *)p->icmpv6h - GET_PKT_DATA(p)));
+                                                      len);
+    }
 
     if (p->level4_comp_csum == p->icmpv6h->csum && cd->valid == 1)
         return 1;