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:dac9e566cd
Signed-off-by: Matthew Smith <mgsmith@netgate.com> Change-Id: I69e951f22044051eb8557da187cb58f5535b54bf (cherry picked from commitff71939c30
)
This commit is contained in:

committed by
Andrew Yourtchenko

parent
1200c799d0
commit
a541cfd31d
@ -607,6 +607,7 @@ esp_encrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
|
|||||||
u32 current_sa_bytes = 0, spi = 0;
|
u32 current_sa_bytes = 0, spi = 0;
|
||||||
u8 esp_align = 4, iv_sz = 0, icv_sz = 0;
|
u8 esp_align = 4, iv_sz = 0, icv_sz = 0;
|
||||||
ipsec_sa_t *sa0 = 0;
|
ipsec_sa_t *sa0 = 0;
|
||||||
|
u8 sa_drop_no_crypto = 0;
|
||||||
vlib_buffer_t *lb;
|
vlib_buffer_t *lb;
|
||||||
vnet_crypto_op_t **crypto_ops = &ptd->crypto_ops;
|
vnet_crypto_op_t **crypto_ops = &ptd->crypto_ops;
|
||||||
vnet_crypto_op_t **integ_ops = &ptd->integ_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);
|
sa0 = ipsec_sa_get (sa_index0);
|
||||||
current_sa_index = sa_index0;
|
current_sa_index = sa_index0;
|
||||||
|
|
||||||
if (PREDICT_FALSE ((sa0->crypto_alg == IPSEC_CRYPTO_ALG_NONE &&
|
sa_drop_no_crypto = ((sa0->crypto_alg == IPSEC_CRYPTO_ALG_NONE &&
|
||||||
sa0->integ_alg == IPSEC_INTEG_ALG_NONE) &&
|
sa0->integ_alg == IPSEC_INTEG_ALG_NONE) &&
|
||||||
!ipsec_sa_is_set_NO_ALGO_NO_DROP (sa0)))
|
!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;
|
|
||||||
}
|
|
||||||
vlib_prefetch_combined_counter (&ipsec_sa_counters, thread_index,
|
vlib_prefetch_combined_counter (&ipsec_sa_counters, thread_index,
|
||||||
current_sa_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);
|
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))
|
if (PREDICT_FALSE ((u16) ~0 == sa0->thread_index))
|
||||||
{
|
{
|
||||||
/* this is the first packet to use this SA, claim the SA
|
/* this is the first packet to use this SA, claim the SA
|
||||||
|
@ -1231,13 +1231,35 @@ class TestIpsec4TunIfEspNoAlgo(TemplateIpsec4TunProtect, TemplateIpsec, IpsecTun
|
|||||||
self.config_sa_tra(p)
|
self.config_sa_tra(p)
|
||||||
self.config_protect(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.send_and_assert_no_replies(self.pg1, tx)
|
||||||
|
|
||||||
self.unconfig_protect(p)
|
self.unconfig_protect(p)
|
||||||
self.unconfig_sa(p)
|
self.unconfig_sa(p)
|
||||||
self.unconfig_network(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
|
@tag_fixme_vpp_workers
|
||||||
class TestIpsec6MultiTunIfEsp(TemplateIpsec6TunProtect, TemplateIpsec, IpsecTun6):
|
class TestIpsec6MultiTunIfEsp(TemplateIpsec6TunProtect, TemplateIpsec, IpsecTun6):
|
||||||
|
Reference in New Issue
Block a user