netdev - Re: [net-next 0/2] net: sched: cls-flower: add support for port-based fragment filtering

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]

Date:   Mon, 1 Jun 2020 00:09:58 -0400
From:   Jason Baron <jbaron@...mai.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
        jiri@...nulli.us
Subject: Re: [net-next 0/2] net: sched: cls-flower: add support for port-based
 fragment filtering



On 5/29/20 7:52 PM, David Miller wrote:
> From: Jason Baron <jbaron@...mai.com>
> Date: Wed, 27 May 2020 16:25:28 -0400
> 
>> Port based allow rules must currently allow all fragments since the
>> port number is not included in the 1rst fragment. We want to restrict
>> allowing all fragments by inclucding the port number in the 1rst
>> fragments.
>>
>> For example, we can now allow fragments for only port 80 via:
>>
>> # tc filter add dev $DEVICE parent ffff: priority 1 protocol ipv4 flower
>>   ip_proto tcp dst_port 80 action pass
>> # tc filter add dev $DEVICE parent ffff: priority 2 protocol ipv4 flower
>>   ip_flags frag/nofirstfrag action pass
>>
>> The first patch includes ports for 1rst fragments.
>> The second patch adds test cases, demonstrating the new behavior.
> 
> But this is only going to drop the first frag right?
>

Yes, only the first frag is dropped, as subsequent packets wouldn't
have the port #. This will prevent re-assembly since the first fragment
is missing. Currently, drop rules on ports will just allow all fragments
through, so this seems to me to be easy to subvert.

> Unless there is logic to toss the rest of the frags this seems
> extremely hackish as best.
> 
> I don't want to apply this as-is, it's a short sighted design
> as far as I am concerned.
> 

The use-case we are using this patch for is to allow specific ports
through the firewall without having to allow all fragments. So the
rules look like:

1) Allow port 80 including first frags

# tc filter add dev $DEVICE parent ffff: priority 1 protocol ipv4 flower
  ip_proto tcp dst_port 80 action pass

2) Allow all non-first frags

# tc filter add dev $DEVICE parent ffff: priority 2 protocol ipv4 flower
  ip_flags frag/nofirstfrag action pass

3) Drop everything left

# tc filter add dev $DEVICE parent ffff: priority 3 flower action drop


This allows one to process fragments without allowing all fragments to
all ports.

I'm certainly open to other ideas on how to improve this.

Thanks,

-Jason