From 5527a78ed96043d2c26e3271066c50b44dd7fc0b Mon Sep 17 00:00:00 2001 From: Benoît Ganne Date: Tue, 18 Jan 2022 15:56:41 +0100 Subject: ipsec: make pre-shared keys harder to misuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using pre-shared keys is usually a bad idea, one should use eg. IKEv2 instead, but one does not always have the choice. For AES-CBC, the IV must be unpredictable (see NIST SP800-38a Appendix C) whereas for AES-CTR or AES-GCM, the IV should never be reused with the same key material (see NIST SP800-38a Appendix B and NIST SP800-38d section 8). If one uses pre-shared keys and VPP is restarted, the IV counter restarts at 0 and the same IVs are generated with the same pre-shared keys materials. To fix those issues we follow the recommendation from NIST SP800-38a and NIST SP800-38d: - we use a PRNG (not cryptographically secured) to generate IVs to avoid generating the same IV sequence between VPP restarts. The PRNG is chosen so that there is a low chance of generating the same sequence - for AES-CBC, the generated IV is encrypted as part of the message. This makes the (predictable) PRNG-generated IV unpredictable as it is encrypted with the secret key - for AES-CTR and GCM, we use the IV as-is as predictable IVs are fine Most of the changes in this patch are caused by the need to shoehorn an additional state of 2 u64 for the PRNG in the 1st cacheline of the SA object. Type: improvement Change-Id: I2af89c21ae4b2c4c33dd21aeffcfb79c13c9d84c Signed-off-by: Benoît Ganne --- src/vnet/ipsec/esp_encrypt.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'src/vnet/ipsec/esp_encrypt.c') diff --git a/src/vnet/ipsec/esp_encrypt.c b/src/vnet/ipsec/esp_encrypt.c index 88e93b9b2d4..861b3e98650 100644 --- a/src/vnet/ipsec/esp_encrypt.c +++ b/src/vnet/ipsec/esp_encrypt.c @@ -219,17 +219,18 @@ esp_get_ip6_hdr_len (ip6_header_t * ip6, ip6_ext_header_t ** ext_hdr) * encryption mode: IVs must be unpredictable for AES-CBC whereas it can * be predictable but should never be reused with the same key material * for CTR and GCM. - * We use a packet counter as the IV for CTR and GCM, and to ensure the - * IV is unpredictable for CBC, it is then encrypted using the same key - * as the message. You can refer to NIST SP800-38a and NIST SP800-38d - * for more details. */ + * To avoid reusing the same IVs between multiple VPP instances and between + * restarts, we use a properly chosen PRNG to generate IVs. To ensure the IV is + * unpredictable for CBC, it is then encrypted using the same key as the + * message. You can refer to NIST SP800-38a and NIST SP800-38d for more + * details. */ static_always_inline void * esp_generate_iv (ipsec_sa_t *sa, void *payload, int iv_sz) { ASSERT (iv_sz >= sizeof (u64)); u64 *iv = (u64 *) (payload - iv_sz); clib_memset_u8 (iv, 0, iv_sz); - *iv = sa->iv_counter++; + *iv = clib_pcg64i_random_r (&sa->iv_prng); return iv; } @@ -434,7 +435,7 @@ esp_prepare_sync_op (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, crypto_len += iv_sz; } - if (lb != b[0]) + if (PREDICT_FALSE (lb != b[0])) { /* is chained */ op->flags |= VNET_CRYPTO_OP_FLAG_CHAINED_BUFFERS; @@ -497,7 +498,7 @@ esp_prepare_async_frame (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, esp_post_data_t *post = esp_post_data (b); u8 *tag, *iv, *aad = 0; u8 flag = 0; - u32 key_index; + const u32 key_index = sa->crypto_key_index; i16 crypto_start_offset, integ_start_offset; u16 crypto_total_len, integ_total_len; @@ -508,8 +509,6 @@ esp_prepare_async_frame (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, crypto_total_len = integ_total_len = payload_len - icv_sz; tag = payload + crypto_total_len; - key_index = sa->linked_key_index; - /* generate the IV in front of the payload */ void *pkt_iv = esp_generate_iv (sa, payload, iv_sz); @@ -523,7 +522,6 @@ esp_prepare_async_frame (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, /* constuct aad in a scratch space in front of the nonce */ aad = (u8 *) nonce - sizeof (esp_aead_t); esp_aad_fill (aad, esp, sa, sa->seq_hi); - key_index = sa->crypto_key_index; } else { @@ -705,7 +703,7 @@ esp_encrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node, is_async = im->async_mode | ipsec_sa_is_set_IS_ASYNC (sa0); } - if (PREDICT_FALSE (~0 == sa0->thread_index)) + if (PREDICT_FALSE ((u16) ~0 == sa0->thread_index)) { /* this is the first packet to use this SA, claim the SA * for this thread. this could happen simultaneously on -- cgit 1.2.3-korg