From fb7e7ed2cd10446d5ecd1b1e8df470e706c448ed Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sun, 3 Nov 2019 07:02:15 -0500 Subject: ipsec: fix padding/alignment for native IPsec encryption Not all ESP crypto algorithms require padding/alignment to be the same as AES block/IV size. CCM, CTR and GCM all have no padding/alignment requirements, and the RFCs indicate that no padding (beyond ESPs 4 octet alignment requirement) should be used unless TFC (traffic flow confidentiality) has been requested. CTR: https://tools.ietf.org/html/rfc3686#section-3.2 GCM: https://tools.ietf.org/html/rfc4106#section-3.2 CCM: https://tools.ietf.org/html/rfc4309#section-3.2 - VPP is incorrectly using the IV/AES block size to pad CTR and GCM. These modes do not require padding (beyond ESPs 4 octet requirement), as a result packets will have unnecessary padding, which will waste bandwidth at least and possibly fail certain network configurations that have finely tuned MTU configurations at worst. Fix this as well as changing the field names from ".*block_size" to ".*block_align" to better represent their actual (and only) use. Rename "block_sz" in esp_encrypt to "esp_align" and set it correctly as well. test: ipsec: Add unit-test to test for RFC correct padding/alignment test: patch scapy to not incorrectly pad ccm, ctr, gcm modes as well - Scapy is also incorrectly using the AES block size of 16 to pad CCM, CTR, and GCM cipher modes. A bug report has been opened with the and acknowledged with the upstream scapy project as well: https://github.com/secdev/scapy/issues/2322 Ticket: VPP-1928 Type: fix Signed-off-by: Christian Hopps Change-Id: Iaa4d6a325a2e99fdcb2c375a3395bcfe7947770e --- src/vnet/ipsec/esp_encrypt.c | 12 ++++++------ src/vnet/ipsec/ipsec.c | 18 +++++++++--------- src/vnet/ipsec/ipsec.h | 2 +- src/vnet/ipsec/ipsec_sa.c | 4 ++-- src/vnet/ipsec/ipsec_sa.h | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/vnet/ipsec/esp_encrypt.c b/src/vnet/ipsec/esp_encrypt.c index 8b722ef541f..c0773e2963f 100644 --- a/src/vnet/ipsec/esp_encrypt.c +++ b/src/vnet/ipsec/esp_encrypt.c @@ -112,7 +112,7 @@ format_esp_post_encrypt_trace (u8 * s, va_list * args) /* pad packet in input buffer */ static_always_inline u8 * esp_add_footer_and_icv (vlib_main_t * vm, vlib_buffer_t ** last, - u8 block_size, u8 icv_sz, + u8 esp_align, u8 icv_sz, u16 * next, vlib_node_runtime_t * node, u16 buffer_data_size, uword total_len) { @@ -122,7 +122,7 @@ esp_add_footer_and_icv (vlib_main_t * vm, vlib_buffer_t ** last, }; u16 min_length = total_len + sizeof (esp_footer_t); - u16 new_length = round_pow2 (min_length, block_size); + u16 new_length = round_pow2 (min_length, esp_align); u8 pad_bytes = new_length - min_length; esp_footer_t *f = (esp_footer_t *) (vlib_buffer_get_current (last[0]) + last[0]->current_length + pad_bytes); @@ -587,7 +587,7 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node, u16 buffer_data_size = vlib_buffer_get_default_data_size (vm); u32 current_sa_index = ~0, current_sa_packets = 0; u32 current_sa_bytes = 0, spi = 0; - u8 block_sz = 0, iv_sz = 0, icv_sz = 0; + u8 esp_align = 4, iv_sz = 0, icv_sz = 0; ipsec_sa_t *sa0 = 0; vlib_buffer_t *lb; vnet_crypto_op_t **crypto_ops = &ptd->crypto_ops; @@ -648,7 +648,7 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node, sa0 = pool_elt_at_index (im->sad, sa_index0); current_sa_index = sa_index0; spi = clib_net_to_host_u32 (sa0->spi); - block_sz = sa0->crypto_block_size; + esp_align = sa0->esp_block_align; icv_sz = sa0->integ_icv_size; iv_sz = sa0->crypto_iv_size; @@ -713,7 +713,7 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node, if (ipsec_sa_is_set_IS_TUNNEL (sa0)) { payload = vlib_buffer_get_current (b[0]); - next_hdr_ptr = esp_add_footer_and_icv (vm, &lb, block_sz, icv_sz, + next_hdr_ptr = esp_add_footer_and_icv (vm, &lb, esp_align, icv_sz, next, node, buffer_data_size, vlib_buffer_length_in_chain @@ -789,7 +789,7 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_buffer_advance (b[0], ip_len); payload = vlib_buffer_get_current (b[0]); - next_hdr_ptr = esp_add_footer_and_icv (vm, &lb, block_sz, icv_sz, + next_hdr_ptr = esp_add_footer_and_icv (vm, &lb, esp_align, icv_sz, next, node, buffer_data_size, vlib_buffer_length_in_chain diff --git a/src/vnet/ipsec/ipsec.c b/src/vnet/ipsec/ipsec.c index 0ef90673596..55f69f584c9 100644 --- a/src/vnet/ipsec/ipsec.c +++ b/src/vnet/ipsec/ipsec.c @@ -450,44 +450,44 @@ ipsec_init (vlib_main_t * vm) a->dec_op_id = VNET_CRYPTO_OP_NONE; a->alg = VNET_CRYPTO_ALG_NONE; a->iv_size = 0; - a->block_size = 1; + a->block_align = 1; a = im->crypto_algs + IPSEC_CRYPTO_ALG_DES_CBC; a->enc_op_id = VNET_CRYPTO_OP_DES_CBC_ENC; a->dec_op_id = VNET_CRYPTO_OP_DES_CBC_DEC; a->alg = VNET_CRYPTO_ALG_DES_CBC; - a->iv_size = a->block_size = 8; + a->iv_size = a->block_align = 8; a = im->crypto_algs + IPSEC_CRYPTO_ALG_3DES_CBC; a->enc_op_id = VNET_CRYPTO_OP_3DES_CBC_ENC; a->dec_op_id = VNET_CRYPTO_OP_3DES_CBC_DEC; a->alg = VNET_CRYPTO_ALG_3DES_CBC; - a->iv_size = a->block_size = 8; + a->iv_size = a->block_align = 8; a = im->crypto_algs + IPSEC_CRYPTO_ALG_AES_CBC_128; a->enc_op_id = VNET_CRYPTO_OP_AES_128_CBC_ENC; a->dec_op_id = VNET_CRYPTO_OP_AES_128_CBC_DEC; a->alg = VNET_CRYPTO_ALG_AES_128_CBC; - a->iv_size = a->block_size = 16; + a->iv_size = a->block_align = 16; a = im->crypto_algs + IPSEC_CRYPTO_ALG_AES_CBC_192; a->enc_op_id = VNET_CRYPTO_OP_AES_192_CBC_ENC; a->dec_op_id = VNET_CRYPTO_OP_AES_192_CBC_DEC; a->alg = VNET_CRYPTO_ALG_AES_192_CBC; - a->iv_size = a->block_size = 16; + a->iv_size = a->block_align = 16; a = im->crypto_algs + IPSEC_CRYPTO_ALG_AES_CBC_256; a->enc_op_id = VNET_CRYPTO_OP_AES_256_CBC_ENC; a->dec_op_id = VNET_CRYPTO_OP_AES_256_CBC_DEC; a->alg = VNET_CRYPTO_ALG_AES_256_CBC; - a->iv_size = a->block_size = 16; + a->iv_size = a->block_align = 16; a = im->crypto_algs + IPSEC_CRYPTO_ALG_AES_GCM_128; a->enc_op_id = VNET_CRYPTO_OP_AES_128_GCM_ENC; a->dec_op_id = VNET_CRYPTO_OP_AES_128_GCM_DEC; a->alg = VNET_CRYPTO_ALG_AES_128_GCM; a->iv_size = 8; - a->block_size = 16; + a->block_align = 1; a->icv_size = 16; a = im->crypto_algs + IPSEC_CRYPTO_ALG_AES_GCM_192; @@ -495,7 +495,7 @@ ipsec_init (vlib_main_t * vm) a->dec_op_id = VNET_CRYPTO_OP_AES_192_GCM_DEC; a->alg = VNET_CRYPTO_ALG_AES_192_GCM; a->iv_size = 8; - a->block_size = 16; + a->block_align = 1; a->icv_size = 16; a = im->crypto_algs + IPSEC_CRYPTO_ALG_AES_GCM_256; @@ -503,7 +503,7 @@ ipsec_init (vlib_main_t * vm) a->dec_op_id = VNET_CRYPTO_OP_AES_256_GCM_DEC; a->alg = VNET_CRYPTO_ALG_AES_256_GCM; a->iv_size = 8; - a->block_size = 16; + a->block_align = 1; a->icv_size = 16; vec_validate (im->integ_algs, IPSEC_INTEG_N_ALG - 1); diff --git a/src/vnet/ipsec/ipsec.h b/src/vnet/ipsec/ipsec.h index 603bc33ba4d..3d84ad3f6ad 100644 --- a/src/vnet/ipsec/ipsec.h +++ b/src/vnet/ipsec/ipsec.h @@ -78,7 +78,7 @@ typedef struct vnet_crypto_op_id_t dec_op_id; vnet_crypto_alg_t alg; u8 iv_size; - u8 block_size; + u8 block_align; u8 icv_size; } ipsec_main_crypto_alg_t; diff --git a/src/vnet/ipsec/ipsec_sa.c b/src/vnet/ipsec/ipsec_sa.c index e5f01bff0d9..2ff3bee1b3f 100644 --- a/src/vnet/ipsec/ipsec_sa.c +++ b/src/vnet/ipsec/ipsec_sa.c @@ -99,12 +99,12 @@ ipsec_sa_set_crypto_alg (ipsec_sa_t * sa, ipsec_crypto_alg_t crypto_alg) ipsec_main_t *im = &ipsec_main; sa->crypto_alg = crypto_alg; sa->crypto_iv_size = im->crypto_algs[crypto_alg].iv_size; - sa->crypto_block_size = im->crypto_algs[crypto_alg].block_size; + sa->esp_block_align = clib_max (4, im->crypto_algs[crypto_alg].block_align); sa->sync_op_data.crypto_enc_op_id = im->crypto_algs[crypto_alg].enc_op_id; sa->sync_op_data.crypto_dec_op_id = im->crypto_algs[crypto_alg].dec_op_id; sa->crypto_calg = im->crypto_algs[crypto_alg].alg; ASSERT (sa->crypto_iv_size <= ESP_MAX_IV_SIZE); - ASSERT (sa->crypto_block_size <= ESP_MAX_BLOCK_SIZE); + ASSERT (sa->esp_block_align <= ESP_MAX_BLOCK_SIZE); if (IPSEC_CRYPTO_ALG_IS_GCM (crypto_alg)) { sa->integ_icv_size = im->crypto_algs[crypto_alg].icv_size; diff --git a/src/vnet/ipsec/ipsec_sa.h b/src/vnet/ipsec/ipsec_sa.h index 5950d6f53b3..5d65238b736 100644 --- a/src/vnet/ipsec/ipsec_sa.h +++ b/src/vnet/ipsec/ipsec_sa.h @@ -113,7 +113,7 @@ typedef struct ipsec_sa_flags_t flags; u8 crypto_iv_size; - u8 crypto_block_size; + u8 esp_block_align; u8 integ_icv_size; u32 encrypt_thread_index; u32 decrypt_thread_index; -- cgit 1.2.3-korg