* [PATCH] xfrm: fix state migration replay sequence numbers
@ 2017-05-18 14:39 Antony Antony
2017-05-18 15:55 ` Richard Guy Briggs
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Antony Antony @ 2017-05-18 14:39 UTC (permalink / raw)
To: netdev, Herbert Xu, Steffen Klassert; +Cc: Richard Guy Briggs, Antony Antony
[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]
During xfrm migration replay and preplay sequence numbers are not
copied from the previous state.
Here is tcpdump output showing the problem.
10.0.10.46 is running vanilla kernel, IKE/IPsec responder.
After the migration it sent wrong sequence number, reset to 1.
The migration is from 10.0.0.52 to 10.0.0.53.
IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7cf), length 136
IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7cf), length 136
IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d0), length 136
IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7d0), length 136
IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d1), length 136
NOTE: next sequence is wrong 0x1
IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x1), length 136
IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d2), length 136
IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x2), length 136
The attached patch fix it by copying replay and preplay.
regards,
-antony
Antony Antony (1):
xfrm: fix state migration replay sequence numbers
net/xfrm/xfrm_state.c | 2 ++
1 file changed, 2 insertions(+)
--
2.9.3
[-- Attachment #2: 0001-xfrm-fix-state-migration.patch --]
[-- Type: text/plain, Size: 1017 bytes --]
>From 1241e8b4c38ad2bf7399599165f763af38aba8d9 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony@phenome.org>
Date: Thu, 18 May 2017 12:19:32 +0200
Subject: [PATCH] xfrm: fix state migration copy replay sequence numbers
To: netdev@vger.kernel.org, Herbert Xu <herbert@gondor.apana.org.au>, Steffen Klassert <steffen.klassert@secunet.com>
Cc: Richard Guy Briggs <rgb@tricolour.ca>
During xfrm migration copy replay and preplay sequence numbers
from the previous state.
Signed-off-by: Antony Antony <antony@phenome.org>
---
net/xfrm/xfrm_state.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index fc3c5aa..2e291bc 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1383,6 +1383,8 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig)
x->curlft.add_time = orig->curlft.add_time;
x->km.state = orig->km.state;
x->km.seq = orig->km.seq;
+ x->replay = orig->replay;
+ x->preplay = orig->preplay;
return x;
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xfrm: fix state migration replay sequence numbers
2017-05-18 14:39 [PATCH] xfrm: fix state migration replay sequence numbers Antony Antony
@ 2017-05-18 15:55 ` Richard Guy Briggs
2017-05-19 9:59 ` Steffen Klassert
2017-05-19 10:47 ` [PATCH v2] xfrm: fix state migration copy " Antony Antony
2 siblings, 0 replies; 5+ messages in thread
From: Richard Guy Briggs @ 2017-05-18 15:55 UTC (permalink / raw)
To: Antony Antony; +Cc: netdev, Herbert Xu, Steffen Klassert
On 2017-05-18 16:39, Antony Antony wrote:
> During xfrm migration replay and preplay sequence numbers are not
> copied from the previous state.
>
> Here is tcpdump output showing the problem.
> 10.0.10.46 is running vanilla kernel, IKE/IPsec responder.
> After the migration it sent wrong sequence number, reset to 1.
> The migration is from 10.0.0.52 to 10.0.0.53.
>
> IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7cf), length 136
> IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7cf), length 136
> IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d0), length 136
> IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7d0), length 136
>
> IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
> IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
> IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
> IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
>
> IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d1), length 136
>
> NOTE: next sequence is wrong 0x1
>
> IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x1), length 136
> IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d2), length 136
> IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x2), length 136
>
> The attached patch fix it by copying replay and preplay.
>
> regards,
> -antony
>
> Antony Antony (1):
> xfrm: fix state migration replay sequence numbers
>
> net/xfrm/xfrm_state.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --
> 2.9.3
>
> >From 1241e8b4c38ad2bf7399599165f763af38aba8d9 Mon Sep 17 00:00:00 2001
> From: Antony Antony <antony@phenome.org>
> Date: Thu, 18 May 2017 12:19:32 +0200
> Subject: [PATCH] xfrm: fix state migration copy replay sequence numbers
> To: netdev@vger.kernel.org, Herbert Xu <herbert@gondor.apana.org.au>, Steffen Klassert <steffen.klassert@secunet.com>
> Cc: Richard Guy Briggs <rgb@tricolour.ca>
>
> During xfrm migration copy replay and preplay sequence numbers
> from the previous state.
>
> Signed-off-by: Antony Antony <antony@phenome.org>
> ---
> net/xfrm/xfrm_state.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index fc3c5aa..2e291bc 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1383,6 +1383,8 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig)
> x->curlft.add_time = orig->curlft.add_time;
> x->km.state = orig->km.state;
> x->km.seq = orig->km.seq;
> + x->replay = orig->replay;
> + x->preplay = orig->preplay;
>
> return x;
>
> --
> 2.9.3
This looks reasonable to me. With a bit more out-of-band information from
Antony and Paul Wouters we have:
https://tools.ietf.org/html/rfc4555#section-3.5
so while it is not explicit about what is to be copied, it only indicates that
the IPsec SA is to be updated with the new address whereas this implementation
creates a new IPsec SA and copies over the values, missing some.
(Note: using "git format-patch --cover-letter --cc ... -o <dir>" and "git
send-email --to ... <dir>" work really well together.)
Reviewed-by: Richard Guy Briggs <rgb@tricolour.ca>
slainte mhath, RGB
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfrm: fix state migration replay sequence numbers
2017-05-18 14:39 [PATCH] xfrm: fix state migration replay sequence numbers Antony Antony
2017-05-18 15:55 ` Richard Guy Briggs
@ 2017-05-19 9:59 ` Steffen Klassert
2017-05-19 10:47 ` [PATCH v2] xfrm: fix state migration copy " Antony Antony
2 siblings, 0 replies; 5+ messages in thread
From: Steffen Klassert @ 2017-05-19 9:59 UTC (permalink / raw)
To: Antony Antony; +Cc: netdev, Herbert Xu, Richard Guy Briggs
On Thu, May 18, 2017 at 04:39:53PM +0200, Antony Antony wrote:
> During xfrm migration replay and preplay sequence numbers are not
> copied from the previous state.
>
> Here is tcpdump output showing the problem.
> 10.0.10.46 is running vanilla kernel, IKE/IPsec responder.
> After the migration it sent wrong sequence number, reset to 1.
> The migration is from 10.0.0.52 to 10.0.0.53.
>
> IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7cf), length 136
> IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7cf), length 136
> IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d0), length 136
> IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7d0), length 136
>
> IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
> IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
> IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
> IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
>
> IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d1), length 136
>
> NOTE: next sequence is wrong 0x1
>
> IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x1), length 136
> IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d2), length 136
> IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x2), length 136
>
> The attached patch fix it by copying replay and preplay.
The patch looks ok, but please do a v2 and put the above
informations into the commit message. This is usefull
information that we would loose otherwise.
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] xfrm: fix state migration copy replay sequence numbers
2017-05-18 14:39 [PATCH] xfrm: fix state migration replay sequence numbers Antony Antony
2017-05-18 15:55 ` Richard Guy Briggs
2017-05-19 9:59 ` Steffen Klassert
@ 2017-05-19 10:47 ` Antony Antony
2017-05-19 11:19 ` Steffen Klassert
2 siblings, 1 reply; 5+ messages in thread
From: Antony Antony @ 2017-05-19 10:47 UTC (permalink / raw)
To: netdev; +Cc: Antony Antony, Richard Guy Briggs, Herbert Xu, Steffen Klassert
During xfrm migration copy replay and preplay sequence numbers
from the previous state.
Here is a tcpdump output showing the problem.
10.0.10.46 is running vanilla kernel, is the IKE/IPsec responder.
After the migration it sent wrong sequence number, reset to 1.
The migration is from 10.0.0.52 to 10.0.0.53.
IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7cf), length 136
IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7cf), length 136
IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d0), length 136
IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7d0), length 136
IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d1), length 136
NOTE: next sequence is wrong 0x1
IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x1), length 136
IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d2), length 136
IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x2), length 136
Signed-off-by: Antony Antony <antony@phenome.org>
---
Changes in v2:
- include tcpdump output showing the problem in the commit message.
net/xfrm/xfrm_state.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index fc3c5aa..2e291bc 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1383,6 +1383,8 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig)
x->curlft.add_time = orig->curlft.add_time;
x->km.state = orig->km.state;
x->km.seq = orig->km.seq;
+ x->replay = orig->replay;
+ x->preplay = orig->preplay;
return x;
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xfrm: fix state migration copy replay sequence numbers
2017-05-19 10:47 ` [PATCH v2] xfrm: fix state migration copy " Antony Antony
@ 2017-05-19 11:19 ` Steffen Klassert
0 siblings, 0 replies; 5+ messages in thread
From: Steffen Klassert @ 2017-05-19 11:19 UTC (permalink / raw)
To: Antony Antony; +Cc: netdev, Richard Guy Briggs, Herbert Xu
On Fri, May 19, 2017 at 12:47:00PM +0200, Antony Antony wrote:
> During xfrm migration copy replay and preplay sequence numbers
> from the previous state.
>
> Here is a tcpdump output showing the problem.
> 10.0.10.46 is running vanilla kernel, is the IKE/IPsec responder.
> After the migration it sent wrong sequence number, reset to 1.
> The migration is from 10.0.0.52 to 10.0.0.53.
>
> IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7cf), length 136
> IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7cf), length 136
> IP 10.0.0.52.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d0), length 136
> IP 10.0.10.46.4500 > 10.0.0.52.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x7d0), length 136
>
> IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
> IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
> IP 10.0.0.53.4500 > 10.0.10.46.4500: NONESP-encap: isakmp: child_sa inf2[I]
> IP 10.0.10.46.4500 > 10.0.0.53.4500: NONESP-encap: isakmp: child_sa inf2[R]
>
> IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d1), length 136
>
> NOTE: next sequence is wrong 0x1
>
> IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x1), length 136
> IP 10.0.0.53.4500 > 10.0.10.46.4500: UDP-encap: ESP(spi=0x43ef462d,seq=0x7d2), length 136
> IP 10.0.10.46.4500 > 10.0.0.53.4500: UDP-encap: ESP(spi=0xca1c282d,seq=0x2), length 136
>
> Signed-off-by: Antony Antony <antony@phenome.org>
Applied, thanks Antony!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-19 11:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-18 14:39 [PATCH] xfrm: fix state migration replay sequence numbers Antony Antony
2017-05-18 15:55 ` Richard Guy Briggs
2017-05-19 9:59 ` Steffen Klassert
2017-05-19 10:47 ` [PATCH v2] xfrm: fix state migration copy " Antony Antony
2017-05-19 11:19 ` Steffen Klassert
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).