[PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Martin Willi]

An IPsec node speaking IKEv2 MUST accept incoming UDP encapsulated
ESP packets, even if no NAT situation is detected. This is important
if MOBIKE is in use. Some implementation keep the encapsulation
mode if they move out of a NAT situation.

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index b4a1317..65bcf09 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -167,11 +167,6 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 			goto drop_unlock;
 		}
 
-		if ((x->encap ? x->encap->encap_type : 0) != encap_type) {
-			XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH);
-			goto drop_unlock;
-		}
-
 		if (x->props.replay_window && xfrm_replay_check(x, skb, seq)) {
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATESEQERROR);
 			goto drop_unlock;
-- 
1.5.6.3

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[David Miller]

From: Martin Willi <martin@strongswan.org>
Date: Thu, 04 Dec 2008 14:18:50 +0100

> An IPsec node speaking IKEv2 MUST accept incoming UDP encapsulated
> ESP packets, even if no NAT situation is detected. This is important
> if MOBIKE is in use. Some implementation keep the encapsulation
> mode if they move out of a NAT situation.

Applied, please provide a proper Signed-off-by line next time.

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

Martin Willi <martin@strongswan.org> wrote:
> An IPsec node speaking IKEv2 MUST accept incoming UDP encapsulated
> ESP packets, even if no NAT situation is detected. This is important
> if MOBIKE is in use. Some implementation keep the encapsulation
> mode if they move out of a NAT situation.
> 
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index b4a1317..65bcf09 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -167,11 +167,6 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>                        goto drop_unlock;
>                }
> 
> -               if ((x->encap ? x->encap->encap_type : 0) != encap_type) {
> -                       XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH);
> -                       goto drop_unlock;
> -               }

This can't work as ESP relies on this check.  Now that it's gone
ESP may touch a UDP header which doesn't exist.

What exactly are you trying to achieve?

Thanks,
-- 
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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 18 Dec 2008 14:47:26 +1100

> Martin Willi <martin@strongswan.org> wrote:
> > An IPsec node speaking IKEv2 MUST accept incoming UDP encapsulated
> > ESP packets, even if no NAT situation is detected. This is important
> > if MOBIKE is in use. Some implementation keep the encapsulation
> > mode if they move out of a NAT situation.
> > 
> > diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> > index b4a1317..65bcf09 100644
> > --- a/net/xfrm/xfrm_input.c
> > +++ b/net/xfrm/xfrm_input.c
> > @@ -167,11 +167,6 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
> >                        goto drop_unlock;
> >                }
> > 
> > -               if ((x->encap ? x->encap->encap_type : 0) != encap_type) {
> > -                       XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH);
> > -                       goto drop_unlock;
> > -               }
> 
> This can't work as ESP relies on this check.  Now that it's gone
> ESP may touch a UDP header which doesn't exist.
> 
> What exactly are you trying to achieve?

IKEv2 daemon needs to be agnostic about this situation.  So we have to
accept everything, regardless of configured encapsulation type.

At least that is my impression after reading the MOBIKE documents.

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

On Wed, Dec 17, 2008 at 07:49:42PM -0800, David Miller wrote:
>
> IKEv2 daemon needs to be agnostic about this situation.  So we have to
> accept everything, regardless of configured encapsulation type.
> 
> At least that is my impression after reading the MOBIKE documents.

The NAT-T encapsulation is an SA-specific attribute, so it's
decided when the SA is negotiated.  There is a mechanism for
detecting changes in the peer's address and updating the SA.

However, I can't see why you would ever need to switch between
having UDP encapsulation and not having it, without negotiating
it with the peer.  That is, the UDP encapsulation works whether
you have NAT or not.

Do you have a specific quote that I can check against?

Thanks,
-- 
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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

On Thu, Dec 18, 2008 at 02:57:58PM +1100, Herbert Xu wrote:
>
> However, I can't see why you would ever need to switch between
> having UDP encapsulation and not having it, without negotiating
> it with the peer.  That is, the UDP encapsulation works whether
> you have NAT or not.
> 
> Do you have a specific quote that I can check against?

A quick google failed to reveal any specific requirements apart
from the need to move in and out of NAT environments.

That isn't actually an issue because when your addresses change
you have to renegotiate with the other side to ensure that this
isn't some kind of an attack.  Afterwards you have to recreate
the SAs at which point you can easily set the encapsulation to
whatever it should be.

