Suggestion for RETURN target

Suggestion for RETURN target

[Brad Fisher]

Would it be possible to modify the RETURN target in such a way that it
could return to an arbitrary chain in the call stack instead of only to
the previous chain?  For example, something like:

    iptables -N chain2
    ... other rules for chain2 ...
    iptables -A chain2 -j RETURN --return-to 2

    iptables -N chain1
    iptables -A chain1 <matches> -j chain2
    iptables -A chain1 -j DROP

    iptables -A FORWARD -j chain1

In the above example, the DROP rule in chain1 would never get executed
when the chain2 rule matched since the RETURN target in chain2 would
"return-to" the 2nd to last chain in the call stack instead of the
calling chain.  Perhaps "return-to" isn't the best name...  Something
like "return-levels" or similar would work just as well for me.  The
current behavior would be maintained if the additional parameter was not
given.

If this isn't a possibility, then what about a way to attach a second
(or third, etc?) target to a rule?  The targets would execute in
sequence, with certain obvious ones such as ACCEPT/DROP/REJECT/etc
stopping target execution.  With this, I could write the example above
as follows:

    iptables -N chain2
    ... other rules for chain2 ...

    iptables -N chain1
    iptables -A chain1 <matches> -j chain2,RETURN
    iptables -A chain1 -j DROP

    iptables -A FORWARD -j chain1

Where the sequence "-j chain2,RETURN" means execute chain2, then if it
returns execute a RETURN to the previous chain.

Either of these two approaches would allow me to reduce my ruleset 1/2
the rules currently required.

Anyway, just thought I'd thow it out there to see what others think.

-Brad

Re: Suggestion for RETURN target

[Henrik Nordstrom]

On Tue, 2 Dec 2003, Brad Fisher wrote:

> Would it be possible to modify the RETURN target in such a way that it
> could return to an arbitrary chain in the call stack instead of only to
> the previous chain?

>From your description it more looks like you need a true jump/goto 
operation, not a "call" operation.. this would be like the current jump 
operation except that the current chain is not saved on the call stack.

Regards
Henrik

Re: Suggestion for RETURN target

[Brad Fisher]

I'm not sure that would do exactly what I require.  If I understand you
correctly, when you fall off the end of a chain called with a "goto"
operation, it would return back 2 chains (or execute the default policy on a
built-in?).  This is different than my suggestion, since as I envision it,
the only time you would go back more than one chain would be if a rule
specifically requested it.  For example,

  iptables -N chain4
  iptables -A chain4 -s 192.168.0.1 -j RETURN --return-to 3

  iptables -N chain3
  iptables -A chain3 -j chain4
  iptables -A chain3 -s 192.168.0.2 -j RETURN --return-to 2
  .. other rules for chain3 here ...

  iptables -N chain2
  iptables -A chain2 -j chain3
  ... other rules for chain2 here ...

  iptables -N chain1
  iptables -A chain1 -j chain2
  ... other rules for chain1 here ...

  iptables -A INPUT -j chain1

In this example, the source ip 192.168.0.1 will cause chain4 to return to
chain1 (the third chain in the call stack), but any other IP will cause it to
fall off the end and return to chain3 (the caller).  After returning to
chain3, an ip of 192.168.0.2 will cause a return to chain1 (the second chain
in the call stack), and any other ip will return to chain2 (the caller).
Well, this assumes any other rules in the respective chains don't chainge
that behavior...

Anyway, this is a contrived example, but hopefully illustrates the idea
better?

Does anyone have any ideas on whether this would be possible to do and if so
would they have any pointers on where I should start looking?  I don't mind
writing the code myself...

-Brad

Henrik Nordstrom wrote:

> On Tue, 2 Dec 2003, Brad Fisher wrote:
>
> > Would it be possible to modify the RETURN target in such a way that it
> > could return to an arbitrary chain in the call stack instead of only to
> > the previous chain?
>
> >From your description it more looks like you need a true jump/goto
> operation, not a "call" operation.. this would be like the current jump
> operation except that the current chain is not saved on the call stack.
>
> Regards
> Henrik

Re: Suggestion for RETURN target

[Henrik Nordstrom]

On Wed, 3 Dec 2003, Brad Fisher wrote:

> I'm not sure that would do exactly what I require.  If I understand you
> correctly, when you fall off the end of a chain called with a "goto"
> operation, it would return back 2 chains (or execute the default policy on a
> built-in?).

