[Bug 294860] pfctl: optimizer removes textually-identical rules without checking for intervening state changes

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

            Bug ID: 294860
           Summary: pfctl: optimizer removes textually-identical rules
                    without checking for intervening state changes
           Product: Base System
           Version: 15.0-RELEASE
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Many People
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: andy@codeedog.com

OS: 15.0-RELEASE-p6 FreeBSD 15.0-RELEASE-p6 GENERIC amd64

Expected behavior:

Optimizer does not make rule changes that modify (conflict with) the filtering
behavior of the original packet filter file.

Observed behavior:

Optimizer removes (deduplicates) meaningful entries in the configuration
resulting in packets that lets through packets that were explicitly blocked.

# Example: pf_tag.conf
block in tagged FOO   # This block rule is skipped (packet untagged)
pass in tag FOO       # Packet tagged
block in tagged FOO   # This block rule will be dropped

> pfctl -vf pf_tag.conf
No ALTQ support in kernel
ALTQ related functions disabled
block drop in all tagged FOO
pass in all flags S/SA keep state tag FOO
block drop in all tagged FOO -- rule was already present

> pfctl -sr
block drop in all tagged FOO
pass in all flags S/SA keep state tag FOO

Packets that the original ruleset blocked (via tag FOO) are now passed.

Other notable deduplication tests:

# Example pf_pass_sandwich.conf:
block in
pass in
block in                   # -- rule was already present

# Example pf_block_sandwich.conf
pass in
block in
pass in                    # -- rule was already present

Discussion

Rule deduplication is based solely on syntactic analysis (memcmp - of rule
structs in pfctl_optimize.c) rather than context analysis of the packet flow.
Context analysis would surface intervening state changes. In the pf_tag example
the intervening "tag FOO" changes the packet's internal state which fires the
second block rule. Although the two block rules appear identical, due to packet
state changes they are not and it is neither safe nor correct to remove the
second block rule.

Changing which duplicate is removed does not solve the problem. Here's an
example in which deduplicating the first rule results in incorrect behavior.

# Example: pf_dedup_first_instance_fails.conf
# 
# If dedup behavior switches to first rule removed, the sequence below
# when optimized lets packets through because second rule doesn't fire
# on packets tagged with "FOO"

pass in tag BAR
block in quick tagged BAR
match in tag FOO
block in quick tagged BAR

Syntax matching alone (memcmp) is insufficient for rule deduplication. Only
semantic analysis determines proper deduplication when examining intervening
state changes.

Workaround

Due to Bug 294858, the optimizer cannot currently be turned off. So, that is
not a workaround.

Instead, adding a unique label to the incorrectly deduplicated rule causes the
rule to be retained. For example:

# Example: pf-with-label.conf
block in
pass in
block in label R3


> pfctl -vf pf-with-label.conf
No ALTQ support in kernel
ALTQ related functions disabled
block drop in all
pass in all flags S/SA keep state
block drop in all label "R3"

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