Patchwork [1/3] xfrm: add incoming interface to selector

login
register
mail settings
Submitter Ulrich Weber
Date Nov. 28, 2011, 8:14 p.m.
Message ID <1322511292-1413-2-git-send-email-ulrich.weber@sophos.com>
Download mbox | patch
Permalink /patch/128075/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Ulrich Weber - Nov. 28, 2011, 8:14 p.m.
Current implementation only allows to match for outgoing
interface in xfrm policy database.

This replaces the user ID (which was never used) with the
incoming interface, so both interfaces can be matched against.

Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com>
---
 include/linux/xfrm.h   |    4 ++--
 net/ipv4/xfrm4_state.c |    3 ++-
 net/ipv6/mip6.c        |    3 ++-
 net/ipv6/xfrm6_state.c |    3 ++-
 net/xfrm/xfrm_policy.c |    6 ++++--
 5 files changed, 12 insertions(+), 7 deletions(-)
David Miller - Nov. 30, 2011, midnight
From: Ulrich Weber <ulrich.weber@Sophos.com>
Date: Mon, 28 Nov 2011 21:14:50 +0100

> Current implementation only allows to match for outgoing
> interface in xfrm policy database.
> 
> This replaces the user ID (which was never used) with the
> incoming interface, so both interfaces can be matched against.
> 
> Signed-off-by: Ulrich Weber <ulrich.weber@sophos.com>

This isn't safe, because we have no idea if existing users are putting
garbage there.  So your change can break things.

You'll have to add a netlink attribute or similar.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulrich Weber - Nov. 30, 2011, 5:33 p.m.
On 30.11.2011 01:00, David Miller wrote:
>
> This isn't safe, because we have no idea if existing users are putting
> garbage there.  So your change can break things.
>
> You'll have to add a netlink attribute or similar.
But a implementation matching xfrm against UID would break
existing programs too, where sel->user is set to garbage.

I checked all common programs, they set sel->user to zero:

iproute 3.1.0: sel->user is shown if set, but its not possible to set it
openswan 2.6.37: xfrm_selector memset to zero
strongswan 4.6.1: ifindex and user set to zero
ipsec-tools 0.8.0: PF_KEY only (memset to zero in net/key/af_key.c)
ike 2.1.7: PF_KEY only
isakmpd 20041012: PF_KEY only

Cheers
Ulrich
David Miller - Nov. 30, 2011, 5:47 p.m.
From: Ulrich Weber <ulrich.weber@Sophos.com>
Date: Wed, 30 Nov 2011 18:33:54 +0100

> On 30.11.2011 01:00, David Miller wrote:
>>
>> This isn't safe, because we have no idea if existing users are putting
>> garbage there.  So your change can break things.
>>
>> You'll have to add a netlink attribute or similar.
> But a implementation matching xfrm against UID would break
> existing programs too, where sel->user is set to garbage.

Such a program would have to be used with other agents which are known
to not modify the sel->user of existing entries.

I'm not letting you reassign this existing structure member for another
use, it's been exposed to userspace for nearly 10 years.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index 22e61fd..bb1bb49 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -54,8 +54,8 @@  struct xfrm_selector {
 	__u8	prefixlen_d;
 	__u8	prefixlen_s;
 	__u8	proto;
-	int	ifindex;
-	__kernel_uid32_t	user;
+	int	oif;
+	int 	iif;
 };
 
 #define XFRM_INF (~(__u64)0)
diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c
index 9258e75..96d9b55 100644
--- a/net/ipv4/xfrm4_state.c
+++ b/net/ipv4/xfrm4_state.c
@@ -36,7 +36,8 @@  __xfrm4_init_tempsel(struct xfrm_selector *sel, const struct flowi *fl)
 	sel->prefixlen_d = 32;
 	sel->prefixlen_s = 32;
 	sel->proto = fl4->flowi4_proto;
-	sel->ifindex = fl4->flowi4_oif;
+	sel->oif = fl4->flowi4_oif;
+	sel->iif = fl4->flowi4_iif;
 }
 
 static void
diff --git a/net/ipv6/mip6.c b/net/ipv6/mip6.c
index 7e1e0fb..c0ebe40 100644
--- a/net/ipv6/mip6.c
+++ b/net/ipv6/mip6.c
@@ -248,7 +248,8 @@  static int mip6_destopt_reject(struct xfrm_state *x, struct sk_buff *skb,
 	sel.sport = xfrm_flowi_sport(fl, &fl6->uli);
 	if (sel.sport)
 		sel.sport_mask = htons(~0);
-	sel.ifindex = fl6->flowi6_oif;
+	sel.oif = fl6->flowi6_oif;
+	sel.iif = fl6->flowi6_iif;
 
 	err = km_report(net, IPPROTO_DSTOPTS, &sel,
 			(hao ? (xfrm_address_t *)&hao->addr : NULL));
diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index 3f2f7c4..bdb13df 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -37,7 +37,8 @@  __xfrm6_init_tempsel(struct xfrm_selector *sel, const struct flowi *fl)
 	sel->prefixlen_d = 128;
 	sel->prefixlen_s = 128;
 	sel->proto = fl6->flowi6_proto;
-	sel->ifindex = fl6->flowi6_oif;
+	sel->oif = fl6->flowi6_oif;
+	sel->iif = fl6->flowi6_iif;
 }
 
 static void
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 4fce1ce..648c9e7 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -66,7 +66,8 @@  __xfrm4_selector_match(const struct xfrm_selector *sel, const struct flowi *fl)
 		!((xfrm_flowi_dport(fl, &fl4->uli) ^ sel->dport) & sel->dport_mask) &&
 		!((xfrm_flowi_sport(fl, &fl4->uli) ^ sel->sport) & sel->sport_mask) &&
 		(fl4->flowi4_proto == sel->proto || !sel->proto) &&
-		(fl4->flowi4_oif == sel->ifindex || !sel->ifindex);
+		(fl4->flowi4_oif == sel->oif || !sel->oif) &&
+		(fl4->flowi4_iif == sel->iif || !sel->iif);
 }
 
 static inline int
@@ -79,7 +80,8 @@  __xfrm6_selector_match(const struct xfrm_selector *sel, const struct flowi *fl)
 		!((xfrm_flowi_dport(fl, &fl6->uli) ^ sel->dport) & sel->dport_mask) &&
 		!((xfrm_flowi_sport(fl, &fl6->uli) ^ sel->sport) & sel->sport_mask) &&
 		(fl6->flowi6_proto == sel->proto || !sel->proto) &&
-		(fl6->flowi6_oif == sel->ifindex || !sel->ifindex);
+		(fl6->flowi6_oif == sel->oif || !sel->oif) &&
+		(fl6->flowi6_iif == sel->iif || !sel->iif);
 }
 
 int xfrm_selector_match(const struct xfrm_selector *sel, const struct flowi *fl,