Skip to contentSkip to conversation

Route match

RFC PATCH In review Phil Oester 8 participants 27 messages Jul 3, 2008 – Jul 4, 2008

Subsystems (3)
Networking netfilter IPv4/IPv6
lore
Share & tools
lore

Phil Oester

author
#0 lore raw

RFC PATCH 1/1 Route match

Attached is a new "route" match, which matches against entries in the kernel routing table. I've had a need for this functionality for quite some time, and finally decided to code it up.

TODO: add IPv6 support to kernel side

Options

[!] --route-src-exists      Route for src exists
    --route-src-eq value    Route for src exists with prefix-length == value
    --route-src-gt value    Route for src exists with prefix-length  > value
    --route-src-lt value    Route for src exists with prefix-length  < value
[!] --route-dst-exists      Route for dst exists
    --route-dst-eq value    Route for dst exists with prefix-length == value
    --route-dst-gt value    Route for dst exists with prefix-length  > value
    --route-dst-lt value    Route for dst exists with prefix-length  < value

Examples:

	Egress filtering:
		iptables -A FORWARD -m route ! --route-src-exists -j DROP

	Ingress filtering:
		iptables -A FORWARD -i $EXTERNAL_IF -m route --route-src-exists -j DROP

	Allow a user to only browse internal websites:
		iptables -A FORWARD -p tcp --dport 80 -m route --route-dst-exists -j ACCEPT

	Allow an SNMP collector to reach all internal routers (/30 or /32 interfaces)
		iptables -A FORWARD -p udp --dport 161 -m route --route-dst-gt 29 -j ACCEPT

Please let me know if anyone else finds this match useful.

Phil

include/linux/netfilter/xt_route.h new file +21 -0
new file mode 100644
index 0000000..0c90494
@@ -0,0 +1,21 @@
+#ifndef _XT_ROUTE_H
+#define _XT_ROUTE_H
+
+enum {
+	XT_ROUTE_SRC_EXISTS = 0,
+	XT_ROUTE_SRC_EQ,
+	XT_ROUTE_SRC_GT,
+	XT_ROUTE_SRC_LT,
+	XT_ROUTE_DST_EXISTS,
+	XT_ROUTE_DST_EQ,
+	XT_ROUTE_DST_GT,
+	XT_ROUTE_DST_LT,
+};
+
+struct xt_route_info {
+	u_int8_t	invert;
+	u_int8_t	mode;
+	u_int8_t	prefixlen;
+};
+
+#endif /*_XT_ROUTE_H*/
net/ipv4/fib_semantics.c +2 -0
index 0d4d728..22bbdd5 100644
@@ -156,6 +156,8 @@ void free_fib_info(struct fib_info *fi)
 	kfree(fi);
 }
 
+EXPORT_SYMBOL_GPL(free_fib_info);
+
 void fib_release_info(struct fib_info *fi)
 {
 	spin_lock_bh(&fib_info_lock);
net/netfilter/Kconfig +10 -0
index aa8d80c..128b186 100644
@@ -724,6 +724,16 @@ config NETFILTER_XT_MATCH_REALM
 	  If you want to compile it as a module, say M here and read
 	  <file:Documentation/kbuild/modules.txt>.  If unsure, say `N'.
 
+config NETFILTER_XT_MATCH_ROUTE
+	tristate '"route" match support'
+	depends on NETFILTER_XTABLES
+	depends on NETFILTER_ADVANCED
+	help
+	  This option adds a `route' match, which allows you to match on
+	  the kernel routing table.  
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config NETFILTER_XT_MATCH_SCTP
 	tristate  '"sctp" protocol match support (EXPERIMENTAL)'
 	depends on NETFILTER_XTABLES && EXPERIMENTAL
net/netfilter/Makefile +1 -0
index 5c4b183..1035372 100644
@@ -76,6 +76,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_POLICY) += xt_policy.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_QUOTA) += xt_quota.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_RATEEST) += xt_rateest.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_REALM) += xt_realm.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_ROUTE) += xt_route.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_SCTP) += xt_sctp.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_STATE) += xt_state.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_STATISTIC) += xt_statistic.o
net/netfilter/xt_route.c new file +141 -0

141 lines not on this page · raw message

extensions/libxt_route.c new file +371 -0

