nat: ICMP handling fixes

If ICMP comes from a router on path, source address must not be
rewritten in o2i path to avoid getting wrong checksum.

Fix ICMP checksum computations.

Type: fix
Signed-off-by: Klement Sekera <ksekera@cisco.com>
Change-Id: I035debccf966d7dbd63c364cb1e43380d641f708

4 files changed, 110 insertions, 24 deletions

diff --git a/src/plugins/nat/nat44-ed/nat44_ed.c b/src/plugins/nat/nat44-ed/nat44_ed.c
index d42d303d95c..1a996cd16f7 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed.c
@@ -3276,7 +3276,7 @@ static_always_inline int nat_6t_flow_icmp_translate (snat_main_t *sm,
 static_always_inline void
 nat_6t_flow_ip4_translate (snat_main_t *sm, vlib_buffer_t *b, ip4_header_t *ip,
 			   nat_6t_flow_t *f, nat_protocol_t proto,
-			   int is_icmp_inner_ip4)
+			   int is_icmp_inner_ip4, int skip_saddr_rewrite)
 {
   udp_header_t *udp = ip4_next_header (ip);
   tcp_header_t *tcp = (tcp_header_t *) udp;
@@ -3319,7 +3319,10 @@ nat_6t_flow_ip4_translate (snat_main_t *sm, vlib_buffer_t *b, ip4_header_t *ip,
     {
       if (!is_icmp_inner_ip4)
 	{ // regular case
-	  ip->src_address = f->rewrite.saddr;
+	  if (!skip_saddr_rewrite)
+	    {
+	      ip->src_address = f->rewrite.saddr;
+	    }
 	  ip->dst_address = f->rewrite.daddr;
 	}
       else
@@ -3329,9 +3332,16 @@ nat_6t_flow_ip4_translate (snat_main_t *sm, vlib_buffer_t *b, ip4_header_t *ip,
 	}
     }
 
-  ip_csum_t ip_sum = ip->checksum;
-  ip_sum = ip_csum_sub_even (ip_sum, f->l3_csum_delta);
-  ip->checksum = ip_csum_fold (ip_sum);
+  if (skip_saddr_rewrite)
+    {
+      ip->checksum = ip4_header_checksum (ip);
+    }
+  else
+    {
+      ip_csum_t ip_sum = ip->checksum;
+      ip_sum = ip_csum_sub_even (ip_sum, f->l3_csum_delta);
+      ip->checksum = ip_csum_fold (ip_sum);
+    }
   if (0xffff == ip->checksum)
     ip->checksum = 0;
   ASSERT (ip4_header_checksum_is_valid (ip));
@@ -3378,16 +3388,53 @@ nat_6t_flow_icmp_translate (snat_main_t *sm, vlib_buffer_t *b,
 	  nat_protocol_t inner_proto =
 	    ip_proto_to_nat_proto (inner_ip->protocol);
 
-	  ip_csum_t icmp_sum = icmp->checksum;
+	  ip_csum_t old_icmp_sum = icmp->checksum;
+	  ip_csum_t old_inner_ip_sum = inner_ip->checksum;
+	  ip_csum_t old_udp_sum;
+	  ip_csum_t old_tcp_sum;
+	  ip_csum_t new_icmp_sum;
+	  udp_header_t *udp;
+	  tcp_header_t *tcp;
 
 	  switch (inner_proto)
 	    {
 	    case NAT_PROTOCOL_UDP:
+	      udp = (udp_header_t *) (inner_ip + 1);
+	      old_udp_sum = udp->checksum;
+	      nat_6t_flow_ip4_translate (sm, b, inner_ip, f, inner_proto,
+					 1 /* is_icmp_inner_ip4 */,
+					 0 /* skip_saddr_rewrite */);
+	      new_icmp_sum = ip_csum_sub_even (old_icmp_sum, f->l3_csum_delta);
+	      new_icmp_sum = ip_csum_sub_even (new_icmp_sum, f->l4_csum_delta);
+	      new_icmp_sum =
+		ip_csum_update (new_icmp_sum, old_inner_ip_sum,
+				inner_ip->checksum, ip4_header_t, checksum);
+	      new_icmp_sum =
+		ip_csum_update (new_icmp_sum, old_udp_sum, udp->checksum,
+				udp_header_t, checksum);
+	      new_icmp_sum = ip_csum_fold (new_icmp_sum);
+	      if (0xffff == new_icmp_sum)
+		new_icmp_sum = 0;
+	      icmp->checksum = new_icmp_sum;
+	      break;
 	    case NAT_PROTOCOL_TCP:
+	      tcp = (tcp_header_t *) (inner_ip + 1);
+	      old_tcp_sum = tcp->checksum;
 	      nat_6t_flow_ip4_translate (sm, b, inner_ip, f, inner_proto,
-					 1 /* is_icmp_inner_ip4 */);
-	      icmp_sum = ip_csum_sub_even (icmp_sum, f->l3_csum_delta);
-	      icmp->checksum = ip_csum_fold (icmp_sum);
+					 1 /* is_icmp_inner_ip4 */,
+					 0 /* skip_saddr_rewrite */);
+	      new_icmp_sum = ip_csum_sub_even (old_icmp_sum, f->l3_csum_delta);
+	      new_icmp_sum = ip_csum_sub_even (new_icmp_sum, f->l4_csum_delta);
+	      new_icmp_sum =
+		ip_csum_update (new_icmp_sum, old_inner_ip_sum,
+				inner_ip->checksum, ip4_header_t, checksum);
+	      new_icmp_sum =
+		ip_csum_update (new_icmp_sum, old_tcp_sum, tcp->checksum,
+				tcp_header_t, checksum);
+	      new_icmp_sum = ip_csum_fold (new_icmp_sum);
+	      if (0xffff == new_icmp_sum)
+		new_icmp_sum = 0;
+	      icmp->checksum = new_icmp_sum;
 	      break;
 	    case NAT_PROTOCOL_ICMP:
 	      if (f->ops & NAT_FLOW_OP_ICMP_ID_REWRITE)
@@ -3417,29 +3464,63 @@ nat_6t_flow_icmp_translate (snat_main_t *sm, vlib_buffer_t *b,
 	    }
 	}
     }
+
   return NAT_ED_TRNSL_ERR_SUCCESS;
 }
 
