netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
@ 2007-12-07 16:04 Paul Moore
  2007-12-09  2:13 ` Herbert Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Moore @ 2007-12-07 16:04 UTC (permalink / raw)
  To: netdev; +Cc: Joy Latten

Hello all,

As part of the IPv6 "gap analysis" that the Linux Foundation is currently 
doing I've been looking at the IPsec auditing requirements as defined in 
RFC4303 and I came across some odd behavior regarding SA sequence number 
overflows ...

RFC4303 states the following:

   3.3.3.  Sequence Number Generation

   The sender's counter is initialized to 0 when an SA is established.
   The sender increments the sequence number (or ESN) counter for this
   SA and inserts the low-order 32 bits of the value into the Sequence
   Number field.  Thus, the first packet sent using a given SA will
   contain a sequence number of 1.

   If anti-replay is enabled (the default), the sender checks to ensure
   that the counter has not cycled before inserting the new value in the
   Sequence Number field.  In other words, the sender MUST NOT send a
   packet on an SA if doing so would cause the sequence number to cycle.
   An attempt to transmit a packet that would result in sequence number
   overflow is an auditable event.  The audit log entry for this event
   SHOULD include the SPI value, current date/time, Source Address,
   Destination Address, and (in IPv6) the cleartext Flow ID.

The related code in net/xfrm/xfrm_output.c:xfrm_output() looks like this:

   if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
           XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
           if (xfrm_aevent_is_on())
                   xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
   }

Which doesn't appear to take into account sequence number overflow at all.  
Granted, it does send notifications to userspace but it doesn't do anything 
to prevent the packet from being sent if the sequence number wraps.  I'm 
still a few years behind in my IPsec specifications so I could be missing 
something here (extended sequence numbers spring to mind and the kernel's 
curious mixing of 32bit and 64bit types for SA sequence number counters) but 
at first glance this appears to be a bug ... yes/no?

If it is a bug, I think the basic fix should be pretty simple, changing the 
above xfrm_output() code to the following:

   if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
           XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
+          if (x->replay.oseq == 0)
+                  goto error;
           if (xfrm_aevent_is_on())
                   xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
   }

-- 
paul moore
linux security @ hp

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

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
  2007-12-07 16:04 IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3) Paul Moore
@ 2007-12-09  2:13 ` Herbert Xu
  2007-12-09 14:37   ` Paul Moore
  0 siblings, 1 reply; 8+ messages in thread
From: Herbert Xu @ 2007-12-09  2:13 UTC (permalink / raw)
  To: Paul Moore; +Cc: netdev, latten

Paul Moore <paul.moore@hp.com> wrote:
>
> If it is a bug, I think the basic fix should be pretty simple, changing the 
> above xfrm_output() code to the following:
> 
>   if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
>           XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
> +          if (x->replay.oseq == 0)
> +                  goto error;

Yes we need this check.

However please add an unlikely around it since it's a 1-in-4
billion event :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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] 8+ messages in thread

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
  2007-12-09  2:13 ` Herbert Xu
@ 2007-12-09 14:37   ` Paul Moore
  2007-12-10  3:06     ` Herbert Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Moore @ 2007-12-09 14:37 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, latten

On Saturday 08 December 2007 9:13:48 pm Herbert Xu wrote:
> Paul Moore <paul.moore@hp.com> wrote:
> > If it is a bug, I think the basic fix should be pretty simple, changing
> > the above xfrm_output() code to the following:
> >
> >   if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
> >           XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
> > +          if (x->replay.oseq == 0)
> > +                  goto error;
>
> Yes we need this check.
>
> However please add an unlikely around it since it's a 1-in-4
> billion event :)

 :)

Thanks for clearing that up, I'll send a patch this week; complete with an 
unlikely (similar to the RFC quality IPsec audit patch I sent on Friday) and 
a decrement to the sequence counter in case of rollover.

-- 
paul moore
linux security @ hp

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

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
  2007-12-09 14:37   ` Paul Moore
@ 2007-12-10  3:06     ` Herbert Xu
  2007-12-10  3:16       ` Patrick McHardy
  0 siblings, 1 reply; 8+ messages in thread
From: Herbert Xu @ 2007-12-10  3:06 UTC (permalink / raw)
  To: Paul Moore; +Cc: netdev, latten

On Sun, Dec 09, 2007 at 09:37:56AM -0500, Paul Moore wrote:
>
> Thanks for clearing that up, I'll send a patch this week; complete with an 
> unlikely (similar to the RFC quality IPsec audit patch I sent on Friday) and 
> a decrement to the sequence counter in case of rollover.

Actually I think we should just use the SA expire mechanism
to do this.  The reason is that the overflow we want to detect
only applies to 32-bit sequence numbers.  In future we will be
making our sequence numbers 64-bit.

When we do that we can no longer just check against wrapping
to zero since we need to know whether ESNs are in use or not.

The easiest fix is to just force the hard_packet_limit to 2^32.
upon SA creation.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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] 8+ messages in thread

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
  2007-12-10  3:06     ` Herbert Xu
