NetBSD Problem Report #58116

From www@netbsd.org  Fri Apr  5 01:00:10 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_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 A68ED1A923B
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  5 Apr 2024 01:00:10 +0000 (UTC)
Message-Id: <20240405005939.0BA0B1A923B@mollari.NetBSD.org>
Date: Fri,  5 Apr 2024 00:59:38 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: npf.conf(5) doesn't allow comments inside lists
X-Send-Pr-Version: www-1.0

>Number:         58116
>Category:       bin
>Synopsis:       npf.conf(5) doesn't allow comments inside lists
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Apr 05 01:05:00 +0000 2024
>Last-Modified:  Thu Mar 20 17:55:01 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, 8, ...
>Organization:
The NetPF Foundation
>Environment:
rainblobular and sleety
>Description:
Can't write this:

$ports = {
    123,    # foo
    456,    # bar
    789,    # baz
}

npf.conf treats line break like `;', and supports `\' continuation lines but only if there's no comment.  It also doesn't like the trailing comma, which would be friendlier to M-x sort-lines.  So although the input can be broken into lines, they can't have comments or be reliably reordered:

$ports = {
    123, \ 
    456, \
    789 \
}
>How-To-Repeat:

>Fix:
Yes, please!

>Audit-Trail:
From: Attaullah <mdattaullahansari152@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/58116: npf.conf(5) doesn't allow comments inside lists
Date: Tue, 18 Mar 2025 03:43:19 +0530

 On Fri Apr 05 2024, Taylor R Campbell wrote:
 >>Number:         58116
 >>Category:       bin
 >>Synopsis:       npf.conf(5) doesn't allow comments inside lists
 >>Confidential:   no
 >>Severity:       serious
 >>Priority:       medium
 >>Responsible:    bin-bug-people
 >>State:          open
 >>Class:          sw-bug
 >>Submitter-Id:   net
 >>Arrival-Date:   Fri Apr 05 01:05:00 +0000 2024
 >>Originator:     Taylor R Campbell
 >>Release:        current, 10, 9, 8, ...
 >>Organization:
 >The NetPF Foundation
 >>Environment:
 >rainblobular and sleety
 >>Description:
 >Can't write this:
 >
 >$ports =3D {
 >    123,    # foo
 >    456,    # bar
 >    789,    # baz
 >}
 >
 >npf.conf treats line break like `;', and supports `\' continuation lines b=
 ut only if there's no comment.  It also doesn't like the trailing comma, wh=
 ich would be friendlier to M-x sort-lines.  So although the input can be br=
 oken into lines, they can't have comments or be reliably reordered:
 >
 >$ports =3D {
 >    123, \
 >    456, \
 >    789 \
 >}
 >>How-To-Repeat:
 >
 >>Fix:
 >Yes, please!

 I have modified the grammar to allow:
 - Skipping any `\n` within curly braces `{}` in variable definitions.
 - Allowing optional trailing commas.

 and updated parser (`npf_parse.y`), lexer (`npf_scan.l`) and revised
 documentation (`npf.conf.5`) accordingly.

 Tested for the following cases
 - NetBSD npf atf test suite
 - Simple lists with comments and optional trailing commas
 - Edge cases (e.g., empty lists, double comma)
 - Old-style lists and continuation lines for backward compatibility.
 - backslash continuation with and without comments.

 Let me know if this approach works for you!

 PATCH:
 diff --git a/usr.sbin/npf/npfctl/npf.conf.5 b/usr.sbin/npf/npfctl/npf.conf.=
 5
 index ed13221d2d44..1f145217c59c 100644
 --- a/usr.sbin/npf/npfctl/npf.conf.5
 +++ b/usr.sbin/npf/npfctl/npf.conf.5
 @@ -76,9 +76,14 @@ Variables are defined by assigning a value to them
 as follows:
  .Dl $var1 =3D 10.0.0.1
  .Pp
  A variable may also be defined as a set:
 +.Bd -literal -offset indent
 +$var2 =3D {
 +    10.0.0.1,   # First host
 +    10.0.0.2,   # Second host
 +}
 +.Ed
  .Pp
 -.Dl $var2 =3D { 10.0.0.1, 10.0.0.2 }
 -.Pp
 +Newlines within curly braces are ignored, and trailing commas are optional=
 .
  Common variable definitions are for IP addresses, networks, ports,
  and interfaces.
  .Ss Tables
 @@ -531,6 +536,8 @@ The backslash
  .Pq Sq \e
  character at the end of a line marks a continuation line,
  i.e., the next line is considered an extension of the present line.
 +Additionally, within curly braces of variable definitions, newlines are
 +allowed without continuation characters.
  .Sh GRAMMAR
  The following is a non-formal BNF-like definition of the grammar.
  The definition is simplified and is intended to be human readable,
 @@ -547,7 +554,7 @@ syntax        =3D var-def | set-param | alg | table-def=
  |

  var-name    =3D "$" . string
  interface    =3D interface-name | var-name
 -var-def        =3D var "=3D" ( var-value | "{" value *[ "," value ] "}" )
 +var-def        =3D var "=3D" ( var-value | "{" value *[ "," value ] [ "," =
 ] "}" )

  # Parameter setting.
  set-param    =3D "set" param-value
 @@ -638,8 +645,15 @@ $int_if =3D { inet4(wm1) }
  table <blocklist> type ipset file "/etc/npf_blocklist"
  table <limited> type lpm

 -$services_tcp =3D { http, https, smtp, domain, 6000, 9022 }
 -$services_udp =3D { domain, ntp, 6000 }
 +$services_tcp =3D {
 +    http,    # Web traffic
 +    https,   # Secure web traffic
 +    smtp,    # Email sending
 +    domain,  # DNS queries
 +    6000,    # Custom service
 +    9022,    # SSH forwarding
 +}
 +$services_udp =3D { domain, ntp, 6000, }
  $localnet =3D { 10.1.1.0/24 }

  alg "icmp"
 diff --git a/usr.sbin/npf/npfctl/npf_parse.y b/usr.sbin/npf/npfctl/npf_pars=
 e.y
 index d77f462cd8c5..169c74beb28c 100644
 --- a/usr.sbin/npf/npfctl/npf_parse.y
 +++ b/usr.sbin/npf/npfctl/npf_parse.y
 @@ -135,6 +135,7 @@ yyerror(const char *fmt, ...)
  %token            IPSET
  %token            LPM
  %token            MAP
 +%token            NEWLINE
  %token            NO_PORTS
  %token            MINUS
  %token            NAME
 @@ -158,7 +159,7 @@ yyerror(const char *fmt, ...)
  %token            RETURNRST
  %token            ROUNDROBIN
  %token            RULESET
 -%token            SEPLINE
 +%token            SEMICOLON
  %token            SET
  %token            SLASH
  %token            STATEFUL
 @@ -193,7 +194,7 @@ yyerror(const char *fmt, ...)
  %type            filt_port filt_port_list port_range icmp_type_and_co=
 de
  %type            filt_addr addr_and_mask tcp_flags tcp_flags_and_mask
  %type            procs proc_call proc_param_list proc_param
 -%type            element list_elems list value filt_addr_list
 +%type            element list_elems list_trail list value filt_addr_l=
 ist
  %type            opt_proto proto proto_elems
  %type    <addrport>    mapseg
  %type    <filtopts>    filt_opts all_or_filt_opts
 @@ -220,7 +221,7 @@ input
      ;

  lines
 -    : lines SEPLINE line
 +    : lines sepline line
      | line
      ;

 @@ -242,6 +243,11 @@ alg
      }
      ;

 +sepline
 +    : NEWLINE
 +    | SEMICOLON
 +    ;
 +
  param_val
      : number    { $$ =3D $1; }
      | ON        { $$ =3D true; }
 @@ -272,18 +278,17 @@ value
      ;

  list
 -    : CURLY_OPEN list_elems CURLY_CLOSE
 +    : CURLY_OPEN opt_nl list_elems CURLY_CLOSE
      {
 -        $$ =3D $2;
 +        $$ =3D $3;
      }
      ;

  list_elems
 -    : list_elems COMMA element
 +    : element list_trail
      {
 -        npfvar_add_elements($1, $3);
 +        $$ =3D npfvar_add_elements($1, $2);
      }
 -    | element
      ;

  element
 @@ -313,6 +318,24 @@ element
      | addr_and_mask        { $$ =3D $1; }
      ;

 +list_trail
 +    : element_sep element list_trail
 +    {
 +        $$ =3D npfvar_add_elements($2, $3);
 +    }
 +    | opt_nl         { $$ =3D NULL; }
 +    | element_sep         { $$ =3D NULL; }
 +    ;
 +
 +element_sep
 +    : opt_nl COMMA opt_nl
 +    ;
 +
 +opt_nl
 +    : opt_nl NEWLINE
 +    |
 +    ;
 +
  /*
   * Table definition.
   */
 @@ -430,7 +453,7 @@ rproc
      ;

  procs
 -    : procs SEPLINE proc_call
 +    : procs sepline proc_call
      {
          $$ =3D npfvar_add_elements($1, $3);
      }
 @@ -531,7 +554,7 @@ ruleset_block
      ;

  ruleset_def
 -    : ruleset_def SEPLINE rule_group
 +    : ruleset_def sepline rule_group
      | rule_group
      ;

 diff --git a/usr.sbin/npf/npfctl/npf_scan.l b/usr.sbin/npf/npfctl/npf_scan.=
 l
 index 28bc5e966574..7dd6b70c8234 100644
 --- a/usr.sbin/npf/npfctl/npf_scan.l
 +++ b/usr.sbin/npf/npfctl/npf_scan.l
 @@ -133,8 +133,8 @@ npt66            return NPT66;
  "-"            return MINUS;
  procedure        return PROCEDURE;
  \\\n            yylineno++; yycolumn =3D 0;
 -\n            yylineno++; yycolumn =3D 0; return SEPLINE;
 -;            return SEPLINE;
 +\n            yylineno++; yycolumn =3D 0; return NEWLINE;
 +;            return SEMICOLON;
  name            return NAME;
  group            return GROUP;
  default            return DEFAULT;

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58116 CVS commit: src/usr.sbin/npf/npfctl
Date: Thu, 20 Mar 2025 13:52:23 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Thu Mar 20 17:52:23 UTC 2025

 Modified Files:
 	src/usr.sbin/npf/npfctl: npf.conf.5 npf_parse.y npf_scan.l

 Log Message:
 PR/58116: Attaulah: Allow comments inside lists in npf.conf


 To generate a diff of this commit:
 cvs rdiff -u -r1.91 -r1.92 src/usr.sbin/npf/npfctl/npf.conf.5
 cvs rdiff -u -r1.51 -r1.52 src/usr.sbin/npf/npfctl/npf_parse.y
 cvs rdiff -u -r1.32 -r1.33 src/usr.sbin/npf/npfctl/npf_scan.l

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

Home	PR Database Search