* [PATCH v3 1/2] scanner: add support for include directories
@ 2017-06-06 11:50 Ismo Puustinen
2017-06-06 11:50 ` [PATCH v3 2/2] tests: test " Ismo Puustinen
2017-06-06 15:58 ` [PATCH v3 1/2] scanner: add support for " Pablo Neira Ayuso
0 siblings, 2 replies; 5+ messages in thread
From: Ismo Puustinen @ 2017-06-06 11:50 UTC (permalink / raw)
To: netfilter-devel; +Cc: Ismo Puustinen
If a string after "include" keyword points to a directory instead of a
file, consider the directory to contain only nft rule files and try to
load them all. This helps with a use case where services drop their own
firewall configuration files into a directory and nft needs to include
those without knowing the exact file names.
File loading order from the include directory is not specified, so the
files inside an include directory should not depend on each other.
Fixes(Bug 1154 - Allow include statement to operate on directories and/or wildcards).
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
---
src/scanner.l | 135 +++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 110 insertions(+), 25 deletions(-)
diff --git a/src/scanner.l b/src/scanner.l
index c2c008d..1beadb1 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -10,6 +10,8 @@
%{
+#include <dirent.h>
+#include <libgen.h>
#include <limits.h>
#include <netinet/in.h>
#include <arpa/inet.h>
@@ -637,8 +639,8 @@ static struct error_record *scanner_push_file(void *scanner, const char *filenam
return NULL;
}
-int scanner_read_file(void *scanner, const char *filename,
- const struct location *loc)
+static int include_file(void *scanner, const char *filename,
+ const struct location *loc)
{
struct parser_state *state = yyget_extra(scanner);
struct error_record *erec;
@@ -655,12 +657,92 @@ int scanner_read_file(void *scanner, const char *filename,
if (erec != NULL)
goto err;
return 0;
+err:
+ erec_queue(erec, state->msgs);
+ return -1;
+}
+int scanner_read_file(void *scanner, const char *filename,
+ const struct location *loc)
+{
+ return include_file(scanner, filename, loc);
+}
+
+static int include_directory(void *scanner, const char *dirname,
+ DIR *directory, const struct location *loc)
+{
+ struct parser_state *state = yyget_extra(scanner);
+ struct error_record *erec;
+ struct dirent *de;
+ FILE *f;
+ int ret;
+
+ if (!dirname[0] || dirname[strlen(dirname)-1] != '/') {
+ erec = error(loc, "Include directory name \"%s\" does not end in '/'",
+ dirname);
+ goto err;
+ }
+
+ /* If the path is a directory, assume that all files there need
+ * to be included.
+ */
+ while ((de = readdir(directory))) {
+ char dirbuf[PATH_MAX];
+
+ ret = snprintf(dirbuf, sizeof(dirbuf), "%s/%s", dirname, de->d_name);
+ if (ret < 0 || ret >= PATH_MAX) {
+ erec = error(loc, "Too long file path \"%s/%s\"\n",
+ dirname, de->d_name);
+ goto err;
+ }
+
+ if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
+ continue;
+
+ f = fopen(dirbuf, "r");
+ if (f == NULL) {
+ erec = error(loc, "Could not open file \"%s\": %s\n",
+ dirbuf, strerror(errno));
+ goto err;
+ }
+
+ erec = scanner_push_file(scanner, de->d_name, f, loc);
+ if (erec != NULL)
+ goto err;
+ }
+ return 0;
err:
erec_queue(erec, state->msgs);
return -1;
}
+static int include_dentry(void *scanner, const char *filename,
+ const struct location *loc)
+{
+ DIR *directory;
+ int ret;
+
+ /* The file can be either a simple file or a directory which
+ * contains files.
+ */
+
+ directory = opendir(filename);
+
+ if (directory == NULL && errno != ENOTDIR)
+ /* Could not access the directory or file, keep on searching.
+ * Return value '1' indicates to the caller that we should still
+ * search in the next include directory.
+ */
+ ret = 1;
+ else if (directory != NULL) {
+ ret = include_directory(scanner, filename, directory, loc);
+ closedir(directory);
+ } else
+ ret = include_file(scanner, filename, loc);
+
+ return ret;
+}
+
static bool search_in_include_path(const char *filename)
{
return (strncmp(filename, "./", strlen("./")) != 0 &&
@@ -671,40 +753,43 @@ static bool search_in_include_path(const char *filename)
int scanner_include_file(void *scanner, const char *filename,
const struct location *loc)
{
- struct parser_state *state = yyget_extra(scanner);
- struct error_record *erec;
char buf[PATH_MAX];
- const char *name = buf;
unsigned int i;
- FILE *f;
+ int ret;
+ struct error_record *erec;
+ struct parser_state *state = yyget_extra(scanner);
- f = NULL;
if (search_in_include_path(filename)) {
for (i = 0; i < INCLUDE_PATHS_MAX; i++) {
if (include_paths[i] == NULL)
break;
- snprintf(buf, sizeof(buf), "%s/%s",
- include_paths[i], filename);
- f = fopen(buf, "r");
- if (f != NULL)
- break;
+ ret = snprintf(buf, sizeof(buf), "%s/%s",
+ include_paths[i], filename);
+ if (ret < 0 || ret >= PATH_MAX) {
+ erec = error(loc, "Too long file path \"%s/%s\"\n",
+ include_paths[i], filename);
+ erec_queue(erec, state->msgs);
+ return -1;
+ }
+
+ ret = include_dentry(scanner, buf, loc);
+ if (ret == 0)
+ return 0;
+ else if (ret != 1)
+ /* error has been processed already */
+ return -1;
}
- } else {
- f = fopen(filename, "r");
- name = filename;
}
- if (f == NULL) {
- erec = error(loc, "Could not open file \"%s\": %s",
- filename, strerror(errno));
- goto err;
+ else {
+ ret = include_dentry(scanner, filename, loc);
+ if (ret == 0)
+ return 0;
+ else if (ret != 1)
+ return -1;
+ /* else fall through to "not found" processing */
}
- erec = scanner_push_file(scanner, name, f, loc);
- if (erec != NULL)
- goto err;
- return 0;
-
-err:
+ erec = error(loc, "Did not find \"%s\"\n", filename);
erec_queue(erec, state->msgs);
return -1;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] tests: test include directories
2017-06-06 11:50 [PATCH v3 1/2] scanner: add support for include directories Ismo Puustinen
@ 2017-06-06 11:50 ` Ismo Puustinen
2017-06-06 15:58 ` Pablo Neira Ayuso
2017-06-06 15:58 ` [PATCH v3 1/2] scanner: add support for " Pablo Neira Ayuso
1 sibling, 1 reply; 5+ messages in thread
From: Ismo Puustinen @ 2017-06-06 11:50 UTC (permalink / raw)
To: netfilter-devel; +Cc: Ismo Puustinen
Add tests for:
* including an empty directory
* including directory with one or two files in it
* testing for required trailing slash in directory name
* testing for detecting non-existent directory
* testing for a broken file in included directory
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
---
tests/shell/testcases/include/0005dir_empty_0 | 29 +++++++++++++
tests/shell/testcases/include/0006dir_single_0 | 36 +++++++++++++++++
tests/shell/testcases/include/0007dir_double_0 | 45 +++++++++++++++++++++
tests/shell/testcases/include/0008dir_no_slash_1 | 29 +++++++++++++
tests/shell/testcases/include/0009dir_nodir_1 | 31 ++++++++++++++
.../shell/testcases/include/0010dir_broken_file_1 | 47 ++++++++++++++++++++++
6 files changed, 217 insertions(+)
create mode 100755 tests/shell/testcases/include/0005dir_empty_0
create mode 100755 tests/shell/testcases/include/0006dir_single_0
create mode 100755 tests/shell/testcases/include/0007dir_double_0
create mode 100755 tests/shell/testcases/include/0008dir_no_slash_1
create mode 100755 tests/shell/testcases/include/0009dir_nodir_1
create mode 100755 tests/shell/testcases/include/0010dir_broken_file_1
diff --git a/tests/shell/testcases/include/0005dir_empty_0 b/tests/shell/testcases/include/0005dir_empty_0
new file mode 100755
index 0000000..f16acf8
--- /dev/null
+++ b/tests/shell/testcases/include/0005dir_empty_0
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+set -e
+
+tmpdir=$(mktemp -d)
+if [ ! -d $tmpdir ] ; then
+ echo "Failed to create tmp directory" >&2
+ exit 0
+fi
+
+tmpfile1=$(mktemp)
+if [ ! -w $tmpfile1 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+# cleanup if aborted
+trap "rm -rf $tmpfile1 && rmdir $tmpdir" EXIT
+
+RULESET1="include \"$tmpdir/\""
+
+echo "$RULESET1" > $tmpfile1
+
+$NFT -f $tmpfile1
+
+if [ $? -ne 0 ] ; then
+ echo "E: unable to load good ruleset" >&2
+ exit 1
+fi
diff --git a/tests/shell/testcases/include/0006dir_single_0 b/tests/shell/testcases/include/0006dir_single_0
new file mode 100755
index 0000000..ae4fd5f
--- /dev/null
+++ b/tests/shell/testcases/include/0006dir_single_0
@@ -0,0 +1,36 @@
+#!/bin/bash
+
+set -e
+
+tmpdir=$(mktemp -d)
+if [ ! -d $tmpdir ] ; then
+ echo "Failed to create tmp directory" >&2
+ exit 0
+fi
+
+tmpfile1=$(mktemp -p $tmpdir)
+if [ ! -w $tmpfile1 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+tmpfile2=$(mktemp)
+if [ ! -w $tmpfile2 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+# cleanup if aborted
+trap "rm -rf $tmpfile1 $tmpfile2 && rmdir $tmpdir" EXIT
+
+RULESET1="add table x"
+RULESET2="include \"$tmpdir/\""
+
+echo "$RULESET1" > $tmpfile1
+echo "$RULESET2" > $tmpfile2
+
+$NFT -f $tmpfile2
+if [ $? -ne 0 ] ; then
+ echo "E: unable to load good ruleset" >&2
+ exit 1
+fi
diff --git a/tests/shell/testcases/include/0007dir_double_0 b/tests/shell/testcases/include/0007dir_double_0
new file mode 100755
index 0000000..0a14ade
--- /dev/null
+++ b/tests/shell/testcases/include/0007dir_double_0
@@ -0,0 +1,45 @@
+#!/bin/bash
+
+set -e
+
+tmpdir=$(mktemp -d)
+if [ ! -d $tmpdir ] ; then
+ echo "Failed to create tmp directory" >&2
+ exit 0
+fi
+
+tmpfile1=$(mktemp -p $tmpdir)
+if [ ! -w $tmpfile1 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+tmpfile2=$(mktemp -p $tmpdir)
+if [ ! -w $tmpfile2 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+tmpfile3=$(mktemp)
+if [ ! -w $tmpfile3 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+# cleanup if aborted
+trap "rm -rf $tmpfile1 $tmpfile2 $tmpfile3 && rmdir $tmpdir" EXIT
+
+RULESET1="add table x"
+RULESET2="add table y"
+RULESET3="include \"$tmpdir/\""
+
+echo "$RULESET1" > $tmpfile1
+echo "$RULESET2" > $tmpfile2
+echo "$RULESET3" > $tmpfile3
+
+$NFT -f $tmpfile3
+
+if [ $? -ne 0 ] ; then
+ echo "E: unable to load good ruleset" >&2
+ exit 1
+fi
diff --git a/tests/shell/testcases/include/0008dir_no_slash_1 b/tests/shell/testcases/include/0008dir_no_slash_1
new file mode 100755
index 0000000..2820dc9
--- /dev/null
+++ b/tests/shell/testcases/include/0008dir_no_slash_1
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+set -e
+
+tmpdir=$(mktemp -d)
+if [ ! -d $tmpdir ] ; then
+ echo "Failed to create tmp directory" >&2
+ exit 0
+fi
+
+tmpfile1=$(mktemp -p $tmpdir)
+if [ ! -w $tmpfile1 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+# cleanup if aborted
+trap "rm -rf $tmpfile1 && rmdir $tmpdir" EXIT
+
+RULESET1="include \"$tmpdir\""
+
+echo "$RULESET1" > $tmpfile1
+
+$NFT -f $tmpfile1
+
+if [ $? -eq 0 ] ; then
+ echo "E: did not catch missing slash in directory name" >&2
+ exit 1
+fi
diff --git a/tests/shell/testcases/include/0009dir_nodir_1 b/tests/shell/testcases/include/0009dir_nodir_1
new file mode 100755
index 0000000..d7407f4
--- /dev/null
+++ b/tests/shell/testcases/include/0009dir_nodir_1
@@ -0,0 +1,31 @@
+#!/bin/bash
+
+set -e
+
+tmpdir=$(mktemp -d)
+if [ ! -d $tmpdir ] ; then
+ echo "Failed to create tmp directory" >&2
+ exit 0
+fi
+
+# remove the directory
+rmdir $tmpdir
+
+tmpfile1=$(mktemp)
+if [ ! -w $tmpfile1 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+# cleanup if aborted
+trap "rm -rf $tmpfile1" EXIT
+
+RULESET1="include \"$tmpdir/\""
+
+echo "$RULESET1" > $tmpfile1
+
+$NFT -f $tmpfile1
+if [ $? -eq 0 ] ; then
+ echo "E: Failed to catch a missing include directory/file" >&2
+ exit 1
+fi
diff --git a/tests/shell/testcases/include/0010dir_broken_file_1 b/tests/shell/testcases/include/0010dir_broken_file_1
new file mode 100755
index 0000000..c093974
--- /dev/null
+++ b/tests/shell/testcases/include/0010dir_broken_file_1
@@ -0,0 +1,47 @@
+#!/bin/bash
+
+set -e
+
+tmpdir=$(mktemp -d)
+if [ ! -d $tmpdir ] ; then
+ echo "Failed to create tmp directory" >&2
+ exit 0
+fi
+
+tmpfile1=$(mktemp -p $tmpdir)
+if [ ! -w $tmpfile1 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+tmpfile2=$(mktemp -p $tmpdir)
+if [ ! -w $tmpfile2 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+tmpfile3=$(mktemp)
+if [ ! -w $tmpfile3 ] ; then
+ echo "Failed to create tmp file" >&2
+ exit 0
+fi
+
+# cleanup if aborted
+trap "rm -rf $tmpfile1 $tmpfile2 $tmpfile3 && rmdir $tmpdir" EXIT
+
+RULESET1="add table x"
+
+# do an error in a file
+RULESET2="intentionally broken file"
+RULESET3="include \"$tmpdir/\""
+
+echo "$RULESET1" > $tmpfile1
+echo "$RULESET2" > $tmpfile2
+echo "$RULESET3" > $tmpfile3
+
+$NFT -f $tmpfile3
+
+if [ $? -eq 0 ] ; then
+ echo "E: didn't catch a broken file in directory" >&2
+ exit 1
+fi
--
2.9.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] scanner: add support for include directories
2017-06-06 11:50 [PATCH v3 1/2] scanner: add support for include directories Ismo Puustinen
2017-06-06 11:50 ` [PATCH v3 2/2] tests: test " Ismo Puustinen
@ 2017-06-06 15:58 ` Pablo Neira Ayuso
2017-06-06 16:01 ` Pablo Neira Ayuso
1 sibling, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 15:58 UTC (permalink / raw)
To: Ismo Puustinen; +Cc: netfilter-devel
On Tue, Jun 06, 2017 at 02:50:09PM +0300, Ismo Puustinen wrote:
> If a string after "include" keyword points to a directory instead of a
> file, consider the directory to contain only nft rule files and try to
> load them all. This helps with a use case where services drop their own
> firewall configuration files into a directory and nft needs to include
> those without knowing the exact file names.
>
> File loading order from the include directory is not specified, so the
> files inside an include directory should not depend on each other.
>
> Fixes(Bug 1154 - Allow include statement to operate on directories and/or wildcards).
Applied, thanks Ismo.
BTW, it would be great if you can send us a patch for the nft(8)
manpage too to document this new feature.
I can also create an account for your at wiki.nftables.org, in case
you want to add there more examples. There are people even slightly
describing their usecase and the way they are solving it. These
patterns will be good for people that are looking into reusing good
ideas when using nftables.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] scanner: add support for include directories
2017-06-06 15:58 ` [PATCH v3 1/2] scanner: add support for " Pablo Neira Ayuso
@ 2017-06-06 16:01 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 16:01 UTC (permalink / raw)
To: Ismo Puustinen; +Cc: netfilter-devel
On Tue, Jun 06, 2017 at 05:58:24PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jun 06, 2017 at 02:50:09PM +0300, Ismo Puustinen wrote:
> > If a string after "include" keyword points to a directory instead of a
> > file, consider the directory to contain only nft rule files and try to
> > load them all. This helps with a use case where services drop their own
> > firewall configuration files into a directory and nft needs to include
> > those without knowing the exact file names.
> >
> > File loading order from the include directory is not specified, so the
> > files inside an include directory should not depend on each other.
> >
> > Fixes(Bug 1154 - Allow include statement to operate on directories and/or wildcards).
>
> Applied, thanks Ismo.
>
> BTW, it would be great if you can send us a patch for the nft(8)
> manpage too to document this new feature.
>
> I can also create an account for your at wiki.nftables.org, in case
> you want to add there more examples. There are people even slightly
> describing their usecase and the way they are solving it. These
> patterns will be good for people that are looking into reusing good
> ideas when using nftables.
BTW, one more question: I still have a minor concern on the
assumptions that people can made on the order that files are included
in this feature.
Please, correct me if wrong, my understanding is at this stage there
is not guarantees in what order they would be loaded at all.
So I'm thinking that it would be good if we provide some good
default, ie. load them in alphanumeric order or similar.
That would be a follow up patch, but it would be good to sort out this
before nft 0.8 gets released.
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-06 16:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-06 11:50 [PATCH v3 1/2] scanner: add support for include directories Ismo Puustinen
2017-06-06 11:50 ` [PATCH v3 2/2] tests: test " Ismo Puustinen
2017-06-06 15:58 ` Pablo Neira Ayuso
2017-06-06 15:58 ` [PATCH v3 1/2] scanner: add support for " Pablo Neira Ayuso
2017-06-06 16:01 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).