Feature #901 - VLAN defrag support.

Take VLAN IDs into account when re-assembling fragments.

Prevents fragments that would otherwise match, but on different
VLANs from being reassembled with each other.

diff --git a/src/defrag-hash.c b/src/defrag-hash.c
index 266e24070a17ebf5680b7157ac89e5cfc34c1be6..ffb957bb7ff3700335906ad870de8589b776d320 100644 (file)
--- a/src/defrag-hash.c
+++ b/src/defrag-hash.c
@@ -86,6 +86,8 @@ static void DefragTrackerInit(DefragTracker *dt, Packet *p) {
         dt->id = (int32_t)IPV6_EXTHDR_GET_FH_ID(p);
         dt->af = AF_INET6;
     }
+    dt->vlan_id[0] = p->vlan_id[0];
+    dt->vlan_id[1] = p->vlan_id[1];
     dt->policy = DefragGetOsPolicy(p);
     TAILQ_INIT(&dt->frags);
     (void) DefragTrackerIncrUsecnt(dt);
@@ -318,8 +320,9 @@ typedef struct DefragHashKey4_ {
         struct {
             uint32_t src, dst;
             uint32_t id;
+            uint16_t vlan_id[2];
         };
-        uint32_t u32[3];
+        uint32_t u32[4];
     };
 } DefragHashKey4;
 
@@ -328,8 +331,9 @@ typedef struct DefragHashKey6_ {
         struct {
             uint32_t src[4], dst[4];
             uint32_t id;
+            uint16_t vlan_id[2];
         };
-        uint32_t u32[9];
+        uint32_t u32[10];
     };
 } DefragHashKey6;
 
@@ -340,6 +344,7 @@ typedef struct DefragHashKey6_ {
  *  source address
  *  destination address
  *  id
+ *  vlan_id
  */
 static inline uint32_t DefragHashGetKey(Packet *p) {
     uint32_t key;
@@ -354,8 +359,10 @@ static inline uint32_t DefragHashGetKey(Packet *p) {
             dhk.dst = p->src.addr_data32[0];
         }
         dhk.id = (uint32_t)IPV4_GET_IPID(p);
+        dhk.vlan_id[0] = p->vlan_id[0];
+        dhk.vlan_id[1] = p->vlan_id[1];
 
-        uint32_t hash = hashword(dhk.u32, 3, defrag_config.hash_rand);
+        uint32_t hash = hashword(dhk.u32, 4, defrag_config.hash_rand);
         key = hash % defrag_config.hash_size;
     } else if (p->ip6h != NULL) {
         DefragHashKey6 dhk;
@@ -379,8 +386,10 @@ static inline uint32_t DefragHashGetKey(Packet *p) {
             dhk.dst[3] = p->src.addr_data32[3];
         }
         dhk.id = IPV6_EXTHDR_GET_FH_ID(p);
+        dhk.vlan_id[0] = p->vlan_id[0];
+        dhk.vlan_id[1] = p->vlan_id[1];
 
-        uint32_t hash = hashword(dhk.u32, 9, defrag_config.hash_rand);
+        uint32_t hash = hashword(dhk.u32, 10, defrag_config.hash_rand);
         key = hash % defrag_config.hash_size;
     } else
         key = 0;
@@ -395,7 +404,9 @@ static inline uint32_t DefragHashGetKey(Packet *p) {
        CMP_ADDR(&(d1)->dst_addr, &(d2)->dst)) || \
       (CMP_ADDR(&(d1)->src_addr, &(d2)->dst) && \
        CMP_ADDR(&(d1)->dst_addr, &(d2)->src))) && \
-     (d1)->id == (id))
+     (d1)->id == (id) && \
+     (d1)->vlan_id[0] == (d2)->vlan_id[0] && \
+     (d1)->vlan_id[1] == (d2)->vlan_id[1])
 
 static inline int DefragTrackerCompare(DefragTracker *t, Packet *p) {
     uint32_t id;

diff --git a/src/defrag.c b/src/defrag.c
index 9cc25a8c05222cfc58df592b5ec495adae49bb43..496997c0e2e26e57c295b7857c71442f0548e125 100644 (file)
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -2156,6 +2156,100 @@ end:
     return ret;
 }
 
