From e7b6eb419efc6972c8af8f5e67f8e4f02db41420 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 30 Jun 2022 06:42:33 +0000 Subject: [PATCH] FROMLIST: xfrm: Skip checking of already-verified secpath entries This change fixes a bug where inbound packets to nested IPsec tunnels fails to pass policy checks due to the inner tunnel's policy checks not having a reference to the outer policy/template. This causes the policy check to fail, since the first entries in the secpath correlate to the outer tunnel, while the templates being verified are for the inner tunnel. In order to ensure that the appropriate policy and template context is searchable, the policy checks must be done incrementally between each decryption step. As such, this marks secpath entries as having been successfully matched, skipping them (treating as optional) on subsequent policy checks By skipping the immediate error return in the case where the secpath entry had previously been validated, this change allows secpath entries that matched a policy/template previously, while still requiring that each searched template find a match in the secpath. For security: - All templates must have matching secpath entries - Unchanged by current patch; templates that do not match any secpath entry still return -1. This patch simply allows skipping earlier blocks of verified secpath entries - All entries (except trailing transport mode entries) must have a matching template - Unvalidated entries, including transport-mode entries still return the errored index if it does not match the correct template. Bug: 236423446 Bug: 277711867 Test: Tested against Android Kernel Unit Tests Test: Tested against Android CTS Link: https://lore.kernel.org/netdev/20220824221252.4130836-2-benedictwong@google.com/ [benedictwong: fixed minor style issues] Signed-off-by: Benedict Wong (cherry picked from commit 970e02667c9689f2fe6ceccfd80596c4b8a368a4) Merged-In: Ic32831cb00151d0de2e465f18ec37d5f7b680e54 Change-Id: Ic32831cb00151d0de2e465f18ec37d5f7b680e54 --- include/net/xfrm.h | 1 + net/xfrm/xfrm_input.c | 1 + net/xfrm/xfrm_policy.c | 12 ++++++++++++ 3 files changed, 14 insertions(+) diff --git a/include/net/xfrm.h b/include/net/xfrm.h index f2fe2b5cd113..bcd57617eca8 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -1026,6 +1026,7 @@ struct xfrm_offload { struct sec_path { int len; int olen; + int verified_cnt; struct xfrm_state *xvec[XFRM_MAX_DEPTH]; struct xfrm_offload ovec[XFRM_MAX_OFFLOAD_DEPTH]; diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c index 05268359cffe..f33fc647b55f 100644 --- a/net/xfrm/xfrm_input.c +++ b/net/xfrm/xfrm_input.c @@ -130,6 +130,7 @@ struct sec_path *secpath_set(struct sk_buff *skb) memset(sp->ovec, 0, sizeof(sp->ovec)); sp->olen = 0; sp->len = 0; + sp->verified_cnt = 0; return sp; } diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index ba58b963f482..96d7987ae06a 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -3274,6 +3274,13 @@ xfrm_policy_ok(const struct xfrm_tmpl *tmpl, const struct sec_path *sp, int star if (xfrm_state_ok(tmpl, sp->xvec[idx], family)) return ++idx; if (sp->xvec[idx]->props.mode != XFRM_MODE_TRANSPORT) { + if (idx < sp->verified_cnt) { + /* Secpath entry previously verified, consider optional and + * continue searching + */ + continue; + } + if (start == -1) start = -2-idx; break; @@ -3652,6 +3659,9 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, * Order is _important_. Later we will implement * some barriers, but at the moment barriers * are implied between each two transformations. + * Upon success, marks secpath entries as having been + * verified to allow them to be skipped in future policy + * checks (e.g. nested tunnels). */ for (i = xfrm_nr-1, k = 0; i >= 0; i--) { k = xfrm_policy_ok(tpp[i], sp, k, family); @@ -3670,6 +3680,8 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, } xfrm_pols_put(pols, npols); + sp->verified_cnt = k; + return 1; } XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLBLOCK);