[PATCH iptables 1/2] xtables-translate: add new field to identify the caller

[PATCH iptables 1/2] xtables-translate: add new field to identify the caller

[Pablo M. Bermudo Garay]

In some cases, xlate functions must print a different result if they are
invoked from nft and not from a xtables-translate command.

This commit adds a new boolean field to the xt_xlate struct. This
variable must be true when a xlate function is called from a nft
command. Additional code is required in nft in order to obtain this
behavior.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 include/xtables.h    |  2 ++
 libxtables/xtables.c | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/xtables.h b/include/xtables.h
index 48be514..fd72623 100644
--- a/include/xtables.h
+++ b/include/xtables.h
@@ -576,6 +576,8 @@ void xt_xlate_add(struct xt_xlate *xl, const char *fmt, ...);
 void xt_xlate_add_comment(struct xt_xlate *xl, const char *comment);
 const char *xt_xlate_get_comment(struct xt_xlate *xl);
 const char *xt_xlate_get(struct xt_xlate *xl);
+void xt_xlate_set_nft_compat(struct xt_xlate *xl, bool nft_compat);
+bool xt_xlate_get_nft_compat(struct xt_xlate *xl);
 
 #ifdef XTABLES_INTERNAL
 
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 921dfe9..1c3f63d 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -2011,6 +2011,7 @@ struct xt_xlate {
 		int	off;
 	} buf;
 	char comment[NFT_USERDATA_MAXLEN];
+	bool nft_compat;
 };
 
 struct xt_xlate *xt_xlate_alloc(int size)
@@ -2029,6 +2030,7 @@ struct xt_xlate *xt_xlate_alloc(int size)
 	xl->buf.rem = size;
 	xl->buf.off = 0;
 	xl->comment[0] = '\0';
+	xl->nft_compat = false;
 
 	return xl;
 }
@@ -2069,3 +2071,13 @@ const char *xt_xlate_get(struct xt_xlate *xl)
 {
 	return xl->buf.data;
 }
+
+void xt_xlate_set_nft_compat(struct xt_xlate *xl, bool nft_compat)
+{
+	xl->nft_compat = nft_compat;
+}
+
+bool xt_xlate_get_nft_compat(struct xt_xlate *xl)
+{
+	return xl->nft_compat;
+}
-- 
2.9.0

[PATCH iptables 2/2] xtables-translate: fix issue with quotes

[Pablo M. Bermudo Garay]

Some translations included escaped quotes when they were called from
nft:

$ sudo nft list ruleset
table ip mangle {
    chain FORWARD {
        type filter hook forward priority -150; policy accept;
        ct helper \"ftp\" counter packets 0 bytes 0
                  ^^   ^^
    }
}

