Page MenuHomeFreeBSD

ipfw: fix compatibility with frag and older rule sets
AbandonedPublic

Authored by 2khramtsov_gmail.com on Sep 8 2020, 2:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jan 12, 2:24 AM
Unknown Object (File)
Dec 16 2022, 1:46 PM
Unknown Object (File)
Nov 27 2022, 4:16 PM
Subscribers

Details

Reviewers
glebius
ae
Group Reviewers
network
Summary

Recent change to frag regressed backward compatibility.
Handle the case when there is an option after "frag" which is not a frag token.

Test Plan

Try ipfw add 00330 deny all from any to any frag in via wlan0
(example from: https://www.freebsd.org/doc/handbook/firewalls-ipfw.html)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

An attempt to also handle "!" with frag options and options like !via
(I hope review comment edits do not make new notifications)

Tested with:

ipfw add deny all from any to any frag
ipfw add deny all from any to any frag df
ipfw add deny all from any to any frag df,mf
ipfw add deny all from any to any frag !df
ipfw add deny all from any to any frag !df,mf
ipfw add deny all from any to any frag !df,!mf

ipfw add deny all from any to any frag in
ipfw add deny all from any to any frag !in
ipfw add deny all from any to any frag out
ipfw add deny all from any to any frag !out

ipfw add deny all from any to any frag via wlan0
ipfw add deny all from any to any frag !via wlan0
ipfw add deny all from any to any frag not via wlan0

I think this patch is too complicated. Can you properly test this patch instead? https://people.freebsd.org/~ae/ipfw_frag.diff

In D26358#586750, @ae wrote:

I think this patch is too complicated. Can you properly test this patch instead? https://people.freebsd.org/~ae/ipfw_frag.diff

That's a good approach.
You need to explicitly set "clear" in the compatibility branch, too.

In D26358#586750, @ae wrote:

I think this patch is too complicated. Can you properly test this patch instead? https://people.freebsd.org/~ae/ipfw_frag.diff

I tested (ipfw add / ipfw show) ipfw_frag.diff and your approach works.
It also looks miles better. When you commit ipfw_frag.diff, please close this review. Thanks.

In D26358#586779, @evgeniy_khramtsov.org wrote:
In D26358#586750, @ae wrote:

I think this patch is too complicated. Can you properly test this patch instead? https://people.freebsd.org/~ae/ipfw_frag.diff

I tested (ipfw add / ipfw show) ipfw_frag.diff and your approach works.
It also looks miles better. When you commit ipfw_frag.diff, please close this review. Thanks.

For reliability, also tested with http://icmpcheckv6.popcount.org/ and http://icmpcheck.popcount.org/.
Rule counter increments and fragments were dropped with a legacy rule.