nat: fixed input validation

Ticket: VPP-1887
Type: fix

Change-Id: I341ac7b455926a106d736f4de6771aae655db82e
Signed-off-by: Filip Varga <fivarga@cisco.com>

7 files changed, 22 insertions, 21 deletions

diff --git a/src/plugins/nat/in2out.c b/src/plugins/nat/in2out.c
index 8616d90ad2b..0c52e50119d 100644
--- a/src/plugins/nat/in2out.c
+++ b/src/plugins/nat/in2out.c
@@ -417,7 +417,7 @@ static_always_inline
     {
       key0.protocol = NAT_PROTOCOL_ICMP;
       key0.addr = ip0->src_address;
-      key0.port = vnet_buffer (b)->ip.reass.l4_src_port;	// TODO fixme should this be dst port?
+      key0.port = vnet_buffer (b)->ip.reass.l4_src_port;
     }
   else
     {
diff --git a/src/plugins/nat/in2out_ed.c b/src/plugins/nat/in2out_ed.c
index 9b10d9df7f6..54c866be74a 100644
--- a/src/plugins/nat/in2out_ed.c
+++ b/src/plugins/nat/in2out_ed.c
@@ -1295,7 +1295,6 @@ nat44_ed_in2out_slow_path_node_fn_inline (vlib_main_t * vm,
 	      goto trace0;
 	    }
 
-	  // move down
 	  make_ed_kv (&ip0->src_address, &ip0->dst_address,
 		      ip0->protocol, rx_fib_index0,
 		      vnet_buffer (b0)->ip.reass.l4_src_port,
diff --git a/src/plugins/nat/nat.c b/src/plugins/nat/nat.c
index eb8f62339c4..55664564e0b 100644
--- a/src/plugins/nat/nat.c
+++ b/src/plugins/nat/nat.c
@@ -443,7 +443,6 @@ nat44_free_session_data (snat_main_t * sm, snat_session_t * s,
   if (snat_is_unk_proto_session (s))
     return;
 
-  // is this correct ?
   if (!is_ha)
     {
       snat_ipfix_logging_nat44_ses_delete (thread_index,
@@ -471,7 +470,6 @@ nat44_free_session_data (snat_main_t * sm, snat_session_t * s,
   if (snat_is_session_static (s))
     return;
 
-  // should be called for every dynamic session
   snat_free_outside_address_and_port (sm->addresses, thread_index,
 				      &s->out2in);
 }
@@ -2601,7 +2599,6 @@ snat_init (vlib_main_t * vm)
     {
       for (i = 0; i < sm->num_workers; i++)
 	bitmap = clib_bitmap_set (bitmap, i, 1);
-      // sets thread indexes for workes
       snat_set_workers (bitmap);
       clib_bitmap_free (bitmap);
     }
@@ -3976,7 +3973,6 @@ nat44_sessions_clear ()
       nat44_db_init (tsm);
 
       ti = tsm->snat_thread_index;
-      // clear per thread session counters
       vlib_set_simple_counter (&sm->total_users, ti, 0, 0);
       vlib_set_simple_counter (&sm->total_sessions, ti, 0, 0);
     }
@@ -4012,7 +4008,6 @@ snat_config (vlib_main_t * vm, unformat_input_t * input)
   u8 static_mapping_only = 0;
   u8 static_mapping_connection_tracking = 0;
 
-  // configurable timeouts
   u32 udp_timeout = SNAT_UDP_TIMEOUT;
   u32 icmp_timeout = SNAT_ICMP_TIMEOUT;
   u32 tcp_transitory_timeout = SNAT_TCP_TRANSITORY_TIMEOUT;
@@ -4072,8 +4067,6 @@ snat_config (vlib_main_t * vm, unformat_input_t * input)
 	;
       else if (unformat (input, "out2in dpo"))
 	sm->out2in_dpo = 1;
-      //else if (unformat (input, "dslite ce"))
-      //dslite_set_ce (dm, 1);
       else if (unformat (input, "endpoint-dependent"))
 	sm->endpoint_dependent = 1;
       else
@@ -4556,8 +4549,6 @@ VLIB_REGISTER_NODE (nat_default_node) = {
   .next_nodes = {
     [NAT_NEXT_DROP] = "error-drop",
     [NAT_NEXT_ICMP_ERROR] = "ip4-icmp-error",
-    //[NAT_NEXT_IN2OUT_PRE] = "nat-pre-in2out",
-    //[NAT_NEXT_OUT2IN_PRE] = "nat-pre-out2in",
     [NAT_NEXT_IN2OUT_ED_FAST_PATH] = "nat44-ed-in2out",
     [NAT_NEXT_IN2OUT_ED_SLOW_PATH] = "nat44-ed-in2out-slowpath",
     [NAT_NEXT_IN2OUT_ED_OUTPUT_SLOW_PATH] = "nat44-ed-in2out-output-slowpath",
diff --git a/src/plugins/nat/nat44_cli.c b/src/plugins/nat/nat44_cli.c
index 9e9751d5bc9..68ed0cb7207 100644
--- a/src/plugins/nat/nat44_cli.c
+++ b/src/plugins/nat/nat44_cli.c
@@ -643,7 +643,6 @@ nat44_show_summary_command_fn (vlib_main_t * vm, unformat_input_t * input,
   if (sm->deterministic || !sm->endpoint_dependent)
     return clib_error_return (0, UNSUPPORTED_IN_DET_OR_NON_ED_MODE_STR);
 
-  // print session configuration values
   vlib_cli_output (vm, "max translations: %u", sm->max_translations);
   vlib_cli_output (vm, "max translations per user: %u",
 		   sm->max_translations_per_user);
@@ -1056,9 +1055,19 @@ add_static_mapping_command_fn (vlib_main_t * vm,
       goto done;
     }
 
-  if (!addr_only && !proto_set)
+  if (addr_only)
     {
-      error = clib_error_return (0, "missing protocol");
+      if (proto_set)
+	{
+	  error =
+	    clib_error_return (0,
+			       "address only mapping doesn't support protocol");
+	  goto done;
+	}
+    }
+  else if (!proto_set)
+    {
+      error = clib_error_return (0, "protocol is required");
       goto done;
     }
 
@@ -2540,16 +2549,19 @@ VLIB_CLI_COMMAND (nat44_show_interfaces_command, static) = {
  *  vpp# nat44 add static mapping tcp local 10.0.0.3 6303 external 4.4.4.4 3606
  * If not runnig "static mapping only" NAT plugin mode use before:
  *  vpp# nat44 add address 4.4.4.4
- * To create static mapping between local and external address use:
+ * To create address only static mapping between local and external address use:
  *  vpp# nat44 add static mapping local 10.0.0.3 external 4.4.4.4
+ * To create ICMP static mapping between local and external with ICMP echo
+ * identifier 10 use:
+ *  vpp# nat44 add static mapping icmp local 10.0.0.3 10 external 4.4.4.4 10
  * @cliexend
 ?*/
 VLIB_CLI_COMMAND (add_static_mapping_command, static) = {
   .path = "nat44 add static mapping",
   .function = add_static_mapping_command_fn,
   .short_help =
-    "nat44 add static mapping tcp|udp|icmp local <addr> [<port>] "
-    "external <addr> [<port>] [vrf <table-id>] [twice-nat|self-twice-nat] "
+    "nat44 add static mapping tcp|udp|icmp local <addr> [<port|icmp-echo-id>] "
+    "external <addr> [<port|icmp-echo-id>] [vrf <table-id>] [twice-nat|self-twice-nat] "
     "[out2in-only] [del]",
 };
 
diff --git a/src/plugins/nat/nat_inlines.h b/src/plugins/nat/nat_inlines.h
index 4dad11bed72..7c28919f781 100644
--- a/src/plugins/nat/nat_inlines.h
+++ b/src/plugins/nat/nat_inlines.h
@@ -545,7 +545,7 @@ get_icmp_i2o_ed_key (vlib_buffer_t * b, ip4_header_t * ip0, u32 rx_fib_index,
       proto = IP_PROTOCOL_ICMP;
       l_addr = &ip0->src_address;
       r_addr = &ip0->dst_address;
-      _l_port = vnet_buffer (b)->ip.reass.l4_src_port;	// TODO should this be src or dst?
+      _l_port = vnet_buffer (b)->ip.reass.l4_src_port;
       _r_port = 0;
     }
   else
@@ -613,7 +613,7 @@ get_icmp_o2i_ed_key (vlib_buffer_t * b, ip4_header_t * ip0, u32 rx_fib_index,
       proto = IP_PROTOCOL_ICMP;
       l_addr = &ip0->dst_address;
       r_addr = &ip0->src_address;
-      _l_port = vnet_buffer (b)->ip.reass.l4_src_port;	// TODO should this be src or dst?
+      _l_port = vnet_buffer (b)->ip.reass.l4_src_port;
       _r_port = 0;
     }
   else
diff --git a/src/plugins/nat/out2in.c b/src/plugins/nat/out2in.c
index 94679fb402c..44f7dcf34f6 100644
--- a/src/plugins/nat/out2in.c
+++ b/src/plugins/nat/out2in.c
@@ -280,7 +280,7 @@ static_always_inline
     {
       key0.protocol = NAT_PROTOCOL_ICMP;
       key0.addr = ip0->dst_address;
-      key0.port = vnet_buffer (b)->ip.reass.l4_src_port;	// TODO should this be dst port?
+      key0.port = vnet_buffer (b)->ip.reass.l4_src_port;
     }
   else
     {
diff --git a/src/plugins/nat/out2in_ed.c b/src/plugins/nat/out2in_ed.c
index abd704579aa..5d759fbfad5 100644
--- a/src/plugins/nat/out2in_ed.c
+++ b/src/plugins/nat/out2in_ed.c
@@ -959,7 +959,6 @@ nat44_ed_out2in_slow_path_node_fn_inline (vlib_main_t * vm,
   u32 n_left_from, *from, *to_next, pkts_processed = 0, stats_node_index;
   nat_next_t next_index;
   snat_main_t *sm = &snat_main;
-  // HERE
   f64 now = vlib_time_now (vm);
   u32 thread_index = vm->thread_index;
   snat_main_per_thread_data_t *tsm = &sm->per_thread_data[thread_index];