netdev - Re: cls_flower vlan matching

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] [day] [month] [year] [list]

Message-ID: <Yjhgi48BpTGh6dig@nanopsycho>
Date:   Mon, 21 Mar 2022 12:24:59 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Vlad Buslov <vladbu@...dia.com>
Cc:     Jiri Pirko <jiri@...dia.com>, netdev@...r.kernel.org,
        Jamal Hadi Salim <hadi@...atatu.com>,
        Maor Dickman <maord@...dia.com>
Subject: Re: cls_flower vlan matching

Mon, Mar 14, 2022 at 06:26:38PM CET, vladbu@...dia.com wrote:
>Hi Jiri,
>
>I've been debugging an issue that we encounter with OvS-created rules
>for single and double-VLAN packets.
>
>Basically, for flows with single VLAN OvS creates following tc filter:
>
>filter protocol 802.1Q pref 2 flower chain 0 
>filter protocol 802.1Q pref 2 flower chain 0 handle 0x1 
>  vlan_id 10                                                                                                            
>  vlan_prio 0             
>  vlan_ethtype ip                                                                                                       
>  dst_mac e4:2c:0b:08:00:02     
>  src_mac b8:ce:f6:05:e7:3a
>  eth_type ipv4
>  ip_flags nofrag                                                                                                       
>  skip_hw                                                                                                               
>  not_in_hw               
>        action order 1: vlan  pop pipe                                                                                  
>         index 2 ref 1 bind 1 installed 11 sec used 0 sec firstused 10 sec
>        Action statistics:                     
>        Sent 860 bytes 10 pkt (dropped 0, overlimits 0 requeues 0) 
>        backlog 0b 0p requeues 0
>        no_percpu
>
>        action order 2: mirred (Egress Redirect to device enp8s0f0_0) stolen
>        index 2 ref 1 bind 1 installed 11 sec used 0 sec firstused 10 sec
>        Action statistics:
>        Sent 860 bytes 10 pkt (dropped 0, overlimits 0 requeues 0) 
>        backlog 0b 0p requeues 0
>        cookie 16a9b603144b3e0c64a887aeb972a269
>        no_percpu
>
>Such rule also matches packets that have additional second VLAN header,
>even though filter has both eth_type and vlan_ethtype set to "ipv4".
>Looking at the code this seems to be mostly an artifact of the way
>flower uses flow dissector. First, even though looking at the uAPI
>eth_type and vlan_ethtype appear like a distinct fields, in flower they
>are all mapped to the same key->basic.n_proto. Second, flow dissector
>skips following VLAN header as no keys for FLOW_DISSECTOR_KEY_CVLAN are
>set and eventually assigns the value of n_proto to last parsed header.
>With these, such filters ignore any headers present between first VLAN
>header and first "non magic" header (ipv4 in this case) that doesn't
>result FLOW_DISSECT_RET_PROTO_AGAIN.
>
>Is described behavior intentional? Any way to enforce matching for
>header following the VLAN header?

Looks very much like a bug to me.

>
>Regards,
>Vlad