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 <neale@graphiant.com>
Change-Id: Iaac78905289a272dc01930d70decd8109cf5e7a5

1 files changed, 8 insertions, 8 deletions

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);