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.
>
> Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx>
> ---
> extensions/libxt_NFLOG.c | 38 ++++++++++++++++++++++++++++----------
> extensions/libxt_NFLOG.h | 12 ++++++++++++
> iptables/nft-shared.c | 17 +++++++++++------
> iptables/nft.c | 10 ++++------
> 4 files changed, 55 insertions(+), 22 deletions(-)
> create mode 100644 extensions/libxt_NFLOG.h
>
> diff --git a/extensions/libxt_NFLOG.c b/extensions/libxt_NFLOG.c
> index 02a1b4aa35a3..6e1482122f11 100644
> --- a/extensions/libxt_NFLOG.c
> +++ b/extensions/libxt_NFLOG.c
> @@ -8,6 +8,8 @@
> #include <linux/netfilter/x_tables.h>
> #include <linux/netfilter/xt_NFLOG.h>
>
> +#include "libxt_NFLOG.h"
> +
> enum {
> O_GROUP = 0,
> O_PREFIX,
> @@ -53,12 +55,17 @@ static void NFLOG_init(struct xt_entry_target *t)
>
> static void NFLOG_parse(struct xt_option_call *cb)
> {
> + struct xt_nflog_state *state = (struct xt_nflog_state *)cb->data;
> +
> 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(state->nf_log_prefix, sizeof(state->nf_log_prefix),
> + "%s", cb->arg);
> break;
> }
> }
> @@ -75,11 +82,26 @@ static void NFLOG_check(struct xt_fcheck_call *cb)
> info->flags |= XT_NFLOG_F_COPY_LEN;
> }
>
> -static void nflog_print(const struct xt_nflog_info *info, char *prefix)
> +static void nflog_print(const void *data, size_t target_size,
> + const char *prefix)
> {
> + size_t data_size = target_size - offsetof(struct xt_entry_target, data);
> + const struct xt_nflog_info *info;
> + const char *nf_log_prefix;
> +
> + if (data_size == XT_ALIGN(sizeof(struct xt_nflog_state))) {
> + const struct xt_nflog_state *state = data;
> +
> + info = &state->info;
> + nf_log_prefix = state->nf_log_prefix;
> + } else {
> + info = data;
> + nf_log_prefix = NULL;
> + }
> +
> if (info->prefix[0] != '\0') {
> printf(" %snflog-prefix ", prefix);
> - xtables_save_string(info->prefix);
> + xtables_save_string(nf_log_prefix ? : info->prefix);
> }
> if (info->group)
> printf(" %snflog-group %u", prefix, info->group);
> @@ -94,16 +116,12 @@ static void nflog_print(const struct xt_nflog_info *info, char *prefix)
> static void NFLOG_print(const void *ip, const struct xt_entry_target *target,
> int numeric)
> {
> - const struct xt_nflog_info *info = (struct xt_nflog_info *)target->data;
> -
> - nflog_print(info, "");
> + nflog_print(target->data, target->u.target_size, "");
> }
>
> static void NFLOG_save(const void *ip, const struct xt_entry_target *target)
> {
> - const struct xt_nflog_info *info = (struct xt_nflog_info *)target->data;
> -
> - nflog_print(info, "--");
> + nflog_print(target->data, target->u.target_size, "--");
> }
>
> static void nflog_print_xlate(const struct xt_nflog_info *info,
> @@ -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.
> .help = NFLOG_help,
> .init = NFLOG_init,
> .x6_fcheck = NFLOG_check,
> diff --git a/extensions/libxt_NFLOG.h b/extensions/libxt_NFLOG.h
> new file mode 100644
> index 000000000000..f3599a77ef2e
> --- /dev/null
> +++ b/extensions/libxt_NFLOG.h
> @@ -0,0 +1,12 @@
> +#ifndef LIBXT_NFLOG_H_INCLUDED
> +#define LIBXT_NFLOG_H_INCLUDED
> +
> +#include <linux/netfilter/nf_log.h>
> +#include <linux/netfilter/xt_NFLOG.h>
> +
> +struct xt_nflog_state {
> + struct xt_nflog_info info;
> + char nf_log_prefix[NF_LOG_PREFIXLEN];
> +};
> +
> +#endif /* !defined(LIBXT_NFLOG_H_INCLUDED) */
> diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> index b5259db07723..0a9c4de034be 100644
> --- a/iptables/nft-shared.c
> +++ b/iptables/nft-shared.c
> @@ -32,6 +32,7 @@
> #include "nft-bridge.h"
> #include "xshared.h"
> #include "nft.h"
> +#include "extensions/libxt_NFLOG.h"
>
> extern struct nft_family_ops nft_family_ops_ipv4;
> extern struct nft_family_ops nft_family_ops_ipv6;
> @@ -621,19 +622,23 @@ 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));
> + struct xt_nflog_state state = { 0 };
> +
> + struct xt_nflog_info *info = &state.info;
> 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.
> + * characters in xtables (in xt_nflog_info specifically), we may end up
> + * truncating the string when parsing it. The longer prefix will be
> + * available in state.nf_log_prefix.
> */
> - strncpy(info->prefix, prefix, sizeof(info->prefix));
> - info->prefix[sizeof(info->prefix) - 1] = '\0';
> + snprintf(info->prefix, sizeof(info->prefix), "%s", prefix);
> +
> + snprintf(state.nf_log_prefix, sizeof(state.nf_log_prefix), "%s", prefix);
>
> - memcpy(&target->t->data, info, target->size);
> + memcpy(&target->t->data, &state, sizeof(state));
>
> ctx->h->ops->parse_target(target, data);
> }
> diff --git a/iptables/nft.c b/iptables/nft.c
> index dce8fe0b4a18..addcfffdd0cc 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -59,6 +59,7 @@
> #include "nft-cache.h"
> #include "nft-shared.h"
> #include "nft-bridge.h" /* EBT_NOPROTO */
> +#include "extensions/libxt_NFLOG.h"
>
> static void *nft_fn;
>
> @@ -1358,18 +1359,15 @@ int add_action(struct nftnl_rule *r, struct iptables_command_state *cs,
> 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;
> + struct xt_nflog_state *state = (struct xt_nflog_state *)cs->target->t->data;
> + struct xt_nflog_info *info = &state->info;
>
> expr = nftnl_expr_alloc("log");
> if (!expr)
> 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, state->nf_log_prefix);
> }
> if (info->group) {
> nftnl_expr_set_u16(expr, NFTNL_EXPR_LOG_GROUP, info->group);
> --
> 2.30.2
Attachment:
signature.asc
Description: PGP signature
