netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfrm: Workaround incompatibility of ESN and async crypto
@ 2012-09-04 10:03 Steffen Klassert
  2012-09-04 12:31 ` Herbert Xu
  0 siblings, 1 reply; 3+ messages in thread
From: Steffen Klassert @ 2012-09-04 10:03 UTC (permalink / raw)
  To: David Miller, Herbert Xu; +Cc: netdev

ESN for esp is defined in RFC 4303. This RFC assumes that the
sequence number counters are always up to date. However,
this is not true if an async crypto algorithm is employed.

If the sequence number counters are not up to date on sequence
number check, we may incorrectly update the upper 32 bit of
the sequence number. This leads to a DOS.

We workaround this by comparing the upper sequence number,
(used for authentication) with the upper sequence number
computed after the async processing. We drop the packet
if these numbers are different.

To do this, we introduce a recheck function that does this
check in the ESN case.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h     |    3 +++
 net/xfrm/xfrm_input.c  |    2 +-
 net/xfrm/xfrm_replay.c |   15 +++++++++++++++
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 976a81a..639dd13 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -273,6 +273,9 @@ struct xfrm_replay {
 	int	(*check)(struct xfrm_state *x,
 			 struct sk_buff *skb,
 			 __be32 net_seq);
+	int	(*recheck)(struct xfrm_state *x,
+			   struct sk_buff *skb,
+			   __be32 net_seq);
 	void	(*notify)(struct xfrm_state *x, int event);
 	int	(*overflow)(struct xfrm_state *x, struct sk_buff *skb);
 };
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 54a0dc2..ab2bb42 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -212,7 +212,7 @@ resume:
 		/* only the first xfrm gets the encap type */
 		encap_type = 0;
 
-		if (async && x->repl->check(x, skb, seq)) {
+		if (async && x->repl->recheck(x, skb, seq)) {
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATESEQERROR);
 			goto drop_unlock;
 		}
diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index 2f6d11d..3efb07d 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -420,6 +420,18 @@ err:
 	return -EINVAL;
 }
 
+static int xfrm_replay_recheck_esn(struct xfrm_state *x,
+				   struct sk_buff *skb, __be32 net_seq)
+{
+	if (unlikely(XFRM_SKB_CB(skb)->seq.input.hi !=
+		     htonl(xfrm_replay_seqhi(x, net_seq)))) {
+			x->stats.replay_window++;
+			return -EINVAL;
+	}
+
+	return xfrm_replay_check_esn(x, skb, net_seq);
+}
+
 static void xfrm_replay_advance_esn(struct xfrm_state *x, __be32 net_seq)
 {
 	unsigned int bitnr, nr, i;
@@ -479,6 +491,7 @@ static void xfrm_replay_advance_esn(struct xfrm_state *x, __be32 net_seq)
 static struct xfrm_replay xfrm_replay_legacy = {
 	.advance	= xfrm_replay_advance,
 	.check		= xfrm_replay_check,
+	.recheck	= xfrm_replay_check,
 	.notify		= xfrm_replay_notify,
 	.overflow	= xfrm_replay_overflow,
 };
@@ -486,6 +499,7 @@ static struct xfrm_replay xfrm_replay_legacy = {
 static struct xfrm_replay xfrm_replay_bmp = {
 	.advance	= xfrm_replay_advance_bmp,
 	.check		= xfrm_replay_check_bmp,
+	.recheck	= xfrm_replay_check_bmp,
 	.notify		= xfrm_replay_notify_bmp,
 	.overflow	= xfrm_replay_overflow_bmp,
 };
@@ -493,6 +507,7 @@ static struct xfrm_replay xfrm_replay_bmp = {
 static struct xfrm_replay xfrm_replay_esn = {
 	.advance	= xfrm_replay_advance_esn,
 	.check		= xfrm_replay_check_esn,
+	.recheck	= xfrm_replay_recheck_esn,
 	.notify		= xfrm_replay_notify_bmp,
 	.overflow	= xfrm_replay_overflow_esn,
 };
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] xfrm: Workaround incompatibility of ESN and async crypto
  2012-09-04 10:03 [PATCH] xfrm: Workaround incompatibility of ESN and async crypto Steffen Klassert
@ 2012-09-04 12:31 ` Herbert Xu
  2012-09-04 18:10   ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2012-09-04 12:31 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, netdev

On Tue, Sep 04, 2012 at 12:03:29PM +0200, Steffen Klassert wrote:
> ESN for esp is defined in RFC 4303. This RFC assumes that the
> sequence number counters are always up to date. However,
> this is not true if an async crypto algorithm is employed.
> 
> If the sequence number counters are not up to date on sequence
> number check, we may incorrectly update the upper 32 bit of
> the sequence number. This leads to a DOS.
> 
> We workaround this by comparing the upper sequence number,
> (used for authentication) with the upper sequence number
> computed after the async processing. We drop the packet
> if these numbers are different.
> 
> To do this, we introduce a recheck function that does this
> check in the ESN case.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

It took me a while to understand the problem but Steffen was
correct again as usual.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] xfrm: Workaround incompatibility of ESN and async crypto
  2012-09-04 12:31 ` Herbert Xu
@ 2012-09-04 18:10   ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2012-09-04 18:10 UTC (permalink / raw)
  To: herbert; +Cc: steffen.klassert, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 4 Sep 2012 05:31:55 -0700

> On Tue, Sep 04, 2012 at 12:03:29PM +0200, Steffen Klassert wrote:
>> ESN for esp is defined in RFC 4303. This RFC assumes that the
>> sequence number counters are always up to date. However,
>> this is not true if an async crypto algorithm is employed.
>> 
>> If the sequence number counters are not up to date on sequence
>> number check, we may incorrectly update the upper 32 bit of
>> the sequence number. This leads to a DOS.
>> 
>> We workaround this by comparing the upper sequence number,
>> (used for authentication) with the upper sequence number
>> computed after the async processing. We drop the packet
>> if these numbers are different.
>> 
>> To do this, we introduce a recheck function that does this
>> check in the ESN case.
>> 
>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> It took me a while to understand the problem but Steffen was
> correct again as usual.

Applied, thanks everyone.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-09-04 18:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-04 10:03 [PATCH] xfrm: Workaround incompatibility of ESN and async crypto Steffen Klassert
2012-09-04 12:31 ` Herbert Xu
2012-09-04 18:10   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).