iptables 1.3.7 doesn't properly test for condition patch

iptables 1.3.7 doesn't properly test for condition patch

[Andrew Schulman]

I have kernel 2.6.21, iptables 1.3.7, and pom-ng 20070527.  I run

./runme --download --batch condition

and the patch applies successfully.  But when I build iptables, the
condition extension isn't included.  This is because
iptables-1.3.7/extensions/.condition-test tests for the existence of

$KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h

But the condition patch doesn't create that file any more; it now creates

$KERNEL_DIR/include/linux/netfilter/xt_condition.h

I guess that .condition-test needs to check for the kernel version that
it's being built against before deciding which header file it wants to look
for.

Thanks,
Andrew.

Re: iptables 1.3.7 doesn't properly test for condition patch

[Andrew Schulman]

> I have kernel 2.6.21, iptables 1.3.7, and pom-ng 20070527.  I run
> 
> ./runme --download --batch condition
> 
> and the patch applies successfully.  But when I build iptables, the
> condition extension isn't included.  This is because
> iptables-1.3.7/extensions/.condition-test tests for the existence of
> 
> $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h
> 
> But the condition patch doesn't create that file any more; it now creates
> 
> $KERNEL_DIR/include/linux/netfilter/xt_condition.h

For the archive, Massimilano Hofer sent me the attached patch, which solves
the problem.  It seems that this patch should be merged into iptables.

Andrew.

diff -Nru iptables-1.3.5-20060922.orig/extensions/.condition-test iptables-1.3.5-20060922.new/extensions/.condition-test
--- iptables-1.3.5-20060922.orig/extensions/.condition-test	2002-11-02 16:00:15.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/.condition-test	2006-09-26 12:56:01.000000000 +0200
@@ -1,3 +1,5 @@
 #!/bin/sh
 # True if condition is applied.
-[ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h ] && echo condition
+( [ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h ] ||
+  [ -f $KERNEL_DIR/include/linux/netfilter/xt_condition.h ] ) &&
+ echo condition
diff -Nru iptables-1.3.5-20060922.orig/extensions/.condition-test6 iptables-1.3.5-20060922.new/extensions/.condition-test6
--- iptables-1.3.5-20060922.orig/extensions/.condition-test6	2003-02-25 12:54:56.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/.condition-test6	2006-09-26 12:55:23.000000000 +0200
@@ -1,3 +1,5 @@
 #!/bin/sh
 # True if condition6 is applied.
-[ -f $KERNEL_DIR/include/linux/netfilter_ipv6/ip6t_condition.h ] && echo condition
+( [ -f $KERNEL_DIR/include/linux/netfilter_ipv6/ip6t_condition.h ] ||
+  [ -f $KERNEL_DIR/include/linux/netfilter/xt_condition.h ] ) &&
+ echo condition
diff -Nru iptables-1.3.5-20060922.orig/extensions/libip6t_condition.c iptables-1.3.5-20060922.new/extensions/libip6t_condition.c
--- iptables-1.3.5-20060922.orig/extensions/libip6t_condition.c	2005-02-14 14:13:04.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libip6t_condition.c	2006-09-26 13:04:09.000000000 +0200
@@ -6,7 +6,14 @@
 #include <ip6tables.h>
 
 #include<linux/netfilter_ipv6/ip6_tables.h>
+
+#ifndef _X_TABLES_H
 #include<linux/netfilter_ipv6/ip6t_condition.h>
+#define condition_info condition6_info
+#define CONDITION_NAME_LEN CONDITION6_NAME_LEN
+#else
+#include<linux/netfilter/xt_condition.h>
+#endif
 
 
 static void
