ipfw: fix compatibility with frag and older rule sets
AbandonedPublic
Actions

Authored by • 2khramtsov_gmail.com on Sep 8 2020, 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 - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

• 2khramtsov_gmail.com created this revision.Sep 8 2020, 2:49 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 8 2020, 2:49 PM

• 2khramtsov_gmail.com requested review of this revision.Sep 8 2020, 2:49 PM

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.