[PATCH v2 ipsec] Clear secpath on loopback_xmit
This patch clears the skb->sp when transmitted over loopback. This
ensures that the loopback-ed packet does not have any secpath
information from the outbound transforms.
At present, this causes XFRM tunnel mode packets to be dropped with
XFRMINNOPOLS, due to the outbound state being in the secpath, without
a matching inbound policy. Clearing the secpath ensures that all states
added to the secpath are exclusively from the inbound processing.
Tests: xfrm tunnel mode tests added for loopback:
https://android-review.googlesource.com/c/kernel/tests/+/777328
Fixes: 8fe7ee2ba983 ("[IPsec]: Strengthen policy checks")
Signed-off-by: Benedict Wong
---
drivers/net/loopback.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 30612497643c..a6bf54df94bd 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include /* For the statistics structure. */
#include /* For ARPHRD_ETHER */
#include
@@ -82,6 +83,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
*/
skb_dst_force(skb);
+ // Clear secpath to ensure xfrm policy check not tainted by outbound
SAs.
+ secpath_reset(skb);
+
skb->protocol = eth_type_trans(skb, dev);
/* it's OK to use per_cpu_ptr() because BHs are off */
--
2.19.0.605.g01d371f741-goog
[PATCH ipsec-next] Clear secpath on loopback_xmit
This patch clears the skb->sp when transmitted over loopback. This ensures that the loopback-ed packet does not have any secpath information from the outbound transforms. At present, this causes XFRM tunnel mode packets to be dropped with XFRMINNOPOLS, due to the outbound state being in the secpath, without a matching inbound policy. Clearing the secpath ensures that all states added to the secpath are exclusively from the inbound processing. Tests: xfrm tunnel mode tests added for loopback: https://android-review.googlesource.com/c/kernel/tests/+/777328 Signed-off-by: Benedict Wong --- drivers/net/loopback.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c index 30612497643c..a6bf54df94bd 100644 --- a/drivers/net/loopback.c +++ b/drivers/net/loopback.c @@ -50,6 +50,7 @@ #include #include #include +#include #include /* For the statistics structure. */ #include /* For ARPHRD_ETHER */ #include @@ -82,6 +83,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb, */ skb_dst_force(skb); + // Clear secpath to ensure xfrm policy check not tainted by outbound SAs. + secpath_reset(skb); + skb->protocol = eth_type_trans(skb, dev); /* it's OK to use per_cpu_ptr() because BHs are off */ -- 2.19.0.605.g01d371f741-goog
[PATCH ipsec-next] xfrm: Return detailed errors from xfrmi_newlink
Currently all failure modes of xfrm interface creation return EEXIST.
This change improves the granularity of errnos provided by also
returning ENODEV or EINVAL if failures happen in looking up the
underlying interface, or a required parameter is not provided.
This change has been tested against the Android Kernel Networking Tests,
with additional xfrmi_newlink tests here:
https://android-review.googlesource.com/c/kernel/tests/+/715755
Signed-off-by: Benedict Wong
---
net/xfrm/xfrm_interface.c | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index ccfe18d67e98..481d7307ab51 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -149,14 +149,18 @@ static struct xfrm_if *xfrmi_create(struct net *net,
struct xfrm_if_parms *p)
char name[IFNAMSIZ];
int err;
- if (p->name[0])
+ if (p->name[0]) {
strlcpy(name, p->name, IFNAMSIZ);
- else
+ } else {
+ err = -EINVAL;
goto failed;
+ }
dev = alloc_netdev(sizeof(*xi), name, NET_NAME_UNKNOWN,
xfrmi_dev_setup);
- if (!dev)
+ if (!dev) {
+ err = -EAGAIN;
goto failed;
+ }
dev_net_set(dev, net);
@@ -165,8 +169,10 @@ static struct xfrm_if *xfrmi_create(struct net *net,
struct xfrm_if_parms *p)
xi->net = net;
xi->dev = dev;
xi->phydev = dev_get_by_index(net, p->link);
- if (!xi->phydev)
+ if (!xi->phydev) {
+ err = -ENODEV;
goto failed_free;
+ }
err = xfrmi_create2(dev);
if (err < 0)
@@ -179,7 +185,7 @@ static struct xfrm_if *xfrmi_create(struct net *net, struct
xfrm_if_parms *p)
failed_free:
free_netdev(dev);
failed:
- return NULL;
+ return ERR_PTR(err);
}
static struct xfrm_if *xfrmi_locate(struct net *net, struct xfrm_if_parms *p,
@@ -194,13 +200,13 @@ static struct xfrm_if *xfrmi_locate(struct net *net,
struct xfrm_if_parms *p,
xip = >next) {
if (xi->p.if_id == p->if_id) {
if (create)
- return NULL;
+ return ERR_PTR(-EEXIST);
return xi;
}
}
if (!create)
- return NULL;
+ return ERR_PTR(-ENODEV);
return xfrmi_create(net, p);
}
@@ -682,8 +688,9 @@ static int xfrmi_newlink(struct net *src_net, struct
net_device *dev,
nla_strlcpy(p->name, tb[IFLA_IFNAME], IFNAMSIZ);
- if (!xfrmi_locate(net, p, 1))
- return -EEXIST;
+ xi = xfrmi_locate(net, p, 1);
+ if (IS_ERR(xi))
+ return PTR_ERR(xi);
return 0;
}
@@ -704,11 +711,12 @@ static int xfrmi_changelink(struct net_device *dev,
struct nlattr *tb[],
xi = xfrmi_locate(net, >p, 0);
- if (xi) {
+ if (IS_ERR_OR_NULL(xi)) {
+ xi = netdev_priv(dev);
+ } else {
if (xi->dev != dev)
return -EEXIST;
- } else
- xi = netdev_priv(dev);
+ }
return xfrmi_update(xi, >p);
}
--
2.18.0.233.g985f88cf7e-goog
[PATCH ipsec-next] xfrm: Remove xfrmi interface ID from flowi
In order to remove performance impact of having the extra u32 in every
single flowi, this change removes the flowi_xfrm struct, prefering to
take the if_id as a method parameter where needed.
In the inbound direction, if_id is only needed during the
__xfrm_check_policy() function, and the if_id can be determined at that
point based on the skb. As such, xfrmi_decode_session() is only called
with the skb in __xfrm_check_policy().
In the outbound direction, the only place where if_id is needed is the
xfrm_lookup() call in xfrmi_xmit2(). With this change, the if_id is
directly passed into the xfrm_lookup_with_ifid() call. All existing
callers can still call xfrm_lookup(), which uses a default if_id of 0.
This change does not change any behavior of XFRMIs except for improving
overall system performance via flowi size reduction.
This change has been tested against the Android Kernel Networking Tests:
https://android.googlesource.com/kernel/tests/+/master/net/test
Signed-off-by: Benedict Wong
---
include/net/dst.h | 14 ++
include/net/flow.h| 9
include/net/xfrm.h| 2 +-
net/xfrm/xfrm_interface.c | 4 +-
net/xfrm/xfrm_policy.c| 98 ++-
net/xfrm/xfrm_state.c | 3 +-
6 files changed, 83 insertions(+), 47 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index b3219cd8a5a1..7f735e76ca73 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -475,6 +475,14 @@ static inline struct dst_entry *xfrm_lookup(struct net
*net,
return dst_orig;
}
+static inline struct dst_entry *
+xfrm_lookup_with_ifid(struct net *net, struct dst_entry *dst_orig,
+ const struct flowi *fl, const struct sock *sk,
+ int flags, u32 if_id)
+{
+ return dst_orig;
+}
+
static inline struct dst_entry *xfrm_lookup_route(struct net *net,
struct dst_entry *dst_orig,
const struct flowi *fl,
@@ -494,6 +502,12 @@ struct dst_entry *xfrm_lookup(struct net *net, struct
dst_entry *dst_orig,
const struct flowi *fl, const struct sock *sk,
int flags);
+struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
+ struct dst_entry *dst_orig,
+ const struct flowi *fl,
+ const struct sock *sk, int flags,
+ u32 if_id);
+
struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry
*dst_orig,
const struct flowi *fl, const struct sock
*sk,
int flags);
diff --git a/include/net/flow.h b/include/net/flow.h
index 187c9bef672f..8ce21793094e 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -26,10 +26,6 @@ struct flowi_tunnel {
__be64 tun_id;
};
-struct flowi_xfrm {
- __u32 if_id;
-};
-
struct flowi_common {
int flowic_oif;
int flowic_iif;
@@ -43,7 +39,6 @@ struct flowi_common {
#define FLOWI_FLAG_SKIP_NH_OIF 0x04
__u32 flowic_secid;
struct flowi_tunnel flowic_tun_key;
- struct flowi_xfrm xfrm;
kuid_t flowic_uid;
};
@@ -83,7 +78,6 @@ struct flowi4 {
#define flowi4_secid __fl_common.flowic_secid
#define flowi4_tun_key __fl_common.flowic_tun_key
#define flowi4_uid __fl_common.flowic_uid
-#define flowi4_xfrm__fl_common.xfrm
/* (saddr,daddr) must be grouped, same order as in IP header */
__be32 saddr;
@@ -115,7 +109,6 @@ static inline void flowi4_init_output(struct flowi4 *fl4,
int oif,
fl4->flowi4_flags = flags;
fl4->flowi4_secid = 0;
fl4->flowi4_tun_key.tun_id = 0;
- fl4->flowi4_xfrm.if_id = 0;
fl4->flowi4_uid = uid;
fl4->daddr = daddr;
fl4->saddr = saddr;
@@ -145,7 +138,6 @@ struct flowi6 {
#define flowi6_secid __fl_common.flowic_secid
#define flowi6_tun_key __fl_common.flowic_tun_key
#define flowi6_uid __fl_common.flowic_uid
-#define flowi6_xfrm__fl_common.xfrm
struct in6_addr daddr;
struct in6_addr saddr;
/* Note: flowi6_tos is encoded in flowlabel, too. */
@@ -193,7 +185,6 @@ struct flowi {
#define flowi_secidu.__fl_common.flowic_secid
#define flowi_tun_key u.__fl_common.flowic_tun_key
#define flowi_uid u.__fl_common.flowic_uid
-#define flowi_xfrm u.__fl_common.xfrm
} __attribute__((__aligned__(BITS_PER_LONG/8)));
static inline struct flowi *flowi4_to_flowi(struct flowi4 *fl4)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 1350e2cf0749..ca820945f30c 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1557,7 +1557,7
[RFC ipsec-next] xfrm: Remove xfrmi interface ID from flowi
In order to remove performance impact of having the extra u32 in every
single flowi, this change removes the flowi_xfrm struct, prefering to
take the if_id as a method parameter where needed.
In the inbound direction, if_id is only needed during the
__xfrm_check_policy() function, and the if_id can be determined at that
point based on the skb. As such, xfrmi_decode_session() is only called
with the skb in __xfrm_check_policy().
In the outbound direction, the only place where if_id is needed is the
xfrm_lookup() call in xfrmi_xmit2(). With this change, the if_id is
directly passed into the xfrm_lookup_with_ifid() call. All existing
callers can still call xfrm_lookup(), which uses a default if_id of 0.
This change does not change any behavior of XFRMIs except for improving
overall system performance via flowi size reduction.
This change has been tested against the Android Kernel Networking Tests:
https://android.googlesource.com/kernel/tests/+/master/net/test
Signed-off-by: Benedict Wong
---
include/net/dst.h | 14 ++
include/net/flow.h| 7 ---
include/net/xfrm.h| 2 +-
net/xfrm/xfrm_interface.c | 4 +-
net/xfrm/xfrm_policy.c| 98 ++-
net/xfrm/xfrm_state.c | 3 +-
6 files changed, 83 insertions(+), 45 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index b3219cd8a5a1..7f735e76ca73 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -475,6 +475,14 @@ static inline struct dst_entry *xfrm_lookup(struct net
*net,
return dst_orig;
}
+static inline struct dst_entry *
+xfrm_lookup_with_ifid(struct net *net, struct dst_entry *dst_orig,
+ const struct flowi *fl, const struct sock *sk,
+ int flags, u32 if_id)
+{
+ return dst_orig;
+}
+
static inline struct dst_entry *xfrm_lookup_route(struct net *net,
struct dst_entry *dst_orig,
const struct flowi *fl,
@@ -494,6 +502,12 @@ struct dst_entry *xfrm_lookup(struct net *net, struct
dst_entry *dst_orig,
const struct flowi *fl, const struct sock *sk,
int flags);
+struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
+ struct dst_entry *dst_orig,
+ const struct flowi *fl,
+ const struct sock *sk, int flags,
+ u32 if_id);
+
struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry
*dst_orig,
const struct flowi *fl, const struct sock
*sk,
int flags);
diff --git a/include/net/flow.h b/include/net/flow.h
index 187c9bef672f..cb1205c526ef 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -26,10 +26,6 @@ struct flowi_tunnel {
__be64 tun_id;
};
-struct flowi_xfrm {
- __u32 if_id;
-};
-
struct flowi_common {
int flowic_oif;
int flowic_iif;
@@ -43,7 +39,6 @@ struct flowi_common {
#define FLOWI_FLAG_SKIP_NH_OIF 0x04
__u32 flowic_secid;
struct flowi_tunnel flowic_tun_key;
- struct flowi_xfrm xfrm;
kuid_t flowic_uid;
};
@@ -115,7 +110,6 @@ static inline void flowi4_init_output(struct flowi4 *fl4,
int oif,
fl4->flowi4_flags = flags;
fl4->flowi4_secid = 0;
fl4->flowi4_tun_key.tun_id = 0;
- fl4->flowi4_xfrm.if_id = 0;
fl4->flowi4_uid = uid;
fl4->daddr = daddr;
fl4->saddr = saddr;
@@ -193,7 +187,6 @@ struct flowi {
#define flowi_secidu.__fl_common.flowic_secid
#define flowi_tun_key u.__fl_common.flowic_tun_key
#define flowi_uid u.__fl_common.flowic_uid
-#define flowi_xfrm u.__fl_common.xfrm
} __attribute__((__aligned__(BITS_PER_LONG/8)));
static inline struct flowi *flowi4_to_flowi(struct flowi4 *fl4)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index a5378613a49c..0721d899ce00 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1557,7 +1557,7 @@ struct xfrm_state *xfrm_state_find(const xfrm_address_t
*daddr,
const struct flowi *fl,
struct xfrm_tmpl *tmpl,
struct xfrm_policy *pol, int *err,
- unsigned short family);
+ unsigned short family, u32 if_id);
struct xfrm_state *xfrm_stateonly_find(struct net *net, u32 mark, u32 if_id,
xfrm_address_t *daddr,
xfrm_address_t *saddr,
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 31cb1c7e3881..ccfe18d67e98 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interfac
Re: [PATCH RFC v2 ipsec-next 0/3] Virtual xfrm interfaces
This set of patches entire resolves issues Android was having with VTIs, specifically that we need multiple IPsec tunnels with the same source/destination endpoints. These tests pass the following regression tests (https://android-review.googlesource.com/c/kernel/tests/+/588259/24): - Outbound encapsulation - Inbound decapsulation - Rekey - ICMP error path - Netfilter rejections of outbound paths. -- Benedict Wong On Tue, Jun 12, 2018 at 12:56 AM Steffen Klassert wrote: > > This patchset introduces new virtual xfrm interfaces. > The design of virtual xfrm interfaces interfaces was > discussed at the Linux IPsec workshop 2018. This patchset > implements these interfaces as the IPsec userspace and > kernel developers agreed. The purpose of these interfaces > is to overcome the design limitations that the existing > VTI devices have. > > The main limitations that we see with the current VTI are the > following: > > - VTI interfaces are L3 tunnels with configurable endpoints. > For xfrm, the tunnel endpoint are already determined by the SA. > So the VTI tunnel endpoints must be either the same as on the > SA or wildcards. In case VTI tunnel endpoints are same as on > the SA, we get a one to one correlation between the SA and > the tunnel. So each SA needs its own tunnel interface. > > On the other hand, we can have only one VTI tunnel with > wildcard src/dst tunnel endpoints in the system because the > lookup is based on the tunnel endpoints. The existing tunnel > lookup won't work with multiple tunnels with wildcard > tunnel endpoints. Some usecases require more than on > VTI tunnel of this type, for example if somebody has multiple > namespaces and every namespace requires such a VTI. > > - VTI needs separate interfaces for IPv4 and IPv6 tunnels. > So when routing to a VTI, we have to know to which address > family this traffic class is going to be encapsulated. > This is a lmitation because it makes routing more complex > and it is not always possible to know what happens behind the > VTI, e.g. when the VTI is move to some namespace. > > - VTI works just with tunnel mode SAs. We need generic interfaces > that ensures transfomation, regardless of the xfrm mode and > the encapsulated address family. > > - VTI is configured with a combination GRE keys and xfrm marks. > With this we have to deal with some extra cases in the generic > tunnel lookup because the GRE keys on the VTI are actually > not GRE keys, the GRE keys were just reused for something else. > All extensions to the VTI interfaces would require to add > even more complexity to the generic tunnel lookup. > > To overcome this, we started with the following design goal: > > - It should be possible to tunnel IPv4 and IPv6 through the same > interface. > > - No limitation on xfrm mode (tunnel, transport and beet). > > - Should be a generic virtual interface that ensures IPsec > transformation, no need to know what happens behind the > interface. > > - Interfaces should be configured with a new key that must match a > new policy/SA lookup key. > > - The lookup logic should stay in the xfrm codebase, no need to > change or extend generic routing and tunnel lookups. > > - Should be possible to use IPsec hardware offloads of the underlying > interface. > > Changes from v1: > > - Document the limitations of VTI interfaces and the design of > the new xfrm interfaces more explicit in the commit messages. > > - No code changes.
