netdev - Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

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]

Message-ID: <9115c9d2-b788-7432-2238-9ac21f632480@iogearbox.net>
Date:   Thu, 15 Mar 2018 18:48:24 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Shmulik Ladkani <shmulik.ladkani@...il.com>
Cc:     Liran Alon <liran.alon@...cle.com>, davem@...emloft.net,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        idan.brown@...cle.com, Yuval Shaia <yuval.shaia@...cle.com>
Subject: Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info
 only when crossing netns

On 03/15/2018 04:54 PM, Shmulik Ladkani wrote:
> On Thu, 15 Mar 2018 16:13:39 +0100 Daniel Borkmann <daniel@...earbox.net> wrote:
>> On 03/15/2018 01:50 PM, Shmulik Ladkani wrote:
>>>
>>> It would be beneficial to have the mark preserved when skb is injected
>>> to the slave device's rx path (especially when it's on the same netns).  
>>
>> Right, I think also here the easiest would be to have a BPF_F_PRESERVE_MARK
>> flag to opt-in in general case (xnet/non-xnet)
> 
> Sounds okay to me.
> 
>> But lets presume for a sec you would _not_ scrub it, then how are users
>> supposed to make use of this? The feature/bug may not be critical enough
>> (well, otherwise it wouldn't have been like this for long time) for stable,
>> so to write an app relying on it the behavior will change from kernel A to
>> kernel B, where you need to end up having a full blown veth run-time test
>> in order to figure it out before you can use it, not really useful either.
> 
> Let's assume BPF_F_PRESERVE_MARK is a feature then, which is available only
> in new kernels.
> As said, this flag will not be honored by older kernels.
> 
> But your "run-time test" argument is true for every new flag-bit
> introduced to bpf functions, for example:
>  BPF_F_SEQ_NUMBER was added after other skb_set_tunnel_key flags,
>  Same for BPF_F_INVALIDATE_HASH (skb_store_bytes), BPF_F_MARK_ENFORCE
>  (l4_csum_replace) and others.
> 
> With every flag addition, the flag mask validation in the corresponding
> bpf function has been relaxed to support it.
> 
> Why is BPF_F_PRESERVE_MARK any different from any previous flag addition?

Not really different than other BPF helpers, but you can simply figure it out
via BPF_PROG_TEST_RUN by calling bpf_redirect() helper on some fake ifindex
and check the return value whether it's shot or redirect. This is definitely
trivial to do and doesn't require any devs to set up compared to otherwise
full blown setup. E.g. test_verifier uses this for the test case array it
contains.

Cheers,
Daniel