Assorted fixes v3

Assorted fixes v3

[Jan Engelhardt]

Removed
  build: also use libtool for install stage
  iptables: reword warning on using an alias
Added:
  build: bump SONAME for libxtables

===
The following changes since commit 817ac5a5e54d083983b7c834194b46c4366d71d2:

  Merge branch 'master' of vishnu.netfilter.org:/data/git/iptables (2013-01-31 20:36:27 +0100)

are available in the git repository at:


  git://git.inai.de/iptables master

for you to fetch changes up to 80722f5b0c2723d34782affec7dc247352d9be33:

  iptables: fall back to using save function when print is not defined (2013-02-06 01:40:43 +0100)

----------------------------------------------------------------
Jan Engelhardt (4):
      build: bump SONAME for libxtables
      libxtables: centralize checking for a .save function
      extensions: eui64: set userspacesize=0
      iptables: fall back to using save function when print is not defined

 configure.ac                |    2 +-
 extensions/libip6t_eui64.c  |    2 +-
 extensions/libxt_standard.c |   14 ++++++++++++++
 iptables/ip6tables.c        |   22 ++++++++--------------
 iptables/iptables.c         |   22 ++++++++--------------
 libxtables/xtables.c        |   21 +++++++++++++++++++++
 tests/options-most.rules    |    2 ++
 7 files changed, 55 insertions(+), 30 deletions(-)

[PATCH 1/4] build: bump SONAME for libxtables

[Jan Engelhardt]

Commit v1.4.17-16-gefcdba4 updated structs in xtables.h, so age must
become 0 and vcurrent be increased. The latter has already happened in
v1.4.17-6-gd1e7922.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 configure.ac |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index ffd088c..27e0b10 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,7 +3,7 @@ AC_INIT([iptables], [1.4.17])
 
 # See libtool.info "Libtool's versioning system"
 libxtables_vcurrent=10
-libxtables_vage=1
+libxtables_vage=0
 
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
-- 
1.7.10.4

[PATCH 2/4] libxtables: centralize checking for a .save function

[Jan Engelhardt]

Both iptables.c and ip6tables.c check for target->save == NULL, which
can be consolidated. In fact, we should also check for match->save ==
NULL, which this patch addds to libxtables.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/libxt_standard.c |   14 ++++++++++++++
 iptables/ip6tables.c        |   12 ------------
 iptables/iptables.c         |   12 ------------
 libxtables/xtables.c        |   21 +++++++++++++++++++++
 4 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/extensions/libxt_standard.c b/extensions/libxt_standard.c
index c64ba29..601e709 100644
--- a/extensions/libxt_standard.c
+++ b/extensions/libxt_standard.c
@@ -9,12 +9,26 @@ static void standard_help(void)
 "(If target is DROP, ACCEPT, RETURN or nothing)\n");
 }
 
+static void standard_save(const void *ip, const struct xt_entry_target *t)
+{
+	/*
+	 * This function left blank intentionally - it only serves to make
+	 * iptables not exit with "target lacks a save function". The
+	 * "standard" target is special, since we do not emit -j standard, but
+	 * -j <verdict>. This is printed by iptables.c's function
+	 * print_rule4(), which ultimately calls TC_GET_TARGET in
+	 * libiptc/libiptc.c that will emit the verdict name based upon data in
+	 * the parameter block (@t->data in this "standard_save" function).
+	 */
+}
+
 static struct xtables_target standard_target = {
 	.family		= NFPROTO_UNSPEC,
 	.name		= "standard",
 	.version	= XTABLES_VERSION,
 	.size		= XT_ALIGN(sizeof(int)),
 	.userspacesize	= XT_ALIGN(sizeof(int)),
+	.save		= standard_save,
 	.help		= standard_help,
 };
 
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 4cfbea3..aeeb62a 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1106,18 +1106,6 @@ void print_rule6(const struct ip6t_entry *e,
 		printf(" -j %s", target->alias ? target->alias(t) : target_name);
 		if (target->save)
 			target->save(&e->ipv6, t);
-		else {
-			/* If the target size is greater than xt_entry_target
-			 * there is something to be saved, we just don't know
-			 * how to print it */
-			if (t->u.target_size !=
-			    sizeof(struct xt_entry_target)) {
-				fprintf(stderr, "Target `%s' is missing "
-						"save function\n",
-					t->u.user.name);
-				exit(1);
-			}
-		}
 	} else if (target_name && (*target_name != '\0'))
 #ifdef IP6T_F_GOTO
 		printf(" -%c %s", e->ipv6.flags & IP6T_F_GOTO ? 'g' : 'j', target_name);
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 085eea1..27bd5b4 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1097,18 +1097,6 @@ void print_rule4(const struct ipt_entry *e,
 		printf(" -j %s", target->alias ? target->alias(t) : target_name);
 		if (target->save)
 			target->save(&e->ip, t);
-		else {
-			/* If the target size is greater than xt_entry_target
-			 * there is something to be saved, we just don't know
-			 * how to print it */
-			if (t->u.target_size !=
-			    sizeof(struct xt_entry_target)) {
-				fprintf(stderr, "Target `%s' is missing "
-						"save function\n",
-					t->u.user.name);
-				exit(1);
-			}
-		}
 	} else if (target_name && (*target_name != '\0'))
 #ifdef IPT_F_GOTO
 		printf(" -%c %s", e->ip.flags & IPT_F_GOTO ? 'g' : 'j', target_name);
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 009ab91..b81013a 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -852,6 +852,16 @@ void xtables_register_match(struct xtables_match *me)
 		xtables_option_metavalidate(me->name, me->x6_options);
 	if (me->extra_opts != NULL)
 		xtables_check_options(me->name, me->extra_opts);
