NAT: fix maximum out of order fragments (VPP-1399)

All fragments should be dropped when max_frag is 1 and 2 non-initial fragments are received before first fragment.

Change-Id: Id0c968f45629698e347e8226c5926f27b48b82d6
Signed-off-by: Matus Fabian <matfabia@cisco.com>

7 files changed, 46 insertions, 14 deletions

diff --git a/src/plugins/nat/in2out.c b/src/plugins/nat/in2out.c
index 0fe36338397..22a346855c6 100755
--- a/src/plugins/nat/in2out.c
+++ b/src/plugins/nat/in2out.c
@@ -2218,7 +2218,7 @@ nat44_in2out_reass_node_fn (vlib_main_t * vm,
             {
               if (PREDICT_FALSE (reass0->sess_index == (u32) ~0))
                 {
-                  if (nat_ip4_reass_add_fragment (reass0, bi0))
+                  if (nat_ip4_reass_add_fragment (reass0, bi0, &fragments_to_drop))
                     {
                       b0->error = node->errors[SNAT_IN2OUT_ERROR_MAX_FRAG];
                       nat_log_notice ("maximum fragments per reassembly exceeded");
diff --git a/src/plugins/nat/nat64_in2out.c b/src/plugins/nat/nat64_in2out.c
index 718c69d7bdf..ddbf5850c7c 100644
--- a/src/plugins/nat/nat64_in2out.c
+++ b/src/plugins/nat/nat64_in2out.c
@@ -1424,7 +1424,8 @@ nat64_in2out_reass_node_fn (vlib_main_t * vm,
 	      ctx0.first_frag = 0;
 	      if (PREDICT_FALSE (reass0->sess_index == (u32) ~ 0))
 		{
-		  if (nat_ip6_reass_add_fragment (reass0, bi0))
+		  if (nat_ip6_reass_add_fragment
+		      (reass0, bi0, &fragments_to_drop))
 		    {
 		      b0->error = node->errors[NAT64_IN2OUT_ERROR_MAX_FRAG];
 		      next0 = NAT64_IN2OUT_NEXT_DROP;
diff --git a/src/plugins/nat/nat64_out2in.c b/src/plugins/nat/nat64_out2in.c
index d4b0c3987f9..54a7e82c7c1 100644
--- a/src/plugins/nat/nat64_out2in.c
+++ b/src/plugins/nat/nat64_out2in.c
@@ -785,7 +785,8 @@ nat64_out2in_reass_node_fn (vlib_main_t * vm, vlib_node_runtime_t * node,
 
 	      if (PREDICT_FALSE (reass0->sess_index == (u32) ~ 0))
 		{
-		  if (nat_ip4_reass_add_fragment (reass0, bi0))
+		  if (nat_ip4_reass_add_fragment
+		      (reass0, bi0, &fragments_to_drop))
 		    {
 		      b0->error = node->errors[NAT64_OUT2IN_ERROR_MAX_FRAG];
 		      next0 = NAT64_OUT2IN_NEXT_DROP;
diff --git a/src/plugins/nat/nat_reass.c b/src/plugins/nat/nat_reass.c
index eb1b4925521..8fd370de2fc 100755
--- a/src/plugins/nat/nat_reass.c
+++ b/src/plugins/nat/nat_reass.c
@@ -249,6 +249,13 @@ nat_ip4_reass_find_or_create (ip4_address_t src, ip4_address_t dst,
 			      srm->ip4_reass_head_index,
 			      reass->lru_list_index);
 	}
+
+      if (reass->flags && NAT_REASS_FLAG_MAX_FRAG_DROP)
+	{
+	  reass = 0;
+	  goto unlock;
+	}
+
       goto unlock;
     }
 
@@ -326,7 +333,8 @@ unlock:
 }
 
 int
-nat_ip4_reass_add_fragment (nat_reass_ip4_t * reass, u32 bi)
+nat_ip4_reass_add_fragment (nat_reass_ip4_t * reass, u32 bi,
+			    u32 ** bi_to_drop)
 {
   nat_reass_main_t *srm = &nat_reass_main;
   dlist_elt_t *elt;
@@ -336,6 +344,8 @@ nat_ip4_reass_add_fragment (nat_reass_ip4_t * reass, u32 bi)
     {
       nat_ipfix_logging_max_fragments_ip4 (srm->ip4_max_frag,
 					   &reass->key.src);
+      reass->flags |= NAT_REASS_FLAG_MAX_FRAG_DROP;
+      nat_ip4_reass_get_frags_inline (reass, bi_to_drop);
       return -1;
     }
 
@@ -446,6 +456,13 @@ nat_ip6_reass_find_or_create (ip6_address_t src, ip6_address_t dst,
 			      srm->ip6_reass_head_index,
 			      reass->lru_list_index);
 	}
+
+      if (reass->flags && NAT_REASS_FLAG_MAX_FRAG_DROP)
+	{
+	  reass = 0;
+	  goto unlock;
+	}
+
       goto unlock;
     }
 
@@ -522,7 +539,8 @@ unlock:
 }
 
 int
-nat_ip6_reass_add_fragment (nat_reass_ip6_t * reass, u32 bi)
+nat_ip6_reass_add_fragment (nat_reass_ip6_t * reass, u32 bi,
+			    u32 ** bi_to_drop)
 {
   nat_reass_main_t *srm = &nat_reass_main;
   dlist_elt_t *elt;
@@ -532,6 +550,8 @@ nat_ip6_reass_add_fragment (nat_reass_ip6_t * reass, u32 bi)
     {
       nat_ipfix_logging_max_fragments_ip6 (srm->ip6_max_frag,
 					   &reass->key.src);
+      reass->flags |= NAT_REASS_FLAG_MAX_FRAG_DROP;
+      nat_ip6_reass_get_frags_inline (reass, bi_to_drop);
       return -1;
     }
 
diff --git a/src/plugins/nat/nat_reass.h b/src/plugins/nat/nat_reass.h
index 4a9137e3255..5b18d1bd7b9 100644
--- a/src/plugins/nat/nat_reass.h
+++ b/src/plugins/nat/nat_reass.h
@@ -30,6 +30,8 @@
 #define NAT_MAX_FRAG_DEFAULT 5
 #define NAT_REASS_HT_LOAD_FACTOR (0.75)
 
+#define NAT_REASS_FLAG_MAX_FRAG_DROP 1
+
 typedef struct
 {
   union
@@ -57,6 +59,7 @@ typedef CLIB_PACKED(struct
   f64 last_heard;
   u32 frags_per_reass_list_head_index;
   u8 frag_n;
+  u8 flags;
 }) nat_reass_ip4_t;
 /* *INDENT-ON* */
 
@@ -86,6 +89,7 @@ typedef CLIB_PACKED(struct
   f64 last_heard;
   u32 frags_per_reass_list_head_index;
   u8 frag_n;
+  u8 flags;
 }) nat_reass_ip6_t;
 /* *INDENT-ON* */
 
@@ -222,10 +226,12 @@ nat_reass_ip4_t *nat_ip4_reass_find_or_create (ip4_address_t src,
  *
  * @param reass Reassembly data.
  * @param bi Buffer index.
+ * @param bi_to_drop Fragments to drop.
  *
  * @returns 0 on success, non-zero value otherwise.
  */
-int nat_ip4_reass_add_fragment (nat_reass_ip4_t * reass, u32 bi);
+int nat_ip4_reass_add_fragment (nat_reass_ip4_t * reass, u32 bi,
+				u32 ** bi_to_drop);
 
 /**
  * @brief Get cached fragments.
@@ -271,10 +277,12 @@ nat_reass_ip6_t *nat_ip6_reass_find_or_create (ip6_address_t src,
  *
  * @param reass Reassembly data.
  * @param bi Buffer index.
+ * @param bi_to_drop Fragments to drop.
  *
  * @returns 0 on success, non-zero value otherwise.
  */
-int nat_ip6_reass_add_fragment (nat_reass_ip6_t * reass, u32 bi);
+int nat_ip6_reass_add_fragment (nat_reass_ip6_t * reass, u32 bi,
+				u32 ** bi_to_drop);
 
 /**
  * @brief Get cached fragments.
diff --git a/src/plugins/nat/out2in.c b/src/plugins/nat/out2in.c
index 5029300dcdb..3d4e9228e67 100755
--- a/src/plugins/nat/out2in.c
+++ b/src/plugins/nat/out2in.c
@@ -1430,7 +1430,7 @@ nat44_out2in_reass_node_fn (vlib_main_t * vm,
             {
               if (PREDICT_FALSE (reass0->sess_index == (u32) ~0))
                 {
-                  if (nat_ip4_reass_add_fragment (reass0, bi0))
+                  if (nat_ip4_reass_add_fragment (reass0, bi0, &fragments_to_drop))
                     {
                       b0->error = node->errors[SNAT_OUT2IN_ERROR_MAX_FRAG];
                       nat_log_notice ("maximum fragments per reassembly exceeded");
diff --git a/test/test_nat.py b/test/test_nat.py
index 47e779b9026..5baadd89863 100644
--- a/test/test_nat.py
+++ b/test/test_nat.py
@@ -3143,7 +3143,7 @@ class TestNAT44(MethodHolder):
         self.vapi.nat44_interface_add_del_feature(self.pg0.sw_if_index)
         self.vapi.nat44_interface_add_del_feature(self.pg1.sw_if_index,
                                                   is_inside=0)
-        self.vapi.nat_set_reass(max_frag=0)
+        self.vapi.nat_set_reass(max_frag=1)
         self.vapi.set_ipfix_exporter(collector_address=self.pg3.remote_ip4n,
                                      src_address=self.pg3.local_ip4n,
                                      path_mtu=512,
@@ -3158,7 +3158,8 @@ class TestNAT44(MethodHolder):
                                        self.tcp_port_in,
                                        20,
                                        data)
-        self.pg0.add_stream(pkts[-1])
+        pkts.reverse()
+        self.pg0.add_stream(pkts)
         self.pg_enable_capture(self.pg_interfaces)
         self.pg_start()
         self.pg1.assert_nothing_captured()
@@ -3181,7 +3182,7 @@ class TestNAT44(MethodHolder):
         for p in capture:
             if p.haslayer(Data):
                 data = ipfix.decode_data_set(p.getlayer(Set))
-                self.verify_ipfix_max_fragments_ip4(data, 0,
+                self.verify_ipfix_max_fragments_ip4(data, 1,
                                                     self.pg0.remote_ip4n)
 
     def test_multiple_outside_vrf(self):
@@ -6972,7 +6973,7 @@ class TestNAT64(MethodHolder):
                                                 self.nat_addr_n)
         self.vapi.nat64_add_del_interface(self.pg0.sw_if_index)
         self.vapi.nat64_add_del_interface(self.pg1.sw_if_index, is_inside=0)
-        self.vapi.nat_set_reass(max_frag=0, is_ip6=1)
+        self.vapi.nat_set_reass(max_frag=1, is_ip6=1)
         self.vapi.set_ipfix_exporter(collector_address=self.pg3.remote_ip4n,
                                      src_address=self.pg3.local_ip4n,
                                      path_mtu=512,
@@ -6983,7 +6984,8 @@ class TestNAT64(MethodHolder):
         data = 'a' * 200
         pkts = self.create_stream_frag_ip6(self.pg0, self.pg1.remote_ip4,
                                            self.tcp_port_in, 20, data)
-        self.pg0.add_stream(pkts[-1])
+        pkts.reverse()
+        self.pg0.add_stream(pkts)
         self.pg_enable_capture(self.pg_interfaces)
         self.pg_start()
         self.pg1.assert_nothing_captured()
@@ -7006,7 +7008,7 @@ class TestNAT64(MethodHolder):
         for p in capture:
             if p.haslayer(Data):
                 data = ipfix.decode_data_set(p.getlayer(Set))
-                self.verify_ipfix_max_fragments_ip6(data, 0,
+                self.verify_ipfix_max_fragments_ip6(data, 1,
                                                     self.pg0.remote_ip6n)
 
     def test_ipfix_bib_ses(self):