Yes, sort of.

> This is different than my suggestion, since as I envision it,
> the only time you would go back more than one chain would be if a rule
> specifically requested it.

Which I see as unneededly complex as it moves the logics to the leaves of 
your rule tree rather than at the branches making it a very interwoven set 
of rules, especially if you call these chains from multiple locations.

>  For example,
> 
>   iptables -N chain4
>   iptables -A chain4 -s 192.168.0.1 -j RETURN --return-to 3
> 
>   iptables -N chain3
>   iptables -A chain3 -j chain4
>   iptables -A chain3 -s 192.168.0.2 -j RETURN --return-to 2
>   .. other rules for chain3 here ...
> 
>   iptables -N chain2
>   iptables -A chain2 -j chain3
>   ... other rules for chain2 here ...
> 
>   iptables -N chain1
>   iptables -A chain1 -j chain2
>   ... other rules for chain1 here ...
> 
>   iptables -A INPUT -j chain1

Yes, and such ruleset becomes very complex to follow and maintain in my
opinion. Only having call/goto type chain operations is more familiar to
most, and also makes the chains neutral on from where they are called.

Try to make a ruleset which makes sense where different levels of return
are actually used. In the above by making the jumps to chain2/3/4 a goto
class jump the return would in all cases return to chain1. The difference
is as you note if you have other levels RETURN statements (including the
implicit return at the end of the chain). But I seriously question if such
ruleset design is sane from both a ruleset design perspective and 
maintainability.

> Does anyone have any ideas on whether this would be possible to do and if so
> would they have any pointers on where I should start looking?  I don't mind
> writing the code myself...

Neither of the two should be very complex to implement, but both requires 
chainging of the iptables core. It can not be done via the match/target 
interface normally used for extending iptables.

Regards
Henrik

Re: Suggestion for RETURN target

[Brad Fisher]

Henrik Nordstrom wrote:

> On Wed, 3 Dec 2003, Brad Fisher wrote:

> > This is different than my suggestion, since as I envision it,
> > the only time you would go back more than one chain would be if a rule
> > specifically requested it.
>
> Which I see as unneededly complex as it moves the logics to the leaves of
> your rule tree rather than at the branches making it a very interwoven set
> of rules, especially if you call these chains from multiple locations.

I agree it does make the chains hard to follow.

> Try to make a ruleset which makes sense where different levels of return
> are actually used. In the above by making the jumps to chain2/3/4 a goto
> class jump the return would in all cases return to chain1. The difference
> is as you note if you have other levels RETURN statements (including the
> implicit return at the end of the chain). But I seriously question if such
> ruleset design is sane from both a ruleset design perspective and
> maintainability.

My main reason for asking for this feature is so I can shortcut the execution of
rules which are guaranteed to never match (ie. the rules following the jump to
another chain).  For this purpose, the goto idea will work.  I guess I thought
that the ability to return an arbitrary number of levels was "cool", but I can
live without it if something like goto existed.

> > Does anyone have any ideas on whether this would be possible to do and if so
> > would they have any pointers on where I should start looking?  I don't mind
> > writing the code myself...
>
> Neither of the two should be very complex to implement, but both requires
> chainging of the iptables core. It can not be done via the match/target
> interface normally used for extending iptables.

Would there be any chance of a patch for the goto functionality being accepted
then?  I wouldn't mind doing the work as long as I don't have to maintain a custom
patch for myself.  If anyone else is interested, then I'd be open to suggestions
on how it should work.  For example:

  # Normal 'jump' (call)
  iptables -j custom_chain

  # Add '-g' (goto) argument to iptables userspace?
  iptables -g custom_chain

  # Add '--goto' (goto) flag to iptables userspace?
  iptables -j custom_chain --goto

And of course there'd have to be a flag somewhere in the rule/target struct itself
which would indicate the calling chain shouldn't be added to the call stack.

> Regards
> Henrik

Thanks for your input!

-Brad

Re: Suggestion for RETURN target

[Henrik Nordstrom]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1046 bytes --]

On Wed, 3 Dec 2003, Brad Fisher wrote:

> Would there be any chance of a patch for the goto functionality being accepted
> then?

Hope so.