+	if (me->userspacesize > 0 && me->save == NULL &&
+	    me->real_name == NULL) {
+		/*
+		 * Catch extensions that have data to be saved, but which
+		 * forgot to define a save method.
+		 */
+		fprintf(stderr, "Match \"%s\" is missing a save function\n",
+		        me->name);
+		exit(1);
+	}
 
 	/* ignore not interested match */
 	if (me->family != afinfo->family && me->family != AF_UNSPEC)
@@ -1010,6 +1020,17 @@ void xtables_register_target(struct xtables_target *me)
 		xtables_option_metavalidate(me->name, me->x6_options);
 	if (me->extra_opts != NULL)
 		xtables_check_options(me->name, me->extra_opts);
+	if (me->userspacesize > 0 && me->save == NULL &&
+	    me->real_name == NULL) {
+		/*
+		 * Catch extensions that have data to be saved, but which
+		 * forgot to define a save method. This only applies to true
+		 * modules (real_name==NULL), not aliases.
+		 */
+		fprintf(stderr, "Target \"%s\" is missing a save function\n",
+		        me->name);
+		exit(1);
+	}
 
 	/* ignore not interested target */
 	if (me->family != afinfo->family && me->family != AF_UNSPEC)
-- 
1.7.10.4

[PATCH 3/4] extensions: eui64: set userspacesize=0

[Jan Engelhardt]

The eui64 match does not use its parameter block at all, so
userspacesize should be 0. This resolves "eui64 is missing a save
function".

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/libip6t_eui64.c |    2 +-
 tests/options-most.rules   |    2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/extensions/libip6t_eui64.c b/extensions/libip6t_eui64.c
index 607bf86..3bd7a8f 100644
--- a/extensions/libip6t_eui64.c
+++ b/extensions/libip6t_eui64.c
@@ -6,7 +6,7 @@ static struct xtables_match eui64_mt6_reg = {
 	.version	= XTABLES_VERSION,
 	.family		= NFPROTO_IPV6,
 	.size		= XT_ALIGN(sizeof(int)),
-	.userspacesize	= XT_ALIGN(sizeof(int)),
+	.userspacesize	= 0,
 };
 
 void _init(void)
diff --git a/tests/options-most.rules b/tests/options-most.rules
index ef4e7f1..c26847e 100644
--- a/tests/options-most.rules
+++ b/tests/options-most.rules
@@ -91,6 +91,8 @@
 -A matches
 -A matches -p esp -m esp --espspi 5:4294967295
 -A matches
+-A matches -m eui64
+-A matches
 -A matches -m hashlimit --hashlimit-upto 1/sec --hashlimit-burst 1 --hashlimit-name mini1 --hashlimit-htable-expire 2000
 -A matches -m hashlimit --hashlimit-upto 1/sec --hashlimit-burst 1 --hashlimit-name mini1
 -A matches -m hashlimit --hashlimit-upto 1/min --hashlimit-burst 1 --hashlimit-name mini2