-nat_translation_error_e
+static_always_inline nat_translation_error_e
 nat_6t_flow_buf_translate (snat_main_t *sm, vlib_buffer_t *b, ip4_header_t *ip,
 			   nat_6t_flow_t *f, nat_protocol_t proto,
-			   int is_output_feature)
+			   int is_output_feature, int is_i2o)
 {
   if (!is_output_feature && f->ops & NAT_FLOW_OP_TXFIB_REWRITE)
     {
       vnet_buffer (b)->sw_if_index[VLIB_TX] = f->rewrite.fib_index;
     }
 
-  nat_6t_flow_ip4_translate (sm, b, ip, f, proto, 0 /* is_icmp_inner_ip4 */);
-
   if (NAT_PROTOCOL_ICMP == proto)
     {
+      if (ip->src_address.as_u32 != f->rewrite.saddr.as_u32)
+	{
+	  // packet is returned from a router, not from destination
+	  // skip source address rewrite if in o2i path
+	  nat_6t_flow_ip4_translate (sm, b, ip, f, proto,
+				     0 /* is_icmp_inner_ip4 */,
+				     !is_i2o /* skip_saddr_rewrite */);
+	}
+      else
+	{
+	  nat_6t_flow_ip4_translate (sm, b, ip, f, proto,
+				     0 /* is_icmp_inner_ip4 */,
+				     0 /* skip_saddr_rewrite */);
+	}
       return nat_6t_flow_icmp_translate (sm, b, ip, f);
     }
 
+  nat_6t_flow_ip4_translate (sm, b, ip, f, proto, 0 /* is_icmp_inner_ip4 */,
+			     0 /* skip_saddr_rewrite */);
+
   return NAT_ED_TRNSL_ERR_SUCCESS;
 }
 
+nat_translation_error_e
+nat_6t_flow_buf_translate_i2o (snat_main_t *sm, vlib_buffer_t *b,
+			       ip4_header_t *ip, nat_6t_flow_t *f,
+			       nat_protocol_t proto, int is_output_feature)
+{
+  return nat_6t_flow_buf_translate (sm, b, ip, f, proto, is_output_feature,
+				    1 /* is_i2o */);
+}
+
+nat_translation_error_e
+nat_6t_flow_buf_translate_o2i (snat_main_t *sm, vlib_buffer_t *b,
+			       ip4_header_t *ip, nat_6t_flow_t *f,
+			       nat_protocol_t proto, int is_output_feature)
+{
+  return nat_6t_flow_buf_translate (sm, b, ip, f, proto, is_output_feature,
+				    0 /* is_i2o */);
+}
+
 u8 *
 format_nat_6t (u8 *s, va_list *args)
 {
diff --git a/src/plugins/nat/nat44-ed/nat44_ed.h b/src/plugins/nat/nat44-ed/nat44_ed.h
index 2710d29cb9d..420f84c591f 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed.h
+++ b/src/plugins/nat/nat44-ed/nat44_ed.h
@@ -1153,9 +1153,14 @@ typedef enum
 } nat_translation_error_e;
 
 nat_translation_error_e
-nat_6t_flow_buf_translate (snat_main_t *sm, vlib_buffer_t *b, ip4_header_t *ip,
-			   nat_6t_flow_t *f, nat_protocol_t proto,
-			   int is_output_feature);
+nat_6t_flow_buf_translate_i2o (snat_main_t *sm, vlib_buffer_t *b,
+			       ip4_header_t *ip, nat_6t_flow_t *f,
+			       nat_protocol_t proto, int is_output_feature);
+
+nat_translation_error_e
+nat_6t_flow_buf_translate_o2i (snat_main_t *sm, vlib_buffer_t *b,
+			       ip4_header_t *ip, nat_6t_flow_t *f,
+			       nat_protocol_t proto, int is_output_feature);
 
 void nat_6t_l3_l4_csum_calc (nat_6t_flow_t *f);
 
