[PATCH 1/2] [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu

[PATCH 1/2] [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu

[Jan Engelhardt]

`git log -p` just flies better than `git-format-patch` I guess.

===
commit 1ed78bbf75455dfe77a028f2da2f351cff4028f2
Author: Jan Engelhardt <jengelh@computergmbh.de>
Date:   Sun Jan 20 13:13:33 2008 +0100

    [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu
    
    The TCPMSS target in Xtables should consider the MTU of the input
    device on forwarded packets as part of the path MTU.
    
    Point in case: IN=ppp0, OUT=eth0. MSS set to 1460 in spite of MTU of
    ppp0 being 1392.
    
    Signed-off-by: Jaco Kroon <jaco@uls.co.za>
    Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>

diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 60e3767..2c446b6 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -41,6 +41,7 @@ optlen(const u_int8_t *opt, unsigned int offset)
 static int
 tcpmss_mangle_packet(struct sk_buff *skb,
 		     const struct xt_tcpmss_info *info,
+		     unsigned int in_mtu,
 		     unsigned int tcphoff,
 		     unsigned int minlen)
 {
@@ -76,7 +77,13 @@ tcpmss_mangle_packet(struct sk_buff *skb,
 				       dst_mtu(skb->dst));
 			return -1;
 		}
-		newmss = dst_mtu(skb->dst) - minlen;
+		if (in_mtu <= minlen) {
+			if (net_ratelimit())
+				printk(KERN_ERR "xt_TCPMSS: unknown or "
+				       "invalid path-MTU (%u)\n", in_mtu);
+			return -1;
+		}
+		newmss = min(dst_mtu(skb->dst), in_mtu) - minlen;
 	} else
 		newmss = info->mss;
 
@@ -146,8 +153,8 @@ tcpmss_tg4(struct sk_buff *skb, const struct net_device *in,
 	__be16 newlen;
 	int ret;
 
-	ret = tcpmss_mangle_packet(skb, targinfo, iph->ihl * 4,
-				   sizeof(*iph) + sizeof(struct tcphdr));
+	ret = tcpmss_mangle_packet(skb, targinfo, (in == NULL) ? ~0U : in->mtu,
+	      iph->ihl * 4, sizeof(*iph) + sizeof(struct tcphdr));
 	if (ret < 0)
 		return NF_DROP;
 	if (ret > 0) {
@@ -174,8 +181,8 @@ tcpmss_tg6(struct sk_buff *skb, const struct net_device *in,
 	tcphoff = ipv6_skip_exthdr(skb, sizeof(*ipv6h), &nexthdr);
 	if (tcphoff < 0)
 		return NF_DROP;
-	ret = tcpmss_mangle_packet(skb, targinfo, tcphoff,
-				   sizeof(*ipv6h) + sizeof(struct tcphdr));
+	ret = tcpmss_mangle_packet(skb, targinfo, (in == NULL) ? ~0U : in->mtu,
+	      tcphoff, sizeof(*ipv6h) + sizeof(struct tcphdr));
 	if (ret < 0)
 		return NF_DROP;
 	if (ret > 0) {

[PATCH 2/2] [NETFILTER]: xt_owner: allow matching UID/GID ranges

[Jan Engelhardt]

commit 1ab123486c698860966193d254db54f8a4d428b4
Author: Jan Engelhardt <jengelh@computergmbh.de>
Date:   Sun Jan 20 13:15:08 2008 +0100

    [NETFILTER]: xt_owner: allow matching UID/GID ranges
    
    Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>

diff --git a/include/linux/netfilter/xt_owner.h b/include/linux/netfilter/xt_owner.h
index eacd34e..c84e52c 100644
--- a/include/linux/netfilter/xt_owner.h
+++ b/include/linux/netfilter/xt_owner.h
@@ -8,8 +8,8 @@ enum {
 };
 
 struct xt_owner_match_info {
-	u_int32_t uid;
-	u_int32_t gid;
+	u_int32_t uid_min, uid_max;
+	u_int32_t gid_min, gid_max;
 	u_int8_t match, invert;
 };
 
diff --git a/net/netfilter/xt_owner.c b/net/netfilter/xt_owner.c
index d382f9c..9059c16 100644
--- a/net/netfilter/xt_owner.c
+++ b/net/netfilter/xt_owner.c
@@ -4,8 +4,8 @@
  *
  * (C) 2000 Marc Boucher <marc@mbsi.ca>
  *
- * Copyright © CC Computer Consultants GmbH, 2007
- * Contact: <jengelh@computergmbh.de>
+ * Copyright © CC Computer Consultants GmbH, 2007 - 2008
+ * <jengelh@computergmbh.de>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -102,13 +102,15 @@ owner_mt(const struct sk_buff *skb, const struct net_device *in,
 		       (XT_OWNER_UID | XT_OWNER_GID)) == 0;
 
 	if (info->match & XT_OWNER_UID)
-		if ((filp->f_uid != info->uid) ^
-		    !!(info->invert & XT_OWNER_UID))
+		if ((filp->f_uid >= info->uid_min &&
+		    filp->f_uid <= info->uid_max) ^
+		    !(info->invert & XT_OWNER_UID))
 			return false;
 
 	if (info->match & XT_OWNER_GID)
-		if ((filp->f_gid != info->gid) ^
-		    !!(info->invert & XT_OWNER_GID))
+		if ((filp->f_gid >= info->gid_min &&
+		    filp->f_gid <= info->gid_max) ^
+		    !(info->invert & XT_OWNER_GID))
 			return false;
 
 	return true;
-
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] [NETFILTER]: xt_owner: allow matching UID/GID ranges

[Patrick McHardy]

Jan Engelhardt wrote:
> commit 1ab123486c698860966193d254db54f8a4d428b4
> Author: Jan Engelhardt <jengelh@computergmbh.de>
> Date:   Sun Jan 20 13:15:08 2008 +0100
> 
>     [NETFILTER]: xt_owner: allow matching UID/GID ranges


Is that actually useful? The GID already allows to match
on entire groups, this seems like a "let do it just because
we can" patch to me.

Re: [PATCH 2/2] [NETFILTER]: xt_owner: allow matching UID/GID ranges

[Jan Engelhardt]

On Jan 20 2008 14:50, Patrick McHardy wrote:
>
> Jan Engelhardt wrote:
>> commit 1ab123486c698860966193d254db54f8a4d428b4
>> Author: Jan Engelhardt <jengelh@computergmbh.de>
>> Date:   Sun Jan 20 13:15:08 2008 +0100
>> 
>>     [NETFILTER]: xt_owner: allow matching UID/GID ranges
>
>
> Is that actually useful? The GID already allows to match
> on entire groups, this seems like a "let do it just because
> we can" patch to me.
>
Of course there is a use case. System with like 2000 students; the
user database is historically grown, so UIDs are 'consecutively
random', i.e. order depends on time the user account was added.
Preallocating an UID range to students is therefore not anymore
possible.

	* I do not want to add 2000 -m owner rules,
	that would just be totally inefficient.

I could add rules for blocks of UIDs (usually they do get added in
batch), but...

	* that's still lots!
	2032-5241, 6010-6185, 10001-10209, 10214, 10235-10422, ...

So, we turn to the GID. Because the GID of a user depends on the
grade (and that changes over time), preallocating UID ranges is not
even feasible. But well, at least the number of rules is down:

	* 1301, 1302, 1303, 1304, 1305...

Can we simplify that? Yes, with GID ranges.

	* 1301-1334
	* 1352-1364 (blame legislation for this new range...)

Two rules, I'm stunned! :-)

Re: [PATCH 2/2] [NETFILTER]: xt_owner: allow matching UID/GID ranges

[Patrick McHardy]

Jan Engelhardt wrote:
> commit 1ab123486c698860966193d254db54f8a4d428b4
> Author: Jan Engelhardt <jengelh@computergmbh.de>
> Date:   Sun Jan 20 13:15:08 2008 +0100
> 
>     [NETFILTER]: xt_owner: allow matching UID/GID ranges


Applied, thanks. Did you already send a patch for userspace for
this?

Re: [PATCH 1/2] [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu

[Patrick McHardy]

Jan Engelhardt wrote:
> commit 1ed78bbf75455dfe77a028f2da2f351cff4028f2
> Author: Jan Engelhardt <jengelh@computergmbh.de>
> Date:   Sun Jan 20 13:13:33 2008 +0100
> 
>     [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu
>     
>     The TCPMSS target in Xtables should consider the MTU of the input
>     device on forwarded packets as part of the path MTU.
>     
>     Point in case: IN=ppp0, OUT=eth0. MSS set to 1460 in spite of MTU of
>     ppp0 being 1392.


> -				   sizeof(*ipv6h) + sizeof(struct tcphdr));
> +	ret = tcpmss_mangle_packet(skb, targinfo, (in == NULL) ? ~0U : in->mtu,
> +	      tcphoff, sizeof(*ipv6h) + sizeof(struct tcphdr));


This is slightly better than what we currently have, but it should
actually use the MTU from the dst_entry for the reverse direction.
I've tried adding this not too long ago and ran into some problems,
but I can't remember the exact details.

Re: [PATCH 1/2] [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu

[Jan Engelhardt]

On Jan 20 2008 14:48, Patrick McHardy wrote:
>> -				   sizeof(*ipv6h) + sizeof(struct tcphdr));
>> +	ret = tcpmss_mangle_packet(skb, targinfo, (in == NULL) ? ~0U : in->mtu,
>> +	      tcphoff, sizeof(*ipv6h) + sizeof(struct tcphdr));
>
>
> This is slightly better than what we currently have, but it should
> actually use the MTU from the dst_entry for the reverse direction.
> I've tried adding this not too long ago and ran into some problems,
> but I can't remember the exact details.
>

Jaco, please test this one (goes on top of the previous MTU patch).

===Patch begins===
commit f75cb772940a1c18e36166a962b54ec60b89b2fa
Author: Jan Engelhardt <jengelh@computergmbh.de>
Date:   Tue Jan 22 21:52:43 2008 +0100

    [NETFILTER]: xt_TCPMSS: Consider reverse route MTU in clamp-to-pmtu
    
    Just using in_dev->mtu is not right; what we need is the reverse
    route MTU.
    
    Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>

diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 2c446b6..e0586e3 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -13,7 +13,10 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/tcp.h>
+#include <net/dst.h>
+#include <net/flow.h>
 #include <net/ipv6.h>
+#include <net/route.h>
 #include <net/tcp.h>
 
 #include <linux/netfilter_ipv4/ip_tables.h>
@@ -144,6 +147,21 @@ tcpmss_mangle_packet(struct sk_buff *skb,
 	return TCPOLEN_MSS;
 }
 
+static u_int16_t tcpmss_reverse_mtu4(const struct iphdr *iph)
+{
+	struct rtable *rt = NULL;
+	struct flowi fl = {.nl_u = {.ip4_u = {
+		.daddr = iph->saddr,
+		.tos   = RT_TOS(iph->tos),
+		.scope = RT_SCOPE_UNIVERSE,
+	}}};
+
+	ip_route_output_key(&rt, &fl);
+	if (rt == NULL)
+		return ~(u_int16_t)0;
+	return dst_mtu(&rt->u.dst);
+}
+
 static unsigned int
 tcpmss_tg4(struct sk_buff *skb, const struct net_device *in,
            const struct net_device *out, unsigned int hooknum,
@@ -153,7 +171,7 @@ tcpmss_tg4(struct sk_buff *skb, const struct net_device *in,
 	__be16 newlen;
 	int ret;
 
-	ret = tcpmss_mangle_packet(skb, targinfo, (in == NULL) ? ~0U : in->mtu,
+	ret = tcpmss_mangle_packet(skb, targinfo, tcpmss_reverse_mtu4(iph),
 	      iph->ihl * 4, sizeof(*iph) + sizeof(struct tcphdr));
 	if (ret < 0)
 		return NF_DROP;
@@ -167,6 +185,22 @@ tcpmss_tg4(struct sk_buff *skb, const struct net_device *in,
 }
 
 #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+static u_int16_t tcpmss_reverse_mtu6(const struct ipv6hdr *iph)
+{
+	struct rtable *rt = NULL;
+	struct flowi fl = {.nl_u = {.ip6_u = {
+		.daddr     = iph->saddr,
+		.flowlabel = ((iph->flow_lbl[0] << 16) |
+		             (iph->flow_lbl[1] << 8) | iph->flow_lbl[2]) &
+		             0x00FFFFFF,
+	}}};
+
+	ip_route_output_key(&rt, &fl);
+	if (rt == NULL)
+		return ~(u_int16_t)0;
+	return dst_mtu(&rt->u.dst);
+}
+
 static unsigned int
 tcpmss_tg6(struct sk_buff *skb, const struct net_device *in,
            const struct net_device *out, unsigned int hooknum,
@@ -181,7 +215,7 @@ tcpmss_tg6(struct sk_buff *skb, const struct net_device *in,
 	tcphoff = ipv6_skip_exthdr(skb, sizeof(*ipv6h), &nexthdr);
 	if (tcphoff < 0)
 		return NF_DROP;
-	ret = tcpmss_mangle_packet(skb, targinfo, (in == NULL) ? ~0U : in->mtu,
+	ret = tcpmss_mangle_packet(skb, targinfo, tcpmss_reverse_mtu6(ipv6h),
 	      tcphoff, sizeof(*ipv6h) + sizeof(struct tcphdr));
 	if (ret < 0)
 		return NF_DROP;

Re: [PATCH 1/2] [NETFILTER]: xt_TCPMSS: Consider incoming device's MTU in clamp-to-pmtu

[Patrick McHardy]

Jan Engelhardt wrote:
> +static u_int16_t tcpmss_reverse_mtu4(const struct iphdr *iph)
> +{
> +	struct rtable *rt = NULL;
> +	struct flowi fl = {.nl_u = {.ip4_u = {
> +		.daddr = iph->saddr,
> +		.tos   = RT_TOS(iph->tos),
> +		.scope = RT_SCOPE_UNIVERSE,
> +	}}};
> +
> +	ip_route_output_key(&rt, &fl);
> +	if (rt == NULL)
> +		return ~(u_int16_t)0;
> +	return dst_mtu(&rt->u.dst);

This leaks the dst_entry.

>  #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
> +static u_int16_t tcpmss_reverse_mtu6(const struct ipv6hdr *iph)
> +{
> +	struct rtable *rt = NULL;
> +	struct flowi fl = {.nl_u = {.ip6_u = {
> +		.daddr     = iph->saddr,
> +		.flowlabel = ((iph->flow_lbl[0] << 16) |
> +		             (iph->flow_lbl[1] << 8) | iph->flow_lbl[2]) &
> +		             0x00FFFFFF,
> +	}}};
> +
> +	ip_route_output_key(&rt, &fl);
> +	if (rt == NULL)
> +		return ~(u_int16_t)0;
> +	return dst_mtu(&rt->u.dst);

This also leaks and you probably want to do IPv6 routing :) But the
patch should use the ->route function from nf_af_info anyways
to avoid adding module dependencies.