nat: fixed input validation 10/27410/2
authorFilip Varga <fivarga@cisco.com>
Wed, 3 Jun 2020 13:26:41 +0000 (15:26 +0200)
committerOle Trøan <otroan@employees.org>
Thu, 4 Jun 2020 10:10:51 +0000 (10:10 +0000)
Ticket: VPP-1887
Type: fix

Change-Id: I341ac7b455926a106d736f4de6771aae655db82e
Signed-off-by: Filip Varga <fivarga@cisco.com>
src/plugins/nat/in2out.c
src/plugins/nat/in2out_ed.c
src/plugins/nat/nat.c
src/plugins/nat/nat44_cli.c
src/plugins/nat/nat_inlines.h
src/plugins/nat/out2in.c
src/plugins/nat/out2in_ed.c

index 8616d90..0c52e50 100644 (file)
@@ -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
     {
index 9b10d9d..54c866b 100644 (file)
@@ -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,
index eb8f623..5566456 100644 (file)
@@ -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",
index 9e9751d..68ed0cb 100644 (file)
@@ -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]",
 };
 
index 4dad11b..7c28919 100644 (file)
@@ -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
index 94679fb..44f7dcf 100644 (file)
@@ -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
     {
index abd7045..5d759fb 100644 (file)
@@ -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];