RFC: enabling pf syncookies by default

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Sat, 24 Sep 2022 13:32:50 UTC
Hi,

During EuroBSDCon 2022 Eirik asked why we don’t enable pf’s 
syncookie feature (in adaptive mode) by default.

I don’t really have a good answer, so I’m inclined to make this 
change.

For those not familiar with it, syncookies are a mechanism to resist syn 
flood DoS attacks. They’re enabled by default in the IP stack, but if 
you’re running pf a syn flood would still exhaust pf’s state table, 
even if the network stack itself could cope.

In adaptive mode all pf does is track the number of half-open states (as 
created when a SYN packet arrives). If that exceeds a set limit the 
syncookie feature activates.
The upside of that is that all we pay for is the counting of the number 
of half-open states, which has no meaningful performance impact, at 
least until we exceed the high water mark.

Does anyone see a good reason not to do so?

Best regards,
Kristof

Proposed patch:

	commit 77b5994c89945e52eb23ec6f5810a1186abc6df4 (HEAD -> main)
	Author: Kristof Provost <kp@FreeBSD.org>
	Date:   Sat Sep 24 14:49:25 2022 +0200

	    pf: default syncookies to adaptive mode

	    The cost of enabling syncookies in adaptive mode is very low 
(basically
	    a single atomic add when we create a new half-open state), and the
	    payoff when under SYN flood is huge.

	    So, enable adaptive mode by default.

	    Suggested by:   Eirik Øverby

	diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
	index 84235031f8e6..4e1eafe016f1 100644
	--- a/sys/netpfil/pf/pf_ioctl.c
	+++ b/sys/netpfil/pf/pf_ioctl.c
	@@ -311,6 +311,8 @@ pfattach_vnet(void)
	 {
	        u_int32_t *my_timeout = V_pf_default_rule.timeout;

	+       bzero(&V_pf_status, sizeof(V_pf_status));
	+
	        pf_initialize();
	        pfr_initialize();
	        pfi_initialize_vnet();
	@@ -379,7 +381,6 @@ pfattach_vnet(void)
	        my_timeout[PFTM_ADAPTIVE_START] = PFSTATE_ADAPT_START;
	        my_timeout[PFTM_ADAPTIVE_END] = PFSTATE_ADAPT_END;

	-       bzero(&V_pf_status, sizeof(V_pf_status));
	        V_pf_status.debug = PF_DEBUG_URGENT;

	        V_pf_pfil_hooked = 0;
	diff --git a/sys/netpfil/pf/pf_syncookies.c 
b/sys/netpfil/pf/pf_syncookies.c
	index 6a375411d8ea..c16d9f3509fd 100644
	--- a/sys/netpfil/pf/pf_syncookies.c
	+++ b/sys/netpfil/pf/pf_syncookies.c
	@@ -126,7 +126,12 @@ pf_syncookies_init(void)
	 {
	        callout_init(&V_pf_syncookie_status.keytimeout, 1);
	        PF_RULES_WLOCK();
	-       pf_syncookies_setmode(PF_SYNCOOKIES_NEVER);
	+       V_pf_syncookie_status.hiwat = PF_SYNCOOKIES_HIWATPCT *
	+           V_pf_limits[PF_LIMIT_STATES].limit / 100;
	+       V_pf_syncookie_status.lowat = PF_SYNCOOKIES_LOWATPCT *
	+           V_pf_limits[PF_LIMIT_STATES].limit / 100;
	+       pf_syncookies_setmode(PF_SYNCOOKIES_ADAPTIVE);
	+
	        PF_RULES_WUNLOCK();
	 }