netfilter buglog - Jun 2017 - [Bug 1154] New: Allow include statement to operate on directories and/or wildcards

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

            Bug ID: 1154
           Summary: Allow include statement to operate on directories
                    and/or wildcards
           Product: nftables
           Version: unspecified
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: nft
          Assignee: pablo at netfilter.org
          Reporter: hoxu at users.sf.net

ferm include statement [1] allows several ways to include multiple files:

```
If you specify a directory (with a trailing '/'), all files in this
directory
are included, sorted alphabetically:

    @include 'ferm.d/';

The function @glob can be used to expand wild cards:

    @include @glob('*.include');

With a trailing pipe symbol, ferm executes a shell command and parses its
output:

    @include "/root/generate_ferm_rules.sh $HOSTNAME|"

ferm aborts, if return code is not 0.
```

I don't personally care about the shell command execution, but being able to
include directories and/or wildcards would be really useful. For example, if
you want to provision various applications to a server, and allow each to
install only the app-specific nftables rules (eg. in
/etc/nftables.d/app.ruleset).

[1]�http://ferm.foo-projects.org/download/2.4/ferm.html#Includes

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170603/53cd85ec/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

hoxu at users.sf.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hoxu at users.sf.net

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170603/023d3449/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

Ismo Puustinen <ismo.puustinen at intel.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ismo.puustinen at intel.com

--- Comment #1 from Ismo Puustinen <ismo.puustinen at intel.com> ---
Hi!

I have the same need. Right now I need to use an external script to construct
the main ruleset file to include all the required (application-installed)
ruleset files. I wrote a patch to nft a long time ago, but it wasn't
accepted
as such:

https://www.spinics.net/lists/netfilter-devel/msg40861.html

