iptables: xtables-eb: fix renaming of chains

Renaming of chains is not working. and ebtables-compat gets:
libnftnl: attribute 0 assertion failed in chain.c:159

This patch brings back the parser code of the original ebtables tool:
http://git.netfilter.org/ebtables.old-history/tree/userspace/ebtables2/ebtables.c#n652

I adaped the original parser code to fit in the new environment, plus fixing style issues.
In the nft backend, some minor changes are needed to support this operation, for example to avoid calling the nf_tables API with NLM_F_EXCL.

I also tried to keep original error messages as much as possible.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 iptables/nft.c                   |   12 +++++++++++-
 iptables/xtables-eb-standalone.c |    2 +-
 iptables/xtables-eb.c            |   33 +++++++++++++++++++++++++++++++--
 3 files changed, 43 insertions(+), 4 deletions(-)

iptables/nft.c +7 -1

index 4ee22c5..7cd56ef 100644
@@ -253,6 +253,7 @@ enum obj_update_type {
 	NFT_COMPAT_CHAIN_USER_ADD,
 	NFT_COMPAT_CHAIN_USER_DEL,
 	NFT_COMPAT_CHAIN_UPDATE,
+	NFT_COMPAT_CHAIN_RENAME,
 	NFT_COMPAT_RULE_APPEND,
 	NFT_COMPAT_RULE_INSERT,
 	NFT_COMPAT_RULE_REPLACE,

@@ -1508,10 +1509,15 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 	uint64_t handle;
 	int ret;
 
+	nft_fn = nft_chain_user_add;
+
 	/* If built-in chains don't exist for this table, create them */
 	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
 		nft_xt_builtin_init(h, table);
 
+	/* Config load changed errno. Ensure genuine info for our callers. */
+	errno = 0;
+
 	/* Find the old chain to be renamed */
 	c = nft_chain_find(h, table, chain);
 	if (c == NULL) {

@@ -1530,7 +1536,7 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 	nft_chain_attr_set_u64(c, NFT_CHAIN_ATTR_HANDLE, handle);
 
 	if (h->batch_support) {
-		ret = batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c);
+		ret = batch_chain_add(h, NFT_COMPAT_CHAIN_RENAME, c);
 	} else {
 		char buf[MNL_SOCKET_BUFFER_SIZE];
 		struct nlmsghdr *nlh;

iptables/xtables-eb-standalone.c +0 -0

 	exit(!ret);
 }

iptables/xtables-eb.c +2 -1

index 0775ee7..e5220c3 100644
@@ -21,6 +21,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include <errno.h>
 #include <getopt.h>
 #include <string.h>
 #include <stdio.h>

@@ -32,6 +33,7 @@
 #include <xtables.h>
 
 #include <linux/netfilter_bridge.h>
+#include <linux/netfilter/nf_tables.h>
 #include <ebtables/ethernetdb.h>
 #include "xshared.h"
 #include "nft.h"

@@ -582,7 +584,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table)
 	struct ebtables_command_state cs;
 	char command = 'h';
 	const char *chain = NULL;
-	const char *newname = NULL;
 	const char *policy = NULL;
 	int exec_style = EXEC_STYLE_PRG;
 	int selected_chain = -1;

@@ -643,7 +644,35 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table)
 			}
 

Quoted 2 lines · Arturo Borrero Gonzalez

Renaming of chains is not working. and ebtables-compat gets:
libnftnl: attribute 0 assertion failed in chain.c:159

Thanks for catching up this.

Quoted 55 lines

This patch brings back the parser code of the original ebtables tool:
http://git.netfilter.org/ebtables.old-history/tree/userspace/ebtables2/ebtables.c#n652

I adaped the original parser code to fit in the new environment, plus fixing style issues.
In the nft backend, some minor changes are needed to support this operation, for example to avoid calling the nf_tables API with NLM_F_EXCL.

I also tried to keep original error messages as much as possible.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 iptables/nft.c                   |   12 +++++++++++-
 iptables/xtables-eb-standalone.c |    2 +-
 iptables/xtables-eb.c            |   33 +++++++++++++++++++++++++++++++--
 3 files changed, 43 insertions(+), 4 deletions(-)

iptables/nft.c +7 -1