The only time when you need this patch is if the other side
unilaterally switched from NAT-T to no NAT-T, or vice versa,
which does not sound like a sane thing to do.

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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 18 Dec 2008 15:14:19 +1100

> A quick google failed to reveal any specific requirements apart
> from the need to move in and out of NAT environments.
> 
> That isn't actually an issue because when your addresses change
> you have to renegotiate with the other side to ensure that this
> isn't some kind of an attack.  Afterwards you have to recreate
> the SAs at which point you can easily set the encapsulation to
> whatever it should be.
> 
> The only time when you need this patch is if the other side
> unilaterally switched from NAT-T to no NAT-T, or vice versa,
> which does not sound like a sane thing to do.

My interpretation of the situation is that when you change (address or
NAT-T) you still have to perform the renegotiation over the old SA.

Or something like that.

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

On Wed, Dec 17, 2008 at 08:17:55PM -0800, David Miller wrote:
>
> My interpretation of the situation is that when you change (address or
> NAT-T) you still have to perform the renegotiation over the old SA.
> 
> Or something like that.

Do you have a pointer to that?

For each connection you have a pair of SAs, they're keyed by

	(dst, spi, proto)

So if one party's address changes then you immediately lose one
direction, which means the SAs are now inoperable as a bi-directional
channel.

In any case, AFAIK negotiations are conducted outside of SAs, on
port 500 or 4500, so this shouldn't matter.

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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Martin Willi]

Hi Herbert,

> This can't work as ESP relies on this check.  Now that it's gone
> ESP may touch a UDP header which doesn't exist.

Hm, this worked perfectly fine in my tests...

> ... when your addresses change
> you have to renegotiate with the other side to ensure that this
> isn't some kind of an attack.  Afterwards you have to recreate
> the SAs at which point you can easily set the encapsulation to
> whatever it should be.

Such address changes are recorded in the IKEv2 daemon and addresses are
updated through MOBIKE (RFC4555). Each peer updates its SA using the new
address. 

> The only time when you need this patch is if the other side
> unilaterally switched from NAT-T to no NAT-T, or vice versa,
> which does not sound like a sane thing to do.

This is a perfectly valid use case. MOBIKE allows you roam your SAs
between different networks, NATed or not. In our implementation, we
switch UDP encapsulation strictly on and off depending on the new NAT
situation. However, other implementations don't [1].

It is a requirement for MOBIKE enabled peers to accept UDP encapsulated
packets in any case (see discussion [2]), and it is currently discussed
to add this requirement to the revised IKEv2 core standard [3]:

> o  Implementations MUST process received UDP-encapsulated ESP packets
>       even when no NAT was detected.

The fact that there is not NAT between peers is no guarantee that the
peer will not use UDP encapsulation.

I don't see any reason to drop ESP packets because of the used UDP
encapsulation mode. What's the point of doing so?

Martin

[1]https://lists.strongswan.org/pipermail/users/2008-December/002936.html
[2]http://www.machshav.com/pipermail/mobike/2006-September/001491.html
[3]http://tools.ietf.org/html/draft-ietf-ipsecme-ikev2bis-01#section-2.23

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

Martin Willi <martin@strongswan.org> wrote:
> 
>> This can't work as ESP relies on this check.  Now that it's gone
>> ESP may touch a UDP header which doesn't exist.
> 
> Hm, this worked perfectly fine in my tests...

Well if you setup an SA with encap != NULL, then send it an
unencapsulated ESP packet it'll try to access the UDP header
(to check for address/port changes) which isn't there.

>> ... when your addresses change
>> you have to renegotiate with the other side to ensure that this
>> isn't some kind of an attack.  Afterwards you have to recreate
>> the SAs at which point you can easily set the encapsulation to
>> whatever it should be.
> 
> Such address changes are recorded in the IKEv2 daemon and addresses are
> updated through MOBIKE (RFC4555). Each peer updates its SA using the new
> address. 

I know.  The point is when such an address change occurs you need
to recreate the kernel SAs anyway, in which case you can reset the
encapsulation status to what it should be.

> This is a perfectly valid use case. MOBIKE allows you roam your SAs
> between different networks, NATed or not. In our implementation, we
> switch UDP encapsulation strictly on and off depending on the new NAT
> situation. However, other implementations don't [1].

