lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bfda402b-c745-4e17-8abe-8b2171605326@strongswan.org>
Date: Tue, 28 Nov 2023 12:33:36 +0100
From: Tobias Brunner <tobias@...ongswan.org>
To: Steffen Klassert <steffen.klassert@...unet.com>,
 Herbert Xu <herbert@...dor.apana.org.au>
Cc: netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH 2/2] xfrm: Remove inner/outer modes from output path

Hi Steffen, Herbert,

> @@ -875,21 +875,10 @@ static int xfrm6_extract_output(struct xfrm_state *x, struct sk_buff *skb)
>  
>  static int xfrm_inner_extract_output(struct xfrm_state *x, struct sk_buff *skb)
>  {
> -	const struct xfrm_mode *inner_mode;
> -
> -	if (x->sel.family == AF_UNSPEC)
> -		inner_mode = xfrm_ip2inner_mode(x,
> -				xfrm_af2proto(skb_dst(skb)->ops->family));
> -	else
> -		inner_mode = &x->inner_mode;
> -
> -	if (inner_mode == NULL)
> -		return -EAFNOSUPPORT;
> -
> -	switch (inner_mode->family) {
> -	case AF_INET:
> +	switch (skb->protocol) {
> +	case htons(ETH_P_IP):
>  		return xfrm4_extract_output(x, skb);
> -	case AF_INET6:
> +	case htons(ETH_P_IPV6):
>  		return xfrm6_extract_output(x, skb);
>  	}

The changes in this function indirectly break tunneling IPv4 packets
sent from RAW sockets.  Such packets currently don't have
skb->protocol set, so this results in EAFNOSUPPORT.  For IPv6, this
isn't a problem because the protocol is set since v3.11, or more
specifically 9c9c9ad5fae7 ("ipv6: set skb->protocol on tcp, raw and
ip6_append_data genereated skbs").  For IPv4, that's not the case.  I
wonder why this hasn't been an issue so far (e.g. with MTU calculation
as mentioned in that commit for IPv6).

To fix this, we basically need the patch below.  The question is how
it should be marked with regards to a Fixes: tag.  Should it actually
reference this xfrm-specific commit?  Or should it even get backported
further (e.g. reference the ipv6 commit above)?  (The question then
would be if this change could have unintended side-effects.)

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 27da9d7294c0..696e1e734729 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -350,6 +350,7 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
 		goto error;
 	skb_reserve(skb, hlen);
 
+	skb->protocol = htons(ETH_P_IP);
 	skb->priority = READ_ONCE(sk->sk_priority);
 	skb->mark = sockc->mark;
 	skb->tstamp = sockc->transmit_time;

Regards,
Tobias


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