[src/trunk]: src/usr.sbin/npf/npfctl Summary: Ensure default TCP flags are ap...

[tih <tih%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/4337a2d4e70e
branches:  trunk
changeset: 450555:4337a2d4e70e
user:      tih <tih%NetBSD.org@localhost>
date:      Wed Apr 17 20:41:58 2019 +0000

description:
Summary: Ensure default TCP flags are applied to rules like 'pass stateful all'

CVS: ----------------------------------------------------------------------
CVS: CVSROOT  cvs.NetBSD.org:/cvsroot
CVS: please use "PR category/123" to have the commitmsg appended to PR 123
CVS:
CVS: Please evaluate your changes and consider the following.
CVS: Abort checkin if you answer no.
CVS: => For all changes:
CVS: Do the changed files compile?
CVS: Has the change been tested?
CVS: => If you are not completely familiar with the changed components:
CVS: Has the change been posted for review?
CVS: Have you allowed enough time for feedback?
CVS: => If the change is major:
CVS: => If the change adds files to, or removes files from $DESTDIR:
CVS: => If you are changing a library or kernel interface:
CVS: Have you successfully run "./build.sh release"?

The documented default "flags S/SAFR" for stateful rules that affect
TCP packets but don't specify any flags, doesn't actually get applied
to a rule like "pass stateful out all". The big problem with this is
that when you then do a "block return-rst" for an incoming packet, the
generated RST packet will create state for the connection attempt it's
blocking, so that a second attempt from the same source will pass.

This change makes the default flags actually apply to such simple
rules.  It also fixes a related bug in the code generation for the
flag matching, where part of the action could erroneously be omitted.

Reviewed by <rmind>
Closes PR bin/54124
Pullup to NetBSD 8

diffstat:

 usr.sbin/npf/npfctl/npf_bpf_comp.c |  8 +++-----
 usr.sbin/npf/npfctl/npf_build.c    |  7 ++++---
 2 files changed, 7 insertions(+), 8 deletions(-)

diffs (56 lines):

diff -r acaf86a6328b -r 4337a2d4e70e usr.sbin/npf/npfctl/npf_bpf_comp.c
--- a/usr.sbin/npf/npfctl/npf_bpf_comp.c        Wed Apr 17 17:44:51 2019 +0000
+++ b/usr.sbin/npf/npfctl/npf_bpf_comp.c        Wed Apr 17 20:41:58 2019 +0000
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_bpf_comp.c,v 1.11 2018/09/29 14:41:36 rmind Exp $");
+__RCSID("$NetBSD: npf_bpf_comp.c,v 1.12 2019/04/17 20:41:58 tih Exp $");
 
 #include <stdlib.h>
 #include <stdbool.h>
@@ -565,10 +565,8 @@
        };
        add_insns(ctx, insns_cmp, __arraycount(insns_cmp));
 
-       if (!checktcp) {
-               uint32_t mwords[] = { BM_TCPFL, 2, tf, tf_mask};
-               done_block(ctx, mwords, sizeof(mwords));
-       }
+       uint32_t mwords[] = { BM_TCPFL, 2, tf, tf_mask};
+       done_block(ctx, mwords, sizeof(mwords));
 }
 
 /*
diff -r acaf86a6328b -r 4337a2d4e70e usr.sbin/npf/npfctl/npf_build.c
--- a/usr.sbin/npf/npfctl/npf_build.c   Wed Apr 17 17:44:51 2019 +0000
+++ b/usr.sbin/npf/npfctl/npf_build.c   Wed Apr 17 20:41:58 2019 +0000
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_build.c,v 1.47 2019/01/19 21:19:32 rmind Exp $");
+__RCSID("$NetBSD: npf_build.c,v 1.48 2019/04/17 20:41:58 tih Exp $");
 
 #include <sys/types.h>
 #define        __FAVOR_BSD
@@ -363,7 +363,7 @@
 npfctl_build_code(nl_rule_t *rl, sa_family_t family, const opt_proto_t *op,
     const filt_opts_t *fopts)
 {
-       bool noproto, noaddrs, noports, need_tcpudp = false;
+       bool noproto, noaddrs, noports, nostate, need_tcpudp = false;
        const addr_port_t *apfrom = &fopts->fo_from;
        const addr_port_t *apto = &fopts->fo_to;
        const int proto = op->op_proto;
@@ -375,7 +375,8 @@
        noproto = family == AF_UNSPEC && proto == -1 && !op->op_opts;
        noaddrs = !apfrom->ap_netaddr && !apto->ap_netaddr;
        noports = !apfrom->ap_portrange && !apto->ap_portrange;
-       if (noproto && noaddrs && noports) {
+       nostate = !(npf_rule_getattr(rl) & NPF_RULE_STATEFUL);
+       if (noproto && noaddrs && noports && nostate) {
                return false;
        }