You didn't understand me correctly.  Of course the peer is allowed
to change addresses.  The point is that after such an address
change occurs, we must replace the existing SAs with a new set of
SAs (otherwise the communication breaks down), at which point you
can set the NAT-T encapsulation status to what it should be.

> It is a requirement for MOBIKE enabled peers to accept UDP encapsulated
> packets in any case (see discussion [2]), and it is currently discussed
> to add this requirement to the revised IKEv2 core standard [3]:

I have absolutely no qualms about that.  But this has nothing to
do with the kernel.  What they were discussing in [2] is that the
IKE manager must be willing to negotiate SAs with/without UDP
encapsulation if NAT is not detected.  The kernel only gets involved
once the negotiation is complete at which point you have agreed
with the other side whether UDP encapsulation is to be used.

Once this negotiation is complete, neither the other side nor us can
unilaterally change the UDP encapsulation status without performing
a new negotiation.  So there is absolutely no need to handle this
in the kernel.

>> o  Implementations MUST process received UDP-encapsulated ESP packets
>>       even when no NAT was detected.
> 
> The fact that there is not NAT between peers is no guarantee that the
> peer will not use UDP encapsulation.

Which is fine.  The kernel is perfectly able to perform ESP-over-UDP
without your patch.  What it doesn't do is automatically switch
from UDP encapsulation to without UDP encapsulation, or vice versa
without key manager instructions.

> I don't see any reason to drop ESP packets because of the used UDP
> encapsulation mode. What's the point of doing so?

For a start, you've introduced a bug in the ESP code as it may try
access a non-existant UDP header and then interpret that as an
address change which will generate bogus events to your daemon.
This will happen on every packet if you move from with encapsulation
to without encapsulation.

More importantly, you've failed to demonstrate why you need this
in the first place.  None of the URLs you've quoted tells us that
the kernel needs to handle an SA switching between with encap and
without encap without the key manager telling it to do so.
 
> [1]https://lists.strongswan.org/pipermail/users/2008-December/002936.html

Right that's what led to this patch.

Reading this it seems that there is either a bug in strongswan or
in its peer.  IKE or IKEv2 is supposed to negotiate UDP encapsulation
explicitly.  So either we did negotiate to use UDP encapsulation,
and Strongswan ignored it (it's buggy), or we didn't and the peer
still chose to use it (the peer is buggy).

In either case it's a serious bug in the key manager.  I don't see
why we should work around such brokenness in the kernel.

Thanks,
-- 
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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Martin Willi]

> IKE or IKEv2 is supposed to negotiate UDP encapsulation
> explicitly.  So either we did negotiate to use UDP encapsulation,
> and Strongswan ignored it (it's buggy), or we didn't and the peer
> still chose to use it (the peer is buggy).

This is just not true. UDP encapsulation is never negotiated explicitly
in IKEv2. It is enabled if the peer thinks it is might help, for example
if it detects a NAT situation. There is no way to say "use UDP
encapsulation".

> More importantly, you've failed to demonstrate why you need this
> in the first place.  None of the URLs you've quoted tells us that
> the kernel needs to handle an SA switching between with encap and
> without encap without the key manager telling it to do so.

The local key manager does not know whether the peer enables UDP
encapsulation, it can't. Most likely it will in NAT situations, but it
might do so even if there is no NAT detected. And this is not a bug, it
is allowed to do so by the protocol.

> In either case it's a serious bug in the key manager.  I don't see
> why we should work around such brokenness in the kernel.

>From the key manager perspective, I can enable or disable UDP
encapsulation, fine. I decide locally what I'll use for outgoing
packets. But how should I know what the peer uses? I can't, it isn't
negotiated. It is, by the standard, perfectly valid to send UDP
encapsulated packets if the peer wants to do so. And there is no need to
communicate this to the key manager, there is actually no such mechanism
in IKEv2. Therefore I need the kernel to accept packet, encapsulated or
not.

> For a start, you've introduced a bug in the ESP code as it may try
> access a non-existant UDP header and then interpret that as an
> address change which will generate bogus events to your daemon.

I agree, I've missed that (because "my" daemon uses Netlink and the
km_new_mapping event was not implemented until recently).

But this is no valid reason to drop that approach in general, it is a
side effect introduced by my specific patch. This can be fixed.

Martin

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