> custom patch for myself.  If anyone else is interested, then I'd be open
> to suggestions on how it should work.  For example:
> 
>   # Normal 'jump' (call)
>   iptables -j custom_chain
> 
>   # Add '-g' (goto) argument to iptables userspace?
>   iptables -g custom_chain

This one (-g/--goto instead of -j/--jump) is what I would prefer.

> And of course there'd have to be a flag somewhere in the rule/target struct itself
> which would indicate the calling chain shouldn't be added to the call stack.

There is plenty of space for flags left..

Attached you can find a patch which implements the above using a new flag
in the ip.flags field. What is missing from the patch is that iptables -L
does not indicate the difference between a jump and a goto. iptables-save
is done. Have tried to make the patch in such manner that binary and 
source compatibility is kept for the userspace.

Regards
Henrik

[-- Attachment #2: kernel patch --]
[-- Type: TEXT/PLAIN, Size: 1070 bytes --]

--- linux-2.4.22-uml/include/linux/netfilter_ipv4/ip_tables.h	Wed Dec  3 23:12:11 2003
+++ linux-2.4.22-goto/include/linux/netfilter_ipv4/ip_tables.h	Wed Dec  3 22:12:30 2003
@@ -104,7 +104,8 @@
 
 /* Values for "flag" field in struct ipt_ip (general ip structure). */
 #define IPT_F_FRAG		0x01	/* Set if rule is a fragment rule */
-#define IPT_F_MASK		0x01	/* All possible flag bits mask. */
+#define IPT_F_GOTO		0x02	/* Set if jump is a goto */
+#define IPT_F_MASK		0x03	/* All possible flag bits mask. */
 
 /* Values for "inv" field in struct ipt_ip. */
 #define IPT_INV_VIA_IN		0x01	/* Invert the sense of IN IFACE. */
--- linux-2.4.22-uml/net/ipv4/netfilter/ip_tables.c	Wed Dec  3 23:12:11 2003
+++ linux-2.4.22-goto/net/ipv4/netfilter/ip_tables.c	Wed Dec  3 22:22:19 2003
@@ -342,7 +342,7 @@
 					continue;
 				}
 				if (table_base + v
-				    != (void *)e + e->next_offset) {
+				    != (void *)e + e->next_offset && !(e->ip.flags & IPT_F_GOTO)) {
 					/* Save old back ptr in next entry */
 					struct ipt_entry *next
 						= (void *)e + e->next_offset;

[-- Attachment #3: readme --]
[-- Type: TEXT/PLAIN, Size: 114 bytes --]

Author: Henrik Nordstrom <hno@marasystems.com>
Status: working

This patch adds kernel support for goto type jumps

[-- Attachment #4: userspace patch --]
[-- Type: TEXT/PLAIN, Size: 3068 bytes --]

Index: iptables.c
===================================================================
RCS file: /cvspublic/netfilter/userspace/iptables.c,v
retrieving revision 1.59
diff -u -w -p -r1.59 iptables.c
--- userspace/iptables.c	31 May 2003 21:30:33 -0000	1.59
+++ userspace/iptables.c	3 Dec 2003 22:26:44 -0000
@@ -138,6 +138,7 @@ static struct option original_opts[] = {
 	{ "line-numbers", 0, 0, '0' },
 	{ "modprobe", 1, 0, 'M' },
 	{ "set-counters", 1, 0, 'c' },
+	{ "goto", 1, 0, 'g' },
 	{ 0 }
 };
 
@@ -395,6 +396,10 @@ exit_printhelp(void)
 "				network interface name ([+] for wildcard)\n"
 "  --jump	-j target\n"
 "				target for rule (may load target extension)\n"
+#ifdef IPT_F_GOTO
+"  --goto	-g chain\n"
+"				jump to chain with no return\n"
+#endif
 "  --match	-m match\n"
 "				extended match (may load extension)\n"
 "  --numeric	-n		numeric output of addresses and ports\n"
@@ -1695,7 +1700,7 @@ int do_command(int argc, char *argv[], c
 	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:xc:g:",
 					   opts, NULL)) != -1) {
 		switch (c) {
 			/*
@@ -1866,6 +1871,15 @@ int do_command(int argc, char *argv[], c
 			fw.nfcache |= NFC_IP_DST;
 			break;
 
+#ifdef IPT_F_GOTO
+		case 'g':
+			set_option(&options, OPT_JUMP, &fw.ip.invflags,
+				   invert);
+			fw.ip.flags |= IPT_F_GOTO;
+			jumpto = parse_target(optarg);
+			break;
+#endif
+
 		case 'j':
 			set_option(&options, OPT_JUMP, &fw.ip.invflags,
 				   invert);
@@ -2216,6 +2230,11 @@ int do_command(int argc, char *argv[], c
 			 * We cannot know if the plugin is corrupt, non
 			 * existant OR if the user just misspelled a
 			 * chain. */
+#ifdef IPT_F_GOTO
+			if (fw.ip.flags & IPT_F_GOTO)
+				exit_error(PARAMETER_PROBLEM,
+					    "goto '%s' is not a chain\n", jumpto);
+#endif
 			find_target(jumpto, LOAD_MUST_SUCCEED);
 		} else {
 			e = generate_entry(&fw, iptables_matches, target->t);
Index: iptables.8
===================================================================
RCS file: /cvspublic/netfilter/userspace/iptables.8,v
retrieving revision 1.38
diff -u -w -p -r1.38 iptables.8
--- userspace/iptables.8	29 Jun 2003 18:00:08 -0000	1.38
+++ userspace/iptables.8	3 Dec 2003 22:26:44 -0000
@@ -274,9 +274,17 @@ one this rule is in), one of the special
 the fate of the packet immediately, or an extension (see
 .B EXTENSIONS
 below).  If this
-option is omitted in a rule, then matching the rule will have no
+option is omitted in a rule (and
+.B -g
+is not used), then matching the rule will have no
 effect on the packet's fate, but the counters on the rule will be
 incremented.
+.TP
+.BI "-g, --goto " "chain"
+This specifies that the processing should continue in a user
+specified chain. Unlike the --jump option return will not continue
+processing in this chain but instead in the chain that called us via
+--jump.
 .TP
 .BR "-i, --in-interface " "[!] \fIname\fP"
 Name of an interface via which a packet is going to be received (only for

Re: Suggestion for RETURN target

[Brad Fisher]

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

Henrik Nordstrom wrote:

> There is plenty of space for flags left..
>
> Attached you can find a patch which implements the above using a new flag
> in the ip.flags field. What is missing from the patch is that iptables -L
> does not indicate the difference between a jump and a goto. iptables-save
> is done. Have tried to make the patch in such manner that binary and
> source compatibility is kept for the userspace.
>
> Regards
> Henrik

I have tested your patch, and it seems to work well.  Attached you will find a modified
version which patches iptables-save, and iptables -L.  It seems a little funny to me to
add the IPT_F_GOTO flag into the ip flags, but it makes sense to make use of the unused
space.  I would think it would make more logical sense to have it attached to the target
structure, but that would break binary compatibility, which I think is important to
preserve.

In all, I think this new patch does what I require, thanks for the quick response and
the great patch.  I had actually started looking at this but hadn't gotten far before
you posted it.  I apppreciate the time you put into it and hope that it at least gets
included in patch-o-matic :)

-Brad

[-- Attachment #2: goto.patch.userspace --]
[-- Type: text/plain, Size: 3505 bytes --]

diff -urN iptables-1.2.9/iptables-save.c iptables-1.2.9-goto/iptables-save.c
--- iptables-1.2.9/iptables-save.c	Sat May  3 13:52:13 2003
+++ iptables-1.2.9-goto/iptables-save.c	Thu Dec  4 12:06:25 2003
@@ -188,8 +188,14 @@
 
 	/* Print target name */	
 	target_name = iptc_get_target(e, h);
-	if (target_name && (*target_name != '\0'))
-		printf("-j %s ", target_name);
+	if (target_name && (*target_name != '\0')) {
+#ifdef IPT_F_GOTO
+		if (e->ip.flags & IPT_F_GOTO)
+			printf("-g %s ", target_name);
+		else
+#endif
+			printf("-j %s ", target_name);
+	}
 
 	/* Print targinfo part */
 	t = ipt_get_target((struct ipt_entry *)e);
diff -urN iptables-1.2.9/iptables.8 iptables-1.2.9-goto/iptables.8
--- iptables-1.2.9/iptables.8	Mon Jun 30 11:16:54 2003
+++ iptables-1.2.9-goto/iptables.8	Wed Dec  3 17:03:04 2003
@@ -274,10 +274,18 @@
 the fate of the packet immediately, or an extension (see
 .B EXTENSIONS
 below).  If this
-option is omitted in a rule, then matching the rule will have no
+option is omitted in a rule (and
+.B -g
+is not used), then matching the rule will have no
 effect on the packet's fate, but the counters on the rule will be
 incremented.
 .TP
+.BI "-g, --goto " "chain"
+This specifies that the processing should continue in a user
+specified chain. Unlike the --jump option return will not continue
+processing in this chain but instead in the chain that called us via
+--jump.
+.TP
 .BR "-i, --in-interface " "[!] \fIname\fP"
 Name of an interface via which a packet is going to be received (only for
 packets entering the 
diff -urN iptables-1.2.9/iptables.c iptables-1.2.9-goto/iptables.c
--- iptables-1.2.9/iptables.c	Sat Jun 14 17:39:35 2003
+++ iptables-1.2.9-goto/iptables.c	Thu Dec  4 12:28:02 2003
@@ -138,6 +138,7 @@
 	{ "line-numbers", 0, 0, '0' },
 	{ "modprobe", 1, 0, 'M' },
 	{ "set-counters", 1, 0, 'c' },
+	{ "goto", 1, 0, 'g' },
 	{ 0 }
 };
 
@@ -395,6 +396,10 @@
 "				network interface name ([+] for wildcard)\n"
 "  --jump	-j target\n"
 "				target for rule (may load target extension)\n"
+#ifdef IPT_F_GOTO
+"  --goto	-g chain\n"
+"				jump to chain with no return\n"
+#endif
 "  --match	-m match\n"
 "				extended match (may load extension)\n"
 "  --numeric	-n		numeric output of addresses and ports\n"
@@ -1269,6 +1274,11 @@
 
 	IPT_MATCH_ITERATE(fw, print_match, &fw->ip, format & FMT_NUMERIC);
 
+#ifdef IPT_F_GOTO
+	if (flags & IPT_F_GOTO)
+		printf("GOTO ");
+#endif
+	
 	if (target) {
 		if (target->print)
 			/* Print the target information. */
@@ -1695,7 +1705,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:xc:g:",
 					   opts, NULL)) != -1) {
 		switch (c) {
 			/*
@@ -1866,6 +1876,15 @@
 			fw.nfcache |= NFC_IP_DST;
 			break;
 
+#ifdef IPT_F_GOTO
+		case 'g':
+			set_option(&options, OPT_JUMP, &fw.ip.invflags,
+				   invert);
+			fw.ip.flags |= IPT_F_GOTO;
+			jumpto = parse_target(optarg);
+			break;
+#endif
+
 		case 'j':
 			set_option(&options, OPT_JUMP, &fw.ip.invflags,
 				   invert);
@@ -2216,6 +2235,11 @@
 			 * We cannot know if the plugin is corrupt, non
 			 * existant OR if the user just misspelled a
 			 * chain. */
+#ifdef IPT_F_GOTO
+			if (fw.ip.flags & IPT_F_GOTO)
+				exit_error(PARAMETER_PROBLEM,
+					    "goto '%s' is not a chain\n", jumpto);
+#endif
 			find_target(jumpto, LOAD_MUST_SUCCEED);
 		} else {
 			e = generate_entry(&fw, iptables_matches, target->t);

Re: Suggestion for RETURN target

[Henrik Nordstrom]

On Thu, 4 Dec 2003, Brad Fisher wrote:

> I have tested your patch, and it seems to work well.  Attached you will find a modified
> version which patches iptables-save, and iptables -L.  It seems a little funny to me to
> add the IPT_F_GOTO flag into the ip flags, but it makes sense to make use of the unused
> space.  I would think it would make more logical sense to have it attached to the target
> structure, but that would break binary compatibility, which I think is important to
> preserve.

Well.. it is not really a target but part of the iptables core so the flag
is fine I think. And not breaking binary compatibility is very desireable 
for a change like this.

On the down side the kernel does not verify the flag fields content, so it 
is possible to use a new binary on a old kernel and it will still accept 
goto rules but will process them like ordinary jumps. This might not be 
all desireable but a acceptable compromise I think.

> I apppreciate the time you put into it and hope that it at least gets
> included in patch-o-matic :)

I would certainly not mind have it included.

As noted in my message I tried to make the userspace changes in such way 
that it is source compatible with kernel includes both with and without 
the changed iptables includes, but requires the updated headers to enable 
the goto flag

Regards
Henrik