diff --git a/src/plugins/nat/nat44-ed/nat44_ed_in2out.c b/src/plugins/nat/nat44-ed/nat44_ed_in2out.c
index 426b6eeba5c..f7257db789f 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed_in2out.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed_in2out.c
@@ -1183,7 +1183,7 @@ nat44_ed_in2out_fast_path_node_fn_inline (vlib_main_t *vm,
 	}
 
       if (NAT_ED_TRNSL_ERR_SUCCESS !=
-	  (translation_error = nat_6t_flow_buf_translate (
+	  (translation_error = nat_6t_flow_buf_translate_i2o (
 	     sm, b0, ip0, f, proto0, is_output_feature)))
 	{
 	  nat_free_session_data (sm, s0, thread_index, 0);
@@ -1331,7 +1331,7 @@ nat44_ed_in2out_slow_path_node_fn_inline (vlib_main_t *vm,
 
 	  if (NAT_NEXT_DROP != next[0] && s0 &&
 	      NAT_ED_TRNSL_ERR_SUCCESS !=
-		(translation_error = nat_6t_flow_buf_translate (
+		(translation_error = nat_6t_flow_buf_translate_i2o (
 		   sm, b0, ip0, &s0->i2o, proto0, is_output_feature)))
 	    {
 	      goto trace0;
@@ -1349,7 +1349,7 @@ nat44_ed_in2out_slow_path_node_fn_inline (vlib_main_t *vm,
 	    now, thread_index, proto0, &s0, is_multi_worker);
 	  if (NAT_NEXT_DROP != next[0] && s0 &&
 	      NAT_ED_TRNSL_ERR_SUCCESS !=
-		(translation_error = nat_6t_flow_buf_translate (
+		(translation_error = nat_6t_flow_buf_translate_i2o (
 		   sm, b0, ip0, &s0->i2o, proto0, is_output_feature)))
 	    {
 	      goto trace0;
@@ -1426,7 +1426,7 @@ nat44_ed_in2out_slow_path_node_fn_inline (vlib_main_t *vm,
       b0->flags |= VNET_BUFFER_F_IS_NATED;
 
       if (NAT_ED_TRNSL_ERR_SUCCESS !=
-	  (translation_error = nat_6t_flow_buf_translate (
+	  (translation_error = nat_6t_flow_buf_translate_i2o (
 	     sm, b0, ip0, &s0->i2o, proto0, is_output_feature)))
 	{
 	  nat_free_session_data (sm, s0, thread_index, 0);
diff --git a/src/plugins/nat/nat44-ed/nat44_ed_out2in.c b/src/plugins/nat/nat44-ed/nat44_ed_out2in.c
index f14311a99e1..995260a2499 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed_out2in.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed_out2in.c
@@ -1026,7 +1026,7 @@ nat44_ed_out2in_fast_path_node_fn_inline (vlib_main_t * vm,
 	}
 
       if (NAT_ED_TRNSL_ERR_SUCCESS !=
-	  (translation_error = nat_6t_flow_buf_translate (
+	  (translation_error = nat_6t_flow_buf_translate_o2i (
 	     sm, b0, ip0, f, proto0, 0 /* is_output_feature */)))
 	{
 	  next[0] = NAT_NEXT_DROP;
@@ -1182,7 +1182,7 @@ nat44_ed_out2in_slow_path_node_fn_inline (vlib_main_t * vm,
 	    }
 	  if (NAT_NEXT_DROP != next[0] && s0 &&
 	      NAT_ED_TRNSL_ERR_SUCCESS !=
-		(translation_error = nat_6t_flow_buf_translate (
+		(translation_error = nat_6t_flow_buf_translate_o2i (
 		   sm, b0, ip0, &s0->o2i, proto0, 0 /* is_output_feature */)))
 	    {
 	      goto trace0;
@@ -1201,7 +1201,7 @@ nat44_ed_out2in_slow_path_node_fn_inline (vlib_main_t * vm,
 
 	  if (NAT_NEXT_DROP != next[0] && s0 &&
 	      NAT_ED_TRNSL_ERR_SUCCESS !=
-		(translation_error = nat_6t_flow_buf_translate (
+		(translation_error = nat_6t_flow_buf_translate_o2i (
 		   sm, b0, ip0, &s0->o2i, proto0, 0 /* is_output_feature */)))
 	    {
 	      goto trace0;
@@ -1311,7 +1311,7 @@ nat44_ed_out2in_slow_path_node_fn_inline (vlib_main_t * vm,
 	}
 
       if (NAT_ED_TRNSL_ERR_SUCCESS !=
-	  (translation_error = nat_6t_flow_buf_translate (
+	  (translation_error = nat_6t_flow_buf_translate_o2i (
 	     sm, b0, ip0, &s0->o2i, proto0, 0 /* is_output_feature */)))
 	{
 	  next[0] = NAT_NEXT_DROP;