[iptables,4/4] xtables: Fix rules print/save after iptables update

Message ID 1520413843-24456-5-git-send-email-serhe.popovych@gmail.com
State Accepted
Delegated to: Pablo Neira
Headers show
Series
  • iptables: Fix [unsupported revision] for matches/targets after update
Related show

Commit Message

Serhey Popovych March 7, 2018, 9:10 a.m.
Updating iptables from 1.4.x to 1.6.x brokes rules print/save output
and causes rules load after reboot to fail. Here is example from
iptables-save(8) output after update:

  -A CHAIN1 -m set [unsupported revision] -j DROP
  -A CHAIN1 -m set [unsupported revision] -j DROP

Similar output could be obtained via iptables -L CHAIN1. While issue
reproduced with xt_set match it is not specific to any match or
target module: it is related on how xtables handles revisions.

In this particular case we have following situation:

  1) Kernel supports revisions from 1 to 4.

  2) Rules configured with iptables 1.4.x supporting only
     revisions from 1 to 3. Choosen highest possible revision 3.

  3) Rules printed/saved with iptables 1.6.x supporting revisions
     from 1 to 4.

  4) Xtables registers matches/targets with highest supported
     revision by the kernel. This is 4 in our case after update to
     iptables 1.6.x.

  5) When printing/saving kernel submits match/target with revision
     it is configured (3), while iptables thinks that rules configured
     with highest supported (4). That's causes revision mismatch in
     during print and "[unsupported revision]" output.

To fix this issue we now store all supported by kernel and xtables
revisions in xt_matches/xt_targets list sorted in descending order.

Introduce helper routines to find match/target with given revision
and use them to find right revision to print submitted by kernel
entry.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 include/xtables.h    |    6 +++++
 iptables/ip6tables.c |   66 ++++++++++++++++++++++++++++++++------------------
 iptables/iptables.c  |   66 ++++++++++++++++++++++++++++++++------------------
 libxtables/xtables.c |   42 ++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 46 deletions(-)

Patch