This behavior is only correct when xlate functions are called from a
xtables-translate command. This patch solves that issue if nft revision
is using the field added to the xt_xlate struct with "xtables-translate:
add new field to identify the caller" commit.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 extensions/libip6t_LOG.c  | 8 ++++++--
 extensions/libipt_LOG.c   | 8 ++++++--
 extensions/libxt_NFLOG.c  | 9 +++++++--
 extensions/libxt_helper.c | 8 ++++++--
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/extensions/libip6t_LOG.c b/extensions/libip6t_LOG.c
index 3c5075e..ec4b9cc 100644
--- a/extensions/libip6t_LOG.c
+++ b/extensions/libip6t_LOG.c
@@ -190,8 +190,12 @@ static int LOG_xlate(const void *ip, const struct xt_entry_target *target,
 			(const struct ip6t_log_info *)target->data;
 
 	xt_xlate_add(xl, "log ");
-	if (strcmp(loginfo->prefix, "") != 0)
-		xt_xlate_add(xl, "prefix \\\"%s\\\" ", loginfo->prefix);
+	if (strcmp(loginfo->prefix, "") != 0) {
+		if (xt_xlate_get_nft_compat(xl))
+			xt_xlate_add(xl, "prefix \\\"%s\\\" ", loginfo->prefix);
+		else
+			xt_xlate_add(xl, "prefix \"%s\" ", loginfo->prefix);
+	}
 
 	for (i = 0; i < ARRAY_SIZE(ip6t_log_xlate_names); ++i)
 		if (loginfo->level == ip6t_log_xlate_names[i].level &&
diff --git a/extensions/libipt_LOG.c b/extensions/libipt_LOG.c
index f81eb8d..c87d5fe 100644
--- a/extensions/libipt_LOG.c
+++ b/extensions/libipt_LOG.c
@@ -190,8 +190,12 @@ static int LOG_xlate(const void *ip, const struct xt_entry_target *target,
 			(const struct ipt_log_info *)target->data;
 
 	xt_xlate_add(xl, "log ");
-	if (strcmp(loginfo->prefix, "") != 0)
-		xt_xlate_add(xl, "prefix \\\"%s\\\" ", loginfo->prefix);
+	if (strcmp(loginfo->prefix, "") != 0) {
+		if (xt_xlate_get_nft_compat(xl))
+			xt_xlate_add(xl, "prefix \\\"%s\\\" ", loginfo->prefix);
+		else
+			xt_xlate_add(xl, "prefix \"%s\" ", loginfo->prefix);
+	}
 
 	for (i = 0; i < ARRAY_SIZE(ipt_log_xlate_names); ++i)
 		if (loginfo->level != LOG_DEFAULT_LEVEL &&
diff --git a/extensions/libxt_NFLOG.c b/extensions/libxt_NFLOG.c
index 8c67066..f0b92de 100644
--- a/extensions/libxt_NFLOG.c
+++ b/extensions/libxt_NFLOG.c
@@ -110,8 +110,13 @@ static void nflog_print_xlate(const struct xt_nflog_info *info,
 			      struct xt_xlate *xl)
 {
 	xt_xlate_add(xl, "log ");
-	if (info->prefix[0] != '\0')
-		xt_xlate_add(xl, "prefix \\\"%s\\\" ", info->prefix);
+	if (info->prefix[0] != '\0') {
+		if (xt_xlate_get_nft_compat(xl))
+			xt_xlate_add(xl, "prefix \\\"%s\\\" ", info->prefix);
+		else
+			xt_xlate_add(xl, "prefix \"%s\" ", info->prefix);
+
+	}
 	if (info->len)
 		xt_xlate_add(xl, "snaplen %u ", info->len);
 	if (info->threshold != XT_NFLOG_DEFAULT_THRESHOLD)
diff --git a/extensions/libxt_helper.c b/extensions/libxt_helper.c
index 26e9569..7b06e50 100644
--- a/extensions/libxt_helper.c
+++ b/extensions/libxt_helper.c
@@ -50,8 +50,12 @@ static int helper_xlate(const void *ip, const struct xt_entry_match *match,
 {
 	const struct xt_helper_info *info = (const void *)match->data;
 
-	xt_xlate_add(xl, "ct helper%s \\\"%s\\\"",
-		   info->invert ? " !=" : "", info->name);
+	if (xt_xlate_get_nft_compat(xl))
+		xt_xlate_add(xl, "ct helper%s \\\"%s\\\"",
+			   info->invert ? " !=" : "", info->name);
+	else
+		xt_xlate_add(xl, "ct helper%s \"%s\"",
+			   info->invert ? " !=" : "", info->name);
 
 	return 1;
 }
-- 
2.9.0

Re: [PATCH iptables 2/2] xtables-translate: fix issue with quotes

[Pablo Neira Ayuso]

On Fri, Jul 22, 2016 at 05:48:34PM +0200, Pablo M. Bermudo Garay wrote:
> Some translations included escaped quotes when they were called from
> nft:

git am /tmp/iptables-2-2-xtables-translate-fix-issue-with-quotes.patch
-s
gApplying: xtables-translate: fix issue with quotes
error: patch failed: extensions/libxt_NFLOG.c:110

This doesn't apply.

You're not working on top of a fresh git repository. Please,
frequently git pull and rebase your work.

Re: [PATCH iptables 1/2] xtables-translate: add new field to identify the caller

[Pablo Neira Ayuso]

On Fri, Jul 22, 2016 at 05:48:33PM +0200, Pablo M. Bermudo Garay wrote:
> In some cases, xlate functions must print a different result if they are
> invoked from nft and not from a xtables-translate command.
> 
> This commit adds a new boolean field to the xt_xlate struct. This
> variable must be true when a xlate function is called from a nft
> command. Additional code is required in nft in order to obtain this
> behavior.

Why not simply pass a new parameter to ->xlate()?

This patch is anyway updating the binary interface and it makes this
less obscure as it is part of the interface.

The xlate structure hides the buffer intentionally to avoid exposing
it the the backend extensions, the comments are also stored there to
use native nf_tables comment support.

But the only reason for this approach is that you're being lazy and
you don't want to upgrade the footprint of every xlate extension ;-)

Re: [PATCH iptables 2/2] xtables-translate: fix issue with quotes

[Pablo Neira Ayuso]

On Fri, Jul 22, 2016 at 05:48:34PM +0200, Pablo M. Bermudo Garay wrote:
> Some translations included escaped quotes when they were called from
> nft:
> 
> $ sudo nft list ruleset
> table ip mangle {
>     chain FORWARD {
>         type filter hook forward priority -150; policy accept;
>         ct helper \"ftp\" counter packets 0 bytes 0
>                   ^^   ^^
>     }
> }
> 
> This behavior is only correct when xlate functions are called from a
> xtables-translate command. This patch solves that issue if nft revision
> is using the field added to the xt_xlate struct with "xtables-translate:
> add new field to identify the caller" commit.

Please, rebase this change on top of:

http://git.netfilter.org/iptables/commit/?id=7a0992da44cfb6cab0ccd1beadcf326df8773552

I'd suggest you add a new parameter for this, something like:

        bool escape_quotes;

Or is there use for this nft_compat field you propose out of this.

Thanks.

Re: [PATCH iptables 2/2] xtables-translate: fix issue with quotes

[Pablo M. Bermudo Garay]

2016-07-25 17:31 GMT+02:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Fri, Jul 22, 2016 at 05:48:34PM +0200, Pablo M. Bermudo Garay wrote:
>> Some translations included escaped quotes when they were called from
>> nft:
>>
>> $ sudo nft list ruleset
>> table ip mangle {
>>     chain FORWARD {
>>         type filter hook forward priority -150; policy accept;
>>         ct helper \"ftp\" counter packets 0 bytes 0
>>                   ^^   ^^
>>     }
>> }
>>
>> This behavior is only correct when xlate functions are called from a
>> xtables-translate command. This patch solves that issue if nft revision
>> is using the field added to the xt_xlate struct with "xtables-translate:
>> add new field to identify the caller" commit.
>
> Please, rebase this change on top of:
>
> http://git.netfilter.org/iptables/commit/?id=7a0992da44cfb6cab0ccd1beadcf326df8773552
>
> I'd suggest you add a new parameter for this, something like:
>
>         bool escape_quotes;
>
> Or is there use for this nft_compat field you propose out of this.

For the moment, the quotes issue is the only use I have found for the
nft_compat field. So the escape_quotes boolean seems a good idea.

Thanks.

Re: [PATCH iptables 2/2] xtables-translate: fix issue with quotes

[Pablo Neira Ayuso]

On Tue, Jul 26, 2016 at 06:22:49PM +0200, Pablo M. Bermudo Garay wrote:
> 2016-07-25 17:31 GMT+02:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> > On Fri, Jul 22, 2016 at 05:48:34PM +0200, Pablo M. Bermudo Garay wrote:
> >> Some translations included escaped quotes when they were called from
> >> nft:
> >>
> >> $ sudo nft list ruleset
> >> table ip mangle {
> >>     chain FORWARD {
> >>         type filter hook forward priority -150; policy accept;
> >>         ct helper \"ftp\" counter packets 0 bytes 0
> >>                   ^^   ^^
> >>     }
> >> }
> >>
> >> This behavior is only correct when xlate functions are called from a
> >> xtables-translate command. This patch solves that issue if nft revision
> >> is using the field added to the xt_xlate struct with "xtables-translate:
> >> add new field to identify the caller" commit.
> >
> > Please, rebase this change on top of:
> >
> > http://git.netfilter.org/iptables/commit/?id=7a0992da44cfb6cab0ccd1beadcf326df8773552
> >
> > I'd suggest you add a new parameter for this, something like:
> >
> >         bool escape_quotes;
> >
> > Or is there use for this nft_compat field you propose out of this.
> 
> For the moment, the quotes issue is the only use I have found for the
> nft_compat field. So the escape_quotes boolean seems a good idea.

Great, I prefer then escape_quotes. We can rename this later on if it
turns out that we need this for something else.