NAT44: fix bug in TCP close with output-feature interface (VPP-1493)

Change-Id: If8c883d6b1ee58de9a03012d3567ec82211a0225
Signed-off-by: Matus Fabian <matfabia@cisco.com>
(cherry picked from commit 6c01dceea5c612373453db7f1ccda589a2cd782e)

5 files changed, 164 insertions, 4 deletions

diff --git a/src/plugins/nat/in2out_ed.c b/src/plugins/nat/in2out_ed.c
index f9f8d776eb4..8c62949b07d 100644
--- a/src/plugins/nat/in2out_ed.c
+++ b/src/plugins/nat/in2out_ed.c
@@ -37,7 +37,8 @@ _(BAD_ICMP_TYPE, "unsupported ICMP type")               \
 _(MAX_SESSIONS_EXCEEDED, "Maximum sessions exceeded")   \
 _(DROP_FRAGMENT, "Drop fragment")                       \
 _(MAX_REASS, "Maximum reassemblies exceeded")           \
-_(MAX_FRAG, "Maximum fragments per reassembly exceeded")
+_(MAX_FRAG, "Maximum fragments per reassembly exceeded")\
+_(NON_SYN, "non-SYN packet try to create session")
 
 typedef enum
 {
@@ -513,7 +514,19 @@ nat44_ed_not_translate_output_feature (snat_main_t * sm, ip4_header_t * ip,
   make_ed_kv (&kv, &ip->src_address, &ip->dst_address, proto, tx_fib_index,
 	      src_port, dst_port);
   if (!clib_bihash_search_16_8 (&tsm->out2in_ed, &kv, &value))
-    return 1;
+    {
+      s = pool_elt_at_index (tsm->sessions, value.value);
+      if (nat44_is_ses_closed (s))
+	{
+	  nat_log_debug ("TCP close connection %U", format_snat_session,
+			 &sm->per_thread_data[thread_index], s);
+	  nat_free_session_data (sm, s, thread_index);
+	  nat44_delete_session (sm, s, thread_index);
+	}
+      else
+	s->flags |= SNAT_SESSION_FLAG_OUTPUT_FEATURE;
+      return 1;
+    }
 
   /* dst NAT check */
   make_ed_kv (&kv, &ip->dst_address, &ip->src_address, proto, rx_fib_index,
@@ -1021,6 +1034,13 @@ nat44_ed_in2out_node_fn_inline (vlib_main_t * vm,
 			goto trace00;
 		    }
 
+		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
+		    {
+		      b0->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
+		      next0 = NAT_IN2OUT_ED_NEXT_DROP;
+		      goto trace00;
+		    }
+
 		  next0 =
 		    slow_path_ed (sm, b0, rx_fib_index0, &kv0, &s0, node,
 				  next0, thread_index, now);
@@ -1225,6 +1245,13 @@ nat44_ed_in2out_node_fn_inline (vlib_main_t * vm,
 			goto trace01;
 		    }
 
+		  if ((proto1 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp1))
+		    {
+		      b1->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
+		      next1 = NAT_IN2OUT_ED_NEXT_DROP;
+		      goto trace01;
+		    }
+
 		  next1 =
 		    slow_path_ed (sm, b1, rx_fib_index1, &kv1, &s1, node,
 				  next1, thread_index, now);
@@ -1458,6 +1485,13 @@ nat44_ed_in2out_node_fn_inline (vlib_main_t * vm,
 			goto trace0;
 		    }
 
+		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
+		    {
+		      b0->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
+		      next0 = NAT_IN2OUT_ED_NEXT_DROP;
+		      goto trace0;
+		    }
+
 		  next0 =
 		    slow_path_ed (sm, b0, rx_fib_index0, &kv0, &s0, node,
 				  next0, thread_index, now);
@@ -1858,6 +1892,13 @@ nat44_ed_in2out_reass_node_fn_inline (vlib_main_t * vm,
 			}
 		    }
 
+		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
+		    {
+		      b0->error = node->errors[NAT_IN2OUT_ED_ERROR_NON_SYN];
+		      next0 = NAT_IN2OUT_ED_NEXT_DROP;
+		      goto trace0;
+		    }
+
 		  next0 = slow_path_ed (sm, b0, rx_fib_index0, &kv0,
 					&s0, node, next0, thread_index, now);
 
diff --git a/src/plugins/nat/nat.h b/src/plugins/nat/nat.h
index 13467203917..02d4aaef9f6 100644
--- a/src/plugins/nat/nat.h
+++ b/src/plugins/nat/nat.h
@@ -174,6 +174,7 @@ typedef enum
 #define SNAT_SESSION_FLAG_ENDPOINT_DEPENDENT   16
 #define SNAT_SESSION_FLAG_FWD_BYPASS           32
 #define SNAT_SESSION_FLAG_AFFINITY             64
+#define SNAT_SESSION_FLAG_OUTPUT_FEATURE       128
 
 /* NAT interface flags */
 #define NAT_INTERFACE_FLAG_IS_INSIDE 1
@@ -673,6 +674,12 @@ unformat_function_t unformat_snat_protocol;
 */
 #define is_lb_static_mapping(sm) (sm->flags & NAT_STATIC_MAPPING_FLAG_LB)
 
+/** \brief Check if client initiating TCP connection (received SYN from client)
+    @param t TCP header
+    @return 1 if client initiating TCP connection
+*/
+#define tcp_is_init(t) ((t->flags & TCP_FLAG_SYN) && !(t->flags & TCP_FLAG_ACK))
+
 /* logging */
 #define nat_log_err(...) \
   vlib_log(VLIB_LOG_LEVEL_ERR, snat_main.log_class, __VA_ARGS__)
diff --git a/src/plugins/nat/nat_inlines.h b/src/plugins/nat/nat_inlines.h
index 4bdb2cb66d9..730d4400e6a 100644
--- a/src/plugins/nat/nat_inlines.h
+++ b/src/plugins/nat/nat_inlines.h
@@ -215,7 +215,8 @@ nat44_set_tcp_session_state_i2o (snat_main_t * sm, snat_session_t * ses,
       if (clib_net_to_host_u32 (tcp->ack_number) > ses->o2i_fin_seq)
 	ses->state |= NAT44_SES_O2I_FIN_ACK;
     }
-  if (nat44_is_ses_closed (ses))
+  if (nat44_is_ses_closed (ses)
+      && !(ses->flags & SNAT_SESSION_FLAG_OUTPUT_FEATURE))
     {
       nat_log_debug ("TCP close connection %U", format_snat_session,
 		     &sm->per_thread_data[thread_index], ses);
diff --git a/src/plugins/nat/out2in_ed.c b/src/plugins/nat/out2in_ed.c
index b2dbc513df6..b4ae6502e0d 100644
--- a/src/plugins/nat/out2in_ed.c
+++ b/src/plugins/nat/out2in_ed.c
@@ -39,7 +39,8 @@ _(NO_TRANSLATION, "No translation")                     \
 _(MAX_SESSIONS_EXCEEDED, "Maximum sessions exceeded")   \
 _(DROP_FRAGMENT, "Drop fragment")                       \
 _(MAX_REASS, "Maximum reassemblies exceeded")           \
-_(MAX_FRAG, "Maximum fragments per reassembly exceeded")
+_(MAX_FRAG, "Maximum fragments per reassembly exceeded")\
+_(NON_SYN, "non-SYN packet try to create session")
 
 typedef enum
 {
@@ -875,6 +876,13 @@ nat44_ed_out2in_node_fn_inline (vlib_main_t * vm,
 		  if (PREDICT_FALSE (identity_nat0))
 		    goto trace00;
 
+		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
+		    {
+		      b0->error = node->errors[NAT_OUT2IN_ED_ERROR_NON_SYN];
+		      next0 = NAT44_ED_OUT2IN_NEXT_DROP;
+		      goto trace00;
+		    }
+
 		  /* Create session initiated by host from external network */
 		  s0 = create_session_for_static_mapping_ed (sm, b0, l_key0,
 							     e_key0, node,
@@ -1097,6 +1105,13 @@ nat44_ed_out2in_node_fn_inline (vlib_main_t * vm,
 		  if (PREDICT_FALSE (identity_nat1))
 		    goto trace01;
 
+		  if ((proto1 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp1))
+		    {
+		      b1->error = node->errors[NAT_OUT2IN_ED_ERROR_NON_SYN];
+		      next1 = NAT44_ED_OUT2IN_NEXT_DROP;
+		      goto trace01;
+		    }
+
 		  /* Create session initiated by host from external network */
 		  s1 = create_session_for_static_mapping_ed (sm, b1, l_key1,
 							     e_key1, node,
@@ -1353,6 +1368,13 @@ nat44_ed_out2in_node_fn_inline (vlib_main_t * vm,
 		  if (PREDICT_FALSE (identity_nat0))
 		    goto trace0;
 
+		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
+		    {
+		      b0->error = node->errors[NAT_OUT2IN_ED_ERROR_NON_SYN];
+		      next0 = NAT44_ED_OUT2IN_NEXT_DROP;
+		      goto trace0;
+		    }
+
 		  /* Create session initiated by host from external network */
 		  s0 = create_session_for_static_mapping_ed (sm, b0, l_key0,
 							     e_key0, node,
@@ -1702,6 +1724,13 @@ nat44_ed_out2in_reass_node_fn (vlib_main_t * vm,
 		      goto trace0;
 		    }
 
+		  if ((proto0 == SNAT_PROTOCOL_TCP) && !tcp_is_init (tcp0))
+		    {
+		      b0->error = node->errors[NAT_OUT2IN_ED_ERROR_NON_SYN];
+		      next0 = NAT44_ED_OUT2IN_NEXT_DROP;
+		      goto trace0;
+		    }
+
 		  /* Create session initiated by host from external network */
 		  s0 = create_session_for_static_mapping_ed (sm, b0, l_key0,
 							     e_key0, node,
diff --git a/test/test_nat.py b/test/test_nat.py
index e26aa27ddbd..22e8903caf5 100644
--- a/test/test_nat.py
+++ b/test/test_nat.py
@@ -4838,6 +4838,88 @@ class TestNAT44EndpointDependent(MethodHolder):
         adresses = self.vapi.nat44_address_dump()
         self.assertEqual(0, len(adresses))
 
+    def test_tcp_close(self):
+        """ Close TCP session from inside network - output feature """
+        self.vapi.nat44_forwarding_enable_disable(1)
+        self.nat44_add_address(self.pg1.local_ip4)
+        twice_nat_addr = '10.0.1.3'
+        service_ip = '192.168.16.150'
+        self.nat44_add_address(twice_nat_addr, twice_nat=1)
+        self.vapi.nat44_interface_add_del_feature(self.pg0.sw_if_index)
+        self.vapi.nat44_interface_add_del_feature(self.pg0.sw_if_index,
+                                                  is_inside=0)
+        self.vapi.nat44_interface_add_del_output_feature(self.pg1.sw_if_index,
+                                                         is_inside=0)
+        self.nat44_add_static_mapping(self.pg0.remote_ip4,
+                                      service_ip,
+                                      80,
+                                      80,
+                                      proto=IP_PROTOS.tcp,
+                                      out2in_only=1,
+                                      twice_nat=1)
+        sessions = self.vapi.nat44_user_session_dump(self.pg0.remote_ip4n, 0)
+        start_sessnum = len(sessions)
+
+        # SYN packet out->in
+        p = (Ether(src=self.pg1.remote_mac, dst=self.pg1.local_mac) /
+             IP(src=self.pg1.remote_ip4, dst=service_ip) /
+             TCP(sport=33898, dport=80, flags="S"))
+        self.pg1.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        capture = self.pg0.get_capture(1)
+        p = capture[0]
+        tcp_port = p[TCP].sport
+
+        # SYN + ACK packet in->out
+        p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) /
+             IP(src=self.pg0.remote_ip4, dst=twice_nat_addr) /
+             TCP(sport=80, dport=tcp_port, flags="SA"))
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        self.pg1.get_capture(1)
+
+        # ACK packet out->in
+        p = (Ether(src=self.pg1.remote_mac, dst=self.pg1.local_mac) /
+             IP(src=self.pg1.remote_ip4, dst=service_ip) /
+             TCP(sport=33898, dport=80, flags="A"))
+        self.pg1.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        self.pg0.get_capture(1)
+
+        # FIN packet in -> out
+        p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) /
+             IP(src=self.pg0.remote_ip4, dst=twice_nat_addr) /
+             TCP(sport=80, dport=tcp_port, flags="FA", seq=100, ack=300))
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        self.pg1.get_capture(1)
+
+        # FIN+ACK packet out -> in
+        p = (Ether(src=self.pg1.remote_mac, dst=self.pg1.local_mac) /
+             IP(src=self.pg1.remote_ip4, dst=service_ip) /
+             TCP(sport=33898, dport=80, flags="FA", seq=300, ack=101))
+        self.pg1.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        self.pg0.get_capture(1)
+
+        # ACK packet in -> out
+        p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) /
+             IP(src=self.pg0.remote_ip4, dst=twice_nat_addr) /
+             TCP(sport=80, dport=tcp_port, flags="A", seq=101, ack=301))
+        self.pg0.add_stream(p)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        self.pg1.get_capture(1)
+
+        sessions = self.vapi.nat44_user_session_dump(self.pg0.remote_ip4n,
+                                                     0)
+        self.assertEqual(len(sessions) - start_sessnum, 0)
+
     def test_tcp_session_close_in(self):
         """ Close TCP session from inside network """
         self.tcp_port_out = 10505