From: Matthew Dillon <dillon@apollo.backplane.com>
Date: Sat, 28 Jun 2014 02:05:33 +0000 (-0700)
Subject: pfctl - Change default keep-policy to bring more in-line with other BSDs
X-Git-Url: https://gitweb.dragonflybsd.org/~mneumann/dragonfly.git/commitdiff_plain/f72d5ff0d8dd6b9afab31a2e1d51d5539bf85b03

pfctl - Change default keep-policy to bring more in-line with other BSDs

* Change the default keep-policy to the equivalent of:

  set keep-policy keep state (pickups, sloppy)

* This is being done because without keep state PF is simply going to be
  too inefficient for any reasonable set of rules, and we no longer want
  to make users set the keep-policy line when keep state is already the
  default in other BSD systems.

* Note that we also set pickups and sloppy by default.  This allows the
  router and/or PF to be restarted and allows packet routing to change
  mid-stream without causing all active TCP connections to drop.  This
  may not be the default in other BSD systems but it should be.  Being
  ultra strict here to improve security against ICMP-based attacks removes
  too much flexibility to be appropriate.  Proper TCP implementations
  already do sequence space checks for RST packets.
---

diff --git a/usr.sbin/pfctl/parse.y b/usr.sbin/pfctl/parse.y
index ebc88b0cfa..47cbc486ba 100644
--- a/usr.sbin/pfctl/parse.y
+++ b/usr.sbin/pfctl/parse.y
@@ -5778,6 +5778,49 @@ parse_config(char *filename, struct pfctl *xpf)
 	blockpolicy = PFRULE_DROP;
 	require_order = 1;
 
+	/*
+	 * Default keep-polifcy is:
+	 *
+	 * set keep-policy keep state (pickups, sloppy)
+	 *
+	 * pickups - Allows TCP connections to be picked up mid-stream,
+	 *	     meaning that router restarts and changes in packet
+	 *	     routing will not destroy TCP connections already in
+	 *	     progress.
+	 *
+	 * sloppy  - Window scale options are only sent with SYN and SYN+ACK,
+	 *	     when picking up a connection in the middle, such as when
+	 *	     the router has been restarted or when multi-pathing
+	 *	     occurs, PF will not understand the window scale.  This
+	 *	     tells PF to ignore the sequence space tests for this case.
+	 *
+	 * While it is true that ICMP attack vectors are easier with these
+	 * options, the problem is that PF just has no business defaulting
+	 * to strict enforcement because it completely destroys the system
+	 * operator's ability to multi-path packets, to change packet routing,
+	 * or to restart the router (or PF) without kicking *ALL* active TCP
+	 * connections that were flowing through the router at the time.  It
+	 * unacceptable to kick active connections by default.
+	 */
+	{
+		struct node_state_opt *opt1 = calloc(1, sizeof(*opt1));
+		struct node_state_opt *opt2 = calloc(1, sizeof(*opt2));
+
+		opt1->type = PF_STATE_OPT_PICKUPS;
+		opt1->data.pickup_mode = PF_PICKUPS_ENABLED;
+		opt1->next = NULL;
+		opt1->tail = opt1;
+
+		opt2->type = PF_STATE_OPT_SLOPPY;
+		opt2->next = NULL;
+		opt2->tail = opt1;
+
+		default_keeppolicy_action = PF_STATE_NORMAL;
+		default_keeppolicy_options = opt1;
+		opt1->next = opt2;
+		opt1->tail = opt2;
+	}
+
 	if ((file = pushfile(filename, 0)) == NULL) {
 		warn("cannot open the main config file!");
 		return (-1);