* [PATCH 1/4] netfilter: more strict TCP flag matching in SYNPROXY
2013-09-04 13:00 [PATCH 0/4] netfilter updates for net-next Pablo Neira Ayuso
@ 2013-09-04 13:00 ` Pablo Neira Ayuso
2013-09-04 13:00 ` [PATCH 2/4] netfilter: synproxy_core: fix warning in __nf_ct_ext_add_length() Pablo Neira Ayuso
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-04 13:00 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Jesper Dangaard Brouer <brouer@redhat.com>
Its seems Patrick missed to incoorporate some of my requested changes
during review v2 of SYNPROXY netfilter module.
Which were, to avoid SYN+ACK packets to enter the path, meant for the
ACK packet from the client (from the 3WHS).
Further there were a bug in ip6t_SYNPROXY.c, for matching SYN packets
that didn't exclude the ACK flag.
Go a step further with SYN packet/flag matching by excluding flags
ACK+FIN+RST, in both IPv4 and IPv6 modules.
The intented usage of SYNPROXY is as follows:
(gracefully describing usage in commit)
iptables -t raw -A PREROUTING -i eth0 -p tcp --dport 80 --syn -j NOTRACK
iptables -A INPUT -i eth0 -p tcp --dport 80 -m state UNTRACKED,INVALID \
-j SYNPROXY --sack-perm --timestamp --mss 1480 --wscale 7 --ecn
echo 0 > /proc/sys/net/netfilter/nf_conntrack_tcp_loose
This does filter SYN flags early, for packets in the UNTRACKED state,
but packets in the INVALID state with other TCP flags could still
reach the module, thus this stricter flag matching is still needed.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/ipv4/netfilter/ipt_SYNPROXY.c | 4 ++--
net/ipv6/netfilter/ip6t_SYNPROXY.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 94371db..90e489e 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -269,7 +269,7 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
synproxy_parse_options(skb, par->thoff, th, &opts);
- if (th->syn && !th->ack) {
+ if (th->syn && !(th->ack || th->fin || th->rst)) {
/* Initial SYN from client */
this_cpu_inc(snet->stats->syn_received);
@@ -285,7 +285,7 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
XT_SYNPROXY_OPT_ECN);
synproxy_send_client_synack(skb, th, &opts);
- } else if (th->ack && !(th->fin || th->rst))
+ } else if (th->ack && !(th->fin || th->rst || th->syn))
/* ACK from client */
synproxy_recv_client_ack(snet, skb, th, &opts, ntohl(th->seq));
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 4270a9b..a5af0bf 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -284,7 +284,7 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
synproxy_parse_options(skb, par->thoff, th, &opts);
- if (th->syn) {
+ if (th->syn && !(th->ack || th->fin || th->rst)) {
/* Initial SYN from client */
this_cpu_inc(snet->stats->syn_received);
@@ -300,7 +300,7 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
XT_SYNPROXY_OPT_ECN);
synproxy_send_client_synack(skb, th, &opts);
- } else if (th->ack && !(th->fin || th->rst))
+ } else if (th->ack && !(th->fin || th->rst || th->syn))
/* ACK from client */
synproxy_recv_client_ack(snet, skb, th, &opts, ntohl(th->seq));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] netfilter: synproxy_core: fix warning in __nf_ct_ext_add_length()
2013-09-04 13:00 [PATCH 0/4] netfilter updates for net-next Pablo Neira Ayuso
2013-09-04 13:00 ` [PATCH 1/4] netfilter: more strict TCP flag matching in SYNPROXY Pablo Neira Ayuso
@ 2013-09-04 13:00 ` Pablo Neira Ayuso
2013-09-04 13:00 ` [PATCH 3/4] netfilter: SYNPROXY: let unrelated packets continue Pablo Neira Ayuso
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-04 13:00 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Patrick McHardy <kaber@trash.net>
With CONFIG_NETFILTER_DEBUG we get the following warning during SYNPROXY init:
[ 80.558906] WARNING: CPU: 1 PID: 4833 at net/netfilter/nf_conntrack_extend.c:80 __nf_ct_ext_add_length+0x217/0x220 [nf_conntrack]()
The reason is that the conntrack template is set to confirmed before adding
the extension and it is invalid to add extensions to already confirmed
conntracks. Fix by adding the extensions before setting the conntrack to
confirmed.
Reported-by: Jesper Dangaard Brouer <jesper.brouer@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_synproxy_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_synproxy_core.c b/net/netfilter/nf_synproxy_core.c
index d23dc79..6fd967c 100644
--- a/net/netfilter/nf_synproxy_core.c
+++ b/net/netfilter/nf_synproxy_core.c
@@ -356,12 +356,12 @@ static int __net_init synproxy_net_init(struct net *net)
goto err1;
}
- __set_bit(IPS_TEMPLATE_BIT, &ct->status);
- __set_bit(IPS_CONFIRMED_BIT, &ct->status);
if (!nfct_seqadj_ext_add(ct))
goto err2;
if (!nfct_synproxy_ext_add(ct))
goto err2;
+ __set_bit(IPS_TEMPLATE_BIT, &ct->status);
+ __set_bit(IPS_CONFIRMED_BIT, &ct->status);
snet->tmpl = ct;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] netfilter: SYNPROXY: let unrelated packets continue
2013-09-04 13:00 [PATCH 0/4] netfilter updates for net-next Pablo Neira Ayuso
2013-09-04 13:00 ` [PATCH 1/4] netfilter: more strict TCP flag matching in SYNPROXY Pablo Neira Ayuso
2013-09-04 13:00 ` [PATCH 2/4] netfilter: synproxy_core: fix warning in __nf_ct_ext_add_length() Pablo Neira Ayuso
@ 2013-09-04 13:00 ` Pablo Neira Ayuso
2013-09-04 13:00 ` [PATCH 4/4] netfilter: xt_TCPMSS: correct return value in tcpmss_mangle_packet Pablo Neira Ayuso
2013-09-04 16:28 ` [PATCH 0/4] netfilter updates for net-next David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-04 13:00 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Jesper Dangaard Brouer <brouer@redhat.com>
Packets reaching SYNPROXY were default dropped, as they were most
likely invalid (given the recommended state matching). This
patch, changes SYNPROXY target to let packets, not consumed,
continue being processed by the stack.
This will be more in line other target modules. As it will allow
more flexible configurations of handling, logging or matching on
packets in INVALID states.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/ipv4/netfilter/ipt_SYNPROXY.c | 8 ++++++--
net/ipv6/netfilter/ip6t_SYNPROXY.c | 8 ++++++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 90e489e..67e17dc 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -285,11 +285,15 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
XT_SYNPROXY_OPT_ECN);
synproxy_send_client_synack(skb, th, &opts);
- } else if (th->ack && !(th->fin || th->rst || th->syn))
+ return NF_DROP;
+
+ } else if (th->ack && !(th->fin || th->rst || th->syn)) {
/* ACK from client */
synproxy_recv_client_ack(snet, skb, th, &opts, ntohl(th->seq));
+ return NF_DROP;
+ }
- return NF_DROP;
+ return XT_CONTINUE;
}
static unsigned int ipv4_synproxy_hook(unsigned int hooknum,
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index a5af0bf..19cfea8 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -300,11 +300,15 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
XT_SYNPROXY_OPT_ECN);
synproxy_send_client_synack(skb, th, &opts);
- } else if (th->ack && !(th->fin || th->rst || th->syn))
+ return NF_DROP;
+
+ } else if (th->ack && !(th->fin || th->rst || th->syn)) {
/* ACK from client */
synproxy_recv_client_ack(snet, skb, th, &opts, ntohl(th->seq));
+ return NF_DROP;
+ }
- return NF_DROP;
+ return XT_CONTINUE;
}
static unsigned int ipv6_synproxy_hook(unsigned int hooknum,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] netfilter: xt_TCPMSS: correct return value in tcpmss_mangle_packet
2013-09-04 13:00 [PATCH 0/4] netfilter updates for net-next Pablo Neira Ayuso
` (2 preceding siblings ...)
2013-09-04 13:00 ` [PATCH 3/4] netfilter: SYNPROXY: let unrelated packets continue Pablo Neira Ayuso
@ 2013-09-04 13:00 ` Pablo Neira Ayuso
2013-09-04 16:28 ` [PATCH 0/4] netfilter updates for net-next David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-04 13:00 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Phil Oester <kernel@linuxace.com>
In commit b396966c4 (netfilter: xt_TCPMSS: Fix missing fragmentation handling),
I attempted to add safe fragment handling to xt_TCPMSS. However, Andy Padavan
of Project N56U correctly points out that returning XT_CONTINUE in this
function does not work. The callers (tcpmss_tg[46]) expect to receive a value
of 0 in order to return XT_CONTINUE.
Signed-off-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_TCPMSS.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 6113cc7..cd24290 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -60,7 +60,7 @@ tcpmss_mangle_packet(struct sk_buff *skb,
/* This is a fragment, no TCP header is available */
if (par->fragoff != 0)
- return XT_CONTINUE;
+ return 0;
if (!skb_make_writable(skb, skb->len))
return -1;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] netfilter updates for net-next
2013-09-04 13:00 [PATCH 0/4] netfilter updates for net-next Pablo Neira Ayuso
` (3 preceding siblings ...)
2013-09-04 13:00 ` [PATCH 4/4] netfilter: xt_TCPMSS: correct return value in tcpmss_mangle_packet Pablo Neira Ayuso
@ 2013-09-04 16:28 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-09-04 16:28 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 4 Sep 2013 15:00:21 +0200
> The following batch contains:
>
> * Three fixes for the new synproxy target available in your
> net-next tree, from Jesper D. Brouer and Patrick McHardy.
>
> * One fix for TCPMSS to correctly handling the fragmentation
> case, from Phil Oester. I'll pass this one to -stable.
Pulled, thanks Pablo.
^ permalink raw reply [flat|nested] 6+ messages in thread