| 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
| ||
|
Date: Mon, 15 Sep 2014 10:09:41 +0200
From: Steffen Klassert <steffen.klassert@...unet.com>
To: Konstantinos Kolelis <k.kolelis@...rix.com>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>,
<kuznet@....inr.ac.ru>, <jmorris@...ei.org>,
<yoshfuji@...ux-ipv6.org>, <kaber@...sh.net>,
<herbert@...dor.apana.org.au>
Subject: Re: [BUG REPORT] Unencrypted packets after SNAT, allthough
IPSEC-Policies are present
On Fri, Sep 12, 2014 at 11:31:43AM +0200, Steffen Klassert wrote:
> On Thu, Sep 11, 2014 at 03:11:17PM +0200, Konstantinos Kolelis wrote:
> > Am 11.09.2014 13:54, schrieb Steffen Klassert:
> > > On Wed, Sep 10, 2014 at 07:26:53PM +0200, Konstantinos Kolelis wrote:
> > >> Hi all,
> > >>
> > >> i' ve observed a problem with xfrm lookups, SNAT, blackhole route and
> > >> missing SAs.
> > >> The problem occures with all Kernels above 3.6.x and might has to do
> > >> with the changes in
> > >> ip4_blackhole_route() function in net/route.c.
> > >
> > > Thanks for the report!
> > >
> > > Is kernel v3.6 the first kernel with this issue? It seems that
> > > we have this problem already longer, at least if my analysis
> > > is correct.
> > >
> >
> > It worked until Kernel 3.4.103, i did not check with v3.5 though.
>
> Hm. I thought the problem exists already for longer, so I did
> a bisect. To my surprise, the following commit introduced the bug:
>
> commit a263b3093641fb1ec377582c90986a7fd0625184
> Author: David S. Miller <davem@...emloft.net>
> Date: Mon Jul 2 02:02:15 2012 -0700
>
> ipv4: Make neigh lookups directly in output packet path.
>
> Do not use the dst cached neigh, we'll be getting rid of that.
>
> Signed-off-by: David S. Miller <davem@...emloft.net>
>
> But this is not really the offending commit. As I said, the problem
> was introduced by git commit 2774c131 ("xfrm: Handle blackhole route
> creation via afinfo.") because the assumption that dst_output() is
> called always after a xfrm_lookup() is wrong. On postrouting nat,
> dst_output() is not called for blackholed packets. But we were in luck
> because ip_finish_output2() tried to use a dst cached neigh entry.
> On blackhole routes we did not cache neigh entries, so the packets
> were dropped in the last moment. The above commit finally opened the
> door by replacing the usage of a dst cached neigh entry by a direct
> lookup.
>
> So we need to ensure that a blackhole route is generated only by the
> route lookup functions.
Can you please try the patch below? This should fix the default case
where xfrm_larval_drop is true. The xfrm_larval_drop false case needs
a separate fix.
Subject: [PATCH] xfrm: Generate blackhole routes only from route lookup
functions
Currently we genarate a blackhole route route whenever we have
matching policies but can not resolve the states. Here we assume
that dst_output() is called to kill the balckholed packets.
Unfortunately this assumption is not true in all cases, so
it is possible that these packets leave the system unwanted.
We fix this by generating blackhole routes only from the
route lookup functions, here we can guarantee a call to
dst_output() afterwards.
Fixes: 2774c131b1d ("xfrm: Handle blackhole route creation via afinfo.")
Reported-by: Konstantinos Kolelis <k.kolelis@...rix.com>
Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>
---
include/net/dst.h | 15 ++++++++++++++-
net/ipv4/route.c | 6 +++---
net/ipv6/ip6_output.c | 4 ++--
net/xfrm/xfrm_policy.c | 16 +++++++++++++++-
4 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index 71c60f4..fa11c90 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -490,7 +490,16 @@ static inline struct dst_entry *xfrm_lookup(struct net *net,
int flags)
{
return dst_orig;
-}
+}
+
+static inline struct dst_entry *xfrm_lookup_route(struct net *net,
+ struct dst_entry *dst_orig,
+ const struct flowi *fl,
+ struct sock *sk,
+ int flags)
+{
+ return dst_orig;
+}
static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
{
@@ -502,6 +511,10 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
const struct flowi *fl, struct sock *sk,
int flags);
+struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry *dst_orig,
+ const struct flowi *fl, struct sock *sk,
+ int flags);
+
/* skb attached with this dst needs transformation if dst->xfrm is valid */
static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
{
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index eaa4b00..173e7ea 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2265,9 +2265,9 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
return rt;
if (flp4->flowi4_proto)
- rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
- flowi4_to_flowi(flp4),
- sk, 0);
+ rt = (struct rtable *)xfrm_lookup_route(net, &rt->dst,
+ flowi4_to_flowi(flp4),
+ sk, 0);
return rt;
}
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 315a55d..0a3448b 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1009,7 +1009,7 @@ struct dst_entry *ip6_dst_lookup_flow(struct sock *sk, struct flowi6 *fl6,
if (final_dst)
fl6->daddr = *final_dst;
- return xfrm_lookup(sock_net(sk), dst, flowi6_to_flowi(fl6), sk, 0);
+ return xfrm_lookup_route(sock_net(sk), dst, flowi6_to_flowi(fl6), sk, 0);
}
EXPORT_SYMBOL_GPL(ip6_dst_lookup_flow);
@@ -1041,7 +1041,7 @@ struct dst_entry *ip6_sk_dst_lookup_flow(struct sock *sk, struct flowi6 *fl6,
if (final_dst)
fl6->daddr = *final_dst;
- return xfrm_lookup(sock_net(sk), dst, flowi6_to_flowi(fl6), sk, 0);
+ return xfrm_lookup_route(sock_net(sk), dst, flowi6_to_flowi(fl6), sk, 0);
}
EXPORT_SYMBOL_GPL(ip6_sk_dst_lookup_flow);
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index beeed60..e041822 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2138,7 +2138,8 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
xfrm_pols_put(pols, drop_pols);
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
- return make_blackhole(net, family, dst_orig);
+ err = -EREMOTE;
+ goto error;
}
err = -EAGAIN;
@@ -2195,6 +2196,19 @@ dropdst:
}
EXPORT_SYMBOL(xfrm_lookup);
+struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry *dst_orig,
+ const struct flowi *fl,
+ struct sock *sk, int flags)
+{
+ struct dst_entry *dst = xfrm_lookup(net, dst_orig, fl, sk, flags);
+
+ if (IS_ERR(dst) && PTR_ERR(dst) == -EREMOTE)
+ return make_blackhole(net, dst_orig->ops->family, dst_orig);
+
+ return dst;
+}
+EXPORT_SYMBOL(xfrm_lookup_route);
+
static inline int
xfrm_secpath_reject(int idx, struct sk_buff *skb, const struct flowi *fl)
{
--
1.9.1
--
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
Powered by blists - more mailing lists