ipsec: Targeted unit testing

Type: fix

1 - big packets; chained buffers and those without enoguh space to add
ESP header
2 - IPv6 extension headers in packets that are encrypted/decrypted
3 - Interface protection with SAs that have null algorithms

Signed-off-by: Neale Ranns <nranns@cisco.com>
Change-Id: Ie330861fb06a9b248d9dcd5c730e21326ac8e973

6 files changed, 114 insertions, 97 deletions

diff --git a/src/vnet/ipsec/esp_decrypt.c b/src/vnet/ipsec/esp_decrypt.c
index 16ae3a3d9eb..ee53b018552 100644
--- a/src/vnet/ipsec/esp_decrypt.c
+++ b/src/vnet/ipsec/esp_decrypt.c
@@ -53,6 +53,7 @@ typedef enum
  _(OVERSIZED_HEADER, "buffer with oversized header (dropped)")  \
  _(NO_TAIL_SPACE, "no enough buffer tail space (dropped)")      \
  _(TUN_NO_PROTO, "no tunnel protocol")                          \
+ _(UNSUP_PAYLOAD, "unsupported payload")                        \
 
 
 typedef enum
@@ -311,9 +312,10 @@ esp_decrypt_inline (vlib_main_t * vm,
       b += 1;
     }
 
-  vlib_increment_combined_counter (&ipsec_sa_counters, thread_index,
-				   current_sa_index, current_sa_pkts,
-				   current_sa_bytes);
+  if (PREDICT_TRUE (~0 != current_sa_index))
+    vlib_increment_combined_counter (&ipsec_sa_counters, thread_index,
+				     current_sa_index, current_sa_pkts,
+				     current_sa_bytes);
 
   if ((n = vec_len (ptd->integ_ops)))
     {
@@ -513,6 +515,8 @@ esp_decrypt_inline (vlib_main_t * vm,
 		      next[0] = ESP_DECRYPT_NEXT_IP6_INPUT;
 		      break;
 		    default:
+		      b[0]->error =
+			node->errors[ESP_DECRYPT_ERROR_UNSUP_PAYLOAD];
 		      next[0] = ESP_DECRYPT_NEXT_DROP;
 		      break;
 		    }
@@ -520,8 +524,7 @@ esp_decrypt_inline (vlib_main_t * vm,
 	      else
 		{
 		  next[0] = ESP_DECRYPT_NEXT_DROP;
-		  b[0]->error =
-		    node->errors[ESP_DECRYPT_ERROR_DECRYPTION_FAILED];
+		  b[0]->error = node->errors[ESP_DECRYPT_ERROR_UNSUP_PAYLOAD];
 		  goto trace;
 		}
 	    }
@@ -530,8 +533,20 @@ esp_decrypt_inline (vlib_main_t * vm,
 	      if (ipsec_sa_is_set_IS_PROTECT (sa0))
 		{
 		  /*
-		   * Check that the reveal IP header matches that
-		   * of the tunnel we are protecting
+		   * There are two encap possibilities
+		   * 1) the tunnel and ths SA are prodiving encap, i.e. it's
+		   *   MAC | SA-IP | TUN-IP | ESP | PAYLOAD
+		   * implying the SA is in tunnel mode (on a tunnel interface)
+		   * 2) only the tunnel provides encap
+		   *   MAC | TUN-IP | ESP | PAYLOAD
+		   * implying the SA is in transport mode.
+		   *
+		   * For 2) we need only strip the tunnel encap and we're good.
+		   *  since the tunnel and crypto ecnap (int the tun=protect
+		   * object) are the same and we verified above that these match
+		   * for 1) we need to strip the SA-IP outer headers, to
+		   * reveal the tunnel IP and then check that this matches
+		   * the configured tunnel.
 		   */
 		  const ipsec_tun_protect_t *itp;
 
diff --git a/src/vnet/ipsec/esp_encrypt.c b/src/vnet/ipsec/esp_encrypt.c
index 4f1bb802bcb..3c2fdf4ec3c 100644
--- a/src/vnet/ipsec/esp_encrypt.c
+++ b/src/vnet/ipsec/esp_encrypt.c
@@ -176,17 +176,19 @@ ext_hdr_is_pre_esp (u8 nexthdr)
 }
 
 static_always_inline u8
-esp_get_ip6_hdr_len (ip6_header_t * ip6)
+esp_get_ip6_hdr_len (ip6_header_t * ip6, ip6_ext_header_t ** ext_hdr)
 {
   /* this code assumes that HbH, route and frag headers will be before
      others, if that is not the case, they will end up encrypted */
-
   u8 len = sizeof (ip6_header_t);
   ip6_ext_header_t *p;
 
   /* if next packet doesn't have ext header */
   if (ext_hdr_is_pre_esp (ip6->protocol) == 0)
-    return len;
+    {
+      *ext_hdr = NULL;
+      return len;
+    }
 
   p = (void *) (ip6 + 1);
   len += ip6_ext_header_len (p);
@@ -197,6 +199,7 @@ esp_get_ip6_hdr_len (ip6_header_t * ip6)
       p = ip6_ext_next_header (p);
     }
 
+  *ext_hdr = p;
   return len;
 }
 
@@ -401,11 +404,12 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
       else			/* transport mode */
 	{
 	  u8 *l2_hdr, l2_len, *ip_hdr, ip_len;
+	  ip6_ext_header_t *ext_hdr;
 	  udp_header_t *udp = 0;
 	  u8 *old_ip_hdr = vlib_buffer_get_current (b[0]);
 
 	  ip_len = is_ip6 ?
-	    esp_get_ip6_hdr_len ((ip6_header_t *) old_ip_hdr) :
+	    esp_get_ip6_hdr_len ((ip6_header_t *) old_ip_hdr, &ext_hdr) :
 	    ip4_header_bytes ((ip4_header_t *) old_ip_hdr);
 
 	  vlib_buffer_advance (b[0], ip_len);
@@ -445,21 +449,27 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	  else
 	    l2_len = 0;
 
-	  clib_memcpy_le64 (ip_hdr, old_ip_hdr, ip_len);
-
 	  if (is_ip6)
 	    {
-	      ip6_header_t *ip6 = (ip6_header_t *) (ip_hdr);
-	      *next_hdr_ptr = ip6->protocol;
-	      ip6->protocol = IP_PROTOCOL_IPSEC_ESP;
+	      ip6_header_t *ip6 = (ip6_header_t *) (old_ip_hdr);
+	      if (PREDICT_TRUE (NULL == ext_hdr))
+		{
+		  *next_hdr_ptr = ip6->protocol;
+		  ip6->protocol = IP_PROTOCOL_IPSEC_ESP;
+		}
+	      else
+		{
+		  *next_hdr_ptr = ext_hdr->next_hdr;
+		  ext_hdr->next_hdr = IP_PROTOCOL_IPSEC_ESP;
+		}
 	      ip6->payload_length =
 		clib_host_to_net_u16 (payload_len + hdr_len - l2_len -
-				      ip_len);
+				      sizeof (ip6_header_t));
 	    }
 	  else
 	    {
 	      u16 len;
-	      ip4_header_t *ip4 = (ip4_header_t *) (ip_hdr);
+	      ip4_header_t *ip4 = (ip4_header_t *) (old_ip_hdr);
 	      *next_hdr_ptr = ip4->protocol;
 	      len = payload_len + hdr_len - l2_len;
 	      if (udp)
@@ -471,6 +481,8 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 		esp_update_ip4_hdr (ip4, len, /* is_transport */ 1, 0);
 	    }
 
+	  clib_memcpy_le64 (ip_hdr, old_ip_hdr, ip_len);
+
 	  if (!is_tun)
 	    next[0] = ESP_ENCRYPT_NEXT_INTERFACE_OUTPUT;
 	}