371 lines not on this page · raw message

include/linux/netfilter/xt_route.h new file +21 -0
new file mode 100644
index 0000000..0c90494
@@ -0,0 +1,21 @@
+#ifndef _XT_ROUTE_H
+#define _XT_ROUTE_H
+
+enum {
+	XT_ROUTE_SRC_EXISTS = 0,
+	XT_ROUTE_SRC_EQ,
+	XT_ROUTE_SRC_GT,
+	XT_ROUTE_SRC_LT,
+	XT_ROUTE_DST_EXISTS,
+	XT_ROUTE_DST_EQ,
+	XT_ROUTE_DST_GT,
+	XT_ROUTE_DST_LT,
+};
+
+struct xt_route_info {
+	u_int8_t	invert;
+	u_int8_t	mode;
+	u_int8_t	prefixlen;
+};
+
+#endif /*_XT_ROUTE_H*/
#1 lore raw
Quoted 3 lines · Phil Oester

Attached is a new "route" match, which matches against entries in the kernel routing table. I've had a need for this functionality for quite some time, and finally decided to code it up.

There is also the 'realm' match..

ip route replace 10.10.96.0/20 dev sis0 realm 5 src 10.10.106.161 iptables -t mangle -A POSTROUTING -m realm 5 -j dowhateveryouwant

Quoted 14 lines

TODO: add IPv6 support to kernel side

Options

[!] --route-src-exists Route for src exists

   --route-src-eq value    Route for src exists with prefix-length == value
   --route-src-gt value    Route for src exists with prefix-length  > value
   --route-src-lt value    Route for src exists with prefix-length  < value
[!] --route-dst-exists      Route for dst exists
   --route-dst-eq value    Route for dst exists with prefix-length == value
   --route-dst-gt value    Route for dst exists with prefix-length  > value
   --route-dst-lt value    Route for dst exists with prefix-length  < value
Quoted 4 lines

Examples:

	Egress filtering:
		iptables -A FORWARD -m route ! --route-src-exists -j DROP

That's what rp_filter is for.

Quoted 2 lines
	Ingress filtering:
		iptables -A FORWARD -i $EXTERNAL_IF -m route --route-src-exists -j DROP

rp_filter again.

Quoted 2 lines
	Allow a user to only browse internal websites:
		iptables -A FORWARD -p tcp --dport 80 -m route --route-dst-exists -j ACCEPT

If there was no fitting dst route, the packet would never get to FORWARD in the first place.

Quoted 2 lines
	Allow an SNMP collector to reach all internal routers (/30 or /32 interfaces)
		iptables -A FORWARD -p udp --dport 161 -m route --route-dst-gt 29 -j ACCEPT

And this could be modeled with -s xxx/30 -d xxx/30 maybe.

#2 lore raw
Quoted 8 lines · Jan Engelhardt
Quoted 7 lines

TODO: add IPv6 support to kernel side

Examples:

	Egress filtering:
		iptables -A FORWARD -m route ! --route-src-exists -j DROP
Quoted 6 lines

That's what rp_filter is for.

Quoted 2 lines
	Ingress filtering:
		iptables -A FORWARD -i $EXTERNAL_IF -m route --route-src-exists -j DROP

rp_filter again.

Quick and not good examples, I'd say. The main problem with rp_filter is that it drops the packets >silently<. The real benefit of such a match is the possibility to log (and drop) faked packets. If the patch had IPv6 support, one could add that additionally (as far as I know) there's no rp_filter for IPv6 at all.

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#3 lore raw
Quoted 11 lines · Jozsef Kadlecsik
Quoted 5 lines
Quoted 3 lines
	Ingress filtering:
		iptables -A FORWARD -i $EXTERNAL_IF -m route --route-src-exists -j DROP

rp_filter again.

Quick and not good examples, I'd say. The main problem with rp_filter is that it drops the packets >silently<. The real benefit of such a match is the possibility to log (and drop) faked packets. If the patch had IPv6 support, one could add that additionally (as far as I know) there's no rp_filter for IPv6 at all.

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