+/**
+ * Test that fragments in different VLANs that would otherwise be
+ * re-assembled, are not re-assembled.  Just use simple in-order
+ * fragments.
+ */
+static int
+DefragVlanTest(void) {
+    Packet *p1 = NULL, *p2 = NULL, *r = NULL;
+    int ret = 0;
+
+    DefragInit();
+
+    p1 = BuildTestPacket(1, 0, 1, 'A', 8);
+    if (p1 == NULL)
+        goto end;
+    p2 = BuildTestPacket(1, 1, 0, 'B', 8);
+    if (p2 == NULL)
+        goto end;
+
+    /* With no VLAN IDs set, packets should re-assemble. */
+    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+        goto end;
+    if ((r = Defrag(NULL, NULL, p2)) == NULL)
+        goto end;
+    SCFree(r);
+
+    /* With mismatched VLANs, packets should not re-assemble. */
+    p1->vlan_id[0] = 1;
+    p2->vlan_id[0] = 2;
+    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+        goto end;
+    if ((r = Defrag(NULL, NULL, p2)) != NULL)
+        goto end;
+
+    /* Pass. */
+    ret = 1;
+
+end:
+    if (p1 != NULL)
+        SCFree(p1);
+    if (p2 != NULL)
+        SCFree(p2);
+    DefragDestroy();
+
+    return ret;
+}
+
+/**
+ * Like DefragVlanTest, but for QinQ, testing the second level VLAN ID.
+ */
+static int
+DefragVlanQinQTest(void) {
+    Packet *p1 = NULL, *p2 = NULL, *r = NULL;
+    int ret = 0;
+
+    DefragInit();
+
+    p1 = BuildTestPacket(1, 0, 1, 'A', 8);
+    if (p1 == NULL)
+        goto end;
+    p2 = BuildTestPacket(1, 1, 0, 'B', 8);
+    if (p2 == NULL)
+        goto end;
+
+    /* With no VLAN IDs set, packets should re-assemble. */
+    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+        goto end;
+    if ((r = Defrag(NULL, NULL, p2)) == NULL)
+        goto end;
+    SCFree(r);
+
+    /* With mismatched VLANs, packets should not re-assemble. */
+    p1->vlan_id[0] = 1;
+    p2->vlan_id[0] = 1;
+    p1->vlan_id[1] = 1;
+    p2->vlan_id[1] = 2;
+    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+        goto end;
+    if ((r = Defrag(NULL, NULL, p2)) != NULL)
+        goto end;
+
+    /* Pass. */
+    ret = 1;
+
+end:
+    if (p1 != NULL)
+        SCFree(p1);
+    if (p2 != NULL)
+        SCFree(p2);
+    DefragDestroy();
+
+    return ret;
+}
+
 #endif /* UNITTESTS */
 
 void
@@ -2199,6 +2293,9 @@ DefragRegisterTests(void)
     UtRegisterTest("IPV6DefragSturgesNovakLastTest",
         IPV6DefragSturgesNovakLastTest, 1);
 
+    UtRegisterTest("DefragVlanTest", DefragVlanTest, 1);
+    UtRegisterTest("DefragVlanQinQTest", DefragVlanQinQTest, 1);
+
     UtRegisterTest("DefragTimeoutTest",
         DefragTimeoutTest, 1);
 #endif /* UNITTESTS */

diff --git a/src/defrag.h b/src/defrag.h
index 6d4b399275f6b02034dbf66452b8c797b47fca79..f9889bea0699c9c1b0e4014d4ad9a7c45de76b89 100644 (file)
--- a/src/defrag.h
+++ b/src/defrag.h
@@ -93,6 +93,8 @@ typedef struct DefragTracker_ {
     SCMutex lock; /**< Mutex for locking list operations on
                            * this tracker. */
 
+    uint16_t vlan_id[2]; /**< VLAN ID tracker applies to. */
+
     uint32_t id; /**< IP ID for this tracker.  32 bits for IPv6, 16
                   * for IPv4. */
 

diff --git a/suricata.yaml.in b/suricata.yaml.in
index 78a3410506dcef12d70e0d0146a5f460bcce381a..7d8fd671bf57655ab77d335c861a016102a54f76 100644 (file)
--- a/suricata.yaml.in
+++ b/suricata.yaml.in
@@ -554,10 +554,10 @@ flow:
   prealloc: 10000
   emergency-recovery: 30
 
-# This option controls the use of vlan ids in the flow hashing. Normally this
-# should be enabled, but in some (broken) setups where both sides of a flow are
-# not tagged with the same vlan tag, we can ignore the vlan id's in the flow
-# hashing.
+# This option controls the use of vlan ids in the flow (and defrag)
+# hashing. Normally this should be enabled, but in some (broken)
+# setups where both sides of a flow are not tagged with the same vlan
+# tag, we can ignore the vlan id's in the flow hashing.
 vlan:
   use-for-tracking: true