-- 
1.7.10.4

[PATCH 4/4] iptables: fall back to using save function when print is not defined

[Jan Engelhardt]

This way we can avoid having to provide two dumping function for new
plugins.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 iptables/ip6tables.c |   10 ++++++++--
 iptables/iptables.c  |   10 ++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index aeeb62a..6a85d83 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -487,10 +487,14 @@ print_match(const struct xt_entry_match *m,
 		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
 
 	if (match) {
-		if (match->print)
+		if (match->print) {
 			match->print(ip, m, numeric);
-		else
+		} else if (match->save != NULL) {
+			printf("-m %s ", match->name);
+			match->save(ip, m);
+		} else {
 			printf("%s ", match->name);
+		}
 	} else {
 		if (m->u.user.name[0])
 			printf("UNKNOWN match `%s' ", m->u.user.name);
@@ -617,6 +621,8 @@ print_firewall(const struct ip6t_entry *fw,
 		if (target->print)
 			/* Print the target information. */
 			target->print(&fw->ipv6, t, format & FMT_NUMERIC);
+		else if (target->save != NULL)
+			target->save(&fw->ipv6, t);
 	} else if (t->u.target_size != sizeof(*t))
 		printf("[%u bytes of unknown target data] ",
 		       (unsigned int)(t->u.target_size - sizeof(*t)));
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 27bd5b4..757d9d3 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -472,10 +472,14 @@ print_match(const struct xt_entry_match *m,
 		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
 
 	if (match) {
-		if (match->print)
+		if (match->print) {
 			match->print(ip, m, numeric);
-		else
+		} else if (match->save != NULL) {
+			printf("-m %s ", match->name);
+			match->save(ip, m);
+		} else {
 			printf("%s ", match->name);
+		}
 	} else {
 		if (m->u.user.name[0])
 			printf("UNKNOWN match `%s' ", m->u.user.name);
@@ -602,6 +606,8 @@ print_firewall(const struct ipt_entry *fw,
 		if (target->print)
 			/* Print the target information. */
 			target->print(&fw->ip, t, format & FMT_NUMERIC);
+		else if (target->save != NULL)
+			target->save(&fw->ip, t);
 	} else if (t->u.target_size != sizeof(*t))
 		printf("[%u bytes of unknown target data] ",
 		       (unsigned int)(t->u.target_size - sizeof(*t)));
-- 
1.7.10.4

Re: Assorted fixes v3

[Pablo Neira Ayuso]

On Wed, Feb 06, 2013 at 01:47:01AM +0100, Jan Engelhardt wrote:
> Removed
>   build: also use libtool for install stage
>   iptables: reword warning on using an alias
> Added:
>   build: bump SONAME for libxtables
> 
> ===
> The following changes since commit 817ac5a5e54d083983b7c834194b46c4366d71d2:
> 
>   Merge branch 'master' of vishnu.netfilter.org:/data/git/iptables (2013-01-31 20:36:27 +0100)
> 
> are available in the git repository at:
> 
> 
>   git://git.inai.de/iptables master
> 
> for you to fetch changes up to 80722f5b0c2723d34782affec7dc247352d9be33:
> 
>   iptables: fall back to using save function when print is not defined (2013-02-06 01:40:43 +0100)
> 
> ----------------------------------------------------------------
> Jan Engelhardt (4):
>       build: bump SONAME for libxtables

Applied this one, thanks a lot for catching up this.

>       libxtables: centralize checking for a .save function
>       extensions: eui64: set userspacesize=0
>       iptables: fall back to using save function when print is not defined

Not applying these, they are not fixes, sorry.

Re: Assorted fixes v3

[Jan Engelhardt]

On Thursday 2013-02-07 19:53, Pablo Neira Ayuso wrote:
>> ----------------------------------------------------------------
>> Jan Engelhardt (4):
>>       build: bump SONAME for libxtables
>
>Applied this one, thanks a lot for catching up this.
>
>>       libxtables: centralize checking for a .save function
>>       extensions: eui64: set userspacesize=0
>>       iptables: fall back to using save function when print is not defined
>
>Not applying these, they are not fixes, sorry.

If you say so. (But it would have been nice to get told that the first time.)

Now, I can't put these onto nf/next without someone having to go through
a merge conflict later on, so besides getting put on hold again,
what is the preferable action?