system/4941: pfctl options ("set skip on") - very tiny race condition during pf.conf reloads

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

system/4941: pfctl options ("set skip on") - very tiny race condition during pf.conf reloads

kloske
>Number:         4941
>Category:       system
>Synopsis:       "set skip on $int_f" during "pfctl -f /etc/pf.conf" causes tcp RST and icmp port unreachable (tiny race condition?)
>Confidential:   yes
>Severity:       non-critical
>Priority:       medium
>Responsible:    bugs
>State:          open
>Quarter:        
>Keywords:      
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Dec 16 07:00:01 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Jon Kloske
>Release:        OPENBSD_3_8
>Organization:
net
>Environment:
        System      : OpenBSD 3.8
        Architecture: OpenBSD.i386
        Machine     : i386
>Description:
Using "set skip on $int_if" (as a quicker way to do "pass on $int_if") seems
to temporarily be ignored during a ruleset reload. This is especially obvious
when attemping to reload the ruleset via a SSH connection over $int_if, since
around %80-90% of the time this results in the connection being reset.

The machine this problem showed up on is a 486DX66 with 36MB of ram, so it
is possible the problem is limited to slower platforms.

I've been told that whilst ruleset reloads are atomic, options are a series
of ioctls, and that if a packet came in during the reloading of the options,
it could be blocked.

Using tcpdump on the $int_if, I was able to show that during reloads of a
ruleset containing the "set skip on $int_if" optimization, pf indeed did
cause TCP RSTs and ICMP port unreachables for a small amount of time:

Dec 16 09:45:32.209876 0:0:c0:54:45:98 0:20:18:a2:37:11 0800 54: 10.0.0.1.22 > 10.0.0.2.4644: R [tcp sum ok] 160:160(0) ack 41 win 0 (DF) [tos 0x10] (ttl 64, id 29371, len 40)
Dec 16 09:45:33.048185 0:0:c0:54:45:98 0:20:18:a2:37:11 0800 70: 10.0.0.1 > 10.0.0.2: icmp: 10.0.0.1 udp port 53 unreachable for 10.0.0.2.4675 > 10.0.0.1.53: [|domain] (ttl 128, id 22605, len 53) (ttl 255, id 6665, len 56)
Dec 16 09:45:33.912518 0:0:c0:54:45:98 0:20:18:a2:37:11 0800 70: 10.0.0.1 > 10.0.0.2: icmp: 10.0.0.1 udp port 53 unreachable for 10.0.0.2.1689 > 10.0.0.1.53: [|domain] (ttl 128, id 22606, len 60) (ttl 255, id 8158, len 56)
Dec 16 09:45:33.914705 0:0:c0:54:45:98 0:20:18:a2:37:11 0800 70: 10.0.0.1 > 10.0.0.2: icmp: 10.0.0.1 udp port 53 unreachable for 10.0.0.2.1689 > 10.0.0.1.53: [|domain] (ttl 128, id 22607, len 60) (ttl 255, id 5660, len 56)
Dec 16 09:45:34.896795 0:0:c0:54:45:98 0:20:18:a2:37:11 0800 54: 10.0.0.1.22 > 10.0.0.2.4601: R [tcp sum ok] 1636:1636(0) ack 21 win 0 (DF) [tos 0x10] (ttl 64, id 31530, len 40)

Additionally, tcpdump on the pflog0 interface revealed this (it must be
noted that this was during a different capture session but the method was
identical):

Dec 16 09:39:05.048155 rule 3/(match) [uid 0, pid 3477] block out on we1: 10.0.0.1.22 > 10.0.0.2.3825: [|tcp] [tos 0x10] (ttl 64, id 29361, len 204)
Dec 16 09:39:05.060534 rule 2/(match) [uid 0, pid 3477] block in on we1: 10.0.0.2.4599 > 10.0.0.1.22: [|tcp] (DF) (ttl 128, id 18647, len 40)

Whilst this problem does appear to be a lot worse for slower machines, it
could theoretially affect any machine. Also, as the loopback interface is
by default listed in the "set skip on" declaration in the distribution
example pf.conf, this could potentially affect the operation of local
services or daemons that rely on local inet domain sockets to communicate.
>How-To-Repeat:
1. Create a ruleset that uses "set skip on" on an interface (the default
pf.conf example ruleset contains this on localhost.)
2. On a slow machine with plenty of traffic, reload the ruleset.
Note that this is likely to affect at least a few people on faster machines
with less traffic, purely on a probability basis.
>Fix:
None known. Workaround involves ensuring that all interfaces that appear
in a "set skip on" declaration also have corresponding "pass on" rules in
the body of the ruleset.


>Release-Note:
>Audit-Trail:
>Unformatted: