netdev - Re: [PATCH] net: Fix invalid ip_route_output

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: <ZB10DlJoNmGhRINM@gauss3.secunet.de>
Date:   Fri, 24 Mar 2023 10:57:34 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Hyunwoo Kim <v4bel@...ori.io>
CC:     Eric Dumazet <edumazet@...gle.com>,
        Taehee Yoo <ap420073@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Dmitry Kozlov <xeb@...l.ru>,
        David Ahern <dsahern@...nel.org>, <tudordana@...gle.com>,
        <netdev@...r.kernel.org>, <imv4bel@...il.com>
Subject: Re: [PATCH] net: Fix invalid ip_route_output_ports() call

On Tue, Mar 21, 2023 at 04:35:09AM -0700, Hyunwoo Kim wrote:
> On Tue, Mar 21, 2023 at 04:19:25AM -0700, Eric Dumazet wrote:
> > On Tue, Mar 21, 2023 at 4:14 AM Hyunwoo Kim <v4bel@...ori.io> wrote:
> > >
> > > I'm not sure what 'ip x p' means, as my understanding of XFRM is limited, sorry.
> > 
> > Since your repro does not set up a private netns.
> > 
> > Please install the iproute2 package (if not there already) and run the
> > following command
> > 
> > sudo ip x p
> > 
> > man ip
> > 
> > IP(8)                                      Linux
> >                IP(8)
> > 
> > NAME
> >        ip - show / manipulate routing, network devices, interfaces and tunnels
> > 
> > SYNOPSIS
> 
> This is the result of creating a new netns, running repro, and then running the ip x p command:
> ```
> src 255.1.0.0/0 dst 0.0.0.0/0
> 	dir out priority 0
> 	mark 0/0x6
> 	tmpl src 0.0.0.0 dst 0.0.0.0
> 		proto comp reqid 0 mode beet
> 		level 16
> 	tmpl src fc00:: dst e000:2::
> 		proto ah reqid 0 mode tunnel
> 		level 32
> 	tmpl src ac14:14bb:: dst ac14:14fa::
> 		proto route2 reqid 0 mode transport
> 		level 3
> 	tmpl src :: dst 2001::1
> 		proto ah reqid 0 mode in_trigger
> 	tmpl src ff01::1 dst 7f00:1::
> 		proto comp reqid 0 mode transport
> ```

I plan to fix this with the patch below. With this, the above policy
should be rejected. It still needs a bit of testing to make sure that
I prohibited no valid usecase with it.

---
Subject: [PATCH RFC ipsec] xfrm: Don't allow optional intermediate templates that
 changes the address family

When an optional intermediate template changes the address family,
it is unclear which family the next template should have. This can
lead to misinterpretations of IPv4/IPv6 addresses. So reject
optional intermediate templates on insertion time.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Hyunwoo Kim <v4bel@...ori.io>
Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>
---
 net/xfrm/xfrm_user.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 103af2b3e986..df4e840b630e 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1770,6 +1770,7 @@ static void copy_templates(struct xfrm_policy *xp, struct xfrm_user_tmpl *ut,
 static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family,
 			 struct netlink_ext_ack *extack)
 {
+	bool opt_family_change;
 	u16 prev_family;
 	int i;
 
@@ -1778,6 +1779,7 @@ static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family,
 		return -EINVAL;
 	}
 
+	opt_family_change = false;
 	prev_family = family;
 
 	for (i = 0; i < nr; i++) {
@@ -1791,9 +1793,16 @@ static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family,
 		if (!ut[i].family)
 			ut[i].family = family;
 
+		if (opt_family_change) {
+			NL_SET_ERR_MSG(extack, "Optional intermediate templates don't support family changes");
+			return -EINVAL;
+		}
+
 		switch (ut[i].mode) {
 		case XFRM_MODE_TUNNEL:
 		case XFRM_MODE_BEET:
+			if (ut[i].optional && ut[i].family != prev_family)
+				opt_family_change = true;
 			break;
 		default:
 			if (ut[i].family != prev_family) {
-- 
2.34.1