netdev - Re: [PATCH net-next] net/sched: cls_flower: Add orig

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:   Thu, 2 Sep 2021 09:48:51 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Boris Sukholitko <boris.sukholitko@...adcom.com>,
        netdev@...r.kernel.org, Jiri Pirko <jiri@...nulli.us>,
        Cong Wang <xiyou.wangcong@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Vladimir Oltean <olteanv@...il.com>,
        Vadym Kochan <vadym.kochan@...ision.eu>,
        Ilya Lifshits <ilya.lifshits@...adcom.com>,
        tom Herbert <tom@...anda.io>,
        Felipe Magno de Almeida <felipe@...ertise.dev>,
        Pedro Tammela <pctammela@...atatu.com>
Subject: Re: [PATCH net-next] net/sched: cls_flower: Add orig_ethtype

On Tue, Aug 31, 2021 at 09:18:16AM -0400, Jamal Hadi Salim wrote:
> You have _not_ been unlucky - it is a design issue with flow dissector
> and the wrapping around flower. Just waiting to happen for more
> other use cases..

I agree. I think the fundamental problem is that flower does not set
'FLOW_DISSECTOR_F_STOP_AT_ENCAP' and simply lets the flow dissector
parse as deep as possible. For example, 'dst_ip' will match on the
inner most destination IP which is not intuitive and probably different
than what most hardware implementations do.

This behavior is also very error prone because it means that if the
kernel learns to dissect a new tunnel protocol, filters can be suddenly
broken (match on outer field now matches on inner field).

I don't think that changing the default behavior is a solution as it can
break user space. Maybe adding a 'stop_encap' flag to flower that user
space will have to set?