From d417fe2616d7b10cd842187aa160516b35831fc7 Mon Sep 17 00:00:00 2001 From: Klement Sekera Date: Wed, 2 Dec 2020 18:27:24 +0000 Subject: nat: fix incorrect session removal case Add a condition where a TCP session in transitory timeout is kept instead of being erroneously deleted. Type: fix Signed-off-by: Klement Sekera Change-Id: Ic625c8c88cc8864293ebd57b0321505652af9380 --- src/plugins/nat/in2out_ed.c | 13 +++++++------ src/plugins/nat/nat.h | 3 +-- src/plugins/nat/test/test_nat44.py | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/plugins/nat/in2out_ed.c b/src/plugins/nat/in2out_ed.c index 5973d36ba19..d9a45dc1398 100644 --- a/src/plugins/nat/in2out_ed.c +++ b/src/plugins/nat/in2out_ed.c @@ -586,7 +586,7 @@ static_always_inline int nat44_ed_not_translate_output_feature (snat_main_t * sm, ip4_header_t * ip, u16 src_port, u16 dst_port, u32 thread_index, u32 rx_sw_if_index, - u32 tx_sw_if_index) + u32 tx_sw_if_index, f64 now) { clib_bihash_kv_16_8_t kv, value; snat_main_per_thread_data_t *tsm = &sm->per_thread_data[thread_index]; @@ -604,13 +604,12 @@ nat44_ed_not_translate_output_feature (snat_main_t * sm, ip4_header_t * ip, s = pool_elt_at_index (tsm->sessions, ed_value_get_session_index (&value)); - if (nat44_is_ses_closed (s)) + if (nat44_is_ses_closed (s) + && (!s->tcp_closed_timestamp || now >= s->tcp_closed_timestamp)) { nat_free_session_data (sm, s, thread_index, 0); nat_ed_session_delete (sm, s, thread_index, 1); } - else - s->flags |= SNAT_SESSION_FLAG_OUTPUT_FEATURE; return 1; } @@ -658,6 +657,7 @@ icmp_match_in2out_ed (snat_main_t * sm, vlib_node_runtime_t * node, vlib_main_t *vm = vlib_get_main (); snat_main_per_thread_data_t *tsm = &sm->per_thread_data[thread_index]; *dont_translate = 0; + f64 now = vlib_time_now (vm); sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX]; rx_fib_index = ip4_fib_table_get_index_for_sw_if_index (sw_if_index); @@ -679,7 +679,7 @@ icmp_match_in2out_ed (snat_main_t * sm, vlib_node_runtime_t * node, if (PREDICT_FALSE (nat44_ed_not_translate_output_feature (sm, ip, l_port, r_port, thread_index, - sw_if_index, vnet_buffer (b)->sw_if_index[VLIB_TX]))) + sw_if_index, vnet_buffer (b)->sw_if_index[VLIB_TX], now))) { *dont_translate = 1; goto out; @@ -1311,7 +1311,8 @@ nat44_ed_in2out_slow_path_node_fn_inline (vlib_main_t * vm, (nat44_ed_not_translate_output_feature (sm, ip0, vnet_buffer (b0)->ip.reass.l4_src_port, vnet_buffer (b0)->ip.reass.l4_dst_port, thread_index, - sw_if_index0, vnet_buffer (b0)->sw_if_index[VLIB_TX]))) + sw_if_index0, vnet_buffer (b0)->sw_if_index[VLIB_TX], + now))) goto trace0; /* diff --git a/src/plugins/nat/nat.h b/src/plugins/nat/nat.h index 710c86fc8d9..a713f7b51cf 100644 --- a/src/plugins/nat/nat.h +++ b/src/plugins/nat/nat.h @@ -199,8 +199,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 -#define SNAT_SESSION_FLAG_EXACT_ADDRESS 256 +#define SNAT_SESSION_FLAG_EXACT_ADDRESS 128 /* NAT interface flags */ #define NAT_INTERFACE_FLAG_IS_INSIDE 1 diff --git a/src/plugins/nat/test/test_nat44.py b/src/plugins/nat/test/test_nat44.py index de4210fa3ff..c71b706cc34 100644 --- a/src/plugins/nat/test/test_nat44.py +++ b/src/plugins/nat/test/test_nat44.py @@ -5726,6 +5726,14 @@ class TestNAT44EndpointDependent(MethodHolder): def test_tcp_close(self): """ Close TCP session from inside network - output feature """ + old_timeouts = self.vapi.nat_get_timeouts() + new_transitory = 2 + self.vapi.nat_set_timeouts( + udp=old_timeouts.udp, + tcp_established=old_timeouts.tcp_established, + icmp=old_timeouts.icmp, + tcp_transitory=new_transitory) + self.vapi.nat44_forwarding_enable_disable(enable=1) self.nat44_add_address(self.pg1.local_ip4) twice_nat_addr = '10.0.1.3' @@ -5808,9 +5816,34 @@ class TestNAT44EndpointDependent(MethodHolder): self.pg_start() self.pg1.get_capture(1) - sessions = self.vapi.nat44_user_session_dump(self.pg0.remote_ip4, - 0) + # session now in transitory timeout + # try SYN packet out->in - should be dropped + 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() + + self.sleep(new_transitory, "wait for transitory timeout") + self.pg0.assert_nothing_captured(0) + + # session should still exist + sessions = self.vapi.nat44_user_session_dump(self.pg0.remote_ip4, 0) + self.assertEqual(len(sessions) - start_sessnum, 1) + + # send FIN+ACK packet out -> in - will cause session to be wiped + # but won't create a new session + 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() + + sessions = self.vapi.nat44_user_session_dump(self.pg0.remote_ip4, 0) self.assertEqual(len(sessions) - start_sessnum, 0) + self.pg0.assert_nothing_captured(0) def test_tcp_session_close_in(self): """ Close TCP session from inside network """ -- cgit 1.2.3-korg