diff --git a/include/xtables.h b/include/xtables.h
index e9bc3b7..0b19e0f 100644
--- a/include/xtables.h
+++ b/include/xtables.h
@@ -464,8 +464,14 @@  extern struct option *xtables_merge_options(struct option *origopts,
 extern int xtables_init_all(struct xtables_globals *xtp, uint8_t nfproto);
 extern struct xtables_match *xtables_find_match(const char *name,
 	enum xtables_tryload, struct xtables_rule_match **match);
+extern struct xtables_match *xtables_find_match_revision(const char *name,
+	enum xtables_tryload tryload, struct xtables_match *match,
+	int revision);
 extern struct xtables_target *xtables_find_target(const char *name,
 	enum xtables_tryload);
+struct xtables_target *xtables_find_target_revision(const char *name,
+	enum xtables_tryload tryload, struct xtables_target *target,
+	int revision);
 extern int xtables_compatible_revision(const char *name, uint8_t revision,
 				       int opt);
 
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 49bd006..6954c25 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -518,19 +518,23 @@  print_match(const struct xt_entry_match *m,
 	    const struct ip6t_ip6 *ip,
 	    int numeric)
 {
-	const struct xtables_match *match =
-		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
+	const char *name = m->u.user.name;
+	const int revision = m->u.user.revision;
+	struct xtables_match *match, *mt;
 
+	match = xtables_find_match(name, XTF_TRY_LOAD, NULL);
 	if (match) {
-		if (match->print && m->u.user.revision == match->revision)
-			match->print(ip, m, numeric);
+		mt = xtables_find_match_revision(name, XTF_TRY_LOAD,
+						 match, revision);
+		if (mt && mt->print)
+			mt->print(ip, m, numeric);
 		else if (match->print)
 			printf("%s%s ", match->name, unsupported_rev);
 		else
 			printf("%s ", match->name);
 	} else {
-		if (m->u.user.name[0])
-			printf("UNKNOWN match `%s' ", m->u.user.name);
+		if (name[0])
+			printf("UNKNOWN match `%s' ", name);
 	}
 	/* Don't stop iterating. */
 	return 0;
@@ -544,7 +548,7 @@  print_firewall(const struct ip6t_entry *fw,
 	       unsigned int format,
 	       struct xtc_handle *const handle)
 {
-	const struct xtables_target *target = NULL;
+	struct xtables_target *target, *tg;
 	const struct xt_entry_target *t;
 	char buf[BUFSIZ];
 
@@ -651,9 +655,13 @@  print_firewall(const struct ip6t_entry *fw,
 	IP6T_MATCH_ITERATE(fw, print_match, &fw->ipv6, format & FMT_NUMERIC);
 
 	if (target) {
-		if (target->print && t->u.user.revision == target->revision)
+		const int revision = t->u.user.revision;
+
+		tg = xtables_find_target_revision(targname, XTF_TRY_LOAD,
+						  target, revision);
+		if (tg && tg->print)
 			/* Print the target information. */
-			target->print(&fw->ipv6, t, format & FMT_NUMERIC);
+			tg->print(&fw->ipv6, t, format & FMT_NUMERIC);
 		else if (target->print)
 			printf(" %s%s", target->name, unsupported_rev);
 	} else if (t->u.target_size != sizeof(*t))
@@ -1035,23 +1043,28 @@  static void print_proto(uint16_t proto, int invert)
 static int print_match_save(const struct xt_entry_match *e,
 			const struct ip6t_ip6 *ip)
 {
-	const struct xtables_match *match =
-		xtables_find_match(e->u.user.name, XTF_TRY_LOAD, NULL);
+	const char *name = e->u.user.name;
+	const int revision = e->u.user.revision;
+	struct xtables_match *match, *mt, *mt2;
 
+	match = xtables_find_match(name, XTF_TRY_LOAD, NULL);
 	if (match) {
-		printf(" -m %s",
-			match->alias ? match->alias(e) : e->u.user.name);
+		mt = mt2 = xtables_find_match_revision(name, XTF_TRY_LOAD,
+						       match, revision);
+		if (!mt2)
+			mt2 = match;
+		printf(" -m %s", mt2->alias ? mt2->alias(e) : name);
 
 		/* some matches don't provide a save function */
-		if (match->save && e->u.user.revision == match->revision)
-			match->save(ip, e);
+		if (mt && mt->save)
+			mt->save(ip, e);
 		else if (match->save)
 			printf(unsupported_rev);
 	} else {
 		if (e->u.match_size) {
 			fprintf(stderr,
 				"Can't find library for match `%s'\n",
-				e->u.user.name);
+				name);
 			exit(1);
 		}
 	}
@@ -1136,18 +1149,25 @@  void print_rule6(const struct ip6t_entry *e,
 	target_name = ip6tc_get_target(e, h);
 	t = ip6t_get_target((struct ip6t_entry *)e);
 	if (t->u.user.name[0]) {
-		struct xtables_target *target =
-			xtables_find_target(t->u.user.name, XTF_TRY_LOAD);
+		const char *name = t->u.user.name;
+		const int revision = t->u.user.revision;
+		struct xtables_target *target, *tg, *tg2;
 
+		target = xtables_find_target(name, XTF_TRY_LOAD);
 		if (!target) {
 			fprintf(stderr, "Can't find library for target `%s'\n",
-				t->u.user.name);
+				name);
 			exit(1);
 		}
 
-		printf(" -j %s", target->alias ? target->alias(t) : target_name);
-		if (target->save && t->u.user.revision == target->revision)
-			target->save(&e->ipv6, t);
+		tg = tg2 = xtables_find_target_revision(name, XTF_TRY_LOAD,
+							target, revision);
+		if (!tg2)
+			tg2 = target;
+		printf(" -j %s", tg2->alias ? tg2->alias(t) : target_name);
+
+		if (tg && tg->save)
+			tg->save(&e->ipv6, t);
 		else if (target->save)
 			printf(unsupported_rev);
 		else {
@@ -1158,7 +1178,7 @@  void print_rule6(const struct ip6t_entry *e,
 			    sizeof(struct xt_entry_target)) {
 				fprintf(stderr, "Target `%s' is missing "
 						"save function\n",
-					t->u.user.name);
+					name);
 				exit(1);
 			}
 		}
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 69d19fe..acacf18 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -502,19 +502,23 @@  print_match(const struct xt_entry_match *m,
 	    const struct ipt_ip *ip,
 	    int numeric)
 {
-	const struct xtables_match *match =
-		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
+	const char *name = m->u.user.name;
+	const int revision = m->u.user.revision;
+	struct xtables_match *match, *mt;
 
+	match = xtables_find_match(name, XTF_TRY_LOAD, NULL);
 	if (match) {
-		if (match->print && m->u.user.revision == match->revision)
-			match->print(ip, m, numeric);
+		mt = xtables_find_match_revision(name, XTF_TRY_LOAD,
+						 match, revision);
+		if (mt && mt->print)
+			mt->print(ip, m, numeric);
 		else if (match->print)
 			printf("%s%s ", match->name, unsupported_rev);
 		else
 			printf("%s ", match->name);
 	} else {
-		if (m->u.user.name[0])
-			printf("UNKNOWN match `%s' ", m->u.user.name);
+		if (name[0])
+			printf("UNKNOWN match `%s' ", name);
 	}
 	/* Don't stop iterating. */
 	return 0;
@@ -528,7 +532,7 @@  print_firewall(const struct ipt_entry *fw,
 	       unsigned int format,
 	       struct xtc_handle *const handle)
 {
-	const struct xtables_target *target = NULL;
+	struct xtables_target *target, *tg;
 	const struct xt_entry_target *t;
 	uint8_t flags;
 	char buf[BUFSIZ];
@@ -635,9 +639,13 @@  print_firewall(const struct ipt_entry *fw,
 	IPT_MATCH_ITERATE(fw, print_match, &fw->ip, format & FMT_NUMERIC);
 
 	if (target) {
-		if (target->print && t->u.user.revision == target->revision)
+		const int revision = t->u.user.revision;
+
+		tg = xtables_find_target_revision(targname, XTF_TRY_LOAD,
+						  target, revision);
+		if (tg && tg->print)
 			/* Print the target information. */
-			target->print(&fw->ip, t, format & FMT_NUMERIC);
+			tg->print(&fw->ip, t, format & FMT_NUMERIC);
 		else if (target->print)
 			printf(" %s%s", target->name, unsupported_rev);
 	} else if (t->u.target_size != sizeof(*t))
@@ -1025,23 +1033,28 @@  print_iface(char letter, const char *iface, const unsigned char *mask,
 static int print_match_save(const struct xt_entry_match *e,
 			const struct ipt_ip *ip)
 {
-	const struct xtables_match *match =
-		xtables_find_match(e->u.user.name, XTF_TRY_LOAD, NULL);
+	const char *name = e->u.user.name;
+	const int revision = e->u.user.revision;
+	struct xtables_match *match, *mt, *mt2;
 
+	match = xtables_find_match(name, XTF_TRY_LOAD, NULL);
 	if (match) {
-		printf(" -m %s",
-			match->alias ? match->alias(e) : e->u.user.name);
+		mt = mt2 = xtables_find_match_revision(name, XTF_TRY_LOAD,
+						       match, revision);
+		if (!mt2)
+			mt2 = match;
+		printf(" -m %s", mt2->alias ? mt2->alias(e) : name);
 
 		/* some matches don't provide a save function */
-		if (match->save && e->u.user.revision == match->revision)
-			match->save(ip, e);
+		if (mt && mt->save)
+			mt->save(ip, e);
 		else if (match->save)
 			printf(unsupported_rev);
 	} else {
 		if (e->u.match_size) {
 			fprintf(stderr,
 				"Can't find library for match `%s'\n",
-				e->u.user.name);
+				name);
 			exit(1);
 		}
 	}
@@ -1125,18 +1138,25 @@  void print_rule4(const struct ipt_entry *e,
 	target_name = iptc_get_target(e, h);
 	t = ipt_get_target((struct ipt_entry *)e);
 	if (t->u.user.name[0]) {
-		const struct xtables_target *target =
-			xtables_find_target(t->u.user.name, XTF_TRY_LOAD);
+		const char *name = t->u.user.name;
+		const int revision = t->u.user.revision;
+		struct xtables_target *target, *tg, *tg2;
 
+		target = xtables_find_target(name, XTF_TRY_LOAD);
 		if (!target) {
 			fprintf(stderr, "Can't find library for target `%s'\n",
-				t->u.user.name);
+				name);
 			exit(1);
 		}
 
-		printf(" -j %s", target->alias ? target->alias(t) : target_name);
-		if (target->save && t->u.user.revision == target->revision)
-			target->save(&e->ip, t);
+		tg = tg2 = xtables_find_target_revision(name, XTF_TRY_LOAD,
+							target, revision);
+		if (!tg2)
+			tg2 = target;
+		printf(" -j %s", tg2->alias ? tg2->alias(t) : target_name);
+
+		if (tg && tg->save)
+			tg->save(&e->ip, t);
 		else if (target->save)
 			printf(unsupported_rev);
 		else {
@@ -1147,7 +1167,7 @@  void print_rule4(const struct ipt_entry *e,
 			    sizeof(struct xt_entry_target)) {
 				fprintf(stderr, "Target `%s' is missing "
 						"save function\n",
-					t->u.user.name);
+					name);
 				exit(1);
 			}
 		}
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 5a115ff..06044a2 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -709,6 +709,27 @@  xtables_find_match(const char *name, enum xtables_tryload tryload,
 	return ptr;
 }
 
+struct xtables_match *
+xtables_find_match_revision(const char *name, enum xtables_tryload tryload,
+			    struct xtables_match *match, int revision)
+{
+	if (!match) {
+		match = xtables_find_match(name, tryload, NULL);
+		if (!match)
+			return NULL;
+	}
+
+	while (1) {
+		if (match->revision == revision)
+			return match;
+		match = match->next;
+		if (!match)
+			return NULL;
+		if (!extension_cmp(name, match->name, match->family))
+			return NULL;
+	}
+}
+
 struct xtables_target *
 xtables_find_target(const char *name, enum xtables_tryload tryload)
 {
@@ -769,6 +790,27 @@  xtables_find_target(const char *name, enum xtables_tryload tryload)
 	return ptr;
 }
 
+struct xtables_target *
+xtables_find_target_revision(const char *name, enum xtables_tryload tryload,
+			     struct xtables_target *target, int revision)
+{
+	if (!target) {
+		target = xtables_find_target(name, tryload);
+		if (!target)
+			return NULL;
+	}
+
+	while (1) {
+		if (target->revision == revision)
+			return target;
+		target = target->next;
+		if (!target)
+			return NULL;
+		if (!extension_cmp(name, target->name, target->family))
+			return NULL;
+	}
+}
+
 int xtables_compatible_revision(const char *name, uint8_t revision, int opt)
 {
 	struct xt_get_revision rev;