# rp_filter
iptables -t route -A ROUTING -m route ! --route-src-exists -j DROP
# policy routing
iptables -t route -A ROUTING -m statistic --mode nth --every 2
-j ROUTE --dev eth0
# ipsec (vaguely)
iptables -t route -A OUTPUT -s 192.168.1.0/24 -d 192.168.2.0/24
-j IPSEC --mode tunnel --right-side 82.83.84.85

That would get rid of the tons of different-kind tables (xt tables, routing tables, routing rules, xfrm policy and xfrm subs) and put it into one thing people understand reasonably well.

#4 lore raw
Quoted 10 lines · Jan Engelhardt

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

# rp_filter
iptables -t route -A ROUTING -m route ! --route-src-exists -j DROP
# policy routing
iptables -t route -A ROUTING -m statistic --mode nth --every 2
-j ROUTE --dev eth0
Quoted 7 lines
# ipsec (vaguely)
iptables -t route -A OUTPUT -s 192.168.1.0/24 -d 192.168.2.0/24
-j IPSEC --mode tunnel --right-side 82.83.84.85

That would get rid of the tons of different-kind tables (xt tables, routing tables, routing rules, xfrm policy and xfrm subs) and put it into one thing people understand reasonably well.

The nf-hipac people also suggested it long time ago.

One can find a lot of smaller and bigger missing pieces, like a new table, new hook, if we'd associate a 'routing table' to a 'chain in the iptables route table', then we'd need a default policy support (i.e. default route) for the user defined chains too, etc.

If iptables were converted to netlink, that'd be a huge step toward such a goal.

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#5 lore raw
Quoted 5 lines · Jozsef Kadlecsik

One can find a lot of smaller and bigger missing pieces, like a new table, new hook, if we'd associate a 'routing table' to a 'chain in the iptables route table', then we'd need a default policy support (i.e. default route) for the user defined chains too, etc.

User-defined chains always have an implicit policy of 'RETURN', and I would not turn a knob on that property anytime. The default policy for the main chain is of course 'UNREACHABLE'.

#6 lore raw
Quoted 9 lines · Jan Engelhardt
Quoted 5 lines · Jozsef Kadlecsik

One can find a lot of smaller and bigger missing pieces, like a new table, new hook, if we'd associate a 'routing table' to a 'chain in the iptables route table', then we'd need a default policy support (i.e. default route) for the user defined chains too, etc.

User-defined chains always have an implicit policy of 'RETURN', and I would not turn a knob on that property anytime.

Routing cannot be replaced by netfilter 'route' table without supporting a 'default policy' (as default route) in the user defined chains (as routing tables).

Quoted 1 line

The default policy for the main chain is of course 'UNREACHABLE'.

Hm, I don't understand you: if we want to replace routing with a 'route' table, then the default policy (i.e. the default route) cannot be 'UNREACHABLE'.

Unless you intend to define the 'default route' as the last rule in any chain...

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#7 lore raw
Quoted 8 lines · Jozsef Kadlecsik
Quoted 7 lines · Jan Engelhardt
Quoted 5 lines · Jozsef Kadlecsik

One can find a lot of smaller and bigger missing pieces, like a new table, new hook, if we'd associate a 'routing table' to a 'chain in the iptables route table', then we'd need a default policy support (i.e. default route) for the user defined chains too, etc.

Quoted 6 lines
Quoted 2 lines

User-defined chains always have an implicit policy of 'RETURN', and I would not turn a knob on that property anytime.

Routing cannot be replaced by netfilter 'route' table without supporting a 'default policy' (as default route) in the user defined chains (as routing tables).

User-defines chains return to the main chain once control runs off their end. Since the main chain has a default policy, I do not see aproblem.

Quoted 5 lines
Quoted 1 line

The default policy for the main chain is of course 'UNREACHABLE'.

Hm, I don't understand you: if we want to replace routing with a 'route' table, then the default policy (i.e. the default route) cannot be 'UNREACHABLE'.

