Re: [PATCH ipsec 1/2] xfrm: Check policy for nested XFRM packets in xfrm_input

[Steffen Klassert <steffen.klassert@secunet.com>]

On Mon, Aug 15, 2022 at 02:25:42PM -0700, Benedict Wong wrote:
> >
> > Hm, shouldn't the xfrm_policy_check called along the
> > packet path for each round after decapsulation?
> >
> > Do you use ESP transformation offload (INET_ESP_OFFLOAD/
> > INET6_ESP_OFFLOAD)?
> 
> Been a while since I've gotten a chance to look through the
> code, but when I previously looked through the stack, it looked
> like we have policy checks in the following places:
> - IPv4/IPv6 deliver to host
> - UDP/TCP/ICMP/L2TP/SCTP/VTI/raw in direct rcv methods
> 
> Additionally, we have a conditional check in XFRM-I, but
> *only if the packet is crossing network namespaces* (which
> in the Android case, it isn't)

Yes, this is because the secpath is cleared when crossing
network namespaces. The inbound policy check in the packet
path would fail in this case. That's why we do the policy
check there.

> Notably, it appears that the missing case is when the outer
> tunnel is an unencap'd ESP packet, which simply calls xfrm_input
> via xfrm(4|6)_rcv_spi. This changes adds that call to ensure
> that the verification is always performed in each packet path.

Please note that all policy checks are done for the traffic
selector of the inner packets. The inbound policy check makes
sure that the inner packets are allowed to pass and really
came through the SA that is recorded in the secpath.

When receiving an ESP packet, the packets IPsec ID (daddr/
SPI/proto) is mached against the SADB. If a matching SA is
there, it is used to decapsulate. The TS of the decapsulated
packet is used to do the policy lookup then.

If the decapsulated packet is not dropped by the policy lookup
and is again an ESP packet, we start with the SADB lookup as
described above.

So I think the behaviour is correct as it is implemented.