From 5b8911020ee5512d76c8daccaa199878ed7cbc01 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Mon, 28 Jun 2021 13:31:28 +0000 Subject: ipsec: Fix setting the hi-sequence number for decrypt Type: fix two problems; 1 - just because anti-reply is not enabled doesn't mean the high sequence number should not be used. - fix, there needs to be some means to detect a wrapped packet, so we use a window size of 2^30. 2 - The SA object was used as a scratch pad for the high-sequence number used during decryption. That means that once the batch has been processed the high-sequence number used is lost. This means it is not possible to distinguish this case: if (seq < IPSEC_SA_ANTI_REPLAY_WINDOW_LOWER_BOUND (tl)) { ... if (post_decrypt) { if (hi_seq_used == sa->seq_hi) /* the high sequence number used to succesfully decrypt this * packet is the same as the last-sequnence number of the SA. * that means this packet did not cause a wrap. * this packet is thus out of window and should be dropped */ return 1; else /* The packet decrypted with a different high sequence number * to the SA, that means it is the wrap packet and should be * accepted */ return 0; } - fix: don't use the SA as a scratch pad, use the 'packet_data' - the same place that is used as the scratch pad for the low sequence number. other consequences: - An SA doesn't have seq and last_seq, it has only seq; the sequence numnber of the last packet tx'd or rx'd. - there's 64bits of space available on the SA's first cache line. move the AES CTR mode IV there. - test the ESN/AR combinations to catch the bugs this fixes. This doubles the amount of tests, but without AR on they only run for 2 seconds. In the AR tests, the time taken to wait for packets that won't arrive is dropped from 1 to 0.2 seconds thus reducing the runtime of these tests from 10-15 to about 5 sceonds. Signed-off-by: Neale Ranns Change-Id: Iaac78905289a272dc01930d70decd8109cf5e7a5 --- src/vnet/ipsec/esp_encrypt.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 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 68aeb60885c..da9c56a7e03 100644 --- a/src/vnet/ipsec/esp_encrypt.c +++ b/src/vnet/ipsec/esp_encrypt.c @@ -379,7 +379,7 @@ esp_encrypt_chain_integ (vlib_main_t * vm, ipsec_per_thread_data_t * ptd, always_inline void esp_prepare_sync_op (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, vnet_crypto_op_t **crypto_ops, - vnet_crypto_op_t **integ_ops, ipsec_sa_t *sa0, + vnet_crypto_op_t **integ_ops, ipsec_sa_t *sa0, u32 seq_hi, u8 *payload, u16 payload_len, u8 iv_sz, u8 icv_sz, u32 bi, vlib_buffer_t **b, vlib_buffer_t *lb, u32 hdr_len, esp_header_t *esp) @@ -408,7 +408,7 @@ esp_prepare_sync_op (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, { /* constuct aad in a scratch space in front of the nonce */ op->aad = (u8 *) nonce - sizeof (esp_aead_t); - op->aad_len = esp_aad_fill (op->aad, esp, sa0); + op->aad_len = esp_aad_fill (op->aad, esp, sa0, seq_hi); op->tag = payload + op->len; op->tag_len = 16; } @@ -465,8 +465,8 @@ esp_prepare_sync_op (vlib_main_t *vm, ipsec_per_thread_data_t *ptd, } else if (ipsec_sa_is_set_USE_ESN (sa0)) { - u32 seq_hi = clib_net_to_host_u32 (sa0->seq_hi); - clib_memcpy_fast (op->digest, &seq_hi, sizeof (seq_hi)); + u32 tmp = clib_net_to_host_u32 (seq_hi); + clib_memcpy_fast (op->digest, &tmp, sizeof (seq_hi)); op->len += sizeof (seq_hi); } } @@ -508,7 +508,7 @@ 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); + esp_aad_fill (aad, esp, sa, sa->seq_hi); key_index = sa->crypto_key_index; } else @@ -956,9 +956,9 @@ esp_encrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node, async_next_node, lb); } else - esp_prepare_sync_op (vm, ptd, crypto_ops, integ_ops, sa0, payload, - payload_len, iv_sz, icv_sz, n_sync, b, lb, - hdr_len, esp); + esp_prepare_sync_op (vm, ptd, crypto_ops, integ_ops, sa0, sa0->seq_hi, + payload, payload_len, iv_sz, icv_sz, n_sync, b, + lb, hdr_len, esp); vlib_buffer_advance (b[0], 0LL - hdr_len); -- cgit 1.2.3-korg