From 02950406c49a743f631395ed52073921744e1afd Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Fri, 20 Dec 2019 00:54:57 +0000 Subject: 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 Change-Id: Ie330861fb06a9b248d9dcd5c730e21326ac8e973 --- src/vnet/ipsec/esp_decrypt.c | 29 +++++++++++++----- src/vnet/ipsec/esp_encrypt.c | 34 ++++++++++++++------- src/vnet/ipsec/ipsec.c | 14 ++++++--- src/vnet/ipsec/ipsec.h | 12 +++++--- src/vnet/ipsec/ipsec_tun.c | 70 +++++++++++++++++++++++++++++++------------ src/vnet/ipsec/ipsec_tun_in.c | 52 -------------------------------- 6 files changed, 114 insertions(+), 97 deletions(-) (limited to 'src/vnet/ipsec') 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: -- cgit 1.2.3-korg