Skip to contentSkip to conversation

xfrm: Accept ESP packets regardless of UDP encapsulation mode

PATCH Applied Martin Willi 3 participants 16 messages Dec 4, 2008 – Dec 19, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfrm: Accept ESP packets regardless of UDP encapsulation mode

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.

net/xfrm/xfrm_input.c +0 -5
index b4a1317..65bcf09 100644
@@ -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;
Signature · 1 line
1.5.6.3
maintainer Applied
#1 lore raw

From: Martin Willi <[email protected]>
Date: Thu, 04 Dec 2008 14:18:50 +0100

Quoted 4 lines

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.

maintainer
#2 lore raw
In reply to Martin Willi
Quoted 10 lines · 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.

net/xfrm/xfrm_input.c +0 -0
index b4a1317..65bcf09 100644

net/xfrm/xfrm_input.c int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) · line 173

Quoted 7 lines
net/xfrm/xfrm_input.c +0 -4
@@ -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,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#3 lore raw

From: Herbert Xu <[email protected]>
Date: Thu, 18 Dec 2008 14:47:26 +1100

Quoted 22 lines
Quoted 10 lines · 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.

net/xfrm/xfrm_input.c +0 -0
index b4a1317..65bcf09 100644

net/xfrm/xfrm_input.c int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) · line 173

Quoted 7 lines
net/xfrm/xfrm_input.c +0 -4
@@ -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.

Quoted 1 line

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.

maintainer
#4 lore raw
Quoted 5 lines · David Miller

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,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#5 lore raw
Quoted 7 lines · Herbert Xu

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,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#6 lore raw

From: Herbert Xu <[email protected]>
Date: Thu, 18 Dec 2008 15:14:19 +1100

Quoted 9 lines

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.

Quoted 3 lines

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.

maintainer
#7 lore raw
Quoted 5 lines · David Miller

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,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#8 lore raw
In reply to Herbert Xu

Hi Herbert,

Quoted 2 lines

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

Quoted 5 lines

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

Quoted 3 lines

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]:

Quoted 2 lines

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

maintainer
#9 lore raw
Quoted 5 lines · Martin Willi
Quoted 3 lines

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.

Quoted 9 lines
Quoted 5 lines

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

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 5 lines
Quoted 2 lines

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.

Quoted 2 lines

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.

Quoted 1 line

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

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#10 lore raw
Quoted 4 lines

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

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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

maintainer
#11 lore raw
Quoted 9 lines · Martin Willi
Quoted 2 lines

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,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#12 lore raw

From: Herbert Xu <[email protected]>
Date: Fri, 19 Dec 2008 07:54:06 +1100

Quoted 15 lines · Martin Willi
Quoted 9 lines
Quoted 2 lines

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.

Quoted 5 lines

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.

#13 lore raw
Quoted 1 line

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

Yes, I'm fine with that.

Quoted 2 lines

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.

net/xfrm/xfrm_input.c +1 -1
index 7527940..ffc32f5 100644
@@ -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;
 		}
Signature · 1 line
1.5.6.3
maintainer
#14 lore raw
Quoted 5 lines · Martin Willi

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,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#15 lore raw
In reply to Herbert Xu
Quoted 10 lines · Herbert Xu
Quoted 2 lines

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

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt