Subject: Re: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec pkt

From: Christophe Gouault <[email-masked]>
Date: Fri, 22 Nov 2013 15:33:22 +0100

On 11/21/2013 01:12 PM, Steffen Klassert wrote:
 > On Wed, Nov 06, 2013 at 09:05:53AM +0100, Christophe Gouault wrote:
 >>
 >> @@ -133,7 +134,13 @@ static int vti_rcv(struct sk_buff *skb)
 >>            * only match policies with this mark.
 >>            */
 >>           skb->mark = be32_to_cpu(tunnel->parms.o_key);
 >> +        /* The packet is decrypted, but not yet decapsulated.
 >> +         * Temporarily make network_header point to the inner header
 >> +         * for policy check.
 >> +         */
 >> +        skb_reset_network_header(skb);
 >>           ret = xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb);
 >
 > If we do it like this, we would do an input policy check even for
 > packets that should be forwarded. I think that's a bit odd.

Admittedly, a forward policy check would be more appropriate for
forwarded traffic.

 > If we really change to match plaintext traffic, we should do
 > it like Fan Du proposed. Remove the policy check here and
 > let the further layers do the policy enforcement. All we
 > have to do is to set the skb mark, then the lookup should
 > match the vti policy.

This solution sounds seductive, however, we must be careful because we
change the skb input device (from the physical interface to the vti
interface). So we are supposed to call skb_scrub_packet as is normally
done when decapsulating a packet from a tunnel. This will reset the skb
secpath and mark, and hence will compromise the deferred inbound policy
check.

 > It is already clear that this packet was IPsec transformed
 > when it enters vti_rcv, so deferring the policy check should
 > be ok.

I had in mind to later support cross netns in vti interfaces like for
ipip tunnels (different netns for the decapsulated and encapsulated
packets). With the deferred inbound policy check, the SA and SP will not
be in the same netns, this will cause problems for the inbound policy check.

Best Regards,
Christophe