[PATCH 0/2] Add syntax and translation for comment module

[PATCH 0/2] Add syntax and translation for comment module

[Shivani Bhardwaj]

This patchset adds the appropriate syntax for comment module that is,
counter should be used as prefix with comment module unlike all other
modules. Also, it adds the appropriate translation to nft.

After applying this patchset, iptables-translate produces correct
translation for comment module to nft.

Shivani Bhardwaj (2):
  iptables: nft-ipv4: Remove suffix counter for comment module
  extensions: libxt_comment: Add translation to nft

 extensions/libxt_comment.c | 17 +++++++++++++++--
 iptables/nft-ipv4.c        | 25 +++++++++++++++----------
 2 files changed, 30 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH 1/2] iptables: nft-ipv4: Remove suffix counter for comment module

[Shivani Bhardwaj]

Remove the counter as suffix for comment module as it should be used as
prefix for this case.

Example:

$ sudo nft add rule ip filter INPUT comment \"random comment\" counter
throws Error: syntax error, unexpected comment

$ sudo nft add rule ip filter INPUT counter comment \"random comment\"
gets accepted as a legit rule in nftables

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 iptables/nft-ipv4.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index f59f630..60720e0 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -100,7 +100,8 @@ static bool nft_ipv4_is_same(const void *data_a,
 	return is_same_interfaces(a->fw.ip.iniface, a->fw.ip.outiface,
 				  a->fw.ip.iniface_mask, a->fw.ip.outiface_mask,
 				  b->fw.ip.iniface, b->fw.ip.outiface,
-				  b->fw.ip.iniface_mask, b->fw.ip.outiface_mask);
+				  b->fw.ip.iniface_mask,
+				  b->fw.ip.outiface_mask);
 }
 
 static void get_frag(struct nft_rule_expr_iter *iter, bool *inv)
