On 2021-08-02, at 17:40:36 +0100, Jeremy Sowden wrote:
> On 2021-08-02, at 12:20:18 +0100, Jeremy Sowden wrote:
> > On 2021-08-01, at 15:14:42 +0100, Jeremy Sowden wrote:
> > > On 2021-07-30, at 13:27:49 -0500, Kyle Bowman wrote:
> > > > On Wed, Jul 28, 2021 at 03:43:47AM +0200, Phil Sutter wrote:
> > > > > You might want to check iptables commit ccf154d7420c0
> > > > > ("xtables: Don't use native nftables comments") for reference,
> > > > > it does the opposite of what you want to do.
> > > >
> > > > I went ahead and looked through this commit and also found found
> > > > the code that initially added this functionality; commit
> > > > d64ef34a9961 ("iptables-compat: use nft built-in comments
> > > > support ").
> > > >
> > > > Additionally I found some other commits that moved code to nft
> > > > native implementations of the xtables counterpart so that proved
> > > > helpful.
> > > >
> > > > After a couple days of research I did end up figuring out what
> > > > to do and have added a (mostly complete) native nft log support
> > > > in iptables-nft. It all seems to work without any kernel changes
> > > > required. The only problem I'm now faced with is that since we
> > > > want to take the string passed into the iptables-nft command and
> > > > add it to the nftnl expression (`NFTNL_EXPR_LOG_PREFIX`) I'm not
> > > > entirely sure where to get the original sized string from aside
> > > > from `argv` in the `struct iptables_command_state`. I would get
> > > > it from the `struct xt_nflog_info`, but that's going to store
> > > > the truncated version and we would like to be able to store 128
> > > > characters of the string as opposed to 64.
> > > >
> > > > Any recommendations about how I might do this safely?
> > >
> > > The xtables_target struct has a `udata` member which I think would
> > > be suitable. libxt_RATEEST does something similar.
> >
> > Actually, if we embed struct xf_nflog_info in another structure
> > along with the longer prefix, we can get iptables-nft to print it
> > untruncated.
>
> > From 3c18555c6356e03731812688d7e6956a04ce820e Mon Sep 17 00:00:00 2001
> > From: Jeremy Sowden <jeremy@xxxxxxxxxx>
> > Date: Sun, 1 Aug 2021 14:47:52 +0100
> > Subject: [PATCH] extensions: libxt_NFLOG: embed struct xt_nflog_info
> > in another structure to store longer prefixes suitable for the nft
> > log statement.
> >
> > NFLOG truncates the log-prefix to 64 characters which is the limit
> > supported by iptables-legacy. We now store the longer 128-character
> > prefix in a new structure, struct xt_nflog_state, alongside the
> > struct xt_nflog_info for use by iptables-nft.
> >
> > [...]
> >
> > @@ -139,8 +157,8 @@ static struct xtables_target nflog_target = {
> > .family = NFPROTO_UNSPEC,
> > .name = "NFLOG",
> > .version = XTABLES_VERSION,
> > - .size = XT_ALIGN(sizeof(struct xt_nflog_info)),
> > - .userspacesize = XT_ALIGN(sizeof(struct xt_nflog_info)),
> > + .size = XT_ALIGN(sizeof(struct xt_nflog_state)),
> > + .userspacesize = XT_ALIGN(sizeof(struct xt_nflog_state)),
>
> Unfortuantely the change in size breaks iptables-legacy. Whoops.
> More thought required.
Right, take three. Firstly, use udata as I previously suggested, and
then use a new struct with a layout compatible with struct xt_nflog_info
just for printing and saving iptables-nft targets.
Seems to work. Doesn't break iptables-legacy.
Patches attached.
J.
From 5fcf518401d71b8821cd1c0a00a958138ad9dca1 Mon Sep 17 00:00:00 2001
From: Jeremy Sowden <jeremy@xxxxxxxxxx>
Date: Sun, 1 Aug 2021 14:47:52 +0100
Subject: [PATCH v3 1/2] extensions: libxt_NFLOG: use udata to store longer
prefixes suitable for the nft log statement.
Hitherto 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: Jeremy Sowden <jeremy@xxxxxxxxxx>
---
extensions/libxt_NFLOG.c | 6 ++++++
iptables/nft.c | 6 +-----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/extensions/libxt_NFLOG.c b/extensions/libxt_NFLOG.c
index 02a1b4aa35a3..9057230d7ee7 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 dce8fe0b4a18..13cbf0a8b87b 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1365,11 +1365,7 @@ int add_log(struct nftnl_rule *r, struct iptables_command_state *cs)
return -ENOMEM;
if (info->prefix != NULL) {
- //char prefix[NF_LOG_PREFIXLEN] = {};
-
- // get prefix here from somewhere...
- // maybe in cs->argv?
- nftnl_expr_set_str(expr, NFTNL_EXPR_LOG_PREFIX, "iff the value at the end is 12 then this string is truncated 123");
+ nftnl_expr_set_str(expr, NFTNL_EXPR_LOG_PREFIX, cs->target->udata);
}
if (info->group) {
nftnl_expr_set_u16(expr, NFTNL_EXPR_LOG_GROUP, info->group);
--
2.30.2
From 06a9108c7a4b9a8b603d2020214b5cf5c5d86880 Mon Sep 17 00:00:00 2001
From: Jeremy Sowden <jeremy@xxxxxxxxxx>
Date: Mon, 2 Aug 2021 22:54:27 +0100
Subject: [PATCH v3 2/2] extensions: libxt_NFLOG: don't truncate log-prefix
when printing and saving iptables-nft nflog targets.
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.
Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx>
---
iptables/nft-shared.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index b5259db07723..842318f95db4 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -20,6 +20,7 @@
#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>
@@ -606,13 +607,26 @@ static void nft_parse_log(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
struct xt_entry_target *t;
size_t target_size;
- void *data = ctx->cs;
+ /*
+ * 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 = { 0 };
target = xtables_find_target("NFLOG", XTF_TRY_LOAD);
if (target == NULL)
return;
- target_size = XT_ALIGN(sizeof(struct xt_entry_target)) + target->size;
+ 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;
@@ -621,21 +635,15 @@ static void nft_parse_log(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
target->t = t;
- struct xt_nflog_info *info = xtables_malloc(sizeof(struct xt_nflog_info));
- info->group = group;
- info->len = snaplen;
- info->threshold = qthreshold;
+ info.group = group;
+ info.len = snaplen;
+ info.threshold = qthreshold;
- /* Here, because we allow 128 characters in nftables but only 64
- * characters in xtables (in xt_nflog_info specifically), we may
- * end up truncating the string when parsing it.
- */
- strncpy(info->prefix, prefix, sizeof(info->prefix));
- info->prefix[sizeof(info->prefix) - 1] = '\0';
+ snprintf(info.prefix, sizeof(info.prefix), "%s", prefix);
- memcpy(&target->t->data, info, target->size);
+ memcpy(&target->t->data, &info, sizeof(info));
- ctx->h->ops->parse_target(target, data);
+ ctx->h->ops->parse_target(target, ctx->cs);
}
static void nft_parse_lookup(struct nft_xt_ctx *ctx, struct nft_handle *h,
--
2.30.2
Attachment:
signature.asc
Description: PGP signature