On Thu, Dec 18, 2008 at 01:36:56PM +0100, Martin Willi wrote:
> 
> >From the key manager perspective, I can enable or disable UDP
> encapsulation, fine. I decide locally what I'll use for outgoing
> packets. But how should I know what the peer uses? I can't, it isn't
> negotiated. It is, by the standard, perfectly valid to send UDP
> encapsulated packets if the peer wants to do so. And there is no need to
> communicate this to the key manager, there is actually no such mechanism
> in IKEv2. Therefore I need the kernel to accept packet, encapsulated or
> not.

Even if the kernel did accept such packets, there is no guarantee
that your return traffic will make it back to the other side because
stateful firewalls may be present.

Responding with unencapsulated ESP traffic when the peer is sending
you UDP-encapsulated traffic is just not going to fly.

BTW I think the IKEv2 draft has stuffed it up on this one (though
luckily it hasn't made it to RFC yet).  I'll open a report on it.

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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

On Thu, Dec 18, 2008 at 10:04:53PM +1100, Herbert Xu wrote:
>
> > [1]https://lists.strongswan.org/pipermail/users/2008-December/002936.html
> 
> Right that's what led to this patch.
> 
> Reading this it seems that there is either a bug in strongswan or
> in its peer.  IKE or IKEv2 is supposed to negotiate UDP encapsulation
> explicitly.  So either we did negotiate to use UDP encapsulation,
> and Strongswan ignored it (it's buggy), or we didn't and the peer
> still chose to use it (the peer is buggy).

You didn't address this part.

What KM/OS was the peer running? How was it configured?

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

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 19 Dec 2008 07:54:06 +1100

> On Thu, Dec 18, 2008 at 01:36:56PM +0100, Martin Willi wrote:
> > 
> > >From the key manager perspective, I can enable or disable UDP
> > encapsulation, fine. I decide locally what I'll use for outgoing
> > packets. But how should I know what the peer uses? I can't, it isn't
> > negotiated. It is, by the standard, perfectly valid to send UDP
> > encapsulated packets if the peer wants to do so. And there is no need to
> > communicate this to the key manager, there is actually no such mechanism
> > in IKEv2. Therefore I need the kernel to accept packet, encapsulated or
> > not.
> 
> Even if the kernel did accept such packets, there is no guarantee
> that your return traffic will make it back to the other side because
> stateful firewalls may be present.
> 
> Responding with unencapsulated ESP traffic when the peer is sending
> you UDP-encapsulated traffic is just not going to fly.
> 
> BTW I think the IKEv2 draft has stuffed it up on this one (though
> luckily it hasn't made it to RFC yet).  I'll open a report on it.

I'm going to revert the change from net-next-2.6 from now
becasue:

1) The general scheme's validity is still suspect and under
   discussion

2) The change has a known bug (the UDP header access issue)

3) Martin can apply the change locally to do testing until we
   work this stuff out.

Thanks guys.

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Martin Willi]

> I'm going to revert the change from net-next-2.6 from now

Yes, I'm fine with that.

> 3) Martin can apply the change locally to do testing until we
>    work this stuff out.

I'll do some testing with a modified patch that accepts encapsulated
packets on a non-encapsulating SA, but not vice-versa. This should solve
that specific issue without introducing that (and hopefully no other)
regression.

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 7527940..ffc32f5 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -166,7 +166,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 			goto drop_unlock;
 		}
 
-		if ((x->encap ? x->encap->encap_type : 0) != encap_type) {
+		if (x->encap && x->encap->encap_type != encap_type) {
 			XFRM_INC_STATS(LINUX_MIB_XFRMINSTATEMISMATCH);
 			goto drop_unlock;
 		}
-- 
1.5.6.3

Re: [PATCH] xfrm: Accept ESP packets regardless of UDP encapsulation mode

[Herbert Xu]

On Fri, Dec 19, 2008 at 11:00:45AM +0100, Martin Willi wrote:
>
> I'll do some testing with a modified patch that accepts encapsulated
> packets on a non-encapsulating SA, but not vice-versa. This should solve
> that specific issue without introducing that (and hopefully no other)
> regression.

Yes this patch looks safe to me from ESP's point of view.

However, I still don't think this makes sense as ESP will no longer
react to an address change (since it may have no x->encap) should a
routing change cause NAT to occur.

Moreover, the fact that we need this patch in the first place
means that the two sides may be sending different encapsulations
to each other which should raise a big red flag since any stateful
firewall in the middle is going to break this completely.

Thanks,
-- 
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