[PATCH 1/2] Versioning (aka release) stuff for iptables

[PATCH 1/2] Versioning (aka release) stuff for iptables

[Pablo Neira]

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

Hi Rusty,

I've been working on the versioning stuff last days. I've tested with 
the mark target.

As I told you, I propose to add an option called --release to the 
current syntax of iptables which works as follows:

a) New version of matches/targets: iptables -I INPUT -t mangle -j MARK 
--release 1 --and-mark 0x1

b) Primitive version. To keep backward compatibility, the syntax is the 
same, no modification: iptables -I INPUT -t mangle -j MARK --set-mark 0x1

Optionally, someone could apply this: iptables -I INPUT -t mangle -j 
MARK --release 0 --set-mark 0x1

To finish, some comments about what I have in mind for next days:

1) Test this stuff in nfsim with a test case based on yours.
2) Clean up the kernel patch that I sent you some weeks ago and rename 
field `version' to `release'
3) port mport match to multiport to test that versioning stuff is 
working fine with matches.
4) More testing...

Please, comments welcome.

--
Pablo

[-- Attachment #2: xmas-version.patch --]
[-- Type: text/x-patch, Size: 12751 bytes --]

Index: iptables.c
===================================================================
--- iptables.c	(revision 3357)
+++ iptables.c	(working copy)
@@ -138,6 +138,7 @@
 	{ "line-numbers", 0, 0, '0' },
 	{ "modprobe", 1, 0, 'M' },
 	{ "set-counters", 1, 0, 'c' },
+	{ "release", 1, 0, 'r' },
 	{ 0 }
 };
 
@@ -664,12 +665,12 @@
 }
 
 struct iptables_match *
-find_match(const char *name, enum ipt_tryload tryload, struct iptables_rule_match **matches)
+find_match(const char *name, u_int8_t rel, enum ipt_tryload tryload, struct iptables_rule_match **matches)
 {
 	struct iptables_match *ptr;
 
 	for (ptr = iptables_matches; ptr; ptr = ptr->next) {
-		if (strcmp(name, ptr->name) == 0)
+		if (strcmp(name, ptr->name) == 0 && ptr->release == rel) 
 			break;
 	}
 
@@ -681,16 +682,16 @@
 		if (dlopen(path, RTLD_NOW)) {
 			/* Found library.  If it didn't register itself,
 			   maybe they specified target as match. */
-			ptr = find_match(name, DONT_LOAD, NULL);
+			ptr = find_match(name, rel, DONT_LOAD, NULL);
 
 			if (!ptr)
 				exit_error(PARAMETER_PROBLEM,
-					   "Couldn't load match `%s'\n",
-					   name);
+					   "Couldn't load match `%s' (r.%d)\n",
+					   name, rel);
 		} else if (tryload == LOAD_MUST_SUCCEED)
 			exit_error(PARAMETER_PROBLEM,
-				   "Couldn't load match `%s':%s\n",
-				   name, dlerror());
+				   "Couldn't load match `%s' (r.%d):%s\n",
+				   name, rel, dlerror());
 	}
 #else
 	if (ptr && !ptr->loaded) {
@@ -701,7 +702,7 @@
 	}
 	if(!ptr && (tryload == LOAD_MUST_SUCCEED)) {
 		exit_error(PARAMETER_PROBLEM,
-			   "Couldn't find match `%s'\n", name);
+			   "Couldn't find match `%s' (r.%d)\n", name, rel);
 	}
 #endif
 
@@ -730,9 +731,10 @@
 		char *protoname = proto_to_name(proto, nolookup);
 
 		if (protoname)
-			return find_match(protoname, tryload, matches);
+			/* standalone matches doesn't support versioning */
+			return find_match(protoname, 0, tryload, matches);
 	} else
-		return find_match(pname, tryload, matches);
+		return find_match(pname, 0, tryload, matches);
 
 	return NULL;
 }
@@ -966,7 +968,7 @@
 }
 
 struct iptables_target *
-find_target(const char *name, enum ipt_tryload tryload)
+find_target(const char *name, u_int8_t rel, enum ipt_tryload tryload)
 {
 	struct iptables_target *ptr;
 
@@ -979,7 +981,7 @@
 		name = "standard";
 
 	for (ptr = iptables_targets; ptr; ptr = ptr->next) {
-		if (strcmp(name, ptr->name) == 0)
+		if (strcmp(name, ptr->name) == 0 && ptr->release == rel)
 			break;
 	}
 
@@ -991,15 +993,15 @@
 		if (dlopen(path, RTLD_NOW)) {
 			/* Found library.  If it didn't register itself,
 			   maybe they specified match as a target. */
-			ptr = find_target(name, DONT_LOAD);
+			ptr = find_target(name, rel, DONT_LOAD);
 			if (!ptr)
 				exit_error(PARAMETER_PROBLEM,
-					   "Couldn't load target `%s'\n",
-					   name);
+					   "Couldn't load target `%s' (r.%d)\n",
+					   name, rel);
 		} else if (tryload == LOAD_MUST_SUCCEED)
 			exit_error(PARAMETER_PROBLEM,
-				   "Couldn't load target `%s':%s\n",
-				   name, dlerror());
+				   "Couldn't load target `%s' (r.%d):%s\n",
+				   name, rel, dlerror());
 	}
 #else
 	if (ptr && !ptr->loaded) {
@@ -1010,7 +1012,7 @@
 	}
 	if(!ptr && (tryload == LOAD_MUST_SUCCEED)) {
 		exit_error(PARAMETER_PROBLEM,
-			   "Couldn't find target `%s'\n", name);
+			   "Couldn't find target `%s' (r.%d)\n", name, rel);
 	}
 #endif
 
