pf: u_int32_t conn counter underflow in pf_src_tree_remove_state()

Dear OpenBSD Team,

I am writing to report a bug in the PF packet filter that causes the
per-source connection counter (conn) to underflow, resulting in legitimate
connection being rejected with source rule tracking applied.

The bug was discovered during investigation of false positive IP blocking
in a production PF firewall environment and has been confirmed through
controlled lab testing.

================================================================
SYSTEM INFORMATION
================================================================
OpenBSD Version:  7.8
Architecture:     amd64

================================================================
AFFECTED FILE
================================================================
File:     sys/net/pf.c
Function: pf_src_tree_remove_state()

================================================================
BUG DESCRIPTION
================================================================

The conn counter (u_int32_t) in struct pf_src_node tracks the number of
established TCP connections from a source IP address. This counter is used
to enforce the max-src-conn rule option.

The counter is:
  INCREMENTED in: pf_src_connlimit()
                  when TCP 3-way handshake completes
                  (st->src.tcp_est flag set to 1)

  DECREMENTED in: pf_src_tree_remove_state()
                  when ANY state is removed where tcp_est=1

The bug is that tcp_est flag is NEVER cleared when a connection transitions
from ESTABLISHED to FIN_WAIT, TIME_WAIT or CLOSING state. The flag remains
set until the state is physically removed from the state table.

At high connection rates, FIN_WAIT states accumulate in the state table.
When these states are eventually removed, pf_src_tree_remove_state()
decrements the conn counter for each removal. Since the conn counter was
only incremented once per connection (at ESTABLISHED), but is decremented
once per state removal (including FIN_WAIT), the total decrements exceed
the total increments, causing unsigned integer underflow.

================================================================
AFFECTED CODE
================================================================

Current code in pf_src_tree_remove_state():

  pf_src_tree_remove_state(struct pf_state *st)
  {
      u_int32_t         timeout;
      struct pf_sn_item *sni;

      while ((sni = SLIST_FIRST(&st->src_nodes)) != NULL) {
          SLIST_REMOVE_HEAD(&st->src_nodes, next);

          if (st->src.tcp_est)
              --sni->sn->conn;        /* BUG: no zero check,
                                         tcp_est never cleared
                                         on FIN_WAIT transition */

          if (--sni->sn->states == 0) {
              timeout = st->rule.ptr->timeout[PFTM_SRC_NODE];
              if (!timeout)
                  timeout =
                      pf_default_rule.timeout[PFTM_SRC_NODE];
              sni->sn->expire = getuptime() + timeout;
          }
          pool_put(&pf_sn_item_pl, sni);
      }
  }

Note: The states counter (--sni->sn->states) also has no explicit zero
check, however it is less susceptible to underflow because states is
incremented for every new state creation (including SYN_SENT) whereas conn
is only incremented at ESTABLISHED, meaning states >= conn in normal
operation.

================================================================
IMPACT
================================================================

When underflow occurs:

  conn counter value:      ~4,294,967,105  (near 2^32)
  max-src-conn threshold:       200,000    (typical value)

  4,294,967,105 >> 200,000

Every subsequent connection from the affected source IP immediately exceeds
max-src-conn threshold, causing:

  1. Source IP added to overload table (<aggressive>)
  2. ALL connections from that IP blocked
  3. IP remains blocked until src.track node expires
     (typically 10+ seconds after last state removed)
  4. If administrator manually removes IP from overload table without
killing states and waiting for src.track expiry, IP is immediately
re-blocked on next connection attempt

This causes legitimate source IPs to be permanently blocked, appearing as
random false positives in production environments.

================================================================
REPRODUCTION
================================================================

PF rule to reproduce:

  table <aggressive> persist

  pass log quick proto tcp from any to <target> port 80 \
      flags S/SA keep state ( \
      max 10000000, \
      max-src-states 999999, \
      max-src-conn 999999, \
      max-src-conn-rate 999999/10, \
      overload <aggressive>, \
      tcp.first 5, tcp.opening 5, \
      tcp.established 3600, tcp.closing 900, \
      tcp.finwait 30, tcp.closed 30, \
      adaptive.start 6000000, \
      adaptive.end 12000000)

Note: No flush option is used. The bug occurs regardless of flush setting
(flush, flush global, or no flush).
Traffic generation (using vegeta):

  echo "GET http://<target>/" | vegeta attack -rate=1000/s
-duration=10s -timeout=5s -header="Connection: close" | vegeta report

Monitoring command to observe underflow:

  pfctl -s Sources

Expected observation:
  During test:   connections counter rises normally
  After test:    FIN_WAIT states accumulate
  During expiry: connections counter underflows to ~2^32
  Final value:   4,294,967,296 - (peak_FIN - peak_conn)

================================================================
PROPOSED FIX
================================================================

Option 1 — Minimal fix (add zero check):

  /* CURRENT (buggy): */
  if (st->src.tcp_est)
      --sni->sn->conn;

  /* FIXED (safe): */
  if (st->src.tcp_est && sni->sn->conn > 0)
      --sni->sn->conn;

  Pros: Simple one-line change
        Prevents underflow completely
        Minimal risk of regression
  Cons: conn may slightly overcount during high-rate
        FIN_WAIT accumulation (benign)


================================================================
TEST ENVIRONMENT
================================================================

Firewall:  OpenBSD
Client:    OpenBSD with vegeta package
Server:    OpenBSD with nginx package
Traffic:   vegeta 12.12.0

================================================================

Thank you for your time and for maintaining OpenBSD and PF. I am happy to
provide additional information, test results, or packet captures if helpful.

Regards,
Janak Trivedi
Stockholm Sweden