netdev - [PATCH v2 ipsec 2/2] xfrm: Ensure policy checked for nested ESP tunnels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220824221252.4130836-3-benedictwong@google.com>
Date:   Wed, 24 Aug 2022 22:12:52 +0000
From:   Benedict Wong <benedictwong@...gle.com>
To:     steffen.klassert@...unet.com, netdev@...r.kernel.org
Cc:     nharold@...gle.com, benedictwong@...gle.com, lorenzo@...gle.com
Subject: [PATCH v2 ipsec 2/2] xfrm: Ensure policy checked for nested ESP tunnels

This change ensures that all nested XFRM packets have their policy
checked before decryption of the next layer, so that policies are
verified at each intermediate step of the decryption process.

Notably, raw ESP/AH packets do not perform policy checks inherently,
whereas all other encapsulated packets (UDP, TCP encapsulated) do policy
checks after calling xfrm_input handling in the respective encapsulation
layer.

This is necessary especially for nested tunnels, as the IP addresses,
protocol and ports may all change, thus not matching the previous
policies. In order to ensure that packets match the relevant inbound
templates, the xfrm_policy_check should be done before handing off to
the inner XFRM protocol to decrypt and decapsulate.

In order to prevent double-checking packets both here and in the
encapsulation layers, this check is currently limited to nested
tunnel-mode transforms and checked prior to decapsulation of inner
tunnel layers (prior to hitting a nested tunnel's xfrm_input, there
is no great way to detect a nested tunnel). This is primarily a
performance consideration, as a general blanket check at the end of
xfrm_input would suffice, but may result in multiple policy checks.

Test: Tested against Android Kernel Unit Tests
Signed-off-by: Benedict Wong <benedictwong@...gle.com>
---
 net/xfrm/xfrm_input.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index bcb9ee25474b..a3b55d109836 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -586,6 +586,20 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 			goto drop;
 		}

+		/* If nested tunnel, check outer states before context is lost.
+		 * Only nested tunnels need to be checked, since IP addresses change
+		 * as a result of the tunnel mode decapsulation. Similarly, this check
+		 * is limited to nested tunnels to avoid performing another policy
+		 * check on non-nested tunnels. On success, this check also updates the
+		 * secpath's verified_cnt variable, skipping future verifications of
+		 * previously-verified secpath entries.
+		 */
+		if ((x->outer_mode.flags & XFRM_MODE_FLAG_TUNNEL) &&
+		    sp->verified_cnt < sp->len &&
+		    !xfrm_policy_check(NULL, XFRM_POLICY_IN, skb, family)) {
+			goto drop;
+		}
+
 		skb->mark = xfrm_smark_get(skb->mark, x);

 		sp->xvec[sp->len++] = x;
-- 
2.37.1.595.g718a3a8f04-goog