I'm not sure if the wildcard support is even needed -- just a feature to
specify directories might be enough.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170605/0ead3e7b/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #2 from Pablo Neira Ayuso <pablo at netfilter.org> ---
We can adopt "directory include" means take all files there from
there. No need
for explicit wildcard. But I would enforce a check for the trailing slash
probably? So we make explicit for the reader/reviewer that someone is importing
a directory, not just a file.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170605/f20bb181/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #3 from hoxu at users.sf.net ---
(In reply to Pablo Neira Ayuso from comment #2)
> We can adopt "directory include" means take all files there from
there. No
> need for explicit wildcard. But I would enforce a check for the trailing
> slash probably? So we make explicit for the reader/reviewer that someone is
> importing a directory, not just a file.
Either that, or a separate includedir directive for clarity?

For comparison, here is how sudo does directory inclusion [1]:


```
The #includedir directive can be used to create a sudo.d direc‐
tory that the system package manager can drop sudoers rules
into as part of package installation.  For example, given:

    #includedir /etc/sudoers.d

sudo will read each file in /etc/sudoers.d, skipping file names
that end in ‘~’ or contain a ‘.’ character to avoid causing
problems with package manager or editor temporary/backup files.
Files are parsed in sorted lexical order.  That is,
/etc/sudoers.d/01_first will be parsed before
/etc/sudoers.d/10_second.  Be aware that because the sorting is
lexical, not numeric, /etc/sudoers.d/1_whoops would be loaded
after /etc/sudoers.d/10_second.  Using a consistent number of
leading zeroes in the file names can be used to avoid such
problems.
```


You may want to consider filtering at least hidden files (.*), which would
nicely exclude vim swapfiles. *~ would do likewise for emacs backup files.

I also like the way debian apache2 does snippet inclusion [2]:


```
# Include generic snippets of statements                                        
IncludeOptional conf-enabled/*.conf
```


Why I'm mentioning apache is to make you consider whether nftables upstream
wants to perhaps nudge downstreams towards - for example - the following
convention:

* /etc/nftables.conf ships with "Include nftables/rules-enabled/*"
* Other packages can ship with suggested rules in /etc/nftables/rules-available
* System admin can cherry-pick rules to symlink to /etc/nftables/rules-enabled
(opening ports by installing a package by default wouldn't work)

Just some random thoughts, in case someone will get brilliant ideas about this.

Personally I would be just happy with a glob include, or failing that, a
directory include.


[1] https://linux.die.net/man/5/sudoers
[2] https://httpd.apache.org/docs/2.4/mod/core.html#includeoptional

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170609/873e6942/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #4 from Ismo Puustinen <ismo.puustinen at intel.com> ---
Hoxu, check if the changes in git are enough to fix the issue. From the man
page concerning include statement:

"If the filename parameter is a directory, then all files in the directory
are
loaded in alphabetical order."

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170609/ac40568c/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #5 from hoxu at users.sf.net ---
https://git.netfilter.org/nftables/commit/?id=443cba72705398c0b12beb0796ba0c5504fb4dd2
https://git.netfilter.org/nftables/commit/?id=593ccac14a5cf85c7d54c3cd7899992c47ef6f25
https://git.netfilter.org/nftables/commit/?id=3facbd92cb93fd584da5ca91083a9c30e5ee0396

Looks good to me (thanks!). But you may want to consider my comment above about
filtering filenames similarly to how sudo does it, because currently (with
default editor settings), nft will be very unhappy of open vi(m) in another
terminal, and emacs backup files will cause the rules to be evaluated twice.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170612/013d17c2/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #6 from Ismo Puustinen <ismo.puustinen at intel.com> ---
I was thinking about globbing... we need to agree on the exact semantics
before the implementation phase.

Consider this directory structure where dir1 and dir2 are include directories
(nft -I dir1 -I dir2 ...):

dir1/
     cba.nft
     acb.nft
dir2/
     abc.nft
     dir3/
          a.nft

What about the following include cases:

include "*"

This means that all files in all include directories get added. However, what
do
we do for directory "dir3"? Should that cause an error or not?

include "a*"

Files starting with 'a' are in both include directories. Should we add
the
files
in alphabetic order [abc.nft, acb.nft] or in include directory order
[acb.nft, abc.nft]?

include "foo*.nft"

There is no match. I think this shouldn't cause an error, because the whole
point of implementing globbing is that we can include files from potentially
empty directories. Do you agree?

include "foo.nft"

There is no match. Should this be an error, or do we think of this as a special
case of glob with no wild cards, and thus not an error?

include "dir4/*"

There is no such directory. Should this be an error? Or what would it mean?

I think the key question is if we want to handle globbing by just running
glob()
on each include directory and merging the results, or do we want to parse the
pattern ourselves and then try to handle several error cases before running the
glob() call.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170615/b0f1ec09/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

Ismo Puustinen <ismo.puustinen at intel.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |arturo at debian.org

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170615/2ba0f315/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #7 from hoxu at users.sf.net ---
(In reply to Ismo Puustinen from comment #6)
> Consider this directory structure where dir1 and dir2 are include
directories
> (nft -I dir1 -I dir2 ...):
> 
> dir1/
>      cba.nft
>      acb.nft
> dir2/
>      abc.nft
>      dir3/
>           a.nft
> 
> What about the following include cases:
> 
> include "*"
> 
> This means that all files in all include directories get added. However,
> what do
> we do for directory "dir3"? Should that cause an error or not?
Would a recursive include cause problems?
> include "a*"
> 
> Files starting with 'a' are in both include directories. Should we
add the
> files
> in alphabetic order [abc.nft, acb.nft] or in include directory order
> [acb.nft, abc.nft]?
I would probably expect the latter; an alphabetical order including the
directories.
> include "foo*.nft"
> 
> There is no match. I think this shouldn't cause an error, because the
whole
> point of implementing globbing is that we can include files from
potentially
> empty directories. Do you agree?
Absolutely!
> include "foo.nft"
> 
> There is no match. Should this be an error, or do we think of this as a
> special
> case of glob with no wild cards, and thus not an error?
This leaves me wondering whether the globbing include should be a distinct
keyword/statement?

Otherwise I'd expect an error.
> include "dir4/*"
> 
> There is no such directory. Should this be an error? Or what would it mean?
I see this as a similar case to the earlier "foo*.nft". No matches
with a
wildcard = no errors.

For example, a distribution could ship /etc/nftables.conf with:

include "nftables/rules-enabled/*.nft"

and that directory wouldn't need to exist by default. (granted, a naive
example)

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170616/244eb1e8/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #8 from hoxu at users.sf.net ---
Just to check, are you planning on calling glob(3) without passing the
GLOB_PERIOD flag?
> GLOB_PERIOD
> Allow a leading period to be matched by metacharacters.  By default,
metacharacters can't match a leading period.
(not passing it, I think, would be preferable, to exclude hidden files
mentioned in my earlier comments)

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170616/d6970188/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #9 from Ismo Puustinen <ismo.puustinen at intel.com> ---
Created attachment 501
  --> https://bugzilla.netfilter.org/attachment.cgi?id=501&action=edit
scanner.l changes

These are the proposed changes to scanner.l. The patch is brand-new, and I
wanted to share it for any comments you might have regarding globbing and
corner case handling before submitting it to wider review.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170626/f8de9088/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #10 from Ismo Puustinen <ismo.puustinen at intel.com> ---
Created attachment 502
  --> https://bugzilla.netfilter.org/attachment.cgi?id=502&action=edit
test updates

Tests for globbing.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170626/15982b6e/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #11 from Pablo Neira Ayuso <pablo at netfilter.org> ---
LGTM.

Please submit this to netfilter-devel at vger.kernel.org 

Thanks Ismo!

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170626/4b27bb1c/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

--- Comment #12 from hoxu at users.sf.net ---
The logic looks sound to me!

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170626/eaf194ac/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1154

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Pablo Neira Ayuso <pablo at netfilter.org> ---
Already patches upstream for this, closing this bug.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20170707/b04ecb72/attachment.html>