[PATCH] xfrm: fix state migration replay sequence numbers
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 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 , Steffen Klassert Cc: Richard Guy Briggs During xfrm migration copy replay and preplay sequence numbers from the previous state. Signed-off-by: Antony Antony --- 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
Re: [PATCH] xfrm: fix state migration replay sequence numbers
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 > 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 , Steffen > Klassert > Cc: Richard Guy Briggs > > During xfrm migration copy replay and preplay sequence numbers > from the previous state. > > Signed-off-by: Antony Antony > --- > 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 " and "git send-email --to ... " work really well together.) Reviewed-by: Richard Guy Briggs slainte mhath, RGB -- Richard Guy Briggs -- ~\-- ~\ -- \___ o \@ @Ride yer bike! Ottawa, ON, CANADA -- Lo_>__M__\\/\%__\\/\% Vote! -- _GTVS6#790__(*)__(*)(*)(*)_
Re: [PATCH] xfrm: fix state migration replay sequence numbers
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!