diff --git a/src/vnet/ipsec/ipsec.c b/src/vnet/ipsec/ipsec.c
index 0f4f2820349..1075fe48d84 100644
--- a/src/vnet/ipsec/ipsec.c
+++ b/src/vnet/ipsec/ipsec.c
@@ -188,9 +188,13 @@ ipsec_register_esp_backend (vlib_main_t * vm, ipsec_main_t * im,
 		  &b->esp6_decrypt_node_index, &b->esp6_decrypt_next_index);
 
   ipsec_add_feature ("ip4-output", esp4_encrypt_node_tun_name,
-		     &b->esp4_encrypt_tun_feature_index);
+		     &b->esp44_encrypt_tun_feature_index);
+  ipsec_add_feature ("ip4-output", esp6_encrypt_node_tun_name,
+		     &b->esp46_encrypt_tun_feature_index);
   ipsec_add_feature ("ip6-output", esp6_encrypt_node_tun_name,
-		     &b->esp6_encrypt_tun_feature_index);
+		     &b->esp66_encrypt_tun_feature_index);
+  ipsec_add_feature ("ip6-output", esp4_encrypt_node_tun_name,
+		     &b->esp64_encrypt_tun_feature_index);
 
   b->check_support_cb = esp_check_support_cb;
   b->add_del_sa_sess_cb = esp_add_del_sa_sess_cb;
