ipsec: check each packet for no algs in esp-encrypt

In esp_encrypt_inline(), if two or more consecutive packets are
associated with the same SA which has no crypto or integrity algorithms
set, only the first one gets dropped. Subsequent packets either get sent
(synchronous crypto) or cause a segv (asynchronous crypto).

The current SA's index and pool entry are cached before it can be
determined whether the packet should be dropped due to no algorithms
being set. The check for no algorithms is only performed when the cached
SA index is different than the SA index for the current packet. So
packets after the first one associated with the "none" alg SA aren't
handled properly.

This was broken by my previous commit ("ipsec: keep esp encrypt pointer
and index synced") which fixed a segv that occurred under a different
set of circumstances.

Check whether each packet should be dropped instead of only checking
when a new SA is encountered.

Update unit tests:
- Add a test for no algs on tunnel interface which enables
  asynchronous crypto.
- Send more than one packet in the tests for no algs.

Type: fix
Fixes: dac9e566cd16fc375fff14280b37cb5135584fc6

Signed-off-by: Matthew Smith <mgsmith@netgate.com>
Change-Id: I69e951f22044051eb8557da187cb58f5535b54bf

diff --git a/src/vnet/ipsec/esp_encrypt.c b/src/vnet/ipsec/esp_encrypt.c
index 8a1ac7e..46f7fe6 100644 (file)
--- a/src/vnet/ipsec/esp_encrypt.c
+++ b/src/vnet/ipsec/esp_encrypt.c
@@ -607,6 +607,7 @@ esp_encrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
   u32 current_sa_bytes = 0, spi = 0;
   u8 esp_align = 4, iv_sz = 0, icv_sz = 0;
   ipsec_sa_t *sa0 = 0;
+  u8 sa_drop_no_crypto = 0;
   vlib_buffer_t *lb;
   vnet_crypto_op_t **crypto_ops = &ptd->crypto_ops;
   vnet_crypto_op_t **integ_ops = &ptd->integ_ops;
@@ -692,16 +693,10 @@ esp_encrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
          sa0 = ipsec_sa_get (sa_index0);
          current_sa_index = sa_index0;
 
-         if (PREDICT_FALSE ((sa0->crypto_alg == IPSEC_CRYPTO_ALG_NONE &&
-                             sa0->integ_alg == IPSEC_INTEG_ALG_NONE) &&
-                            !ipsec_sa_is_set_NO_ALGO_NO_DROP (sa0)))
-           {
-             err = ESP_ENCRYPT_ERROR_NO_ENCRYPTION;
-             esp_encrypt_set_next_index (b[0], node, thread_index, err,
-                                         n_noop, noop_nexts, drop_next,
-                                         sa_index0);
-             goto trace;
-           }
+         sa_drop_no_crypto = ((sa0->crypto_alg == IPSEC_CRYPTO_ALG_NONE &&
+                               sa0->integ_alg == IPSEC_INTEG_ALG_NONE) &&
+                              !ipsec_sa_is_set_NO_ALGO_NO_DROP (sa0));
+
          vlib_prefetch_combined_counter (&ipsec_sa_counters, thread_index,
                                          current_sa_index);
 
@@ -715,6 +710,14 @@ 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 (sa_drop_no_crypto != 0))
+       {
+         err = ESP_ENCRYPT_ERROR_NO_ENCRYPTION;
+         esp_encrypt_set_next_index (b[0], node, thread_index, err, n_noop,
+                                     noop_nexts, drop_next, sa_index0);
+         goto trace;
+       }
+
       if (PREDICT_FALSE ((u16) ~0 == sa0->thread_index))
        {
          /* this is the first packet to use this SA, claim the SA

diff --git a/test/test_ipsec_tun_if_esp.py b/test/test_ipsec_tun_if_esp.py
index 5131fbe..a7f91b9 100644 (file)
--- a/test/test_ipsec_tun_if_esp.py
+++ b/test/test_ipsec_tun_if_esp.py
@@ -1231,13 +1231,35 @@ class TestIpsec4TunIfEspNoAlgo(TemplateIpsec4TunProtect, TemplateIpsec, IpsecTun
         self.config_sa_tra(p)
         self.config_protect(p)
 
-        tx = self.gen_pkts(self.pg1, src=self.pg1.remote_ip4, dst=p.remote_tun_if_host)
+        tx = self.gen_pkts(
+            self.pg1, src=self.pg1.remote_ip4, dst=p.remote_tun_if_host, count=127
+        )
         self.send_and_assert_no_replies(self.pg1, tx)
 
         self.unconfig_protect(p)
         self.unconfig_sa(p)
         self.unconfig_network(p)
 
+    def test_tun_44_async(self):
+        """IPSec SA with NULL algos using async crypto"""
+        p = self.ipv4_params
+
+        self.vapi.ipsec_set_async_mode(async_enable=True)
+        self.config_network(p)
+        self.config_sa_tra(p)
+        self.config_protect(p)
+
+        tx = self.gen_pkts(
+            self.pg1, src=self.pg1.remote_ip4, dst=p.remote_tun_if_host, count=127
+        )
+        self.send_and_assert_no_replies(self.pg1, tx)
+
+        self.unconfig_protect(p)
+        self.unconfig_sa(p)
+        self.unconfig_network(p)
+
+        self.vapi.ipsec_set_async_mode(async_enable=False)
+
 
 @tag_fixme_vpp_workers
 class TestIpsec6MultiTunIfEsp(TemplateIpsec6TunProtect, TemplateIpsec, IpsecTun6):