Support ping from outside network in 1:1 NAT (VPP-695)

Change-Id: Iec8fd4c2bd26874bd8bda82172af797e9b92592c
Signed-off-by: Juraj Sloboda <jsloboda@cisco.com>

5 files changed, 106 insertions, 37 deletions

diff --git a/src/plugins/snat/in2out.c b/src/plugins/snat/in2out.c
index 8825158aee1..0d5ce4201cd 100644
--- a/src/plugins/snat/in2out.c
+++ b/src/plugins/snat/in2out.c
@@ -94,7 +94,7 @@ _(UNSUPPORTED_PROTOCOL, "Unsupported protocol")         \
 _(IN2OUT_PACKETS, "Good in2out packets processed")      \
 _(OUT_OF_PORTS, "Out of ports")                         \
 _(BAD_OUTSIDE_FIB, "Outside VRF ID not found")          \
-_(BAD_ICMP_TYPE, "icmp type not echo-request")          \
+_(BAD_ICMP_TYPE, "unsupported ICMP type")               \
 _(NO_TRANSLATION, "No translation")
   
 typedef enum {
@@ -177,7 +177,7 @@ snat_not_translate (snat_main_t * sm, snat_runtime_t * rt, u32 sw_if_index0,
   if (clib_bihash_search_8_8 (&sm->out2in, &kv0, &value0))
     {
       /* or is static mappings */
-      if (!snat_static_mapping_match(sm, key0, &sm0, 1))
+      if (!snat_static_mapping_match(sm, key0, &sm0, 1, 0))
         return 0;
     }
   else
@@ -333,7 +333,7 @@ static u32 slow_path (snat_main_t *sm, vlib_buffer_t *b0,
       u8 static_mapping = 1;
 
       /* First try to match static mapping by local address and port */
-      if (snat_static_mapping_match (sm, *key0, &key1, 0))
+      if (snat_static_mapping_match (sm, *key0, &key1, 0, 0))
         {
           static_mapping = 0;
           /* Try to create dynamic translation */
@@ -432,8 +432,6 @@ snat_in2out_error_t icmp_get_key(ip4_header_t *ip0,
 
   if (!icmp_is_error_message (icmp0))
     {
-      if (PREDICT_FALSE(icmp0->type != ICMP4_echo_request))
-        return SNAT_IN2OUT_ERROR_BAD_ICMP_TYPE;
       key0.protocol = SNAT_PROTOCOL_ICMP;
       key0.addr = ip0->src_address;
       key0.port = echo0->identifier;
@@ -520,8 +518,9 @@ u32 icmp_match_in2out_slow(snat_main_t *sm, vlib_node_runtime_t *node,
           goto out;
         }
 
-      if (icmp_is_error_message (icmp0))
+      if (PREDICT_FALSE(icmp0->type != ICMP4_echo_request))
         {
+          b0->error = node->errors[SNAT_IN2OUT_ERROR_BAD_ICMP_TYPE];
           next0 = SNAT_IN2OUT_NEXT_DROP;
           goto out;
         }
@@ -533,8 +532,19 @@ u32 icmp_match_in2out_slow(snat_main_t *sm, vlib_node_runtime_t *node,
         goto out;
     }
   else
-    s0 = pool_elt_at_index (sm->per_thread_data[cpu_index].sessions,
-                            value0.value);
+    {
+      if (PREDICT_FALSE(icmp0->type != ICMP4_echo_request &&
+                        icmp0->type != ICMP4_echo_reply &&
+                        !icmp_is_error_message (icmp0)))
+        {
+          b0->error = node->errors[SNAT_IN2OUT_ERROR_BAD_ICMP_TYPE];
+          next0 = SNAT_IN2OUT_NEXT_DROP;
+          goto out;
+        }
+
+      s0 = pool_elt_at_index (sm->per_thread_data[cpu_index].sessions,
+                              value0.value);
+    }
 
 out:
   *p_key = key0;
@@ -572,6 +582,7 @@ u32 icmp_match_in2out_fast(snat_main_t *sm, vlib_node_runtime_t *node,
   snat_session_key_t key0;
   snat_session_key_t sm0;
   u8 dont_translate = 0;
+  u8 is_addr_only;
   u32 next0 = ~0;
   int err;
 
@@ -590,7 +601,7 @@ u32 icmp_match_in2out_fast(snat_main_t *sm, vlib_node_runtime_t *node,
     }
   key0.fib_index = rx_fib_index0;
 
-  if (snat_static_mapping_match(sm, key0, &sm0, 0))
+  if (snat_static_mapping_match(sm, key0, &sm0, 0, &is_addr_only))
     {
       if (PREDICT_FALSE(snat_not_translate(sm, rt, sw_if_index0, ip0,
           IP_PROTOCOL_ICMP, rx_fib_index0)))
@@ -610,6 +621,15 @@ u32 icmp_match_in2out_fast(snat_main_t *sm, vlib_node_runtime_t *node,
       goto out;
     }
 
+  if (PREDICT_FALSE(icmp0->type != ICMP4_echo_request &&
+                    (icmp0->type != ICMP4_echo_reply || !is_addr_only) &&
+                    !icmp_is_error_message (icmp0)))
+    {
+      b0->error = node->errors[SNAT_IN2OUT_ERROR_BAD_ICMP_TYPE];
+      next0 = SNAT_IN2OUT_NEXT_DROP;
+      goto out;
+    }
+
 out:
   *p_value = sm0;
 out2:
@@ -778,7 +798,7 @@ snat_hairpinning (snat_main_t *sm,
   if (clib_bihash_search_8_8 (&sm->out2in, &kv0, &value0))
     {
       /* or static mappings */
-      if (!snat_static_mapping_match(sm, key0, &sm0, 1))
+      if (!snat_static_mapping_match(sm, key0, &sm0, 1, 0))
         {
           new_dst_addr0 = sm0.addr.as_u32;
           new_dst_port0 = sm0.port;
@@ -2166,7 +2186,6 @@ snat_in2out_fast_static_map_fn (vlib_main_t * vm,
   snat_in2out_next_t next_index;
   u32 pkts_processed = 0;
   snat_main_t * sm = &snat_main;
-  snat_runtime_t * rt = (snat_runtime_t *)node->runtime_data;
   u32 stats_node_index;
 
   stats_node_index = snat_in2out_fast_node.index;
@@ -2225,10 +2244,6 @@ snat_in2out_fast_static_map_fn (vlib_main_t * vm,
 
           if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
             {
-              if (PREDICT_FALSE(snat_not_translate(sm, rt, sw_if_index0, ip0,
-                  proto0, rx_fib_index0)))
-                goto trace0;
-
               next0 = icmp_in2out(sm, b0, ip0, icmp0, sw_if_index0,
                                   rx_fib_index0, node, next0, ~0, 0);
               goto trace0;
@@ -2238,7 +2253,7 @@ snat_in2out_fast_static_map_fn (vlib_main_t * vm,
           key0.port = udp0->src_port;
           key0.fib_index = rx_fib_index0;
 
-          if (snat_static_mapping_match(sm, key0, &sm0, 0))
+          if (snat_static_mapping_match(sm, key0, &sm0, 0, 0))
             {
               b0->error = node->errors[SNAT_IN2OUT_ERROR_NO_TRANSLATION];
               next0= SNAT_IN2OUT_NEXT_DROP;
diff --git a/src/plugins/snat/out2in.c b/src/plugins/snat/out2in.c
index f0555a1b2d3..7de85eb3cb5 100644
--- a/src/plugins/snat/out2in.c
+++ b/src/plugins/snat/out2in.c
@@ -86,7 +86,7 @@ vlib_node_registration_t snat_det_out2in_node;
 #define foreach_snat_out2in_error                       \
 _(UNSUPPORTED_PROTOCOL, "Unsupported protocol")         \
 _(OUT2IN_PACKETS, "Good out2in packets processed")      \
-_(BAD_ICMP_TYPE, "icmp type not echo-reply")            \
+_(BAD_ICMP_TYPE, "unsupported ICMP type")               \
 _(NO_TRANSLATION, "No translation")
   
 typedef enum {
@@ -324,6 +324,7 @@ u32 icmp_match_out2in_slow(snat_main_t *sm, vlib_node_runtime_t *node,
   snat_session_t *s0 = 0;
   u8 dont_translate = 0;
   clib_bihash_kv_8_8_t kv0, value0;
+  u8 is_addr_only;
   u32 next0 = ~0;
   int err;
 
@@ -347,7 +348,7 @@ u32 icmp_match_out2in_slow(snat_main_t *sm, vlib_node_runtime_t *node,
     {
       /* Try to match static mapping by external address and port,
          destination address and port in packet */
-      if (snat_static_mapping_match(sm, key0, &sm0, 1))
+      if (snat_static_mapping_match(sm, key0, &sm0, 1, &is_addr_only))
         {
           /* Don't NAT packet aimed at the intfc address */
           if (is_interface_addr(sm, node, sw_if_index0,
@@ -361,8 +362,10 @@ u32 icmp_match_out2in_slow(snat_main_t *sm, vlib_node_runtime_t *node,
           goto out;
         }
 
-      if (icmp_is_error_message (icmp0))
+      if (PREDICT_FALSE(icmp0->type != ICMP4_echo_reply &&
+                        (icmp0->type != ICMP4_echo_request || !is_addr_only)))
         {
+          b0->error = node->errors[SNAT_OUT2IN_ERROR_BAD_ICMP_TYPE];
           next0 = SNAT_OUT2IN_NEXT_DROP;
           goto out;
         }
@@ -378,8 +381,19 @@ u32 icmp_match_out2in_slow(snat_main_t *sm, vlib_node_runtime_t *node,
         }
     }
   else
-    s0 = pool_elt_at_index (sm->per_thread_data[cpu_index].sessions,
-                            value0.value);
+    {
+      if (PREDICT_FALSE(icmp0->type != ICMP4_echo_reply &&
+                        icmp0->type != ICMP4_echo_request &&
+                        !icmp_is_error_message (icmp0)))
+        {
+          b0->error = node->errors[SNAT_OUT2IN_ERROR_BAD_ICMP_TYPE];
+          next0 = SNAT_OUT2IN_NEXT_DROP;
+          goto out;
+        }
+
+      s0 = pool_elt_at_index (sm->per_thread_data[cpu_index].sessions,
+                              value0.value);
+    }
 
 out:
   *p_key = key0;
@@ -410,15 +424,18 @@ u32 icmp_match_out2in_fast(snat_main_t *sm, vlib_node_runtime_t *node,
                            u8 *p_dont_translate, void *d)
 {
   ip4_header_t *ip0;
+  icmp46_header_t *icmp0;
   u32 sw_if_index0;
   u32 rx_fib_index0;
   snat_session_key_t key0;
   snat_session_key_t sm0;
   u8 dont_translate = 0;
+  u8 is_addr_only;
   u32 next0 = ~0;
   int err;
 
   ip0 = vlib_buffer_get_current (b0);
+  icmp0 = (icmp46_header_t *) ip4_next_header (ip0);
   sw_if_index0 = vnet_buffer(b0)->sw_if_index[VLIB_RX];
   rx_fib_index0 = ip4_fib_table_get_index_for_sw_if_index (sw_if_index0);
 
@@ -431,7 +448,7 @@ u32 icmp_match_out2in_fast(snat_main_t *sm, vlib_node_runtime_t *node,
     }
   key0.fib_index = rx_fib_index0;
 
-  if (snat_static_mapping_match(sm, key0, &sm0, 1))
+  if (snat_static_mapping_match(sm, key0, &sm0, 1, &is_addr_only))
     {
       /* Don't NAT packet aimed at the intfc address */
       if (is_interface_addr(sm, node, sw_if_index0, ip0->dst_address.as_u32))
@@ -444,6 +461,15 @@ u32 icmp_match_out2in_fast(snat_main_t *sm, vlib_node_runtime_t *node,
       goto out;
     }
 
+  if (PREDICT_FALSE(icmp0->type != ICMP4_echo_reply &&
+                    (icmp0->type != ICMP4_echo_request || !is_addr_only) &&
+                    !icmp_is_error_message (icmp0)))
+    {
+      b0->error = node->errors[SNAT_OUT2IN_ERROR_BAD_ICMP_TYPE];
+      next0 = SNAT_OUT2IN_NEXT_DROP;
+      goto out;
+    }
+
 out:
   *p_value = sm0;
 out2:
@@ -484,14 +510,6 @@ static inline u32 icmp_out2in (snat_main_t *sm,
   if (next0 == SNAT_OUT2IN_NEXT_DROP || dont_translate)
     goto out;
 
-  if (PREDICT_FALSE(icmp0->type != ICMP4_echo_reply &&
-                    !icmp_is_error_message (icmp0)))
-    {
-      b0->error = node->errors[SNAT_OUT2IN_ERROR_BAD_ICMP_TYPE];
-      next0 = SNAT_OUT2IN_NEXT_DROP;
-      goto out;
-    }
-
   sum0 = ip_incremental_checksum (0, icmp0,
                                   ntohs(ip0->length) - ip4_header_bytes (ip0));
   checksum0 = ~ip_csum_fold (sum0);
@@ -727,7 +745,7 @@ snat_out2in_node_fn (vlib_main_t * vm,
             {
               /* Try to match static mapping by external address and port,
                  destination address and port in packet */
-              if (snat_static_mapping_match(sm, key0, &sm0, 1))
+              if (snat_static_mapping_match(sm, key0, &sm0, 1, 0))
                 {
                   b0->error = node->errors[SNAT_OUT2IN_ERROR_NO_TRANSLATION];
                   /* 
@@ -862,7 +880,7 @@ snat_out2in_node_fn (vlib_main_t * vm,
             {
               /* Try to match static mapping by external address and port,
                  destination address and port in packet */
-              if (snat_static_mapping_match(sm, key1, &sm1, 1))
+              if (snat_static_mapping_match(sm, key1, &sm1, 1, 0))
                 {
                   b1->error = node->errors[SNAT_OUT2IN_ERROR_NO_TRANSLATION];
                   /* 
@@ -1031,7 +1049,7 @@ snat_out2in_node_fn (vlib_main_t * vm,
             {
               /* Try to match static mapping by external address and port,
                  destination address and port in packet */
-              if (snat_static_mapping_match(sm, key0, &sm0, 1))
+              if (snat_static_mapping_match(sm, key0, &sm0, 1, 0))
                 {
                   b0->error = node->errors[SNAT_OUT2IN_ERROR_NO_TRANSLATION];
                   /* 
@@ -1816,7 +1834,7 @@ snat_out2in_fast_node_fn (vlib_main_t * vm,
           key0.port = udp0->dst_port;
           key0.fib_index = rx_fib_index0;
 
-          if (snat_static_mapping_match(sm, key0, &sm0, 1))
+          if (snat_static_mapping_match(sm, key0, &sm0, 1, 0))
             {
               b0->error = node->errors[SNAT_OUT2IN_ERROR_NO_TRANSLATION];
               goto trace00;
diff --git a/src/plugins/snat/snat.c b/src/plugins/snat/snat.c
index 70b6a6e285e..b635bee9894 100644
--- a/src/plugins/snat/snat.c
+++ b/src/plugins/snat/snat.c
@@ -1838,13 +1838,15 @@ void snat_free_outside_address_and_port (snat_main_t * sm,
  * @param mapping     External or local address and port of the matched mapping.
  * @param by_external If 0 match by local address otherwise match by external
  *                    address.
+ * @param is_addr_only If matched mapping is address only
  *
  * @returns 0 if match found otherwise 1.
  */
 int snat_static_mapping_match (snat_main_t * sm,
                                snat_session_key_t match,
                                snat_session_key_t * mapping,
-                               u8 by_external)
+                               u8 by_external,
+                               u8 *is_addr_only)
 {
   clib_bihash_kv_8_8_t kv, value;
   snat_static_mapping_t *m;
@@ -1890,6 +1892,9 @@ int snat_static_mapping_match (snat_main_t * sm,
       mapping->fib_index = sm->outside_fib_index;
     }
 
+  if (PREDICT_FALSE(is_addr_only != 0))
+    *is_addr_only = m->addr_only;
+
   return 0;
 }
 
diff --git a/src/plugins/snat/snat.h b/src/plugins/snat/snat.h
index 017825c0904..95e72be3a50 100644
--- a/src/plugins/snat/snat.h
+++ b/src/plugins/snat/snat.h
@@ -342,7 +342,8 @@ int snat_alloc_outside_address_and_port (snat_main_t * sm,
 int snat_static_mapping_match (snat_main_t * sm,
                                snat_session_key_t match,
                                snat_session_key_t * mapping,
-                               u8 by_external);
+                               u8 by_external,
+                               u8 *is_addr_only);
 
 void snat_add_del_addr_to_fib (ip4_address_t * addr,
                                u8 p_len,
diff --git a/test/test_snat.py b/test/test_snat.py
index e1dd576e991..c07d8c4621b 100644
--- a/test/test_snat.py
+++ b/test/test_snat.py
@@ -548,7 +548,7 @@ class TestSNAT(VppTestCase):
         self.verify_capture_out_with_icmp_errors(capture)
 
     def test_ping_out_interface_from_outside(self):
-        """ Ping SNAT out interface from outside """
+        """ Ping SNAT out interface from outside network """
 
         self.snat_add_address(self.snat_addr)
         self.vapi.snat_interface_add_del_feature(self.pg0.sw_if_index)
@@ -575,6 +575,36 @@ class TestSNAT(VppTestCase):
                                   "(outside network):", packet))
             raise
 
+    def test_ping_internal_host_from_outside(self):
+        """ Ping internal host from outside network """
+
+        self.snat_add_static_mapping(self.pg0.remote_ip4, self.snat_addr)
+        self.vapi.snat_interface_add_del_feature(self.pg0.sw_if_index)
+        self.vapi.snat_interface_add_del_feature(self.pg1.sw_if_index,
+                                                 is_inside=0)
+
+        # out2in
+        pkt = (Ether(dst=self.pg1.local_mac, src=self.pg1.remote_mac) /
+               IP(src=self.pg1.remote_ip4, dst=self.snat_addr, ttl=64) /
+               ICMP(id=self.icmp_id_out, type='echo-request'))
+        self.pg1.add_stream(pkt)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        capture = self.pg0.get_capture(1)
+        self.verify_capture_in(capture, self.pg0, packet_num=1)
+        self.assert_equal(capture[0][IP].proto, IP_PROTOS.icmp)
+
+        # in2out
+        pkt = (Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac) /
+               IP(src=self.pg0.remote_ip4, dst=self.pg1.remote_ip4, ttl=64) /
+               ICMP(id=self.icmp_id_in, type='echo-reply'))
+        self.pg0.add_stream(pkt)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        capture = self.pg1.get_capture(1)
+        self.verify_capture_out(capture, same_port=True, packet_num=1)
+        self.assert_equal(capture[0][IP].proto, IP_PROTOS.icmp)
+
     def test_static_in(self):
         """ SNAT 1:1 NAT initialized from inside network """