@@ -29,8 +36,12 @@
       const struct ip6t_entry *entry, unsigned int *nfcache,
       struct ip6t_entry_match **match)
 {
-	struct condition6_info *info =
-	    (struct condition6_info *) (*match)->data;
+	static const char * const forbidden_names[]={ "", ".", ".." };
+	const char *name;
+	int i;
+
+	struct condition_info *info =
+	    (struct condition_info *) (*match)->data;
 
 	if (c == 'X') {
 		if (*flags)
@@ -39,12 +50,26 @@
 
 		check_inverse(optarg, &invert, &optind, 0);
 
-		if (strlen(argv[optind - 1]) < CONDITION6_NAME_LEN)
-			strcpy(info->name, argv[optind - 1]);
-		else
+		name = argv[optind - 1];
+		/* We don't want a '/' in a proc file name. */
+		for (i=0; i < CONDITION_NAME_LEN && name[i] != '\0'; i++)
+			if (name[i] == '/')
+				exit_error(PARAMETER_PROBLEM,
+					   "Can't have a '/' in a condition name");
+
+		/* We can't handle file names longer than CONDITION_NAME_LEN and */
+		/* we want a NULL terminated string. */
+		if (i == CONDITION_NAME_LEN)
 			exit_error(PARAMETER_PROBLEM,
 				   "File name too long");
 
+		/* We don't want certain reserved names. */
+		for (i=0; i < sizeof(forbidden_names)/sizeof(char *); i++)
+			if(strcmp(name, forbidden_names[i])==0)
+				exit_error(PARAMETER_PROBLEM,
+					   "Forbidden condition name");
+
+		strcpy(info->name, name);
 		info->invert = invert;
 		*flags = 1;
 		return 1;
@@ -67,8 +92,8 @@
 print(const struct ip6t_ip6 *ip,
 		  const struct ip6t_entry_match *match, int numeric)
 {
-	const struct condition6_info *info =
-	    (const struct condition6_info *) match->data;
+	const struct condition_info *info =
+	    (const struct condition_info *) match->data;
 
 	printf("condition %s%s ", (info->invert) ? "!" : "", info->name);
 }
@@ -78,8 +103,8 @@
 save(const struct ip6t_ip6 *ip,
 		 const struct ip6t_entry_match *match)
 {
-	const struct condition6_info *info =
-	    (const struct condition6_info *) match->data;
+	const struct condition_info *info =
+	    (const struct condition_info *) match->data;
 
 	printf("--condition %s\"%s\" ", (info->invert) ? "! " : "", info->name);
 }
@@ -88,8 +113,8 @@
 static struct ip6tables_match condition = {
 	.name = "condition",
 	.version = IPTABLES_VERSION,
-	.size = IP6T_ALIGN(sizeof(struct condition6_info)),
-	.userspacesize = IP6T_ALIGN(sizeof(struct condition6_info)),
+	.size = IP6T_ALIGN(sizeof(struct condition_info)),
+	.userspacesize = IP6T_ALIGN(sizeof(struct condition_info)),
 	.help = &help,
 	.parse = &parse,
 	.final_check = &final_check,
diff -Nru iptables-1.3.5-20060922.orig/extensions/libip6t_condition.man iptables-1.3.5-20060922.new/extensions/libip6t_condition.man
--- iptables-1.3.5-20060922.orig/extensions/libip6t_condition.man	2006-01-30 09:50:09.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libip6t_condition.man	2006-09-26 09:31:40.000000000 +0200
@@ -1,4 +1,4 @@
 This matches if a specific /proc filename is '0' or '1'.
 .TP
 .BR "--condition " "[!] \fIfilename"
-Match on boolean value stored in /proc/net/ip6t_condition/filename file
+Match on boolean value stored in /proc/net/nf_condition/filename file
diff -Nru iptables-1.3.5-20060922.orig/extensions/libipt_condition.c iptables-1.3.5-20060922.new/extensions/libipt_condition.c
--- iptables-1.3.5-20060922.orig/extensions/libipt_condition.c	2005-02-14 14:13:04.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libipt_condition.c	2006-09-26 12:01:57.000000000 +0200
@@ -6,7 +6,12 @@
 #include <iptables.h>
 
 #include<linux/netfilter_ipv4/ip_tables.h>
+
+#ifndef _X_TABLES_H
 #include<linux/netfilter_ipv4/ipt_condition.h>
+#else
+#include<linux/netfilter/xt_condition.h>
+#endif
 
 
 static void
@@ -29,6 +34,10 @@
       const struct ipt_entry *entry, unsigned int *nfcache,
       struct ipt_entry_match **match)
 {
+	static const char * const forbidden_names[]={ "", ".", ".." };
+	const char *name;
+	int i;
+
 	struct condition_info *info =
 	    (struct condition_info *) (*match)->data;
 
@@ -39,12 +48,26 @@
 
 		check_inverse(optarg, &invert, &optind, 0);
 
-		if (strlen(argv[optind - 1]) < CONDITION_NAME_LEN)
-			strcpy(info->name, argv[optind - 1]);
-		else
+		name = argv[optind - 1];
+		/* We don't want a '/' in a proc file name. */
+		for (i=0; i < CONDITION_NAME_LEN && name[i] != '\0'; i++)
+			if (name[i] == '/')
+				exit_error(PARAMETER_PROBLEM,
+					   "Can't have a '/' in a condition name");
+
+		/* We can't handle file names longer than CONDITION_NAME_LEN and */
+		/* we want a NULL terminated string. */
+		if (i == CONDITION_NAME_LEN)
 			exit_error(PARAMETER_PROBLEM,
 				   "File name too long");
 
+		/* We don't want certain reserved names. */
+		for (i=0; i < sizeof(forbidden_names)/sizeof(char *); i++)
+			if(strcmp(name, forbidden_names[i])==0)
+				exit_error(PARAMETER_PROBLEM,
+					   "Forbidden condition name");
+
+		strcpy(info->name, name);
 		info->invert = invert;
 		*flags = 1;
 		return 1;
diff -Nru iptables-1.3.5-20060922.orig/extensions/libipt_condition.man iptables-1.3.5-20060922.new/extensions/libipt_condition.man
--- iptables-1.3.5-20060922.orig/extensions/libipt_condition.man	2006-01-30 09:50:09.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libipt_condition.man	2006-09-26 09:31:42.000000000 +0200
@@ -1,4 +1,4 @@
 This matches if a specific /proc filename is '0' or '1'.
 .TP
 .BI "--condition " "[!] \fIfilename\fP"
-Match on boolean value stored in /proc/net/ipt_condition/filename file
+Match on boolean value stored in /proc/net/nf_condition/filename file

Re: iptables 1.3.7 doesn't properly test for condition patch

[Jan Engelhardt]

cc nf-dev

On May 29 2007 06:34, Andrew Schulman wrote:
>
>For the archive, Massimilano Hofer sent me the attached patch, which solves
>the problem.  It seems that this patch should be merged into iptables.
>
>Andrew.
>
>diff -Nru iptables-1.3.5-20060922.orig/extensions/.condition-test iptables-1.3.5-20060922.new/extensions/.condition-test
>--- iptables-1.3.5-20060922.orig/extensions/.condition-test	2002-11-02 16:00:15.000000000 +0100
>+++ iptables-1.3.5-20060922.new/extensions/.condition-test	2006-09-26 12:56:01.000000000 +0200
>@@ -1,3 +1,5 @@
> #!/bin/sh
> # True if condition is applied.
>-[ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h ] && echo condition
>+( [ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h ] ||
>+  [ -f $KERNEL_DIR/include/linux/netfilter/xt_condition.h ] ) &&
>+ echo condition

While it is valid, why not use one '[ ]' test instead of two?

[ -f "$KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h" -o \
  -f "$KERNEL_DIR/include/linux/netfilter/xt_condition.h" ] && \
	echo condition;

Of course, the common prefix can be merged, i.e.:

S="$KERNEL_DIR/include/linux";
[ -f "$S/netfilter_ipv4/ipt_condition.h" -o "$S/netfilter/xt_condition.h" ] ...


	Jan
-- 

Re: iptables 1.3.7 doesn't properly test for condition patch

[Pablo Neira Ayuso]

Jan Engelhardt wrote:
> cc nf-dev
> 
> On May 29 2007 06:34, Andrew Schulman wrote:
>> For the archive, Massimilano Hofer sent me the attached patch, which solves
>> the problem.  It seems that this patch should be merged into iptables.

I don't find this patch in the archives, did it get lost or never post
it to netfilter-devel? Massimiliano?

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: iptables 1.3.7 doesn't properly test for condition patch

[Massimiliano Hofer]

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

On Wednesday 30 May 2007, Pablo Neira Ayuso wrote:

> Jan Engelhardt wrote:
> > cc nf-dev
> >
> > On May 29 2007 06:34, Andrew Schulman wrote:
> >> For the archive, Massimilano Hofer sent me the attached patch, which
> >> solves the problem.  It seems that this patch should be merged into
> >> iptables.
>
> I don't find this patch in the archives, did it get lost or never post
> it to netfilter-devel? Massimiliano?

Sorry for the dalay. I couldn't read the mailing list in the last several 
weeks.
I sent my patch several months ago, but received no reply. I suppose it got 
lost in the noise.
I attach it again. Just a few fixes and a little tidying.

I have no objections to Jan's variants. Choose the one you like better.

-- 
Saluti,
   Massimiliano Hofer

[-- Attachment #2: iptables-xt_condition.patch --]
[-- Type: text/x-diff, Size: 7374 bytes --]

diff -Nru iptables-1.3.5-20060922.orig/extensions/.condition-test iptables-1.3.5-20060922.new/extensions/.condition-test
--- iptables-1.3.5-20060922.orig/extensions/.condition-test	2002-11-02 16:00:15.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/.condition-test	2006-09-26 12:56:01.000000000 +0200
@@ -1,3 +1,5 @@
 #!/bin/sh
 # True if condition is applied.
-[ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h ] && echo condition
+( [ -f $KERNEL_DIR/include/linux/netfilter_ipv4/ipt_condition.h ] ||
+  [ -f $KERNEL_DIR/include/linux/netfilter/xt_condition.h ] ) &&
+ echo condition
diff -Nru iptables-1.3.5-20060922.orig/extensions/.condition-test6 iptables-1.3.5-20060922.new/extensions/.condition-test6
--- iptables-1.3.5-20060922.orig/extensions/.condition-test6	2003-02-25 12:54:56.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/.condition-test6	2006-09-26 12:55:23.000000000 +0200
@@ -1,3 +1,5 @@
 #!/bin/sh
 # True if condition6 is applied.
-[ -f $KERNEL_DIR/include/linux/netfilter_ipv6/ip6t_condition.h ] && echo condition
+( [ -f $KERNEL_DIR/include/linux/netfilter_ipv6/ip6t_condition.h ] ||
+  [ -f $KERNEL_DIR/include/linux/netfilter/xt_condition.h ] ) &&
+ echo condition
diff -Nru iptables-1.3.5-20060922.orig/extensions/libip6t_condition.c iptables-1.3.5-20060922.new/extensions/libip6t_condition.c
--- iptables-1.3.5-20060922.orig/extensions/libip6t_condition.c	2005-02-14 14:13:04.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libip6t_condition.c	2006-09-26 13:04:09.000000000 +0200
@@ -6,7 +6,14 @@
 #include <ip6tables.h>
 
 #include<linux/netfilter_ipv6/ip6_tables.h>
+
+#ifndef _X_TABLES_H
 #include<linux/netfilter_ipv6/ip6t_condition.h>
+#define condition_info condition6_info
+#define CONDITION_NAME_LEN CONDITION6_NAME_LEN
+#else
+#include<linux/netfilter/xt_condition.h>
+#endif
 
 
 static void
@@ -29,8 +36,12 @@
       const struct ip6t_entry *entry, unsigned int *nfcache,
       struct ip6t_entry_match **match)
 {
-	struct condition6_info *info =
-	    (struct condition6_info *) (*match)->data;
+	static const char * const forbidden_names[]={ "", ".", ".." };
+	const char *name;
+	int i;
+
+	struct condition_info *info =
+	    (struct condition_info *) (*match)->data;
 
 	if (c == 'X') {
 		if (*flags)
@@ -39,12 +50,26 @@
 
 		check_inverse(optarg, &invert, &optind, 0);
 
-		if (strlen(argv[optind - 1]) < CONDITION6_NAME_LEN)
-			strcpy(info->name, argv[optind - 1]);
-		else
+		name = argv[optind - 1];
+		/* We don't want a '/' in a proc file name. */
+		for (i=0; i < CONDITION_NAME_LEN && name[i] != '\0'; i++)
+			if (name[i] == '/')
+				exit_error(PARAMETER_PROBLEM,
+					   "Can't have a '/' in a condition name");
+
+		/* We can't handle file names longer than CONDITION_NAME_LEN and */
+		/* we want a NULL terminated string. */
+		if (i == CONDITION_NAME_LEN)
 			exit_error(PARAMETER_PROBLEM,
 				   "File name too long");
 
+		/* We don't want certain reserved names. */
+		for (i=0; i < sizeof(forbidden_names)/sizeof(char *); i++)
+			if(strcmp(name, forbidden_names[i])==0)
+				exit_error(PARAMETER_PROBLEM,
+					   "Forbidden condition name");
+
+		strcpy(info->name, name);
 		info->invert = invert;
 		*flags = 1;
 		return 1;
@@ -67,8 +92,8 @@
 print(const struct ip6t_ip6 *ip,
 		  const struct ip6t_entry_match *match, int numeric)
 {
-	const struct condition6_info *info =
-	    (const struct condition6_info *) match->data;
+	const struct condition_info *info =
+	    (const struct condition_info *) match->data;
 
 	printf("condition %s%s ", (info->invert) ? "!" : "", info->name);
 }
@@ -78,8 +103,8 @@
 save(const struct ip6t_ip6 *ip,
 		 const struct ip6t_entry_match *match)
 {
-	const struct condition6_info *info =
-	    (const struct condition6_info *) match->data;
+	const struct condition_info *info =
+	    (const struct condition_info *) match->data;
 
 	printf("--condition %s\"%s\" ", (info->invert) ? "! " : "", info->name);
 }
@@ -88,8 +113,8 @@
 static struct ip6tables_match condition = {
 	.name = "condition",
 	.version = IPTABLES_VERSION,
-	.size = IP6T_ALIGN(sizeof(struct condition6_info)),
-	.userspacesize = IP6T_ALIGN(sizeof(struct condition6_info)),
+	.size = IP6T_ALIGN(sizeof(struct condition_info)),
+	.userspacesize = IP6T_ALIGN(sizeof(struct condition_info)),
 	.help = &help,
 	.parse = &parse,
 	.final_check = &final_check,
diff -Nru iptables-1.3.5-20060922.orig/extensions/libip6t_condition.man iptables-1.3.5-20060922.new/extensions/libip6t_condition.man
--- iptables-1.3.5-20060922.orig/extensions/libip6t_condition.man	2006-01-30 09:50:09.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libip6t_condition.man	2006-09-26 09:31:40.000000000 +0200
@@ -1,4 +1,4 @@
 This matches if a specific /proc filename is '0' or '1'.
 .TP
 .BR "--condition " "[!] \fIfilename"
-Match on boolean value stored in /proc/net/ip6t_condition/filename file
+Match on boolean value stored in /proc/net/nf_condition/filename file
diff -Nru iptables-1.3.5-20060922.orig/extensions/libipt_condition.c iptables-1.3.5-20060922.new/extensions/libipt_condition.c
--- iptables-1.3.5-20060922.orig/extensions/libipt_condition.c	2005-02-14 14:13:04.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libipt_condition.c	2006-09-26 12:01:57.000000000 +0200
@@ -6,7 +6,12 @@
 #include <iptables.h>
 
 #include<linux/netfilter_ipv4/ip_tables.h>
+
+#ifndef _X_TABLES_H
 #include<linux/netfilter_ipv4/ipt_condition.h>
+#else
+#include<linux/netfilter/xt_condition.h>
+#endif
 
 
 static void
@@ -29,6 +34,10 @@
       const struct ipt_entry *entry, unsigned int *nfcache,
       struct ipt_entry_match **match)
 {
+	static const char * const forbidden_names[]={ "", ".", ".." };
+	const char *name;
+	int i;
+
 	struct condition_info *info =
 	    (struct condition_info *) (*match)->data;
 
@@ -39,12 +48,26 @@
 
 		check_inverse(optarg, &invert, &optind, 0);
 
-		if (strlen(argv[optind - 1]) < CONDITION_NAME_LEN)
-			strcpy(info->name, argv[optind - 1]);
-		else
+		name = argv[optind - 1];
+		/* We don't want a '/' in a proc file name. */
+		for (i=0; i < CONDITION_NAME_LEN && name[i] != '\0'; i++)
+			if (name[i] == '/')
+				exit_error(PARAMETER_PROBLEM,
+					   "Can't have a '/' in a condition name");
+
+		/* We can't handle file names longer than CONDITION_NAME_LEN and */
+		/* we want a NULL terminated string. */
+		if (i == CONDITION_NAME_LEN)
 			exit_error(PARAMETER_PROBLEM,
 				   "File name too long");
 
+		/* We don't want certain reserved names. */
+		for (i=0; i < sizeof(forbidden_names)/sizeof(char *); i++)
+			if(strcmp(name, forbidden_names[i])==0)
+				exit_error(PARAMETER_PROBLEM,
+					   "Forbidden condition name");
+
+		strcpy(info->name, name);
 		info->invert = invert;
 		*flags = 1;
 		return 1;
diff -Nru iptables-1.3.5-20060922.orig/extensions/libipt_condition.man iptables-1.3.5-20060922.new/extensions/libipt_condition.man
--- iptables-1.3.5-20060922.orig/extensions/libipt_condition.man	2006-01-30 09:50:09.000000000 +0100
+++ iptables-1.3.5-20060922.new/extensions/libipt_condition.man	2006-09-26 09:31:42.000000000 +0200
@@ -1,4 +1,4 @@
 This matches if a specific /proc filename is '0' or '1'.
 .TP
 .BI "--condition " "[!] \fIfilename\fP"
-Match on boolean value stored in /proc/net/ipt_condition/filename file
+Match on boolean value stored in /proc/net/nf_condition/filename file