netdev - Re: [PATCH ipsec-next] Clear secpath on loopback

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: <20181008064039.GJ3823@gauss3.secunet.de>
Date:   Mon, 8 Oct 2018 08:40:39 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Benedict Wong <benedictwong@...gle.com>
CC:     <netdev@...r.kernel.org>, <nharold@...gle.com>,
        <lorenzo@...gle.com>
Subject: Re: [PATCH ipsec-next] Clear secpath on loopback_xmit

On Fri, Oct 05, 2018 at 11:23:28AM -0700, Benedict Wong wrote:
> This patch clears the skb->sp when transmitted over loopback. This
> ensures that the loopback-ed packet does not have any secpath
> information from the outbound transforms.
> 
> At present, this causes XFRM tunnel mode packets to be dropped with
> XFRMINNOPOLS, due to the outbound state being in the secpath, without
> a matching inbound policy. Clearing the secpath ensures that all states
> added to the secpath are exclusively from the inbound processing.

This looks like a fix, so the target should be the ipsec tree,
not ipsec-next.

> 
> Tests: xfrm tunnel mode tests added for loopback:
>     https://android-review.googlesource.com/c/kernel/tests/+/777328
> Signed-off-by: Benedict Wong <benedictwong@...gle.com>

Please add a 'Fixes:' tag.

> ---
>  drivers/net/loopback.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
> index 30612497643c..a6bf54df94bd 100644
> --- a/drivers/net/loopback.c
> +++ b/drivers/net/loopback.c
> @@ -50,6 +50,7 @@
>  #include <linux/ethtool.h>
>  #include <net/sock.h>
>  #include <net/checksum.h>
> +#include <net/xfrm.h>
>  #include <linux/if_ether.h>	/* For the statistics structure. */
>  #include <linux/if_arp.h>	/* For ARPHRD_ETHER */
>  #include <linux/ip.h>
> @@ -82,6 +83,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
>  	 */
>  	skb_dst_force(skb);
>  
> +	// Clear secpath to ensure xfrm policy check not tainted by outbound SAs.

Comments should use /* ... */, like it is done below.

> +	secpath_reset(skb);
> +
>  	skb->protocol = eth_type_trans(skb, dev);
>  
>  	/* it's OK to use per_cpu_ptr() because BHs are off */
> -- 
> 2.19.0.605.g01d371f741-goog