[PATCH iptables 1/2] xtables-translate: add escape_quotes option to comment_xlate

[PATCH iptables 1/2] xtables-translate: add escape_quotes option to comment_xlate

[Pablo M. Bermudo Garay]

The comment_xlate function was not supporting this option that is
necessary in some situations.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 extensions/libxt_comment.c | 9 ++++++++-
 iptables/nft-ipv4.c        | 2 +-
 iptables/nft-ipv6.c        | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/extensions/libxt_comment.c b/extensions/libxt_comment.c
index 0e31edd..bf9a039 100644
--- a/extensions/libxt_comment.c
+++ b/extensions/libxt_comment.c
@@ -52,9 +52,16 @@ static int comment_xlate(struct xt_xlate *xl,
 			 const struct xt_xlate_mt_params *params)
 {
 	struct xt_comment_info *commentinfo = (void *)params->match->data;
+	char comment[XT_MAX_COMMENT_LEN];
 
 	commentinfo->comment[XT_MAX_COMMENT_LEN - 1] = '\0';
-	xt_xlate_add_comment(xl, commentinfo->comment);
+	if (params->escape_quotes)
+		snprintf(comment, XT_MAX_COMMENT_LEN, "comment \\\"%s\\\"",
+			 commentinfo->comment);
+	else
+		snprintf(comment, XT_MAX_COMMENT_LEN, "comment \"%s\"",
+			 commentinfo->comment);
+	xt_xlate_add_comment(xl, comment);
 
 	return 1;
 }
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 50706cb..f5c0d95 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -490,7 +490,7 @@ static int nft_ipv4_xlate(const void *data, struct xt_xlate *xl)
 
 	comment = xt_xlate_get_comment(xl);
 	if (comment)
-		xt_xlate_add(xl, "comment \\\"%s\\\" ", comment);
+		xt_xlate_add(xl, "%s", comment);
 
 	ret = xlate_action(cs, !!(cs->fw.ip.flags & IPT_F_GOTO), xl);
 
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 8ca523c..3792c68 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -439,7 +439,7 @@ static int nft_ipv6_xlate(const void *data, struct xt_xlate *xl)
 
 	comment = xt_xlate_get_comment(xl);
 	if (comment)
-		xt_xlate_add(xl, "comment \\\"%s\\\" ", comment);
+		xt_xlate_add(xl, "%s", comment);
 
 	ret = xlate_action(cs, !!(cs->fw6.ipv6.flags & IP6T_F_GOTO), xl);
 
-- 
2.9.3

[PATCH iptables 2/2] xtables-translate-restore: do not escape quotes

[Pablo M. Bermudo Garay]

If quotes are escaped, nft -f is unable to parse and load the translated
ruleset.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 iptables/xtables-translate.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c
index 3c577ed..914d3b1 100644
--- a/iptables/xtables-translate.c
+++ b/iptables/xtables-translate.c
@@ -72,6 +72,11 @@ int xlate_action(const struct iptables_command_state *cs, bool goto_set,
 				.numeric	= numeric,
 				.escape_quotes	= true,
 			};
+			if (!strcmp(xtables_globals.program_name,
+				   "iptables-translate-restore") ||
+			    !strcmp(xtables_globals.program_name,
+				   "ip6tables-translate-restore"))
+				params.escape_quotes = false;
 			ret = cs->target->xlate(xl, &params);
 		}
 		else
@@ -100,6 +105,12 @@ int xlate_matches(const struct iptables_command_state *cs, struct xt_xlate *xl)
 			.escape_quotes	= true,
 		};
 
+		if (!strcmp(xtables_globals.program_name,
+			   "iptables-translate-restore") ||
+		    !strcmp(xtables_globals.program_name,
+			   "ip6tables-translate-restore"))
+			params.escape_quotes = false;
+
 		if (!matchp->match->xlate)
 			return 0;
 
-- 
2.9.3

Re: [PATCH iptables 1/2] xtables-translate: add escape_quotes option to comment_xlate

[Pablo Neira Ayuso]

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

On Tue, Aug 16, 2016 at 07:44:32PM +0200, Pablo M. Bermudo Garay wrote:
> The comment_xlate function was not supporting this option that is
> necessary in some situations.

I have applied what I'm attaching to this email, that is more simple
than this and makes sure buffer is nul-terminated (given snprintf
doesn't guarantee this).

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1229 bytes --]