@@ -252,8 +256,10 @@ ipsec_select_esp_backend (ipsec_main_t * im, u32 backend_idx)
   im->esp6_encrypt_next_index = b->esp6_encrypt_next_index;
   im->esp6_decrypt_next_index = b->esp6_decrypt_next_index;
 
-  im->esp4_encrypt_tun_feature_index = b->esp4_encrypt_tun_feature_index;
-  im->esp6_encrypt_tun_feature_index = b->esp6_encrypt_tun_feature_index;
+  im->esp44_encrypt_tun_feature_index = b->esp44_encrypt_tun_feature_index;
+  im->esp64_encrypt_tun_feature_index = b->esp64_encrypt_tun_feature_index;
+  im->esp46_encrypt_tun_feature_index = b->esp46_encrypt_tun_feature_index;
+  im->esp66_encrypt_tun_feature_index = b->esp66_encrypt_tun_feature_index;
 
   return 0;
 }
diff --git a/src/vnet/ipsec/ipsec.h b/src/vnet/ipsec/ipsec.h
index af758418322..65b888e51a2 100644
--- a/src/vnet/ipsec/ipsec.h
+++ b/src/vnet/ipsec/ipsec.h
@@ -61,8 +61,10 @@ typedef struct
   u32 esp6_decrypt_node_index;
   u32 esp6_encrypt_next_index;
   u32 esp6_decrypt_next_index;
-  u32 esp4_encrypt_tun_feature_index;
-  u32 esp6_encrypt_tun_feature_index;
+  u32 esp44_encrypt_tun_feature_index;
+  u32 esp46_encrypt_tun_feature_index;
+  u32 esp66_encrypt_tun_feature_index;
+  u32 esp64_encrypt_tun_feature_index;
 } ipsec_esp_backend_t;
 
 typedef struct
@@ -135,8 +137,10 @@ typedef struct
   u32 ah6_decrypt_next_index;
 
   /* tun encrypt arcs and feature nodes */
-  u32 esp4_encrypt_tun_feature_index;
-  u32 esp6_encrypt_tun_feature_index;
+  u32 esp44_encrypt_tun_feature_index;
+  u32 esp64_encrypt_tun_feature_index;
+  u32 esp46_encrypt_tun_feature_index;
+  u32 esp66_encrypt_tun_feature_index;
 
   /* tun nodes to drop packets when no crypto alg set on outbound SA */
   u32 esp4_no_crypto_tun_feature_index;
diff --git a/src/vnet/ipsec/ipsec_tun.c b/src/vnet/ipsec/ipsec_tun.c
index ca0091b3545..f6b09d65f2c 100644
--- a/src/vnet/ipsec/ipsec_tun.c
+++ b/src/vnet/ipsec/ipsec_tun.c
@@ -35,35 +35,61 @@ typedef struct ipsec_protect_db_t_
 
 static ipsec_protect_db_t ipsec_protect_db;
 
-static int
+static void
 ipsec_tun_protect_feature_set (ipsec_tun_protect_t * itp, u8 enable)
 {
-  u32 sai = itp->itp_out_sa;
-  int rv;
+  u32 sai;
 
-  const char *enc_node = (ip46_address_is_ip4 (&itp->itp_tun.src) ?
-			  "esp4-encrypt-tun" : "esp6-encrypt-tun");
+  sai = itp->itp_out_sa;
 
   if (itp->itp_flags & IPSEC_PROTECT_L2)
     {
-      rv = vnet_feature_enable_disable ("ethernet-output",
-					enc_node,
-					itp->itp_sw_if_index, enable,
-					&sai, sizeof (sai));
+      /* l2-GRE only supported by the vnet ipsec code */
+      vnet_feature_enable_disable ("ethernet-output",
+				   (ip46_address_is_ip4 (&itp->itp_tun.src) ?
+				    "esp4-encrypt-tun" :
+				    "esp6-encrypt-tun"),
+				   itp->itp_sw_if_index, enable,
+				   &sai, sizeof (sai));
     }
   else
     {
-      rv = vnet_feature_enable_disable ("ip4-output",
-					enc_node,
-					itp->itp_sw_if_index, enable,
-					&sai, sizeof (sai));
-      rv = vnet_feature_enable_disable ("ip6-output",
-					enc_node,
-					itp->itp_sw_if_index, enable,
-					&sai, sizeof (sai));
+      ipsec_main_t *im;
+      ipsec_sa_t *sa;
+      u32 fi4, fi6;
+
+      im = &ipsec_main;
+      sa = ipsec_sa_get (sai);
+
+      if (sa->crypto_alg == IPSEC_CRYPTO_ALG_NONE &&
+	  sa->integ_alg == IPSEC_INTEG_ALG_NONE)
+	{
+	  fi4 = im->esp4_no_crypto_tun_feature_index;
+	  fi6 = im->esp6_no_crypto_tun_feature_index;
+	}
+      else
+	{
+	  if (ip46_address_is_ip4 (&itp->itp_tun.src))
+	    {
+	      /* tunnel destination is v4 so we need the Xo4 indexes */
+	      fi4 = im->esp44_encrypt_tun_feature_index;
+	      fi6 = im->esp64_encrypt_tun_feature_index;
+	    }
+	  else
+	    {
+	      /* tunnel destination is v6 so we need the Xo6 indexes */
+	      fi4 = im->esp46_encrypt_tun_feature_index;
+	      fi6 = im->esp66_encrypt_tun_feature_index;
+	    }
+	}
+
+      vnet_feature_enable_disable_with_index
+	(vnet_get_feature_arc_index ("ip4-output"),
+	 fi4, itp->itp_sw_if_index, enable, &sai, sizeof (sai));
+      vnet_feature_enable_disable_with_index
+	(vnet_get_feature_arc_index ("ip6-output"),
+	 fi6, itp->itp_sw_if_index, enable, &sai, sizeof (sai));
     }
-  ASSERT (!rv);
-  return (rv);
 }
 
 static void
