[PATCH 1/3] extensions: libtxt_NFLOG: use nft built-in logging instead of xt_NFLOG

[PATCH 1/3] extensions: libtxt_NFLOG: use nft built-in logging instead of xt_NFLOG

[Kyle Bowman]

Replaces the use of xt_NFLOG with the nft built-in log statement.

This additionally adds support for using longer log prefixes of 128
characters in size. Until now NFLOG has truncated the log-prefix to the
64-character limit supported by iptables-legacy. We now use the struct
xtables_target's udata member to store the longer 128-character prefix
supported by iptables-nft.

Signed-off-by: Kyle Bowman <kbowman@cloudflare.com>
Signed-off-by: Alex Forster <aforster@cloudflare.com>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/libxt_NFLOG.c |  6 ++++++
 iptables/nft.c           | 28 ++++++++++++++++++++++++++++
 iptables/nft.h           |  1 +
 3 files changed, 35 insertions(+)

diff --git a/extensions/libxt_NFLOG.c b/extensions/libxt_NFLOG.c
index 02a1b4aa..2b78e278 100644
--- a/extensions/libxt_NFLOG.c
+++ b/extensions/libxt_NFLOG.c
@@ -5,6 +5,7 @@
 #include <getopt.h>
 #include <xtables.h>
 
+#include <linux/netfilter/nf_log.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_NFLOG.h>
 
@@ -53,12 +54,16 @@ static void NFLOG_init(struct xt_entry_target *t)
 
 static void NFLOG_parse(struct xt_option_call *cb)
 {
+	char *nf_log_prefix = cb->udata;
+
 	xtables_option_parse(cb);
 	switch (cb->entry->id) {
 	case O_PREFIX:
 		if (strchr(cb->arg, '\n') != NULL)
 			xtables_error(PARAMETER_PROBLEM,
 				   "Newlines not allowed in --log-prefix");
+
+		snprintf(nf_log_prefix, NF_LOG_PREFIXLEN, "%s", cb->arg);
 		break;
 	}
 }
@@ -149,6 +154,7 @@ static struct xtables_target nflog_target = {
 	.save		= NFLOG_save,
 	.x6_options	= NFLOG_opts,
 	.xlate		= NFLOG_xlate,
+	.udata_size	= NF_LOG_PREFIXLEN
 };
 
 void _init(void)
diff --git a/iptables/nft.c b/iptables/nft.c
index 795dff86..aebbf674 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -39,6 +39,7 @@
 #include <linux/netfilter/nf_tables_compat.h>
 
 #include <linux/netfilter/xt_limit.h>
+#include <linux/netfilter/xt_NFLOG.h>
 
 #include <libmnl/libmnl.h>
 #include <libnftnl/gen.h>
