netdev - Re: [PATCH ipsec 2/2] xfrm interface: fix packet tx through bpf

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 9 Jan 2020 09:40:09 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
CC:     <davem@...emloft.net>, <netdev@...r.kernel.org>
Subject: Re: [PATCH ipsec 2/2] xfrm interface: fix packet tx through
 bpf_redirect()

On Tue, Dec 31, 2019 at 05:56:54PM +0100, Nicolas Dichtel wrote:
> With an ebpf program that redirects packets through a xfrm interface,
> packets are dropped because no dst is attached to skb.
> 
> This could also be reproduced with an AF_PACKET socket, with the following
> python script (xfrm1 is a xfrm interface):
> 
>  import socket
>  send_s = socket.socket(socket.AF_PACKET, socket.SOCK_RAW, 0)
>  # scapy
>  # p = IP(src='10.100.0.2', dst='10.200.0.1')/ICMP(type='echo-request')
>  # raw(p)
>  req = b'E\x00\x00\x1c\x00\x01\x00\x00@\x01e\xb2\nd\x00\x02\n\xc8\x00\x01\x08\x00\xf7\xff\x00\x00\x00\x00'
>  send_s.sendto(req, ('xfrm1', 0x800, 0, 0))
> 
> It was also not possible to send an ip packet through an AF_PACKET socket
> because a LL header was expected. Let's remove those LL header constraints.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
> ---
>  net/xfrm/xfrm_interface.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
> index 7ac1542feaf8..55978a1501ec 100644
> --- a/net/xfrm/xfrm_interface.c
> +++ b/net/xfrm/xfrm_interface.c
> @@ -343,6 +343,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>  	struct xfrm_if *xi = netdev_priv(dev);
>  	struct net_device_stats *stats = &xi->dev->stats;
> +	struct dst_entry *dst = skb_dst(skb);
>  	struct flowi fl;
>  	int ret;
>  
> @@ -352,10 +353,26 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
>  	case htons(ETH_P_IPV6):
>  		xfrm_decode_session(skb, &fl, AF_INET6);
>  		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
> +		if (!dst) {
> +			dst = ip6_route_output(dev_net(dev), NULL, &fl.u.ip6);
> +			if (dst->error) {
> +				dst_release(dst);
> +				goto tx_err;
> +			}
> +			skb_dst_set(skb, dst);
> +		}
>  		break;
>  	case htons(ETH_P_IP):
>  		xfrm_decode_session(skb, &fl, AF_INET);
>  		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
> +		if (!dst) {
> +			struct rtable *rt = __ip_route_output_key(dev_net(dev),
> +								  &fl.u.ip4);
> +
> +			if (IS_ERR(rt))
> +				goto tx_err;

With this change, the 'if (!dst)' in xfrmi_xmit2() is meaningless
and we don't handle this error as a link failure anymore.

Please make sure that the error path is not changed.