It can (well, we'd have to make UNREACHABLE an acceptable default policy). This is what routing does today. For example:

$ ip r g 2001::1
unreachable 2001::1 from :: dev lo  table unspec  proto none  src ::1  metric
-1  error -101 hoplimit 255
Quoted 2 lines

Unless you intend to define the 'default route' as the last rule in any chain...

No, default route would be

	iptables -t route -A ROUTING <no further conditions> -j ROUTE --via

my-default-gw-ip

Because traditional routing tables can have no default route, xtables should not be forced to have one either.

#8 lore raw
Quoted 12 lines · Jan Engelhardt
Quoted 11 lines · Jozsef Kadlecsik
Quoted 9 lines · Jan Engelhardt
Quoted 5 lines · Jozsef Kadlecsik

One can find a lot of smaller and bigger missing pieces, like a new table, new hook, if we'd associate a 'routing table' to a 'chain in the iptables route table', then we'd need a default policy support (i.e. default route) for the user defined chains too, etc.

User-defined chains always have an implicit policy of 'RETURN', and I would not turn a knob on that property anytime.

Quoted 6 lines
Quoted 3 lines

Routing cannot be replaced by netfilter 'route' table without supporting a 'default policy' (as default route) in the user defined chains (as routing tables).

User-defines chains return to the main chain once control runs off their end. Since the main chain has a default policy, I do not see aproblem.

That means we'd not support multiple routing tables with default routes. We'd loose an important functionality, which is unacceptable.

[...]

Quoted 10 lines
Quoted 2 lines

Unless you intend to define the 'default route' as the last rule in any chain...

No, default route would be

	iptables -t route -A ROUTING <no further conditions> -j ROUTE --via

my-default-gw-ip

Because traditional routing tables can have no default route, xtables should not be forced to have one either.

Routing tables can have default route. Most of the time this is the main point when using multiple routing tables: source routing via another default route.

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#9 lore raw
Quoted 8 lines · Jozsef Kadlecsik
Quoted 4 lines

User-defines chains return to the main chain once control runs off their end. Since the main chain has a default policy, I do not see aproblem.

That means we'd not support multiple routing tables with default routes. We'd loose an important functionality, which is unacceptable.

Routing tables become xtables chains. We would not lose anything. In fact, you would gain the possibilty to jump to further chains, something not possible in routing today.

rt input:

ip rule fwmark 5 table 5
ip rule fwmark 6 table 6
ip route add 10.10.96.2/32 dev eth2 table 5
ip route add via 10.10.96.1 dev eth0 table 5 # default route ip route add via 10.11.96.1 dev eth1 table 6 # default orute

in xtables (ROUTE is a terminating target):

-N table5
-N table6
-A table5 -d 10.10.96.2 -j ROUTE --dev eth2
-A table5 -j ROUTE --via 10.10.96.1 --dev eth0 # default route! -A table6 -j ROUTE --via 10.11.96.1 --dev eth1 # also a default -A ROUTING -m mark --mark 5 -j table5
-A ROUTING -m mark --mark 6 -j table6

Quoted 6 lines
Quoted 2 lines

Because traditional routing tables can have no default route, xtables should not be forced to have one either.

Routing tables can have default route. Most of the time this is the main point when using multiple routing tables: source routing via another default route.

It is very easy to not have a default gateway.

ip route flush
ip route add 10.10.96.0/20 dev eth0

Try pinging 134.76.12.5 with that. Will fail with Network Unreachable. The xtables representation henceforth:

-A ROUTING -d 10.10.96.0/20 -j ROUTE --dev eth0
-A ROUTING -j ROUTE --unreachable

or alternatively something like -P ROUTING UNREACHABLE.

#10 lore raw
Quoted 14 lines · Jan Engelhardt

Routing tables become xtables chains. We would not lose anything. In fact, you would gain the possibilty to jump to further chains, something not possible in routing today.

rt input:

ip rule fwmark 5 table 5
ip rule fwmark 6 table 6
ip route add 10.10.96.2/32 dev eth2 table 5
ip route add via 10.10.96.1 dev eth0 table 5 # default route ip route add via 10.11.96.1 dev eth1 table 6 # default orute

in xtables (ROUTE is a terminating target):

Review

Quoted 7 lines
(quoted diff) +0 -7
-N table5
-N table6
-A table5 -d 10.10.96.2 -j ROUTE --dev eth2
-A table5 -j ROUTE --via 10.10.96.1 --dev eth0  # default route!
-A table6 -j ROUTE --via 10.11.96.1 --dev eth1  # also a default
-A ROUTING -m mark --mark 5 -j table5
-A ROUTING -m mark --mark 6 -j table6

Wouldn't this break with normal routing concepts, since xtables traverses the chain linearly, while routing lookups use longest prefix match (most specific route wins)? IOW, unless xtables automatically inserted the rule into the correct spot in the chain (as opposed to just appending to the end of the chain like -A implies), you could run into situations where specifying the rules out of order would send the packet to the wrong next-hop.

Regards,
James

#11 lore raw
Quoted 12 lines · James King

Review

Quoted 8 lines
(quoted diff) +0 -7
-N table5
-N table6
-A table5 -d 10.10.96.2 -j ROUTE --dev eth2
-A table5 -j ROUTE --via 10.10.96.1 --dev eth0  # default route!
-A table6 -j ROUTE --via 10.11.96.1 --dev eth1  # also a default
-A ROUTING -m mark --mark 5 -j table5
-A ROUTING -m mark --mark 6 -j table6

Wouldn't this break with normal routing concepts, since xtables traverses the chain linearly, while routing lookups use longest prefix match (most specific route wins)?

Very well observed. However,

- the kernel is likely to do a linear rule list search (in ip ru l) too, because I would not imagine how you could optimize _that_ more than Xtables when it has to compare src/dst addresses, fwmarks, etc.

- you are right, the *routing* table is sorted by longest prefix, and that is probably why people tout it as "optimized for the case". But I would not see anything wrong with the user doing the sorting (or even Xtables).

(Maybe I just want a new command-line interface for routing instead
of a new in-core architecture...)

Quoted 5 lines

IOW, unless xtables automatically
inserted the rule into the correct spot in the chain (as opposed to just appending to the end of the chain like -A implies), you could run into situations where specifying the rules out of order would send the packet to the wrong next-hop.

Or you could actually optimize it. If, say 95% of all your packets go out on eth0, the following route table (which is dependent upon cabling), with its ordering, is suboptimal:

	192.168.1.1/32 dev isp0
	192.168.1.0/24 dev eth0

Relaxing the longest-prefix ordering would mean less traversal time through the chains (i.e. minimizing the time taken for routing table traversal for randomly distributed packets, but weighted according to network situation, over a given time period.)

#12 lore raw
In reply to Jan Engelhardt
Quoted 8 lines · Jan Engelhardt
Quoted 6 lines · Jozsef Kadlecsik
Quoted 5 lines
Quoted 3 lines
	Ingress filtering:
		iptables -A FORWARD -i $EXTERNAL_IF -m route --route-src-exists -j DROP

rp_filter again.

Quoted 8 lines
Quoted 5 lines

Quick and not good examples, I'd say. The main problem with rp_filter is that it drops the packets >silently<. The real benefit of such a match is the possibility to log (and drop) faked packets. If the patch had IPv6 support, one could add that additionally (as far as I know) there's no rp_filter for IPv6 at all.

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

Because routing in a firewall is slow and too complicated?

Best regards,

Krzysztof Olędzki

#13 lore raw
Quoted 5 lines · Krzysztof Oledzki
Quoted 3 lines

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

Because routing in a firewall is slow and too complicated?

Proof?

#14 lore raw
Quoted 9 lines · Jan Engelhardt
Quoted 5 lines · Krzysztof Oledzki
Quoted 3 lines

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

Because routing in a firewall is slow and too complicated?

Proof?

Proof of what? Of a performance penalty?

http://people.netfilter.org/kadlec/nftest.pdf

# ip route show table all|wc -l
456292

Best regards,

Krzysztof Olędzki

#15 lore raw
In reply to Jan Engelhardt
Quoted 9 lines · Jan Engelhardt
Quoted 5 lines · Krzysztof Oledzki
Quoted 3 lines

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

Because routing in a firewall is slow and too complicated?

Proof?

Its obvious, routing lookups are optimized for the specific case while iptables is trying to be generic.

#16 lore raw
In reply to Krzysztof Oledzki
Quoted 11 lines · Krzysztof Oledzki
Quoted 9 lines
Quoted 6 lines

Quick and not good examples, I'd say. The main problem with rp_filter is that it drops the packets >silently<. The real benefit of such a match is the possibility to log (and drop) faked packets. If the patch had IPv6 support, one could add that additionally (as far as I know) there's no rp_filter for IPv6 at all.

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

Because routing in a firewall is slow and too complicated?

[Citation needed]

I fail to see why Xtables (not just a firewall) should be slower than the routing table (without routing cache, to make it fair).

Iterating over an IPv4 table compares, at a minimum, interface name(s), src/dst address and an L4 protocol, plus extra flags like inversion.

Routing? Well, let's see, it can match src address, src interface, out interface, dst address, and so on. Sounds pretty much like the same.

#17 lore raw
Quoted 2 lines · Jan Engelhardt

I fail to see why Xtables (not just a firewall) should be slower than the routing table (without routing cache, to make it fair).

What about the routing cache, though?

Our Linux-based routers handle considerably less than 1Gbps peak, and they drop packets when the cache is flushed. (So I had to set the automatic flush to 86400 seconds instead of 10).

It would be necessary to either reinvent the route cache or come up with something better. Preferably the latter, of course.

/Benny

#18 lore raw
In reply to Jan Engelhardt
Quoted 2 lines · Jan Engelhardt

You know what's been bugging me... why don't we replace the entire routing infrastructure by an xtables "route" table, with something like

It's been discussed off-list many times.

The problem with the routing table is that it's more to that table than what's visible. It's heavily cached and annotated by the kernel beyond just the plain routing table, and used in a number of different ways such as when assigning the source address on outgoing connections, way before there even is a packet.

Regards
Henrik

#19 lore raw
In reply to Jozsef Kadlecsik
Quoted 8 lines · Jozsef Kadlecsik
Quoted 6 lines · Jan Engelhardt
Quoted 6 lines

TODO: add IPv6 support to kernel side

Examples:

	Egress filtering:
		iptables -A FORWARD -m route ! --route-src-exists -j DROP
Quoted 7 lines
Quoted 5 lines

That's what rp_filter is for.

Quoted 2 lines
	Ingress filtering:
		iptables -A FORWARD -i $EXTERNAL_IF -m route --route-src-exists -j DROP

rp_filter again.

Quick and not good examples, I'd say. The main problem with rp_filter is that it drops the packets >silently<. The real benefit of such a match is the possibility to log (and drop) faked packets. If the patch had IPv6 support, one could add that additionally (as far as I know) there's no rp_filter for IPv6 at all.

I agree. While I find rp_filter highly annoying (Debian has it enabled by default), this patch at least allows you to notice whats going on easily. I'm also sure there are people who would like to use rp_filter like functionality for IPv6.

#20 lore raw
Quoted 22 lines · Patrick McHardy
Quoted 13 lines · Jozsef Kadlecsik
Quoted 11 lines · Jan Engelhardt
Quoted 6 lines

TODO: add IPv6 support to kernel side

Examples:

	Egress filtering:
		iptables -A FORWARD -m route ! --route-src-exists -j DROP

That's what rp_filter is for.

Quoted 3 lines

Ingress filtering:
iptables -A FORWARD -i $EXTERNAL_IF -m route
--route-src-exists -j DROP

Quoted 7 lines
Quoted 1 line

rp_filter again.

Quick and not good examples, I'd say. The main problem with rp_filter is that it drops the packets >silently<. The real benefit of such a match is the possibility to log (and drop) faked packets. If the patch had IPv6 support, one could add that additionally (as far as I know) there's no rp_filter for IPv6 at all.

Quoted 4 lines

I agree. While I find rp_filter highly annoying (Debian has it enabled by default), this patch at least allows you to notice whats going on easily. I'm also sure there are people who would like to use rp_filter like functionality for IPv6.

A single loop in the routing paths is enough and rp_filter enabled ruins everything. :-)