index 4ee22c5..7cd56ef 100644
@@ -253,6 +253,7 @@ enum obj_update_type {
      NFT_COMPAT_CHAIN_USER_ADD,
      NFT_COMPAT_CHAIN_USER_DEL,
      NFT_COMPAT_CHAIN_UPDATE,
+     NFT_COMPAT_CHAIN_RENAME,
      NFT_COMPAT_RULE_APPEND,
      NFT_COMPAT_RULE_INSERT,
      NFT_COMPAT_RULE_REPLACE,

@@ -1508,10 +1509,15 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
      uint64_t handle;
      int ret;
 
+     nft_fn = nft_chain_user_add;
+
      /* If built-in chains don't exist for this table, create them */
      if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
              nft_xt_builtin_init(h, table);
 
+     /* Config load changed errno. Ensure genuine info for our callers. */
+     errno = 0;
+
      /* Find the old chain to be renamed */
      c = nft_chain_find(h, table, chain);
      if (c == NULL) {

@@ -1530,7 +1536,7 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
      nft_chain_attr_set_u64(c, NFT_CHAIN_ATTR_HANDLE, handle);
 
      if (h->batch_support) {
-             ret = batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c);
+             ret = batch_chain_add(h, NFT_COMPAT_CHAIN_RENAME, c);
      } else {
              char buf[MNL_SOCKET_BUFFER_SIZE];
              struct nlmsghdr *nlh;

iptables/nft.c static int nft_action(struct nft_handle *h, int action) · line 2294

Quoted 10 lines

iptables/nft.c +4 -0

@@ -2279,6 +2285,10 @@ static int nft_action(struct nft_handle *h, int action)
                                                   NLM_F_CREATE : 0,
                                                 seq++, n->chain);
                      break;
+             case NFT_COMPAT_CHAIN_RENAME:
+                     nft_compat_chain_batch_add(h, NFT_MSG_NEWCHAIN, 0,
+                                                seq++, n->chain);
+                     break;
              case NFT_COMPAT_RULE_APPEND:
                      nft_compat_rule_batch_add(h, NFT_MSG_NEWRULE,
                                                NLM_F_CREATE | NLM_F_APPEND,

Please, send me a separated patch so I can merge this to master.

Quoted 7 lines

iptables/xtables-eb-standalone.c +0 -0

index 1c3cbf0..740a420 100644
@@ -84,7 +84,7 @@ int xtables_eb_main(int argc, char *argv[])
              ret = nft_commit(&h);
 

iptables/xtables-eb-standalone.c int xtables_eb_main(int argc, char *argv[]) · line 87

Quoted 3 lines

iptables/xtables-eb-standalone.c +1 -1

      if (!ret)
-             fprintf(stderr, "%s\n", nft_strerror(errno));
+             xtables_error(OTHER_PROBLEM, "%s\n", nft_strerror(errno));

IIRC error reporting in ebtables differs from iptables. The output should look the same. We're currently using nft_strerror() but I guess we'll need a ebt_strerror() function.

Quoted 7 lines · Pablo Neira Ayuso

Quoted 7 lines

iptables/xtables-eb-standalone.c +0 -0

index 1c3cbf0..740a420 100644
@@ -84,7 +84,7 @@ int xtables_eb_main(int argc, char *argv[])
              ret = nft_commit(&h);
 

Quoted 8 lines

iptables/xtables-eb-standalone.c int xtables_eb_main(int argc, char *argv[]) · line 87

Quoted 3 lines

iptables/xtables-eb-standalone.c +1 -1

      if (!ret)
-             fprintf(stderr, "%s\n", nft_strerror(errno));
+             xtables_error(OTHER_PROBLEM, "%s\n", nft_strerror(errno));

IIRC error reporting in ebtables differs from iptables. The output should look the same. We're currently using nft_strerror() but I guess we'll need a ebt_strerror() function.

I've tried to collect ebtables original errors. Most of them uses arguments, for example:
* Can't delete the chain 'test', it's referenced in chain 'FORWARD', rule 0.
* Loop from chain 'test2' to chain 'test'.

So, that ebt_strerror() function seems almost useless.

I will resend this patch with the changes you requested. I guess we can revisit that issue in the future.