netdev - Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware offloading

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] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 14 Jan 2019 09:06:33 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Raed Salem <raeds@...lanox.com>
CC:     Boris Pismenny <borisp@...lanox.com>,
        Yossi Kuperman <yossiku@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH ipsec] xfrm: fix non-GRO codepath for IPsec hardware
 offloading

On Mon, Jan 07, 2019 at 09:53:31AM +0000, Raed Salem wrote:
> 
> tested based on kernel v4.20-rc7 with the patch it works when configured with hw offload and without gro:
> topology used:
> Server A (192.168.8.2) <--> GW C <---> GW D <--> Server B (192.168.9.4):
> Server A (vm) connected to gateway C (host) via linux bridge (bripsec - 192.168.8.1)
> Gateway C connected to gateway D back to back with ipsec hw offload capable NICs (interface names [ip] respectfully p6p1 [192.168.7.2] /enp4s0 [192.168.7.9])
> Server B (vm) connected to gateway D (host) via linux bridge (bripsec - 192.168.9.1)
> 
> test flow:
> 1- configure ipsec tunnel with hw offload on and appropriate ip route at gateways C+D
> 2- Server A send ping traffic to server B using the tunnel

Thanks a lot for doing this tests!

Unfortunately it seems that we have problem
with our policy lookups. The problem is not
related to your patch, but I can't apply
the pach as is because of that (see below).

> 
> Server A config:
> ip route:
> 192.168.7.0/24 via 192.168.8.1 dev ens6
> 192.168.8.0/24 dev ens6 proto kernel scope link src 192.168.8.2
> 192.168.9.0/24 via 192.168.8.1 dev ens6
> 
> GW C:
> ip route:
> 192.168.7.0/24 dev p6p1 proto kernel scope link src 192.168.7.2
> 192.168.8.0/24 dev bripsec proto kernel scope link src 192.168.8.1
> 192.168.9.0/24 via 192.168.7.9 dev p6p1
> ip x s s:
> src 192.168.7.9 dst 192.168.7.2
>         proto esp spi 0x052dada4 reqid 86879652 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16747cd9 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev p6p1 dir in
>         sel src 192.168.9.2/24 dst 192.168.8.2/24
> src 192.168.7.2 dst 192.168.7.9
>         proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be7677952954644 128
>         anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
>         crypto offload parameters: dev p6p1 dir out
>         sel src 192.168.8.2/24 dst 192.168.9.2/24
> ip x p s:
> src 192.168.9.2/24 dst 192.168.8.2/24
>         dir fwd priority 0 ptype main
>         tmpl src 192.168.7.9 dst 192.168.7.2
>                 proto esp reqid 86879652 mode tunnel
> src 192.168.9.2/24 dst 192.168.8.2/24
>         dir in priority 0 ptype main
>         tmpl src 192.168.7.9 dst 192.168.7.2
>                 proto esp reqid 86879652 mode tunnel
> src 192.168.8.2/24 dst 192.168.9.2/24
>         dir out priority 0 ptype main
>         tmpl src 192.168.7.2 dst 192.168.7.9
>                 proto esp reqid 726637921 mode tunnel
> 				
> GW D:
> ip route:
> 192.168.7.0/24 dev enp4s0 proto kernel scope link src 192.168.7.9
> 192.168.8.0/24 via 192.168.7.2 dev enp4s0
> 192.168.9.0/24 dev bripsec proto kernel scope link src 192.168.9.1
> ip x s s:
> src 192.168.7.9 dst 192.168.7.2
>         proto esp spi 0x052dada4 reqid 86879652 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0xa04f6b814ce4b8738ced2adc2e1abea3152822281df5338560fe9f50cf473cfa16747cd9 128
>         anti-replay context: seq 0x0, oseq 0x2d, bitmap 0x00000000
>         crypto offload parameters: dev enp4s0 dir out
>         sel src 192.168.9.2/24 dst 192.168.8.2/24
> src 192.168.7.2 dst 192.168.7.9
>         proto esp spi 0x2b4f9d61 reqid 726637921 mode tunnel
>         replay-window 0
>         aead rfc4106(gcm(aes)) 0x4f580ac2e9b959c195c212451498edfc54204dcf152dac387baaf3aa6be7677952954644 128
>         anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
>         crypto offload parameters: dev enp4s0 dir in
>         sel src 192.168.8.2/24 dst 192.168.9.2/24

With HW IPsec offload, the packet is decrypted but not
decapsulated. So for the SA above, the outer packet
header should have src 192.168.7.2 and dst 192.168.7.9.
This does not match the selector for that SA, so the
packet must be discarded. Unfortunately this does not
happen and it took me quite some time to find out
why this is the case. The problem is that we set
the no_policy flag for IPsec protocols. If this flag
is set, we skip the inbound policy check (including the
SA selector check). The patch that introduced this
behaviour predates our git history and I think it
was ok until we started to support tunnel in tunnel
scenarios (and now HW offload).

I'm thinking about removing the no_policy flag
from the IPsec protocols to actually do the inbound
policy check for these protocols too, but have to make
sure that this has no side effects first.

For HW offload, we should either refuse to do it
if GRO is disabled, or to do the inbound policy
check against the inner headers (they are valid
in this case).