@@ -505,6 +531,12 @@ ipsec_tunnel_protect_init (vlib_main_t * vm)
 					     sizeof (u64));
   im->tun4_protect_by_key = hash_create (0, sizeof (u64));
 
+  /* set up feature nodes to drop outbound packets with no crypto alg set */
+  ipsec_add_feature ("ip4-output", "esp4-no-crypto",
+		     &im->esp4_no_crypto_tun_feature_index);
+  ipsec_add_feature ("ip6-output", "esp6-no-crypto",
+		     &im->esp6_no_crypto_tun_feature_index);
+
   return 0;
 }
 
diff --git a/src/vnet/ipsec/ipsec_tun_in.c b/src/vnet/ipsec/ipsec_tun_in.c
index f25a76319f1..e6ad67b433a 100644
--- a/src/vnet/ipsec/ipsec_tun_in.c
+++ b/src/vnet/ipsec/ipsec_tun_in.c
@@ -311,58 +311,6 @@ ipsec_tun_protect_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	      n_bytes = len0;
 	    }
 
-	  /*
-	   * compare the packet's outer IP headers to that of the tunnels
-	   */
-	  if (is_ip6)
-	    {
-	      if (PREDICT_FALSE
-		  (!ip46_address_is_equal_v6
-		   (&itp0->itp_crypto.dst, &ip60->src_address)
-		   || !ip46_address_is_equal_v6 (&itp0->itp_crypto.src,
-						 &ip60->dst_address)))
-		{
-		  b[0]->error =
-		    node->errors
-		    [IPSEC_TUN_PROTECT_INPUT_ERROR_TUNNEL_MISMATCH];
-		  next[0] = IPSEC_INPUT_NEXT_DROP;
-		  goto trace00;
-		}
-	    }
-	  else
-	    {
-	      if (PREDICT_FALSE
-		  (!ip46_address_is_equal_v4
-		   (&itp0->itp_crypto.dst, &ip40->src_address)
-		   || !ip46_address_is_equal_v4 (&itp0->itp_crypto.src,
-						 &ip40->dst_address)))
-		{
-		  b[0]->error =
-		    node->errors
-		    [IPSEC_TUN_PROTECT_INPUT_ERROR_TUNNEL_MISMATCH];
-		  next[0] = IPSEC_INPUT_NEXT_DROP;
-		  goto trace00;
-		}
-	    }
-
-	  /*
-	   * There are two encap possibilities
-	   * 1) the tunnel and ths SA are prodiving encap, i.e. it's
-	   *   MAC | SA-IP | TUN-IP | ESP | PAYLOAD
-	   * implying the SA is in tunnel mode (on a tunnel interface)
-	   * 2) only the tunnel provides encap
-	   *   MAC | TUN-IP | ESP | PAYLOAD
-	   * implying the SA is in transport mode.
-	   *
-	   * For 2) we need only strip the tunnel encap and we're good.
-	   *  since the tunnel and crypto ecnap (int the tun=protect
-	   * object) are the same and we verified above that these match
-	   * for 1) we need to strip the SA-IP outer headers, to
-	   * reveal the tunnel IP and then check that this matches
-	   * the configured tunnel. this we can;t do here since it
-	   * involves a lookup in the per-tunnel-type DB - so ship
-	   * the packet to the tunnel-types provided node to do that
-	   */
 	  next[0] = IPSEC_TUN_PROTECT_NEXT_DECRYPT;
 	}
     trace00: