NPF: imply SYN-only check for the stateful rules by default (when inspecting

TCP packets).  Many users trip here.  This behaviour can be overriden with the
explicit "flags" keyword, but other configuration does not really make sense.

usr.sbin/npf/npfctl/npf.conf.5

@@ -1,4 +1,4 @@
-.\"    $NetBSD: npf.conf.5,v 1.39 2014/02/14 01:52:58 rmind Exp $
+.\"    $NetBSD: npf.conf.5,v 1.40 2014/05/15 02:34:29 rmind Exp $
 .\"
 .\" Copyright (c) 2009-2014 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd February 14, 2014
+.Dd May 15, 2014
 .Dt NPF.CONF 5
 .Os
 .Sh NAME
@@ -155,6 +155,25 @@ block out final pcap-filter "tcp and dst 10.1.1.252"
 .Pp
 Fragments are not selectable since NPF always reassembles packets
 before further processing.
+.Ss Stateful
+Stateful packet inspection is enabled using
+.Cd stateful
+or
+.Cd stateful-ends
+keywords.
+The former creates a state which is uniquely identified by a 5-tuple (source
+and destination IP addresses, port numbers and an interface identifier).
+The latter excludes the interface identifier and must be used with
+precaution.
+In both cases, a full TCP state tracking is performed for TCP connections
+and a limited tracking for message-based protocols (UDP and ICMP).
+.Pp
+By default, stateful rule implies SYN-only flag check ("flags S/SAFR")
+for the TCP packets.
+It is not advisable to change this behavior, however,
+it can be overriden with
+.Cd flags
+keyword.
 .Ss Map
 Network Address Translation (NAT) is expressed in a form of segment mapping.
 The translation may be dynamic (stateful) or static (stateless).
@@ -252,7 +271,8 @@ rule-list	= [ rule new-line ] rule-list
 
 npf-filter	= [ "family" family-opt ] [ "proto" protocol [ proto-opts ] ]
 		  ( "all" | filt-opts )
-static-rule	= ( "block" [ block-opts ] | "pass" ) [ "stateful" ]
+static-rule	= ( "block" [ block-opts ] | "pass" )
+		  [ "stateful" | "stateful-ends" ]
 		  [ "in" | out" ] [ "final" ] [ "on" interface ]
 		  ( npf-filter | "pcap-filter" pcap-filter-expr )
 		  [ "apply" proc-name ]

usr.sbin/npf/npfctl/npf_bpf_comp.c

@@ -1,7 +1,7 @@
-/*	$NetBSD: npf_bpf_comp.c,v 1.4 2014/03/15 08:46:01 rmind Exp $	*/
+/*	$NetBSD: npf_bpf_comp.c,v 1.5 2014/05/15 02:34:29 rmind Exp $	*/
 
 /*-
- * Copyright (c) 2010-2013 The NetBSD Foundation, Inc.
+ * Copyright (c) 2010-2014 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This material is based upon work partially supported by The
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_bpf_comp.c,v 1.4 2014/03/15 08:46:01 rmind Exp $");
+__RCSID("$NetBSD: npf_bpf_comp.c,v 1.5 2014/05/15 02:34:29 rmind Exp $");
 
 #include <stdlib.h>
 #include <stdbool.h>
@@ -513,12 +513,23 @@ npfctl_bpf_ports(npf_bpf_t *ctx, u_int opts, in_port_t from, in_port_t to)
  * npfctl_bpf_tcpfl: code block to match TCP flags.
  */
 void
-npfctl_bpf_tcpfl(npf_bpf_t *ctx, uint8_t tf, uint8_t tf_mask)
+npfctl_bpf_tcpfl(npf_bpf_t *ctx, uint8_t tf, uint8_t tf_mask, bool checktcp)
 {
 	const u_int tcpfl_off = offsetof(struct tcphdr, th_flags);
 
 	/* X <- IP header length */
 	fetch_l3(ctx, AF_UNSPEC, X_EQ_L4OFF);
+	if (checktcp) {
+		const u_int jf = (tf_mask != tf) ? 3 : 2;
+		assert(ctx->ingroup == false);
+
+		/* A <- L4 protocol; A == TCP?  If not, jump out. */
+		struct bpf_insn insns_tcp[] = {
+			BPF_STMT(BPF_LD+BPF_W+BPF_MEM, BPF_MW_L4PROTO),
+			BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, IPPROTO_TCP, 0, jf),
+		};
+		add_insns(ctx, insns_tcp, __arraycount(insns_tcp));
+	}
 
 	struct bpf_insn insns_tf[] = {
 		/* A <- TCP flags */
@@ -540,8 +551,10 @@ npfctl_bpf_tcpfl(npf_bpf_t *ctx, uint8_t tf, uint8_t tf_mask)
 	};
 	add_insns(ctx, insns_cmp, __arraycount(insns_cmp));
 
-	uint32_t mwords[] = { BM_TCPFL, 2, tf, tf_mask};
+	if (!checktcp) {
-	done_block(ctx, mwords, sizeof(mwords));
+		uint32_t mwords[] = { BM_TCPFL, 2, tf, tf_mask};
+		done_block(ctx, mwords, sizeof(mwords));
+	}
 }
 
 /*

usr.sbin/npf/npfctl/npf_build.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_build.c,v 1.36 2014/02/13 03:34:40 rmind Exp $	*/
+/*	$NetBSD: npf_build.c,v 1.37 2014/05/15 02:34:29 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2011-2014 The NetBSD Foundation, Inc.
@@ -34,11 +34,12 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_build.c,v 1.36 2014/02/13 03:34:40 rmind Exp $");
+__RCSID("$NetBSD: npf_build.c,v 1.37 2014/05/15 02:34:29 rmind Exp $");
 
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
+#include <netinet/tcp.h>
 
 #include <stdlib.h>
 #include <inttypes.h>
@@ -267,7 +268,7 @@ npfctl_build_proto(npf_bpf_t *ctx, sa_family_t family, const opt_proto_t *op)
 			assert(npfvar_get_count(popts) == 2);
 			tf = npfvar_get_data(popts, NPFVAR_TCPFLAG, 0);
 			tf_mask = npfvar_get_data(popts, NPFVAR_TCPFLAG, 1);
-			npfctl_bpf_tcpfl(ctx, *tf, *tf_mask);
+			npfctl_bpf_tcpfl(ctx, *tf, *tf_mask, false);
 		}
 		break;
 	case IPPROTO_ICMP:
@@ -328,6 +329,16 @@ npfctl_build_code(nl_rule_t *rl, sa_family_t family, const opt_proto_t *op,
 	/* Build layer 4 protocol blocks. */
 	npfctl_build_proto(bc, family, op);
 
+	/*
+	 * If this is a stateful rule and TCP flags are not specified,
+	 * then add "flags S/SAFR" filter for TCP protocol case.
+	 */
+	if ((npf_rule_getattr(rl) & NPF_RULE_STATEFUL) != 0 &&
+	    (proto == -1 || (proto == IPPROTO_TCP && !op->op_opts))) {
+		npfctl_bpf_tcpfl(bc, TH_SYN,
+		    TH_SYN | TH_ACK | TH_FIN | TH_RST, proto == -1);
+	}
+
 	/* Build IP address blocks. */
 	npfctl_build_vars(bc, family, apfrom->ap_netaddr, MATCH_SRC);
 	npfctl_build_vars(bc, family, apto->ap_netaddr, MATCH_DST);

usr.sbin/npf/npfctl/npfctl.h

@@ -1,4 +1,4 @@
-/*	$NetBSD: npfctl.h,v 1.36 2014/02/13 03:34:40 rmind Exp $	*/
+/*	$NetBSD: npfctl.h,v 1.37 2014/05/15 02:34:29 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2009-2013 The NetBSD Foundation, Inc.
@@ -166,7 +166,7 @@ void		npfctl_bpf_proto(npf_bpf_t *, sa_family_t, int);
 void		npfctl_bpf_cidr(npf_bpf_t *, u_int, sa_family_t,
 		    const npf_addr_t *, const npf_netmask_t);
 void		npfctl_bpf_ports(npf_bpf_t *, u_int, in_port_t, in_port_t);
-void		npfctl_bpf_tcpfl(npf_bpf_t *, uint8_t, uint8_t);
+void		npfctl_bpf_tcpfl(npf_bpf_t *, uint8_t, uint8_t, bool);
 void		npfctl_bpf_icmp(npf_bpf_t *, int, int);
 void		npfctl_bpf_table(npf_bpf_t *, u_int, u_int);