@@ -180,7 +181,7 @@ static void nft_ipv4_parse_payload(struct nft_rule_expr_iter *iter,
 {
 	struct iptables_command_state *cs = data;
 
-	switch(offset) {
+	switch (offset) {
 	struct in_addr addr;
 	uint8_t proto;
 	bool inv;
@@ -235,26 +236,26 @@ static void print_ipv4_addr(const struct iptables_command_state *cs,
 
 	fputc(cs->fw.ip.invflags & IPT_INV_SRCIP ? '!' : ' ', stdout);
 	if (cs->fw.ip.smsk.s_addr == 0L && !(format & FMT_NUMERIC))
-		printf(FMT("%-19s ","%s "), "anywhere");
+		printf(FMT("%-19s ", "%s "), "anywhere");
 	else {
 		if (format & FMT_NUMERIC)
 			strcpy(buf, xtables_ipaddr_to_numeric(&cs->fw.ip.src));
 		else
 			strcpy(buf, xtables_ipaddr_to_anyname(&cs->fw.ip.src));
 		strcat(buf, xtables_ipmask_to_numeric(&cs->fw.ip.smsk));
-		printf(FMT("%-19s ","%s "), buf);
+		printf(FMT("%-19s ", "%s "), buf);
 	}
 
 	fputc(cs->fw.ip.invflags & IPT_INV_DSTIP ? '!' : ' ', stdout);
 	if (cs->fw.ip.dmsk.s_addr == 0L && !(format & FMT_NUMERIC))
-		printf(FMT("%-19s ","-> %s"), "anywhere");
+		printf(FMT("%-19s ", "-> %s"), "anywhere");
 	else {
 		if (format & FMT_NUMERIC)
 			strcpy(buf, xtables_ipaddr_to_numeric(&cs->fw.ip.dst));
 		else
 			strcpy(buf, xtables_ipaddr_to_anyname(&cs->fw.ip.dst));
 		strcat(buf, xtables_ipmask_to_numeric(&cs->fw.ip.dmsk));
-		printf(FMT("%-19s ","-> %s"), buf);
+		printf(FMT("%-19s ", "-> %s"), buf);
 	}
 }
 
@@ -422,13 +423,13 @@ static int nft_ipv4_xlate(const void *data, struct xt_buf *buf)
 	}
 	if (cs->fw.ip.outiface[0] != '\0') {
 		xt_buf_add(buf, "oifname %s%s ",
-			   cs->fw.ip.invflags & IPT_INV_VIA_OUT? "!= " : "",
+			   cs->fw.ip.invflags & IPT_INV_VIA_OUT ? "!= " : "",
 			   cs->fw.ip.outiface);
 	}
 
 	if (cs->fw.ip.flags & IPT_F_FRAG) {
 		xt_buf_add(buf, "ip frag-off %s%x ",
-			   cs->fw.ip.invflags & IPT_INV_FRAG? "" : "!= ", 0);
+			   cs->fw.ip.invflags & IPT_INV_FRAG ? "" : "!= ", 0);
 	}
 
 	if (cs->fw.ip.proto != 0) {
@@ -462,8 +463,12 @@ static int nft_ipv4_xlate(const void *data, struct xt_buf *buf)
 	if (!ret)
 		return ret;
 
-	/* Always add counters per rule, as in iptables */
-	xt_buf_add(buf, "counter ");
+	/*
+	 * Always add counters as suffix per rule as in iptables
+	 * except for comment where it should be prefix
+	 */
+	if (strcmp(cs->matches->match->name, "comment"))
+		xt_buf_add(buf, "counter ");
 
 	ret = xlate_action(cs, !!(cs->fw.ip.flags & IPT_F_GOTO), buf);
 
-- 
1.9.1

[PATCH 2/2] extensions: libxt_comment: Add translation to nft

[Shivani Bhardwaj]

Add translation for comment module to nftables.

Examples:

$ sudo iptables-translate -A INPUT -m comment --comment "random comment"
nft add rule ip filter INPUT counter comment \"random comment\"

$ sudo iptables-translate -A INPUT -i eth1 -m comment --comment "Local LAN"
nft add rule ip filter INPUT iifname eth1 counter comment \"Local LAN\"

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 extensions/libxt_comment.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/extensions/libxt_comment.c b/extensions/libxt_comment.c
index 6ed2ff9..bbbd6cb 100644
--- a/extensions/libxt_comment.c
+++ b/extensions/libxt_comment.c
@@ -48,6 +48,18 @@ comment_save(const void *ip, const struct xt_entry_match *match)
 	xtables_save_string(commentinfo->comment);
 }
 
+static int
+comment_xlate(const struct xt_entry_match *match,
+	      struct xt_buf *buf, int numeric)
+{
+	struct xt_comment_info *commentinfo = (void *)match->data;
+
+	commentinfo->comment[XT_MAX_COMMENT_LEN-1] = '\0';
+	xt_buf_add(buf, "counter comment \\\"%s\\\" ", commentinfo->comment);
+
+	return 1;
+}
+
 static struct xtables_match comment_match = {
 	.family		= NFPROTO_UNSPEC,
 	.name		= "comment",
@@ -55,10 +67,11 @@ static struct xtables_match comment_match = {
 	.size		= XT_ALIGN(sizeof(struct xt_comment_info)),
 	.userspacesize	= XT_ALIGN(sizeof(struct xt_comment_info)),
 	.help		= comment_help,
-	.print 		= comment_print,
-	.save 		= comment_save,
+	.print		= comment_print,
+	.save		= comment_save,
 	.x6_parse	= xtables_option_parse,
 	.x6_options	= comment_opts,
+	.xlate		= comment_xlate,
 };
 
 void _init(void)
-- 
1.9.1

Re: [PATCH 2/2] extensions: libxt_comment: Add translation to nft

[Pablo Neira Ayuso]

On Tue, Dec 22, 2015 at 03:48:34PM +0530, Shivani Bhardwaj wrote:
> Add translation for comment module to nftables.
> 
> Examples:
> 
> $ sudo iptables-translate -A INPUT -m comment --comment "random comment"
> nft add rule ip filter INPUT counter comment \"random comment\"
> 
> $ sudo iptables-translate -A INPUT -i eth1 -m comment --comment "Local LAN"
> nft add rule ip filter INPUT iifname eth1 counter comment \"Local LAN\"
> 
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
>  extensions/libxt_comment.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/extensions/libxt_comment.c b/extensions/libxt_comment.c
> index 6ed2ff9..bbbd6cb 100644
> --- a/extensions/libxt_comment.c
> +++ b/extensions/libxt_comment.c
> @@ -48,6 +48,18 @@ comment_save(const void *ip, const struct xt_entry_match *match)
>  	xtables_save_string(commentinfo->comment);
>  }
>  
> +static int
> +comment_xlate(const struct xt_entry_match *match,
> +	      struct xt_buf *buf, int numeric)
> +{
> +	struct xt_comment_info *commentinfo = (void *)match->data;
> +
> +	commentinfo->comment[XT_MAX_COMMENT_LEN-1] = '\0';
> +	xt_buf_add(buf, "counter comment \\\"%s\\\" ", commentinfo->comment);
                         ^^^^^^^

We have to find a better way to fix this from the core infrastructure.

Re: [PATCH 2/2] extensions: libxt_comment: Add translation to nft

[Shivani Bhardwaj]

On Tue, Dec 22, 2015 at 10:34 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Dec 22, 2015 at 03:48:34PM +0530, Shivani Bhardwaj wrote:
>> Add translation for comment module to nftables.
>>
>> Examples:
>>
>> $ sudo iptables-translate -A INPUT -m comment --comment "random comment"
>> nft add rule ip filter INPUT counter comment \"random comment\"
>>
>> $ sudo iptables-translate -A INPUT -i eth1 -m comment --comment "Local LAN"
>> nft add rule ip filter INPUT iifname eth1 counter comment \"Local LAN\"
>>
>> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> ---
>>  extensions/libxt_comment.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/extensions/libxt_comment.c b/extensions/libxt_comment.c
>> index 6ed2ff9..bbbd6cb 100644
>> --- a/extensions/libxt_comment.c
>> +++ b/extensions/libxt_comment.c
>> @@ -48,6 +48,18 @@ comment_save(const void *ip, const struct xt_entry_match *match)
>>       xtables_save_string(commentinfo->comment);
>>  }
>>
>> +static int
>> +comment_xlate(const struct xt_entry_match *match,
>> +           struct xt_buf *buf, int numeric)
>> +{
>> +     struct xt_comment_info *commentinfo = (void *)match->data;
>> +
>> +     commentinfo->comment[XT_MAX_COMMENT_LEN-1] = '\0';
>> +     xt_buf_add(buf, "counter comment \\\"%s\\\" ", commentinfo->comment);
>                          ^^^^^^^
>
> We have to find a better way to fix this from the core infrastructure.

Is removing counter at all for this one going to be a good option?