If there's dynamic routing (say OSPF), then sometimes it's not so easy at all to write (static) iptables rules for egress/ingress filtering. That is also a case where the route match can be important (besides the possibility to log nasty packets when the routing is trivial).

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#21 lore raw
Quoted 4 lines · Jozsef Kadlecsik

If there's dynamic routing (say OSPF), then sometimes it's not so easy at all to write (static) iptables rules for egress/ingress filtering. That is also a case where the route match can be important (besides the possibility to log nasty packets when the routing is trivial).

I have to correct myself: when rp_filter produces false results then the route match does also at egress/ingress filtering.

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#22 lore raw

Re: [PATCH,RFC] Route match, kernel

In reply to Phil Oester
Quoted 6 lines
net/netfilter/xt_route.c new file +5 -0
+	case XT_ROUTE_SRC_EXISTS:
+		fl.nl_u.ip4_u.daddr = iph->saddr;
+		ret = fib_lookup(dev_net(in), &fl, &res);
+		if (ret == 0 && res.prefixlen != 0) {
+			fib_res_put(&res);

net/netfilter/xt_route.c line 50

Quoted 10 lines
net/netfilter/xt_route.c new file +10 -0
+			return true ^ info->invert;
+		}
+	case XT_ROUTE_SRC_EQ:
+		fl.nl_u.ip4_u.daddr = iph->saddr;
+		ret = fib_lookup(dev_net(in), &fl, &res);
+		if (ret == 0 && res.prefixlen == info->prefixlen) {
+			fib_res_put(&res);
+			return true;
+		}
+		break;