commit 3317b14f0d6fa0e460e4e758b7e3010f940d07bc
Author: Pablo M. Bermudo Garay <pablombg@gmail.com>
Date:   Tue Aug 16 19:44:32 2016 +0200

    xtables-translate: add escape_quotes option to comment_xlate
    
    The comment_xlate function was not supporting this option that is
    necessary in some situations.
    
    Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/extensions/libxt_comment.c b/extensions/libxt_comment.c
index 0e31edd..b635d16 100644
--- a/extensions/libxt_comment.c
+++ b/extensions/libxt_comment.c
@@ -52,9 +52,18 @@ static int comment_xlate(struct xt_xlate *xl,
 			 const struct xt_xlate_mt_params *params)
 {
 	struct xt_comment_info *commentinfo = (void *)params->match->data;
+	char comment[XT_MAX_COMMENT_LEN];
 
 	commentinfo->comment[XT_MAX_COMMENT_LEN - 1] = '\0';
-	xt_xlate_add_comment(xl, commentinfo->comment);
+	if (params->escape_quotes)
+		snprintf(comment, XT_MAX_COMMENT_LEN, "\\\"%s\\\"",
+			 commentinfo->comment);
+	else
+		snprintf(comment, XT_MAX_COMMENT_LEN, "\"%s\"",
+			 commentinfo->comment);
+
+	comment[XT_MAX_COMMENT_LEN - 1] = '\0';
+	xt_xlate_add_comment(xl, comment);
 
 	return 1;
 }

Re: [PATCH iptables 2/2] xtables-translate-restore: do not escape quotes

[Pablo Neira Ayuso]

On Tue, Aug 16, 2016 at 07:44:33PM +0200, Pablo M. Bermudo Garay wrote:
> If quotes are escaped, nft -f is unable to parse and load the translated
> ruleset.
> 
> Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
> ---
>  iptables/xtables-translate.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c
> index 3c577ed..914d3b1 100644
> --- a/iptables/xtables-translate.c
> +++ b/iptables/xtables-translate.c
> @@ -72,6 +72,11 @@ int xlate_action(const struct iptables_command_state *cs, bool goto_set,
>  				.numeric	= numeric,
>  				.escape_quotes	= true,
>  			};
> +			if (!strcmp(xtables_globals.program_name,
> +				   "iptables-translate-restore") ||
> +			    !strcmp(xtables_globals.program_name,
> +				   "ip6tables-translate-restore"))
> +				params.escape_quotes = false;
>  			ret = cs->target->xlate(xl, &params);
>  		}
>  		else

I can see a 'bool restore' in do_command_xlate() provides this
context, you can probably propagate this.

This strcmp() is ugly using the global.

Re: [PATCH iptables 1/2] xtables-translate: add escape_quotes option to comment_xlate

[Pablo M. Bermudo Garay]

2016-08-17 16:23 GMT+02:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Tue, Aug 16, 2016 at 07:44:32PM +0200, Pablo M. Bermudo Garay wrote:
>> The comment_xlate function was not supporting this option that is
>> necessary in some situations.
>
> I have applied what I'm attaching to this email, that is more simple
> than this and makes sure buffer is nul-terminated (given snprintf
> doesn't guarantee this).

Ok, the simplification seems great. But AFAIK both standards (C99+ and
POSIX.1-2001+) guarantee that the string will be null-terminated. Am I
wrong about that?

Re: [PATCH iptables 1/2] xtables-translate: add escape_quotes option to comment_xlate

[Pablo Neira Ayuso]

On Thu, Aug 18, 2016 at 05:59:30PM +0200, Pablo M. Bermudo Garay wrote:
> 2016-08-17 16:23 GMT+02:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> > On Tue, Aug 16, 2016 at 07:44:32PM +0200, Pablo M. Bermudo Garay wrote:
> >> The comment_xlate function was not supporting this option that is
> >> necessary in some situations.
> >
> > I have applied what I'm attaching to this email, that is more simple
> > than this and makes sure buffer is nul-terminated (given snprintf
> > doesn't guarantee this).
> 
> Ok, the simplification seems great. But AFAIK both standards (C99+ and
> POSIX.1-2001+) guarantee that the string will be null-terminated. Am I
> wrong about that?

If you write as many bytes as you can fit into the buffer, then
snprintf doesn't guarantee a nul-terminated string.