netdev - Re: [PATCH][IPSEC][6/7] inter address family ipsec tunnel

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: <20061203.182814.74748972.davem@davemloft.net>
Date:	Sun, 03 Dec 2006 18:28:14 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	kazunori@...azawa.org
Cc:	miika@....fi, Diego.Beltrami@...t.fi, herbert@...dor.apana.org.au,
	netdev@...r.kernel.org, usagi-core@...ux-ipv6.org
Subject: Re: [PATCH][IPSEC][6/7] inter address family ipsec tunnel

From: David Miller <davem@...emloft.net>
Date: Sun, 03 Dec 2006 17:58:47 -0800 (PST)

> Kazunori, a bug from the changes I did apply:
> 
> [  761.318131] kernel BUG at net/key/af_key.c:1925!

I found the problem, it's because of the xfrm_user.c change where
we clobber the xp->family value with ut->family.

But we never ever verified nor cared about the ut->family value
because previously templates were all of the same family as the
policy, so there was no reason to check or verify the ut->family
value.

So applications left it at zero.

This means you did no testing of the xfrm_user.c netlink changes.

We can "fix" this with some patch like the below, changing
ut->family to xp->family if it is left at zero, but it is clear
that since we've never checked this value it can be any value.
What if it is left uninitialized by the application and the
garbage value just happens to be AF_INET6 or something?

To me this means that ut->family is %100 unreliable and we cannot
count on it in any way, and we'll need to specify the family in
some other way.

BTW, is it OK to clobber the entire policy's xp->family with the
top-most ut->family?  Shouldn't the application set the policy's
family to AF_INET6 if it wants the outer-most template to be
AF_INET6?

How can changing the policy family be valid?  Doing this means we'll
interpret the selectors of the policy differently from what the
application originally provided.  This setting of xp->family therefore
cannot make any sense, it must remain at whatever value the
application gave us.

I really regret applying these patches, they are in a very bad shape
and poorly designed.  Now every openswan user will get an OOPS
when they try to bring up their tunnels with Linus's current tree.

I think instead of the patch below, I'm going to revert at least
the xfrm_user part of these changes. :-/

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 6f97665..76c7cdc 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -857,6 +857,11 @@ static void copy_templates(struct xfrm_p
 {
 	int i;

+
+	/* Backward compat for older applications.  */
+	if (ut->family == 0)
+		ut->family = xp->family;
+
 	xp->xfrm_nr = nr;
 	xp->family = ut->family;
 	for (i = 0; i < nr; i++, ut++) {

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html