Should not these be true ^ info->invert too?

I would probably combine mode and invert, to save a byte.

net/netfilter/xt_route.c line 111

Quoted 8 lines
net/netfilter/xt_route.c new file +8 -0
+static bool
+route_mt6(const struct sk_buff *skb, const struct net_device *in,
+           const struct net_device *out, const struct xt_match *match,
+           const void *matchinfo, int offset, unsigned int protoff,
+           bool *hotdrop)
+{
+	return 1;
+}

If you do not support IPv6, do not write a function for it ;-)

author
#23 lore raw

Re: [PATCH,RFC] Route match, kernel

Quoted 10 lines · Jan Engelhardt
Quoted 10 lines
net/netfilter/xt_route.c new file +9 -0
+	case XT_ROUTE_SRC_EXISTS:
+		fl.nl_u.ip4_u.daddr = iph->saddr;
+		ret = fib_lookup(dev_net(in), &fl, &res);
+		if (ret == 0 && res.prefixlen != 0) {
+			fib_res_put(&res);
+			return true ^ info->invert;
+		}
+	case XT_ROUTE_SRC_EQ:
+		fl.nl_u.ip4_u.daddr = iph->saddr;
Quoted 8 lines

net/netfilter/xt_route.c line 50

Quoted 6 lines
net/netfilter/xt_route.c new file +6 -0
+		ret = fib_lookup(dev_net(in), &fl, &res);
+		if (ret == 0 && res.prefixlen == info->prefixlen) {
+			fib_res_put(&res);
+			return true;
+		}
+		break;