@@ -1055,7 +1057,7 @@
 		exit(1);
 	}
 
-	if (find_match(me->name, DONT_LOAD, NULL)) {
+	if (find_match(me->name, me->release, DONT_LOAD, NULL)) {
 		fprintf(stderr, "%s: match `%s' already registered.\n",
 			program_name, me->name);
 		exit(1);
@@ -1085,7 +1087,7 @@
 		exit(1);
 	}
 
-	if (find_target(me->name, DONT_LOAD)) {
+	if (find_target(me->name, me->release, DONT_LOAD)) {
 		fprintf(stderr, "%s: target `%s' already registered.\n",
 			program_name, me->name);
 		exit(1);
@@ -1185,7 +1187,7 @@
 	    const struct ipt_ip *ip,
 	    int numeric)
 {
-	struct iptables_match *match = find_match(m->u.user.name, TRY_LOAD, NULL);
+	struct iptables_match *match = find_match(m->u.user.name, m->u.user.version, TRY_LOAD, NULL);
 
 	if (match) {
 		if (match->print)
@@ -1213,12 +1215,12 @@
 	u_int8_t flags;
 	char buf[BUFSIZ];
 
+	t = ipt_get_target((struct ipt_entry *)fw);
 	if (!iptc_is_chain(targname, handle))
-		target = find_target(targname, TRY_LOAD);
+		target = find_target(targname, t->u.user.version, TRY_LOAD);
 	else
-		target = find_target(IPT_STANDARD_TARGET, LOAD_MUST_SUCCEED);
+		target = find_target(IPT_STANDARD_TARGET, t->u.user.version, LOAD_MUST_SUCCEED);
 
-	t = ipt_get_target((struct ipt_entry *)fw);
 	flags = fw->ip.flags;
 
 	if (format & FMT_LINENUMBERS)
@@ -1715,6 +1717,9 @@
 	char *protocol = NULL;
 	const char *modprobe = NULL;
 	int proto_used = 0;
+	int release = 0, match_option = 0, target_option = 0;
+	char *match_name = NULL, *target_name = NULL;
+	static struct option *old_opts;
 
 	memset(&fw, 0, sizeof(fw));
 
@@ -1737,7 +1742,7 @@
 	opterr = 0;
 
 	while ((c = getopt_long(argc, argv,
-	   "-A:D:R:I:L::M:F::Z::N:X::E:P:Vh::o:p:s:d:j:i:fbvnt:m:xc:",
+	   "-A:D:R:I:L::M:F::Z::N:X::E:P:Vh::o:p:s:d:j:i:fbvnt:m:r:xc:",
 					   opts, NULL)) != -1) {
 		switch (c) {
 			/*
@@ -1815,7 +1820,7 @@
 				exit_error(PARAMETER_PROBLEM,
 					   "chain name not allowed to start "
 					   "with `-'\n");
-			if (find_target(optarg, TRY_LOAD))
+			if (find_target(optarg, 0, TRY_LOAD))
 				exit_error(PARAMETER_PROBLEM,
 					   "chain name may not clash "
 					   "with target name\n");
@@ -1866,7 +1871,7 @@
 
 			/* iptables -p icmp -h */
 			if (!matches && protocol)
-				find_match(protocol, TRY_LOAD, &matches);
+				find_match(protocol, 0, TRY_LOAD, &matches);
 
 			exit_printhelp(matches);
 
@@ -1913,19 +1918,14 @@
 				   invert);
 			jumpto = parse_target(optarg);
 			/* TRY_LOAD (may be chain name) */
-			target = find_target(jumpto, TRY_LOAD);
+			target = find_target(jumpto, 0, TRY_LOAD);
 
 			if (target) {
-				size_t size;
-
-				size = IPT_ALIGN(sizeof(struct ipt_entry_target))
-					+ target->size;
-
-				target->t = fw_calloc(1, size);
-				target->t->u.target_size = size;
-				strcpy(target->t->u.user.name, jumpto);
-				target->init(target->t, &fw.nfcache);
+				old_opts = opts;
 				opts = merge_options(opts, target->extra_opts, &target->option_offset);
+				target_name = jumpto;
+				target_option = 1;
+				release = 0;
 			}
 			break;
 
@@ -1964,24 +1964,76 @@
 			verbose++;
 			break;
 
-		case 'm': {
-			size_t size;
-
+		case 'm':
 			if (invert)
 				exit_error(PARAMETER_PROBLEM,
 					   "unexpected ! flag before --match");
 
-			m = find_match(optarg, LOAD_MUST_SUCCEED, &matches);
-			size = IPT_ALIGN(sizeof(struct ipt_entry_match))
-					 + m->size;
-			m->m = fw_calloc(1, size);
-			m->m->u.match_size = size;
-			strcpy(m->m->u.user.name, m->name);
-			m->init(m->m, &fw.nfcache);
-			opts = merge_options(opts, m->extra_opts, &m->option_offset);
-		}
+			/* Backward compatibility: load release 0 */
+			m = find_match(optarg, release, LOAD_MUST_SUCCEED, NULL);
+
+			/* Merge option of release 0, but keep a 
+			 * pointer to the old structure */
+			old_opts = opts;
+			opts = merge_options(opts, m->extra_opts, 
+					     &m->option_offset);
+			match_name = optarg;
+			match_option = 1;
+			release = 0;
 		break;
 
+		case 'r':
+			release = atoi(optarg);
+
+			if (release < 0 || release > 256)
+				exit_error(PARAMETER_PROBLEM,
+					   "release must be >= 0 and =< 255");
+
+			if (match_option) {
+				size_t s;
+
+				/* restore old options */
+				free(opts);
+				opts = old_opts;
+
+				/* load a specific release of a match */
+				m = find_match(match_name, release, 
+					       LOAD_MUST_SUCCEED, &matches);
+				s = IPT_ALIGN(sizeof(struct ipt_entry_match))
+					      + m->size;
+				m->m = fw_calloc(1, s);
+				m->m->u.match_size = s;
+				strcpy(m->m->u.user.name, m->name);
+				m->m->u.user.version = m->release;
+				m->init(m->m, &fw.nfcache);
+				opts = merge_options(opts, m->extra_opts, 
+						     &m->option_offset);
+				/* reset flag */
+				match_option = 0;
+			} else if (target_option) {
+				size_t s;
+
+				/* restore old options */
+				free(opts);
+				opts = old_opts;
+
+				/* load a specific release of a target */
+				target = find_target(target_name, release, TRY_LOAD);
+
+				s = IPT_ALIGN(sizeof(struct ipt_entry_target))
+					      + target->size;
+
+				target->t = fw_calloc(1, s);
+				target->t->u.target_size = s;
+				strcpy(target->t->u.user.name, target_name);
+				target->t->u.user.version = target->release;
+				target->init(target->t, &fw.nfcache);
+				opts = merge_options(opts, target->extra_opts, &target->option_offset);
+				/* reset flag */
+				target_option = 0;
+			}
+			break;
+			
 		case 'n':
 			set_option(&options, OPT_NUMERIC, &fw.ip.invflags,
 				   invert);
@@ -2056,6 +2108,41 @@
 			exit_tryhelp(2);
 
 		default:
+			if (match_option) {
+				/* No release specified, so use release 0 */
+				size_t s;
+
+				m = find_match(match_name, release, 
+					       LOAD_MUST_SUCCEED, &matches);
+				s = IPT_ALIGN(sizeof(struct ipt_entry_match))
+					      + m->size;
+				m->m = fw_calloc(1, s);
+				m->m->u.match_size = s;
+				strcpy(m->m->u.user.name, m->name);
+				m->m->u.user.version = 0;
+				m->init(m->m, &fw.nfcache);
+				
+				/* reset flag */
+				match_option = 0;
+			} else if (target_option) {
+				/* targets with parameters like SNAT need
+				 * this stuff */
+				size_t s;
+	
+				s = IPT_ALIGN(sizeof(struct ipt_entry_target))
+					      + target->size;
+
+				target->t = fw_calloc(1, s);
+				target->t->u.target_size = s;
+				strcpy(target->t->u.user.name, target_name);
+				target->t->u.user.version = 0;
+				target->init(target->t, &fw.nfcache);
+
+				/* reset flag */
+				target_option = 0;
+			}
+ 
+
 			/* FIXME: This scheme doesn't allow two of the same
 			   matches --RR */
 			if (!target
@@ -2135,6 +2222,23 @@
 		invert = FALSE;
 	}
 
+	if (target_option) {
+		/* This target has no parameters */
+		size_t s;
+				
+		s = IPT_ALIGN(sizeof(struct ipt_entry_target))
+ 			      + target->size;
+
+		target->t = fw_calloc(1, s);
+		target->t->u.target_size = s;
+		strcpy(target->t->u.user.name, target_name);
+		target->t->u.user.version = 0;
+		target->init(target->t, &fw.nfcache);
+
+		/* reset flag */
+		target_option = 0;
+	}
+
 	for (matchp = matches; matchp; matchp = matchp->next)
 		matchp->match->final_check(matchp->match->mflags);
 
@@ -2239,7 +2343,9 @@
 			|| iptc_is_chain(jumpto, *handle))) {
 			size_t size;
 
+			/* standard doesn't support versioning */
 			target = find_target(IPT_STANDARD_TARGET,
+					     0,
 					     LOAD_MUST_SUCCEED);
 
 			size = sizeof(struct ipt_entry_target)
@@ -2255,7 +2361,7 @@
 			 * We cannot know if the plugin is corrupt, non
 			 * existant OR if the user just misspelled a
 			 * chain. */
-			find_target(jumpto, LOAD_MUST_SUCCEED);
+			find_target(jumpto, 0, LOAD_MUST_SUCCEED);
 		} else {
 			e = generate_entry(&fw, matches, target->t);
 			free(target->t);
Index: include/iptables.h
===================================================================
--- include/iptables.h	(revision 3309)
+++ include/iptables.h	(working copy)
@@ -22,8 +22,12 @@
 
 	ipt_chainlabel name;
 
+	/* compatible with a certain iptables version */
 	const char *version;
 
+	/* release number of this match */
+	u_int8_t release;
+
 	/* Size of match data. */
 	size_t size;
 
@@ -74,6 +78,8 @@
 
 	const char *version;
 
+	u_int8_t release;
+
 	/* Size of target data. */
 	size_t size;
 
@@ -143,8 +149,8 @@
 	LOAD_MUST_SUCCEED
 };
 
-extern struct iptables_target *find_target(const char *name, enum ipt_tryload);
-extern struct iptables_match *find_match(const char *name, enum ipt_tryload, struct iptables_rule_match **match);
+extern struct iptables_target *find_target(const char *name, const u_int8_t version, enum ipt_tryload);
+extern struct iptables_match *find_match(const char *name, const u_int8_t version, enum ipt_tryload, struct iptables_rule_match **match);
 
 extern int delete_chain(const ipt_chainlabel chain, int verbose,
 			iptc_handle_t *handle);
Index: libiptc/libiptc.c
===================================================================
--- libiptc/libiptc.c	(revision 3309)
+++ libiptc/libiptc.c	(working copy)
@@ -1246,7 +1246,11 @@
 	/* memset to all 0 for your memcmp convenience. */
 	memset(t->u.user.name + strlen(t->u.user.name),
 	       0,
-	       FUNCTION_MAXNAMELEN - strlen(t->u.user.name));
+	       FUNCTION_MAXNAMELEN - strlen(t->u.user.name) - 1);
+	/* fcuk (tm), this is what happens when you apply 
+	 * dirty hacks... that memset has been knocking me for
+	 * hours... arrrg! --pablo
+	 */
 
 	set_changed(handle);
 	return 1;

Re: [PATCH 1/2] Versioning (aka release) stuff for iptables

[Rusty Russell]

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

On Sat, 2004-12-25 at 22:31 +0100, Pablo Neira wrote:
> Hi Rusty,
> 
> I've been working on the versioning stuff last days. I've tested with 
> the mark target.

Hi Pablo,

	I've taken your patches and hacked on them again.  I thought for a long
time about the compatibility question, and decided that we need a kernel
mechanism for querying versions of things as well.  This makes life much
easier, as then all versions can register with iptables and it can
figure out which one to use.

Enclosed is a series of four kernel patches, and one iptables patch:

iptables-cleanup-kmod-handling.patch.gz:
        Clean up the kmod handling in iptables, inspired by reading your
        patch 8)

compat-pabli.patch.gz:
        Your "steal the byte" versioning code.  I used the term
        "revision", but it's the same code.

iptables-revisions.patch.gz:
        New getsockopts which test whether a revision is available, and
        return the maximum revision available.

compat-MARK.patch.gz:
        The new version of ipt_MARK.

iptables-userspace.patch.gz:
        The iptables.c code to handle different revisions, and the
        libipt_MARK.c revision 1.  Has some other crap in there (SVN
        seems broken?).
        
It seems to work here... thoughts?
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

[-- Attachment #2: compat-MARK.patch.gz --]
[-- Type: application/x-gzip, Size: 1558 bytes --]

[-- Attachment #3: compat-pablo.patch.gz --]
[-- Type: application/x-gzip, Size: 1663 bytes --]

[-- Attachment #4: iptables-cleanup-kmod-handling.patch.gz --]
[-- Type: application/x-gzip, Size: 2938 bytes --]

[-- Attachment #5: iptables-revisions.patch.gz --]
[-- Type: application/x-gzip, Size: 1330 bytes --]

[-- Attachment #6: iptables-userspace.patch.gz --]
[-- Type: application/x-gzip, Size: 3702 bytes --]

Re: [PATCH 1/2] Versioning (aka release) stuff for iptables

[Pablo Neira]

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

Rusty Russell wrote:

>On Sat, 2004-12-25 at 22:31 +0100, Pablo Neira wrote:
>  
>
>>Hi Rusty,
>>
>>I've been working on the versioning stuff last days. I've tested with 
>>the mark target.
>>    
>>
>
>Hi Pablo,
>
>	I've taken your patches and hacked on them again.  I thought for a long
>time about the compatibility question, and decided that we need a kernel
>mechanism for querying versions of things as well.  This makes life much
>easier, as then all versions can register with iptables and it can
>figure out which one to use.
>  
>

I agree, I like your autoprobing version stuff.

>Enclosed is a series of four kernel patches, and one iptables patch:
>
>[...snip...]
>        
>It seems to work here... thoughts?
>  
>

I've given some spins to your patches on x86 box and everything works 
fine for me. I've also cook a patch to add a new version to multiport to 
test that versioning stuff works fine with matches.

multiport-remove-late-check.patch:
Remove a checking in kernel space which makes sures that the parameters 
passed are correct, that's already done in user space where this thing 
really belongs.

multiport_v2.patch:
the revision 1 of multiport. Actually this merges current mport in the 
SVN repository to multiport.

iptables-multiport_v2.patch:
iptables user space part.

multiport.sim:
a testsuite to check that new version works fine (incomplete).

Hope to see the versioning stuff pushed forward soon.

BTW, if you need this. Signed-off-by: Pablo Neira Ayuso <pablo@eurodev.net>

--
Pablo

[-- Attachment #2: multiport-remove-late-check.patch --]
[-- Type: text/x-patch, Size: 958 bytes --]

===== net/ipv4/netfilter/ipt_multiport.c 1.8 vs edited =====
--- 1.8/net/ipv4/netfilter/ipt_multiport.c	2004-08-19 02:14:53 +02:00
+++ edited/net/ipv4/netfilter/ipt_multiport.c	2005-01-01 18:58:25 +01:00
@@ -88,19 +88,7 @@
 	   unsigned int matchsize,
 	   unsigned int hook_mask)
 {
-	const struct ipt_multiport *multiinfo = matchinfo;
-
-	if (matchsize != IPT_ALIGN(sizeof(struct ipt_multiport)))
-		return 0;
-
-	/* Must specify proto == TCP/UDP, no unknown flags or bad count */
-	return (ip->proto == IPPROTO_TCP || ip->proto == IPPROTO_UDP)
-		&& !(ip->invflags & IPT_INV_PROTO)
-		&& matchsize == IPT_ALIGN(sizeof(struct ipt_multiport))
-		&& (multiinfo->flags == IPT_MULTIPORT_SOURCE
-		    || multiinfo->flags == IPT_MULTIPORT_DESTINATION
-		    || multiinfo->flags == IPT_MULTIPORT_EITHER)
-		&& multiinfo->count <= IPT_MULTI_PORTS;
+	return (matchsize == IPT_ALIGN(sizeof(struct ipt_multiport)));
 }
 
 static struct ipt_match multiport_match = {

[-- Attachment #3: multiport_v2.patch --]
[-- Type: text/x-patch, Size: 4725 bytes --]

--- linux-2.5/net/ipv4/netfilter/ipt_multiport.c	2005-01-01 19:03:25.000000000 +0100
+++ revision/net/ipv4/netfilter/ipt_multiport.c	2005-01-01 19:03:13.000000000 +0100
@@ -46,16 +46,55 @@
 	return 0;
 }
 
-static int
-match(const struct sk_buff *skb,
-      const struct net_device *in,
-      const struct net_device *out,
-      const void *matchinfo,
-      int offset,
-      int *hotdrop)
+/* Returns 1 if the port is matched by the test, 0 otherwise. */
+static inline int
+ports_match_v1(const struct ipt_multiport_v1 *minfo,
+	       u_int16_t src, u_int16_t dst)
+{
+	unsigned int i;
+	u_int16_t s, e;
+
+	for (i=0; i < minfo->count; i++) {
+		s = minfo->ports[i];
+
+		if (minfo->pflags[i]) {
+			/* range port matching */
+			e = minfo->ports[++i];
+			duprintf("src or dst matches with %d-%d?\n", s, e);
+
+			if (minfo->flags == IPT_MULTIPORT_SOURCE
+			    && src >= s && src <= e)
+				return 1;
+			if (minfo->flags == IPT_MULTIPORT_DESTINATION
+			    && dst >= s && dst <= e)
+				return 1;
+			if (minfo->flags == IPT_MULTIPORT_EITHER
+			    && dst >= s && dst <= e)
+				return 1;
+		} else {
+			/* exact port matching */
+			duprintf("src or dst matches with %d?\n", s);
+			if (minfo->flags == IPT_MULTIPORT_SOURCE
+			    && src == s)
+				return 1;
+			if (minfo->flags == IPT_MULTIPORT_DESTINATION
+			    && dst == s)
+				return 1;
+			if (minfo->flags == IPT_MULTIPORT_EITHER
+			    && (src == s || dst == s))
+				return 1;
+		}
+	}
+ 
+ 	return 0;
+}
+
+static u16 *
+__match(const struct sk_buff *skb,
+	const void *matchinfo,
+	int offset)
 {
 	u16 _ports[2], *pptr;
-	const struct ipt_multiport *multiinfo = matchinfo;
 
 	/* Must not be a fragment. */
 	if (offset)
@@ -65,6 +104,21 @@
            them at the start). */
 	pptr = skb_header_pointer(skb, skb->nh.iph->ihl * 4,
 				  sizeof(_ports), &_ports[0]);
+	return pptr;
+}
+
+static int
+match(const struct sk_buff *skb,
+      const struct net_device *in,
+      const struct net_device *out,
+      const void *matchinfo,
+      int offset,
+      int *hotdrop)
+{
+	u16 *pptr;
+	const struct ipt_multiport *multiinfo = matchinfo;
+
+	pptr = __match(skb, matchinfo, offset);
 	if (pptr == NULL) {
 		/* We've been asked to examine this packet, and we
 		 * can't.  Hence, no choice but to drop.
@@ -80,6 +134,31 @@
 			   ntohs(pptr[0]), ntohs(pptr[1]));
 }
 
+static int
+match_v1(const struct sk_buff *skb,
+	 const struct net_device *in,
+	 const struct net_device *out,
+	 const void *matchinfo,
+	 int offset,
+	 int *hotdrop)
+{
+	u16 *pptr;
+	const struct ipt_multiport_v1 *multiinfo = matchinfo;
+
+	pptr = __match(skb, matchinfo, offset);
+	if (pptr == NULL) {
+		/* We've been asked to examine this packet, and we
+		 * can't.  Hence, no choice but to drop.
+		 */
+		duprintf("ipt_multiport:"
+			 " Dropping evil offset=0 tinygram.\n");
+		*hotdrop = 1;
+		return 0;
+	}
+
+	return ports_match_v1(multiinfo, ntohs(pptr[0]), ntohs(pptr[1]));
+}
+
 /* Called when user tries to insert an entry of this type. */
 static int
 checkentry(const char *tablename,
@@ -91,21 +170,50 @@
 	return (matchsize == IPT_ALIGN(sizeof(struct ipt_multiport)));
 }
 
+static int
+checkentry_v1(const char *tablename,
+	      const struct ipt_ip *ip,
+	      void *matchinfo,
+	      unsigned int matchsize,
+	      unsigned int hook_mask)
+{
+	return (matchsize == IPT_ALIGN(sizeof(struct ipt_multiport_v1)));
+}
+
 static struct ipt_match multiport_match = {
 	.name		= "multiport",
+	.revision	= 0,
 	.match		= &match,
 	.checkentry	= &checkentry,
 	.me		= THIS_MODULE,
 };
 
+static struct ipt_match multiport_match_v1 = {
+	.name		= "multiport",
+	.revision	= 1,
+	.match		= &match_v1,
+	.checkentry	= &checkentry_v1,
+	.me		= THIS_MODULE,
+};
+
 static int __init init(void)
 {
-	return ipt_register_match(&multiport_match);
+	int err;
+
+	err = ipt_register_match(&multiport_match);
+	if (!err) {
+		err = ipt_register_match(&multiport_match_v1);
+		if (err)
+			ipt_unregister_match(&multiport_match);
+	}
+
+	return err;
 }
 
 static void __exit fini(void)
 {
 	ipt_unregister_match(&multiport_match);
+	ipt_unregister_match(&multiport_match_v1);
 }
 
 module_init(init);
--- linux-2.5/include/linux/netfilter_ipv4/ipt_multiport.h	2005-01-01 19:03:21.000000000 +0100
+++ revision/include/linux/netfilter_ipv4/ipt_multiport.h	2005-01-01 19:03:13.000000000 +0100
@@ -18,4 +18,12 @@
 	u_int8_t count;				/* Number of ports */
 	u_int16_t ports[IPT_MULTI_PORTS];	/* Ports */
 };
+
+struct ipt_multiport_v1
+{
+	u_int8_t flags;				/* Type of comparison */
+	u_int8_t count;				/* Number of ports */
+	u_int16_t ports[IPT_MULTI_PORTS];	/* Ports */
+	u_int8_t pflags[IPT_MULTI_PORTS];	/* Port flags */
+};
 #endif /*_IPT_MULTIPORT_H*/

[-- Attachment #4: iptables-multiport_v2.patch --]
[-- Type: text/x-patch, Size: 7113 bytes --]

Index: include/linux/netfilter_ipv4/ipt_multiport.h
===================================================================
--- include/linux/netfilter_ipv4/ipt_multiport.h	(revision 0)
+++ include/linux/netfilter_ipv4/ipt_multiport.h	(revision 0)
@@ -0,0 +1,28 @@
+#ifndef _IPT_MULTIPORT_H
+#define _IPT_MULTIPORT_H
+
+enum ipt_multiport_flags
+{
+	IPT_MULTIPORT_SOURCE,
+	IPT_MULTIPORT_DESTINATION,
+	IPT_MULTIPORT_EITHER
+};
+
+#define IPT_MULTI_PORTS	15
+
+/* Must fit inside union ipt_matchinfo: 16 bytes */
+struct ipt_multiport
+{
+	u_int8_t flags;				/* Type of comparison */
+	u_int8_t count;				/* Number of ports */
+	u_int16_t ports[IPT_MULTI_PORTS];	/* Ports */
+};
+
+struct ipt_multiport_v1
+{
+	u_int8_t flags;				/* Type of comparison */
+	u_int8_t count;				/* Number of ports */
+	u_int16_t ports[IPT_MULTI_PORTS];	/* Ports */
+	u_int8_t pflags[IPT_MULTI_PORTS];	/* Port flags */
+};
+#endif /*_IPT_MULTIPORT_H*/
Index: extensions/libipt_multiport.c
===================================================================
--- extensions/libipt_multiport.c	(revision 3514)
+++ extensions/libipt_multiport.c	(working copy)
@@ -5,7 +5,8 @@
 #include <stdlib.h>
 #include <getopt.h>
 #include <iptables.h>
-#include <linux/netfilter_ipv4/ipt_multiport.h>
+/* To ensure that iptables compiles with an old kernel */
+#include "../include/linux/netfilter_ipv4/ipt_multiport.h"
 
 /* Function which prints out usage message. */
 static void
@@ -24,6 +25,22 @@
 IPTABLES_VERSION);
 }
 
+static void
+help_v1(void)
+{
+	printf(
+"multiport v%s options:\n"
+" --source-ports port[,port:port,port...]\n"
+" --sports ...\n"
+"				match source port(s)\n"
+" --destination-ports port[,port:port,port...]\n"
+" --dports ...\n"
+"				match destination port(s)\n"
+" --ports port[,port:port,port]\n"
+"				match both source and destination port(s)\n",
+IPTABLES_VERSION);
+}
+
 static struct option opts[] = {
 	{ "source-ports", 1, 0, '1' },
 	{ "sports", 1, 0, '1' }, /* synonym */
@@ -77,6 +94,46 @@
 	return i;
 }
 
+static void
+parse_multi_ports_v1(const char *portstring, 
+		     struct ipt_multiport_v1 *multiinfo,
+		     const char *proto)
+{
+	char *buffer, *cp, *next, *range;
+	unsigned int i;
+	u_int16_t m;
+
+	buffer = strdup(portstring);
+	if (!buffer) exit_error(OTHER_PROBLEM, "strdup failed");
+
+	for (i=0; i<IPT_MULTI_PORTS; i++)
+		multiinfo->pflags[i] = 0;
+ 
+	for (cp=buffer, i=0; cp && i<IPT_MULTI_PORTS; cp=next, i++) {
+		next=strchr(cp, ',');
+ 		if (next) *next++='\0';
+		range = strchr(cp, ':');
+		if (range) {
+			if (i == IPT_MULTI_PORTS-1)
+				exit_error(PARAMETER_PROBLEM,
+					   "too many ports specified");
+			*range++ = '\0';
+		}
+		multiinfo->ports[i] = parse_port(cp, proto);
+		if (range) {
+			multiinfo->pflags[i] = 1;
+			multiinfo->ports[++i] = parse_port(range, proto);
+			if (multiinfo->ports[i-1] >= multiinfo->ports[i])
+				exit_error(PARAMETER_PROBLEM,
+					   "invalid portrange specified");
+			m <<= 1;
+		}
+ 	}
+	multiinfo->count = i;
+ 	if (cp) exit_error(PARAMETER_PROBLEM, "too many ports specified");
+ 	free(buffer);
+}
+
 /* Initialize the match. */
 static void
 init(struct ipt_entry_match *m, unsigned int *nfcache)
@@ -153,6 +210,56 @@
 	return 1;
 }
 
+static int
+parse_v1(int c, char **argv, int invert, unsigned int *flags,
+	 const struct ipt_entry *entry,
+	 unsigned int *nfcache,
+	 struct ipt_entry_match **match)
+{
+	const char *proto;
+	struct ipt_multiport_v1 *multiinfo
+		= (struct ipt_multiport_v1 *)(*match)->data;
+
+	switch (c) {
+	case '1':
+		check_inverse(argv[optind-1], &invert, &optind, 0);
+		proto = check_proto(entry);
+		parse_multi_ports_v1(argv[optind-1], multiinfo, proto);
+		multiinfo->flags = IPT_MULTIPORT_SOURCE;
+		*nfcache |= NFC_IP_SRC_PT;
+		break;
+
+	case '2':
+		check_inverse(argv[optind-1], &invert, &optind, 0);
+		proto = check_proto(entry);
+		parse_multi_ports_v1(argv[optind-1], multiinfo, proto);
+		multiinfo->flags = IPT_MULTIPORT_DESTINATION;
+		*nfcache |= NFC_IP_DST_PT;
+		break;
+
+	case '3':
+		check_inverse(argv[optind-1], &invert, &optind, 0);
+		proto = check_proto(entry);
+		parse_multi_ports_v1(argv[optind-1], multiinfo, proto);
+		multiinfo->flags = IPT_MULTIPORT_EITHER;
+		*nfcache |= NFC_IP_SRC_PT | NFC_IP_DST_PT;
+		break;
+
+	default:
+		return 0;
+	}
+
+	if (invert)
+		exit_error(PARAMETER_PROBLEM,
+			   "multiport does not support invert");
+
+	if (*flags)
+		exit_error(PARAMETER_PROBLEM,
+			   "multiport can only have one option");
+	*flags = 1;
+	return 1;
+}
+
 /* Final check; must specify something. */
 static void
 final_check(unsigned int flags)
@@ -221,6 +328,46 @@
 	printf(" ");
 }
 
+static void
+print_v1(const struct ipt_ip *ip,
+	 const struct ipt_entry_match *match,
+	 int numeric)
+{
+	const struct ipt_multiport_v1 *multiinfo
+		= (const struct ipt_multiport_v1 *)match->data;
+	unsigned int i;
+
+	printf("multiport ");
+
+	switch (multiinfo->flags) {
+	case IPT_MULTIPORT_SOURCE:
+		printf("sports ");
+		break;
+
+	case IPT_MULTIPORT_DESTINATION:
+		printf("dports ");
+		break;
+
+	case IPT_MULTIPORT_EITHER:
+		printf("ports ");
+		break;
+
+	default:
+		printf("ERROR ");
+		break;
+	}
+
+	for (i=0; i < multiinfo->count; i++) {
+		printf("%s", i ? "," : "");
+		print_port(multiinfo->ports[i], ip->proto, numeric);
+		if (multiinfo->pflags[i]) {
+			printf(":");
+			print_port(multiinfo->ports[++i], ip->proto, numeric);
+		}
+	}
+	printf(" ");
+}
+
 /* Saves the union ipt_matchinfo in parsable form to stdout. */
 static void save(const struct ipt_ip *ip, const struct ipt_entry_match *match)
 {
@@ -249,9 +396,42 @@
 	printf(" ");
 }
 
+static void save_v1(const struct ipt_ip *ip, 
+		    const struct ipt_entry_match *match)
+{
+	const struct ipt_multiport_v1 *multiinfo
+		= (const struct ipt_multiport_v1 *)match->data;
+	unsigned int i;
+
+	switch (multiinfo->flags) {
+	case IPT_MULTIPORT_SOURCE:
+		printf("--sports ");
+		break;
+
+	case IPT_MULTIPORT_DESTINATION:
+		printf("--dports ");
+		break;
+
+	case IPT_MULTIPORT_EITHER:
+		printf("--ports ");
+		break;
+	}
+
+	for (i=0; i < multiinfo->count; i++) {
+		printf("%s", i ? "," : "");
+		print_port(multiinfo->ports[i], ip->proto, 1);
+		if (multiinfo->pflags[i]) {
+			printf(":");
+			print_port(multiinfo->ports[++i], ip->proto, 1);
+		}
+	}
+	printf(" ");
+}
+
 static struct iptables_match multiport = { 
 	.next		= NULL,
 	.name		= "multiport",
+	.revision	= 0,
 	.version	= IPTABLES_VERSION,
 	.size		= IPT_ALIGN(sizeof(struct ipt_multiport)),
 	.userspacesize	= IPT_ALIGN(sizeof(struct ipt_multiport)),
@@ -264,8 +444,25 @@
 	.extra_opts	= opts
 };
 
+static struct iptables_match multiport_v1 = { 
+	.next		= NULL,
+	.name		= "multiport",
+	.version	= IPTABLES_VERSION,
+	.revision	= 1,
+	.size		= IPT_ALIGN(sizeof(struct ipt_multiport_v1)),
+	.userspacesize	= IPT_ALIGN(sizeof(struct ipt_multiport_v1)),
+	.help		= &help_v1,
+	.init		= &init,
+	.parse		= &parse_v1,
+	.final_check	= &final_check,
+	.print		= &print_v1,
+	.save		= &save_v1,
+	.extra_opts	= opts
+};
+
 void
 _init(void)
 {
 	register_match(&multiport);
+	register_match(&multiport_v1);
 }

[-- Attachment #5: multiport.sim --]
[-- Type: text/plain, Size: 3361 bytes --]

#test old style

iptables -I INPUT -p 6 -m multiport --sports 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -j DROP
iptables -D INPUT -p tcp -m multiport --source-ports 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -j DROP

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.
168.0.1 0 6 15 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 15 1 SYN

#test new version

iptables -I INPUT -p 6 -m multiport --sports 1:10,1000 -j DROP
expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 4 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 4 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 1 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_ACCEPT {IPv4 192.168.0.2 192.168.0.1 0 6 11 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 11 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_ACCEPT {IPv4 192.168.0.2 192.168.0.1 0 6 999 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 999 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_ACCEPT {IPv4 192.168.0.2 192.168.0.1 0 6 1001 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1001 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 1000 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1000 1 SYN

iptables -D INPUT -p 6 -m multiport --sports 1:10,1000 -j DROP

iptables -I INPUT -p 6 -m multiport --sports 1:10,1000,1500:1501,2000:2005 -j DROP
expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 1500 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1500 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 1501 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1501 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_ACCEPT {IPv4 192.168.0.2 192.168.0.1 0 6 1499 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1499 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_ACCEPT {IPv4 192.168.0.2 192.168.0.1 0 6 1502 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1502 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 2000 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 2000 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 2003 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 2003 1 SYN

iptables -D INPUT -p 6 -m multiport --sports 1:10,1000,1500:1501,2000:2005 -j DROP

iptables -I INPUT -p 6 -m multiport --ports 1:10,1000,1500:1501,2000:2005 -j DROP

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 2003 1 SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 2003 1 SYN

expect gen_ip hook:NF_IP_LOCAL_IN iptable_filter NF_DROP {IPv4 192.168.0.2 192.168.0.1 0 6 1 2003  SYN}
gen_ip IF=eth0 192.168.0.2 192.168.0.1 0 6 1 2003 SYN

iptables -D INPUT -p 6 -m multiport --sports 1:10,1000,1500:1501,2000:2005 -j DROP

iptables -I INPUT -p 6 -m multiport --sports 1:10,20:30,40:50,60,70,80,90,100,110,120:1200,15000 -j DROP

iptables -D INPUT -p 6 -m multiport --sports 1:10,20:30,40:50,60,70,80,90,100,110,120:1200,15000 -j DROP

expect iptables iptables: command failed
iptables -I INPUT -p 6 -m multiport --sports 1:10:30,40 -j DROP

Re: [PATCH 1/2] Versioning (aka release) stuff for iptables

[Rusty Russell]

On Sat, 2005-01-01 at 22:56 +0100, Pablo Neira wrote:
> multiport-remove-late-check.patch:
> Remove a checking in kernel space which makes sures that the parameters 
> passed are correct, that's already done in user space where this thing 
> really belongs.

I've rolled this into the other kernel patch, after some thought.  The
previous policy was that iptables should not be able to crash the
kernel, however that means a lot of gratuitous checking in the kernel
that is far more useful if done in userspace (where the error messages
can be more informative).

> multiport_v2.patch:
> the revision 1 of multiport. Actually this merges current mport in the 
> SVN repository to multiport.

I've put this in my patch collection.

> iptables-multiport_v2.patch:
> iptables user space part.

Applied and committed.

> multiport.sim:
> a testsuite to check that new version works fine (incomplete).

I reworked this to be thorough and test one thing at a time, and split
the test into revision 0 things, and revision 1 things.  As a result, I
found a bug in the userspace part (checks protocol, but not that it
isn't inverted), and in the kernel part (--ports only examined
destination ports, not source).

Fixed, and thanks!

> BTW, if you need this. Signed-off-by: Pablo Neira Ayuso <pablo@eurodev.net>

Please read part (11) of Documentation/SubmittingPatches in the kernel
source tree.  If you're fine by that, attach it to future patches.

Thanks!
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman