NetBSD Problem Report #54124

From www@netbsd.org  Tue Apr 16 14:49:37 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 3251D7A169
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 16 Apr 2019 14:49:37 +0000 (UTC)
Message-Id: <20190416144935.C57707A1B2@mollari.NetBSD.org>
Date: Tue, 16 Apr 2019 14:49:35 +0000 (UTC)
From: venture37@geeklan.co.uk
Reply-To: venture37@geeklan.co.uk
To: gnats-bugs@NetBSD.org
Subject: npfctl only applies 'flags S/SAFR' to a stateful rule if the protocol is explicitly TCP, not when proto is omitted
X-Send-Pr-Version: www-1.0

>Number:         54124
>Category:       bin
>Synopsis:       npfctl only applies 'flags S/SAFR' to a stateful rule if the protocol is explicitly TCP, not when proto is omitted
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    tih
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 16 14:50:00 +0000 2019
>Closed-Date:    Fri Apr 19 13:24:59 +0000 2019
>Last-Modified:  Fri Apr 19 13:24:59 +0000 2019
>Originator:     Sevan Janiyan
>Release:        NetBSD-HEAD
>Organization:
>Environment:
NetBSD 8.99.37 amd64 x86_64
>Description:
When using a generic 'pass stateful out all' rule without explicitly specifying a protocol and a default "block return all' rule, a state is created for all tcp related traffic, including for traffic which was blocked.
>How-To-Repeat:
Use the following npf ruleset:

alg "icmp"

procedure "log" {
    # Send log events to npflog0, see npfd(8)
    log: npflog0
}

group default {
    # Default deny, otherwise last matching rule wins
    block return all apply "log"

    # Don't block loopback
    pass on lo0 all

    # Allow incoming DHCP server responses
    pass in family inet4 proto udp from any port bootps to any port bootpc
    pass in family inet6 proto udp from any to any port "dhcpv6-client"

    # Allow IPv6 ICMP
    pass family inet6 proto ipv6-icmp all

    # Allow incoming IPv4 pings
    pass in family inet4 proto icmp icmp-type echo all

    # Allow being tracerouted
    pass in proto udp to any port 33434-33600

    # Allow incoming mDNS traffic from neighbours
    pass in proto udp to any port mdns

    # Allow all outbound traffic
    pass stateful out all
}

load ruleset and check stats with 'npfctl stats', note the ruleset block and state allocations count

from another host, run 'nmap -v -A $netbsdhost' where $netbsdhost is the machine with the npf rules set loaded.

Run 'npfctl stats' again and you should see a rise in both ruleset block and state allocations count.
>Fix:
As a workaround, one has to supplement the 'pass stateful out all' rule with
pass out proto tcp all
pass stateful out proto tcp all

npfctl validate will show the generated rules set as

pass stateful out all # id="0"
pass out proto tcp # id="0"
pass stateful out proto tcp flags S/FSRA # id="0"

Repeating the nmap test, with these rules in place, only the ruleset block count rises.

>Release-Note:

>Audit-Trail:
From: Sevan Janiyan <venture37@geeklan.co.uk>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/54124: npfctl only applies 'flags S/SAFR' to a stateful rule
 if the protocol is explicitly TCP, not when proto is omitted
Date: Wed, 17 Apr 2019 11:32:07 +0100

 Proposed fix by <tih>

 https://mail-index.netbsd.org/tech-net/2019/04/17/msg007314.html

From: "Tom Ivar Helbekkmo" <tih@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54124 CVS commit: src/usr.sbin/npf/npfctl
Date: Wed, 17 Apr 2019 20:41:59 +0000

 Module Name:	src
 Committed By:	tih
 Date:		Wed Apr 17 20:41:59 UTC 2019

 Modified Files:
 	src/usr.sbin/npf/npfctl: npf_bpf_comp.c npf_build.c

 Log Message:
 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


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 src/usr.sbin/npf/npfctl/npf_bpf_comp.c
 cvs rdiff -u -r1.47 -r1.48 src/usr.sbin/npf/npfctl/npf_build.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54124 CVS commit: [netbsd-8] src/usr.sbin/npf/npfctl
Date: Fri, 19 Apr 2019 09:10:50 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Apr 19 09:10:50 UTC 2019

 Modified Files:
 	src/usr.sbin/npf/npfctl [netbsd-8]: npf_bpf_comp.c npf_build.c

 Log Message:
 Pull up following revision(s) (requested by tih in ticket #1232):

 	usr.sbin/npf/npfctl/npf_build.c: revision 1.48
 	usr.sbin/npf/npfctl/npf_bpf_comp.c: revision 1.12

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

 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


 To generate a diff of this commit:
 cvs rdiff -u -r1.10 -r1.10.6.1 src/usr.sbin/npf/npfctl/npf_bpf_comp.c
 cvs rdiff -u -r1.44 -r1.44.4.1 src/usr.sbin/npf/npfctl/npf_build.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: bin-bug-people->tih
Responsible-Changed-By: sevan@NetBSD.org
Responsible-Changed-When: Fri, 19 Apr 2019 13:24:59 +0000
Responsible-Changed-Why:
tih fixed it


State-Changed-From-To: open->closed
State-Changed-By: sevan@NetBSD.org
State-Changed-When: Fri, 19 Apr 2019 13:24:59 +0000
State-Changed-Why:
Resolved


>Unformatted:

Home	PR Database Search