Should not these be true ^ info->invert too?

No - invert is only supported on the *_EXISTS case. Didn't see any point in supporting invert on gt/lt, but now that I think of it perhaps supporting it on _EQ may be useful. But can you see a point on gt/lt?

Quoted 1 line

I would probably combine mode and invert, to save a byte.

Possible.

Quoted 10 lines

net/netfilter/xt_route.c line 111

Quoted 8 lines
net/netfilter/xt_route.c new file +8 -0
+static bool
+route_mt6(const struct sk_buff *skb, const struct net_device *in,
+           const struct net_device *out, const struct xt_match *match,
+           const void *matchinfo, int offset, unsigned int protoff,
+           bool *hotdrop)
+{
+	return 1;
+}

If you do not support IPv6, do not write a function for it ;-)

OK, so I should have #if 0 it...but this is just an RFC and that was listed in the TODO ;-)

Thanks for the review BTW.

Phil

#24 lore raw

Re: [PATCH,RFC] Route match, user

In reply to Phil Oester

extensions/libxt_route.c line 39

Quoted 12 lines
extensions/libxt_route.c new file +11 -0
+static const struct option route_opts[] = {
+	{ "route-src-exists", 0, NULL, '1' },
+	{ "route-src-eq", 1, NULL, '2' },
+	{ "route-src-gt", 1, NULL, '3' },
+	{ "route-src-lt", 1, NULL, '4' },
+	{ "route-dst-exists", 0, NULL, '5' },
+	{ "route-dst-eq", 1, NULL, '6' },
+	{ "route-dst-gt", 1, NULL, '7' },
+	{ "route-dst-lt", 1, NULL, '8' },
+	{ .name = NULL }
+};

You are mixing non-C99 and C99 style.

extensions/libxt_route.c line 47

Quoted 7 lines
extensions/libxt_route.c new file +7 -0
+static int route_parse(int c, char **argv, int invert, unsigned int *flags,
+                       const void *entry, struct xt_entry_match **match)
+{
+	struct xt_route_info *info = (struct xt_route_info *) (*match)->data;
+	unsigned int value;
+
+	check_inverse(optarg, &invert, &optind, 0);

I would not bother supporting intraplaced ! at all. Just remove that line.

extensions/libxt_route.c line 215

Quoted 12 lines
extensions/libxt_route.c new file +12 -0
+		case '6':
+			if (invert) 
+				exit_error(PARAMETER_PROBLEM,
+						"route: unexpected `!'");
+
+			if (string_to_number(optarg, 1, 128, &value) == -1)
+				exit_error(PARAMETER_PROBLEM,
+				           "route: Expected prefix between 1 and 128");
+
+			info->mode = XT_ROUTE_DST_EQ;
+			info->value = value;
+			break;

There is a lot of code to unify between route_mt_parse and route_mt6_parse.

author
#25 lore raw

Re: [PATCH,RFC] Route match, user

Quoted 14 lines · Jan Engelhardt

extensions/libxt_route.c line 39

Quoted 12 lines
extensions/libxt_route.c new file +11 -0
+static const struct option route_opts[] = {
+	{ "route-src-exists", 0, NULL, '1' },
+	{ "route-src-eq", 1, NULL, '2' },
+	{ "route-src-gt", 1, NULL, '3' },
+	{ "route-src-lt", 1, NULL, '4' },
+	{ "route-dst-exists", 0, NULL, '5' },
+	{ "route-dst-eq", 1, NULL, '6' },
+	{ "route-dst-gt", 1, NULL, '7' },
+	{ "route-dst-lt", 1, NULL, '8' },
+	{ .name = NULL }
+};

You are mixing non-C99 and C99 style.

Copy/paste from other iptables code. Guess a new match should remain consistent.

Quoted 9 lines

extensions/libxt_route.c line 47

Quoted 7 lines
extensions/libxt_route.c new file +7 -0
+static int route_parse(int c, char **argv, int invert, unsigned int *flags,
+                       const void *entry, struct xt_entry_match **match)
+{
+	struct xt_route_info *info = (struct xt_route_info *) (*match)->data;
+	unsigned int value;
+
+	check_inverse(optarg, &invert, &optind, 0);

I would not bother supporting intraplaced ! at all. Just remove that line.

Sorry..."intraplaced"?

Quoted 6 lines
Quoted 6 lines
extensions/libxt_route.c new file +6 -0
+		case '6':
+			if (invert) 
+				exit_error(PARAMETER_PROBLEM,
+						"route: unexpected `!'");
+
+			if (string_to_number(optarg, 1, 128, &value) == -1)
Quoted 8 lines

extensions/libxt_route.c line 215

Quoted 6 lines
extensions/libxt_route.c new file +6 -0
+				exit_error(PARAMETER_PROBLEM,
+				           "route: Expected prefix between 1 and 128");
+
+			info->mode = XT_ROUTE_DST_EQ;
+			info->value = value;
+			break;

There is a lot of code to unify between route_mt_parse and route_mt6_parse.

I suppose I could pass a 32 or 128 to a common function, yes.

Phil

#26 lore raw

Re: [PATCH,RFC] Route match, user

Quoted 11 lines · Phil Oester
Quoted 10 lines

extensions/libxt_route.c line 47

Quoted 8 lines
extensions/libxt_route.c new file +7 -0
+static int route_parse(int c, char **argv, int invert, unsigned int *flags,
+                       const void *entry, struct xt_entry_match **match)
+{
+	struct xt_route_info *info = (struct xt_route_info *) (*match)->data;
+	unsigned int value;
+
+	check_inverse(optarg, &invert, &optind, 0);

I would not bother supporting intraplaced ! at all. Just remove that line.

Quoted 1 line

Sorry..."intraplaced"?

intra = within/inside

There is

	iptables ! -s 192.168.1.5	(normal)
and
	iptables -s ! 192.168.1.5	(intraplaced)

Considering that most extensions only support the normal form, I would not add any more intraplace support to new modules to reduce the number of supported styles. Real robustness can only be achieved when one is strict.