@ 2007-12-10  3:16       ` Patrick McHardy
  2007-12-10  3:43         ` Herbert Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick McHardy @ 2007-12-10  3:16 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Paul Moore, netdev, latten

Herbert Xu wrote:
> On Sun, Dec 09, 2007 at 09:37:56AM -0500, Paul Moore wrote:
>   
>> Thanks for clearing that up, I'll send a patch this week; complete with an 
>> unlikely (similar to the RFC quality IPsec audit patch I sent on Friday) and 
>> a decrement to the sequence counter in case of rollover.
>>     
>
> Actually I think we should just use the SA expire mechanism
> to do this.  The reason is that the overflow we want to detect
> only applies to 32-bit sequence numbers.  In future we will be
> making our sequence numbers 64-bit.
>
> When we do that we can no longer just check against wrapping
> to zero since we need to know whether ESNs are in use or not.
>
> The easiest fix is to just force the hard_packet_limit to 2^32.
> upon SA creation.

Won't this break with manually installed SAs (without a keying
daemon)?



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

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
  2007-12-10  3:16       ` Patrick McHardy
@ 2007-12-10  3:43         ` Herbert Xu
  2007-12-18 16:14           ` Paul Moore
  0 siblings, 1 reply; 8+ messages in thread
From: Herbert Xu @ 2007-12-10  3:43 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Paul Moore, netdev, latten

On Mon, Dec 10, 2007 at 04:16:36AM +0100, Patrick McHardy wrote:
>
> Won't this break with manually installed SAs (without a keying
> daemon)?

Well what's being suggested here will already break that anyway :)

Alternatively we can take the interpretation that it's the KM's
responsibility to set the appropriate hard life time if ESNs are
not in use.

Either way is fine with me.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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] 8+ messages in thread

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
  2007-12-10  3:43         ` Herbert Xu
@ 2007-12-18 16:14           ` Paul Moore
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Moore @ 2007-12-18 16:14 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Patrick McHardy, netdev, latten

On Sunday 09 December 2007 10:43:56 pm Herbert Xu wrote:
> On Mon, Dec 10, 2007 at 04:16:36AM +0100, Patrick McHardy wrote:
> > Won't this break with manually installed SAs (without a keying
> > daemon)?
>
> Well what's being suggested here will already break that anyway :)
>
> Alternatively we can take the interpretation that it's the KM's
> responsibility to set the appropriate hard life time if ESNs are
> not in use.
>
> Either way is fine with me.
>
> Cheers,

Sorry for the delay, I got distracted ...

Rereading the thread it's unclear to me which solution was deemed "correct".  
I'm not a big fan of fiddling/forcing SA lifetimes unless we have no other 
option; if someone is foolish enough to use manual keying with replay 
protection and no mechanism to catch rollover then they most likely have 
larger problems.  It's the whole "we'll provide you with the gun, but you 
have to shoot yourself" argument as applied to SA lifetimes.

However, you guys have to deal with this code more often than I do so I'll 
deffer to your better judgment.

-- 
paul moore
linux security @ hp

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

* Re: IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3)
@ 2007-12-19  6:36 Joy Latten
  0 siblings, 0 replies; 8+ messages in thread
From: Joy Latten @ 2007-12-19  6:36 UTC (permalink / raw)
  To: paul.moore; +Cc: herbert, kaber, latten, netdev

>Rereading the thread it's unclear to me which solution was deemed "correct".  
>I'm not a big fan of fiddling/forcing SA lifetimes unless we have no other 
>option; if someone is foolish enough to use manual keying with replay 
>protection and no mechanism to catch rollover then they most likely have 
>larger problems.  It's the whole "we'll provide you with the gun, but you 
>have to shoot yourself" argument as applied to SA lifetimes.

Also, the ipsec rfc require auotmated SA management when 
using anti-replay service and that the option be disabled 
when SAs are manually setup.

It may not stop anyone, but we can always point to rfc. :-)

Joy


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

end of thread, other threads:[~2007-12-19  6:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-07 16:04 IPsec replay sequence number overflow behavior? (RFC4303 section 3.3.3) Paul Moore
2007-12-09  2:13 ` Herbert Xu
2007-12-09 14:37   ` Paul Moore
2007-12-10  3:06     ` Herbert Xu
2007-12-10  3:16       ` Patrick McHardy
2007-12-10  3:43         ` Herbert Xu
2007-12-18 16:14           ` Paul Moore
  -- strict thread matches above, loose matches on Subject: below --
2007-12-19  6:36 Joy Latten

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).