Page MenuHomeFreeBSD

ipfw: fix compatibility with frag and older rule sets
AbandonedPublic

Authored by evgeniy_khramtsov.org on Tue, Sep 8, 2:49 PM.

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
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

evgeniy_khramtsov.org requested review of this revision.Tue, Sep 8, 2:49 PM
evgeniy_khramtsov.org updated this revision to Diff 76778.EditedTue, Sep 8, 5:29 PM
evgeniy_khramtsov.org added a reviewer: network.

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
ae added a comment.Thu, Sep 10, 10:09 AM

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#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.

ae added a comment.Fri, Sep 11, 10:15 AM

Committed as rS365628.