* [RFC PATCH ipsec] xfrm: don't pass too short packets to userspace with ESPINUDP encap
@ 2020-07-24 14:46 Sabrina Dubroca
2020-07-27 9:28 ` Steffen Klassert
0 siblings, 1 reply; 4+ messages in thread
From: Sabrina Dubroca @ 2020-07-24 14:46 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Paul Wouters, Andrew Cagney, Steffen Klassert,
Tobias Brunner
Currently, any UDP-encapsulated packet of 8 bytes or less will be
passed to userspace, whether it starts with the non-ESP prefix or
not (except keepalives). This includes:
- messages of 1, 2, 3 bytes
- messages of 4 to 8 bytes not starting with 00 00 00 00
This patch changes that behavior, so that only properly-formed non-ESP
messages are passed to userspace. Messages of 8 bytes or less that
don't contain a full non-ESP prefix followed by some data (at least
one byte) will be dropped and counted as XfrmInHdrError.
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
net/ipv4/xfrm4_input.c | 9 +++++++--
net/ipv6/xfrm6_input.c | 9 +++++++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index ad2afeef4f10..2a2bb38ac798 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -114,9 +114,14 @@ int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
} else if (len > sizeof(struct ip_esp_hdr) && udpdata32[0] != 0) {
/* ESP Packet without Non-ESP header */
len = sizeof(struct udphdr);
- } else
- /* Must be an IKE packet.. pass it through */
+ } else if (len > 4 && udpdata32[0] == 0) {
+ /* IKE packet: pass it through */
return 1;
+ } else {
+ /* incomplete packet, drop */
+ XFRM_INC_STATS(dev_net(skb->dev), LINUX_MIB_XFRMINHDRERROR);
+ goto drop;
+ }
break;
case UDP_ENCAP_ESPINUDP_NON_IKE:
/* Check if this is a keepalive packet. If so, eat it. */
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 04cbeefd8982..7e14d59d55cb 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -110,9 +110,14 @@ int xfrm6_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
} else if (len > sizeof(struct ip_esp_hdr) && udpdata32[0] != 0) {
/* ESP Packet without Non-ESP header */
len = sizeof(struct udphdr);
- } else
- /* Must be an IKE packet.. pass it through */
+ } else if (len > 4 && udpdata32[0] == 0) {
+ /* IKE packet: pass it through */
return 1;
+ } else {
+ /* incomplete packet, drop */
+ XFRM_INC_STATS(dev_net(skb->dev), LINUX_MIB_XFRMINHDRERROR);
+ goto drop;
+ }
break;
case UDP_ENCAP_ESPINUDP_NON_IKE:
/* Check if this is a keepalive packet. If so, eat it. */
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH ipsec] xfrm: don't pass too short packets to userspace with ESPINUDP encap
2020-07-24 14:46 [RFC PATCH ipsec] xfrm: don't pass too short packets to userspace with ESPINUDP encap Sabrina Dubroca
@ 2020-07-27 9:28 ` Steffen Klassert
2020-07-27 9:40 ` Tobias Brunner
2020-07-27 11:07 ` Paul Wouters
0 siblings, 2 replies; 4+ messages in thread
From: Steffen Klassert @ 2020-07-27 9:28 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev, Paul Wouters, Andrew Cagney, Tobias Brunner
On Fri, Jul 24, 2020 at 04:46:07PM +0200, Sabrina Dubroca wrote:
> Currently, any UDP-encapsulated packet of 8 bytes or less will be
> passed to userspace, whether it starts with the non-ESP prefix or
> not (except keepalives). This includes:
> - messages of 1, 2, 3 bytes
> - messages of 4 to 8 bytes not starting with 00 00 00 00
>
> This patch changes that behavior, so that only properly-formed non-ESP
> messages are passed to userspace. Messages of 8 bytes or less that
> don't contain a full non-ESP prefix followed by some data (at least
> one byte) will be dropped and counted as XfrmInHdrError.
I'm ok with that change. But it affects userspace, so the *swan
people have to tell if that's ok for them.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH ipsec] xfrm: don't pass too short packets to userspace with ESPINUDP encap
2020-07-27 9:28 ` Steffen Klassert
@ 2020-07-27 9:40 ` Tobias Brunner
2020-07-27 11:07 ` Paul Wouters
1 sibling, 0 replies; 4+ messages in thread
From: Tobias Brunner @ 2020-07-27 9:40 UTC (permalink / raw)
To: Steffen Klassert, Sabrina Dubroca; +Cc: netdev, Paul Wouters, Andrew Cagney
>> Currently, any UDP-encapsulated packet of 8 bytes or less will be
>> passed to userspace, whether it starts with the non-ESP prefix or
>> not (except keepalives). This includes:
>> - messages of 1, 2, 3 bytes
>> - messages of 4 to 8 bytes not starting with 00 00 00 00
>>
>> This patch changes that behavior, so that only properly-formed non-ESP
>> messages are passed to userspace. Messages of 8 bytes or less that
>> don't contain a full non-ESP prefix followed by some data (at least
>> one byte) will be dropped and counted as XfrmInHdrError.
>
> I'm ok with that change. But it affects userspace, so the *swan
> people have to tell if that's ok for them.
Yes, no problem from strongSwan's side. Packets shorter than 4 bytes
are immediately dropped anyway, the others when attempting to parse as
IKE messages (already the initiator IKE SPI, with which they start, is 8
bytes long).
Regards,
Tobias
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH ipsec] xfrm: don't pass too short packets to userspace with ESPINUDP encap
2020-07-27 9:28 ` Steffen Klassert
2020-07-27 9:40 ` Tobias Brunner
@ 2020-07-27 11:07 ` Paul Wouters
1 sibling, 0 replies; 4+ messages in thread
From: Paul Wouters @ 2020-07-27 11:07 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Sabrina Dubroca, netdev, Andrew Cagney, Tobias Brunner
On Jul 27, 2020, at 05:28, Steffen Klassert <steffen.klassert@secunet.com> wrote:
>
>
>>
>> This patch changes that behavior, so that only properly-formed non-ESP
>> messages are passed to userspace. Messages of 8 bytes or less that
>> don't contain a full non-ESP prefix followed by some data (at least
>> one byte) will be dropped and counted as XfrmInHdrError.
>
> I'm ok with that change. But it affects userspace, so the *swan
> people have to tell if that's ok for them.
Libreswan is okay with this, we actually discussed this with Sabrina as a result of the TCP work where she noticed the difference.
Paul
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-27 11:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24 14:46 [RFC PATCH ipsec] xfrm: don't pass too short packets to userspace with ESPINUDP encap Sabrina Dubroca
2020-07-27 9:28 ` Steffen Klassert
2020-07-27 9:40 ` Tobias Brunner
2020-07-27 11:07 ` Paul Wouters
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.