[Bug 238796] ipfilter: fix unremovable rules and rules checksum for comparison

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238796

            Bug ID: 238796
           Summary: ipfilter: fix unremovable rules and rules checksum for
                    comparison
           Product: Base System
           Version: 12.0-STABLE
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: kern
          Assignee: bugs at FreeBSD.org
          Reporter: msl0000023508 at gmail.com

Created attachment 205322
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=205322&action=edit
freebsd-ipfilter-rule-compare-fix.diff

This patch fix 2 bugs.

1. Unremovable rules:

A filter rule could becomes non-removable if it contains 'route-to' (displayed
as 'to' in ipfstat(8) output), 'reply-to' or 'due-to' keyword to specify an
interface name for routing.

For example:

[root at x ~]# ipfstat -Rion
# empty list for ipfilter(out)
@1 ...
@2 ...
@3 ...
@4 pass in quick on vboxnet0 to tun0:10.1.202.11 inet proto tcp from
10.12.4.0/24 port = 22 to any
@5 pass in quick on vboxnet0 to tun0:10.1.202.11 inet from 10.0.5.52/32 to any
[root at x ~]# echo "pass in quick on vboxnet0 to tun0:10.1.202.11 inet proto tcp
from 10.12.4.0/24 port = 22 to any" | ipf -r -f -
29:1:ioctl(delete rule): rule not found for removing
[root at x ~]# echo "pass in quick on vboxnet0 to tun0:10.1.202.11 inet proto tcp
from 10.12.4.0/24 port = 22 to any" | ipf -f -
[root at x ~]# ipfstat -Rion
# empty list for ipfilter(out)
@1 ...
@2 ...
@3 ...
@4 pass in quick on vboxnet0 to tun0:10.1.202.11 inet proto tcp from
10.12.4.0/24 port = 22 to any
@5 pass in quick on vboxnet0 to tun0:10.1.202.11 inet from 10.0.5.52/32 to any
@6 pass in quick on vboxnet0 to tun0:10.1.202.11 inet proto tcp from
10.12.4.0/24 port = 22 to any

As showing by the output, the rule @4 cannot be removed by using 'ipf -r';
trying to add the exactly same rule succeed, as rule @6; but duplicated rules
are not allowed by the ipfilter design.
Rule @5 has the same issue.

The cause of this bug is when comparing 2 rules, the code failed to exclude
some volatile variables such as pointers and index numbers to a volatile array.
The pointers included in rules comparison are 'fd_ptr' in 'frdest_t', which are
turn be included as 'fr_tifs' and 'fr_dif' in 'struct frentry', the rule entry
structure. The index numbers are 'fr_ifnames' in 'struct frentry', and
'fd_name', 'fr_tifs', 'fr_dif'; all those numbers are indexing strings in array
'fr_names' in 'struct frentry'; the actual strings should be compared instead
of the indexes, since the string sequence inside 'fr_ifnames' may differ even
between 2 same rules.
Another variable should be excluded from comparison is 'fd_local' in
'frdest_t'. This variable is a hit for the code to determine whether an address
is at local; it shouldn't be compared, because this could be changed during
runtime (an address was added to an interface after a rule was added).


2. Inefficient rule checksum

There is a member 'fr_cksum' in 'struct frentry'; it was designed to speedup
rules comparison; see
https://svnweb.freebsd.org/base/stable/12/sys/contrib/ipfilter/netinet/fil.c?revision=349223&view=markup#l4922

This above code calculates first part of the checksum starting from member
'fr_func', ending at 'fr_chsum'. However in ipfilter revision '2580062 from/to
targets should be able to use any interface name; 2605045 destination lists
aren't loaded; 2605049 destination lists need testing; 2637667 pool stats
structures should not have pointers; 2644504 cannot list configured destination
lists; 2644536 destination lists need more selection policies' branch
'v5-1-RELEASE' on 2009-03-08 09:08:32, the member 'fr_chsum' was moved, sitting
before 'fr_func', causing this calculation be skipped.

-- 
You are receiving this mail because:
You are the assignee for the bug.