@@ -1340,6 +1341,8 @@ int add_action(struct nftnl_rule *r, struct iptables_command_state *cs,
 		       ret = add_verdict(r, NF_DROP);
 	       else if (strcmp(cs->jumpto, XTC_LABEL_RETURN) == 0)
 		       ret = add_verdict(r, NFT_RETURN);
+	       else if (strcmp(cs->jumpto, "NFLOG") == 0)
+		       ret = add_log(r, cs);
 	       else
 		       ret = add_target(r, cs->target->t);
        } else if (strlen(cs->jumpto) > 0) {
@@ -1352,6 +1355,31 @@ int add_action(struct nftnl_rule *r, struct iptables_command_state *cs,
        return ret;
 }
 
+int add_log(struct nftnl_rule *r, struct iptables_command_state *cs)
+{
+	struct nftnl_expr *expr;
+	struct xt_nflog_info *info = (struct xt_nflog_info *)cs->target->t->data;
+
+	expr = nftnl_expr_alloc("log");
+	if (!expr)
+		return -ENOMEM;
+
+	if (info->prefix[0] != '\0') {
+		nftnl_expr_set_str(expr, NFTNL_EXPR_LOG_PREFIX, cs->target->udata);
+	}
+
+	nftnl_expr_set_u16(expr, NFTNL_EXPR_LOG_GROUP, info->group);
+	if (info->flags & XT_NFLOG_F_COPY_LEN)
+		nftnl_expr_set_u32(expr, NFTNL_EXPR_LOG_SNAPLEN,
+				info->len);
+	if (info->threshold)
+		nftnl_expr_set_u16(expr, NFTNL_EXPR_LOG_QTHRESHOLD,
+				info->threshold);
+
+	nftnl_rule_add_expr(r, expr);
+	return 0;
+}
+
 static void nft_rule_print_debug(struct nftnl_rule *r, struct nlmsghdr *nlh)
 {
 #ifdef NLDEBUG
diff --git a/iptables/nft.h b/iptables/nft.h
index 4ac7e009..28dc81b7 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -193,6 +193,7 @@ int add_match(struct nft_handle *h, struct nftnl_rule *r, struct xt_entry_match
 int add_target(struct nftnl_rule *r, struct xt_entry_target *t);
 int add_jumpto(struct nftnl_rule *r, const char *name, int verdict);
 int add_action(struct nftnl_rule *r, struct iptables_command_state *cs, bool goto_set);
+int add_log(struct nftnl_rule *r, struct iptables_command_state *cs);
 char *get_comment(const void *data, uint32_t data_len);
 
 enum nft_rule_print {
-- 
2.20.1

[PATCH 2/3] extensions: libxt_NFLOG: dont truncate log prefix on print/save

[Kyle Bowman]

When parsing the rule, use a struct with a layout compatible to that of
struct xt_nflog_info, but with a buffer large enough to contain the
whole 128-character nft prefix.

We always send the nflog-group to the kernel since, for nft, log and
nflog targets are handled by the same kernel module, and are
distinguished by whether they define an nflog-group. Therefore, we must
send the group even if it is zero, or the kernel will configure the
target as a log, not an nflog.

Changes to nft_is_expr_compatible were made since only targets which
have an `nflog-group` are compatible. Since nflog targets are
distinguished by having an nflog-group, we ignore targets without one.

We also set the copy-len flag if the snap-len is set since without this,
iptables will mistake `nflog-size` for `nflog-range`.

Signed-off-by: Kyle Bowman <kbowman@cloudflare.com>
Signed-off-by: Alex Forster <aforster@cloudflare.com>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 iptables/nft-shared.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 iptables/nft.c        |  4 +++
 2 files changed, 62 insertions(+)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 4253b081..c164d140 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -20,8 +20,10 @@
 
 #include <xtables.h>
 
+#include <linux/netfilter/nf_log.h>
 #include <linux/netfilter/xt_comment.h>
 #include <linux/netfilter/xt_limit.h>
+#include <linux/netfilter/xt_NFLOG.h>
 
 #include <libmnl/libmnl.h>
 #include <libnftnl/rule.h>
@@ -595,6 +597,60 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		ctx->h->ops->parse_match(match, ctx->cs);
 }
 
+static void nft_parse_log(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
+{
+	struct xtables_target *target;
+	struct xt_entry_target *t;
+	size_t target_size;
+	/*
+	* In order to handle the longer log-prefix supported by nft, instead of
+	* using struct xt_nflog_info, we use a struct with a compatible layout, but
+	* a larger buffer for the prefix.
+	*/
+	struct xt_nflog_info_nft {
+		__u32 len;
+		__u16 group;
+		__u16 threshold;
+		__u16 flags;
+		__u16 pad;
+		char  prefix[NF_LOG_PREFIXLEN];
+	} info = {
+		.group     = nftnl_expr_get_u16(e, NFTNL_EXPR_LOG_GROUP),
+		.threshold = nftnl_expr_get_u16(e, NFTNL_EXPR_LOG_QTHRESHOLD),
+	};
+	if (nftnl_expr_is_set(e, NFTNL_EXPR_LOG_SNAPLEN)) {
+		info.len = nftnl_expr_get_u32(e, NFTNL_EXPR_LOG_SNAPLEN);
+		info.flags = XT_NFLOG_F_COPY_LEN;
+	}
+	if (nftnl_expr_is_set(e, NFTNL_EXPR_LOG_PREFIX)) {
+		snprintf(info.prefix, sizeof(info.prefix), "%s",
+				nftnl_expr_get_str(e, NFTNL_EXPR_LOG_PREFIX));
+	}
+
+	target = xtables_find_target("NFLOG", XTF_TRY_LOAD);
+	if (target == NULL)
+		return;
+
+	target_size = XT_ALIGN(sizeof(struct xt_entry_target)) +
+		XT_ALIGN(sizeof(struct xt_nflog_info_nft));
+
+	t = xtables_calloc(1, target_size);
+	t->u.target_size = target_size;
+	strcpy(t->u.user.name, target->name);
+	t->u.user.revision = target->revision;
+
+	target->t = t;
+
+	struct xt_nflog_info *info = xtables_malloc(sizeof(struct xt_nflog_info));
+	info->group = group;
+	info->len = snaplen;
+	info->threshold = qthreshold;
+
+	memcpy(&target->t->data, &info, sizeof(info));
+
+	ctx->h->ops->parse_target(target, ctx->cs);
+}
+
 static void nft_parse_lookup(struct nft_xt_ctx *ctx, struct nft_handle *h,
 			     struct nftnl_expr *e)
 {
@@ -644,6 +700,8 @@ void nft_rule_to_iptables_command_state(struct nft_handle *h,
 			nft_parse_limit(&ctx, expr);
 		else if (strcmp(name, "lookup") == 0)
 			nft_parse_lookup(&ctx, h, expr);
+		else if (strcmp(name, "log") == 0)
+			nft_parse_log(&ctx, expr);
 
 		expr = nftnl_expr_iter_next(iter);
 	}
diff --git a/iptables/nft.c b/iptables/nft.c
index aebbf674..e9875f28 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -3515,6 +3515,10 @@ static int nft_is_expr_compatible(struct nftnl_expr *expr, void *data)
 	    nftnl_expr_get_u32(expr, NFTNL_EXPR_LIMIT_FLAGS) == 0)
 		return 0;
 
+	if (!strcmp(name, "log") &&
+	    nftnl_expr_is_set(expr, NFTNL_EXPR_LOG_GROUP))
+		return 0;
+
 	return -1;
 }
 
-- 
2.20.1

[PATCH 3/3] extensions: libxf_NFLOG: remove `--nflog-range` Python unit-tests.

[Kyle Bowman]

nft has no equivalent to `--nflog-range`, so we cannot emulate it and
the Python unit-tests for it fail.  However, since `--nflog-range` is
broken and doesn't do anything, the tests are not testing anything
useful.

Signed-off-by: Kyle Bowman <kbowman@cloudflare.com>
Signed-off-by: Alex Forster <aforster@cloudflare.com>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/libxt_NFLOG.t | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/extensions/libxt_NFLOG.t b/extensions/libxt_NFLOG.t
index 933fa221..33a15c06 100644
--- a/extensions/libxt_NFLOG.t
+++ b/extensions/libxt_NFLOG.t
@@ -3,10 +3,6 @@
 -j NFLOG --nflog-group 65535;=;OK
 -j NFLOG --nflog-group 65536;;FAIL
 -j NFLOG --nflog-group 0;-j NFLOG;OK
--j NFLOG --nflog-range 1;=;OK
--j NFLOG --nflog-range 4294967295;=;OK
--j NFLOG --nflog-range 4294967296;;FAIL
--j NFLOG --nflog-range -1;;FAIL
 -j NFLOG --nflog-size 0;=;OK
 -j NFLOG --nflog-size 1;=;OK
 -j NFLOG --nflog-size 4294967295;=;OK
-- 
2.20.1

Re: [PATCH 1/3] extensions: libtxt_NFLOG: use nft built-in logging instead of xt_NFLOG

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

On 2021-08-09, at 14:42:41 -0500, Kyle Bowman wrote:
> Replaces the use of xt_NFLOG with the nft built-in log statement.
>
> This additionally adds support for using longer log prefixes of 128
> characters in size. Until now NFLOG has truncated the log-prefix to
> the 64-character limit supported by iptables-legacy. We now use the
> struct xtables_target's udata member to store the longer 128-character
> prefix supported by iptables-nft.
>
> Signed-off-by: Kyle Bowman <kbowman@cloudflare.com>
> Signed-off-by: Alex Forster <aforster@cloudflare.com>
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>

Hi Core Team,

It's been the better part of two months since this patch series was
posted and there has been no feedback.  I was wondering if one of you
might be in a position to review it in the not too distant future.  I
see that it is delegated to Pablo in Patchwork, but then so is every-
thing else. :)

Cheers,

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/3] extensions: libtxt_NFLOG: use nft built-in logging instead of xt_NFLOG

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

On 2021-09-30, at 08:16:07 +0100, Jeremy Sowden wrote:
> On 2021-08-09, at 14:42:41 -0500, Kyle Bowman wrote:
> > Replaces the use of xt_NFLOG with the nft built-in log statement.
> >
> > This additionally adds support for using longer log prefixes of 128
> > characters in size. Until now NFLOG has truncated the log-prefix to
> > the 64-character limit supported by iptables-legacy. We now use the
> > struct xtables_target's udata member to store the longer
> > 128-character prefix supported by iptables-nft.
> >
> > Signed-off-by: Kyle Bowman <kbowman@cloudflare.com>
> > Signed-off-by: Alex Forster <aforster@cloudflare.com>
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
>
> Hi Core Team,
>
> It's been the better part of two months since this patch series was
> posted and there has been no feedback.  I was wondering if one of you
> might be in a position to review it in the not too distant future.  I
> see that it is delegated to Pablo in Patchwork, but then so is every-
> thing else. :)

Having asked for feedback, I've spotted a bug. :)  Will fix and send out
v2.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]