| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1338322661.7747.17.camel@localhost>
Date: Tue, 29 May 2012 22:17:41 +0200
From: Jesper Dangaard Brouer <brouer@...hat.com>
To: christoph.paasch@...ouvain.be
Cc: netdev@...r.kernel.org, Eric Dumazet <eric.dumazet@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Martin Topholm <mph@...h.dk>, Florian Westphal <fw@...len.de>,
opurdila@...acom.com,
Hans Schillstrom <hans.schillstrom@...csson.com>,
Andi Kleen <andi@...stfloor.org>
Subject: Re: [RFC PATCH 0/2] Faster/parallel SYN handling to mitigate SYN
floods
On Mon, 2012-05-28 at 18:14 +0200, Christoph Paasch wrote:
> On 05/28/2012 01:52 PM, Jesper Dangaard Brouer wrote:
> > The following series is a RFC (Request For Comments) for implementing
> > a faster and parallel handling of TCP SYN connections, to mitigate SYN
> > flood attacks. This is against DaveM's net (f0d1b3c2bc), as net-next
> > is closed, as DaveM has mentioned numerous times ;-)
> >
> > Only IPv4 TCP is handled here. The IPv6 TCP code also need to be
> > updated, but I'll deal with that part after we have agreed on a
> > solution for IPv4 TCP.
> >
> > Patch 1/2: Is a cleanup, where I split out the SYN cookie handling
> > from tcp_v4_conn_request() into tcp_v4_syn_conn_limit().
> >
> > Patch 2/2: Move tcp_v4_syn_conn_limit() outside bh_lock_sock() in
> > tcp_v4_rcv(). I would like some input on, (1) if this safe without
> > the lock, (2) if we need to do some sock lookup, before calling
> > tcp_v4_syn_conn_limit() (Christoph Paasch
> > <christoph.paasch@...ouvain.be> mentioned something about SYN
> > retransmissions)
>
> Concerning (1):
> I think, there are places where you may have troube because you don't
> hold the lock.
> E.g., in tcp_make_synack (called by tcp_v4_send_synack from your
> tcp_v4_syn_conn_limit) there is:
>
> if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
> (req->window_clamp > tcp_full_space(sk) ||
> req->window_clamp == 0))
> req->window_clamp = tcp_full_space(sk);
>
> Thus, tcp_full_space(sk) may have different values between the check and
> setting req->window_clamp.
This should be simply solved by using a local stack variable, for
storing the result from tcp_full_space(sk). Its likely that GCC already
does this behind our back.
> Concerning (2):
>
> Imagine, a SYN coming in, when the reqsk-queue is not yet full. A
> request-sock will be added to the reqsk-queue. Then, a retransmission of
> this SYN comes in and the queue got full by the time. This time
> tcp_v4_syn_conn_limit will do syn-cookies and thus generate a different
> seq-number for the SYN/ACK.
I have addressed your issue, by checking the reqsk_queue in
tcp_v4_syn_conn_limit() before allocating a new req via
inet_reqsk_alloc().
If I find an existing reqsk, I choose to drop it, so the SYN cookie
SYN-ACK takes precedence, as the path/handling of the last ACK doesn't
find this reqsk. This is done under the lock.
Test results show that I can provoke the SYN retransmit situation, and
that performance is still very good. Func call inet_csk_search_req()
only sneaks up to a top 20 on perf report.
Patch on top of this patch:
[RFC PATCH 3/2] tcp: Detect SYN retransmits during SYN flood
Check for existing connection request (reqsk) as this might
be a retransmitted SYN which have gotten into the
reqsk_queue. If so, we choose to drop the reqsk, and use
SYN cookies to restore the state later.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7480fc2..e0c9ba3 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1274,8 +1274,10 @@ static const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops = {
*/
int tcp_v4_syn_conn_limit(struct sock *sk, struct sk_buff *skb)
{
- struct request_sock *req;
+ struct request_sock *req = NULL;
struct inet_request_sock *ireq;
+ struct request_sock *exist_req;
+ struct request_sock **prev;
struct tcp_options_received tmp_opt;
__be32 saddr = ip_hdr(skb)->saddr;
__be32 daddr = ip_hdr(skb)->daddr;
@@ -1303,6 +1305,22 @@ int tcp_v4_syn_conn_limit(struct sock *sk, struct sk_buff *skb)
if (!tcp_syn_flood_action(sk, skb, "TCP"))
goto drop; /* Not enabled, indicate drop, due to queue full */
+ /* Check for existing connection request (reqsk) as this might
+ * be a retransmitted SYN which have gotten into the
+ * reqsk_queue. If so, we choose to drop the reqsk, and use
+ * SYN cookies to restore the state later.
+ */
+ bh_lock_sock(sk);
+ exist_req = inet_csk_search_req(sk, &prev, tcp_hdr(skb)->source, saddr, daddr);
+ if (exist_req) { /* Drop existing reqsk */
+ if (TCP_SKB_CB(skb)->seq == tcp_rsk(exist_req)->rcv_isn)
+ net_warn_ratelimited("Retransmitted SYN from %pI4"
+ " (orig reqsk dropped)", &saddr);
+
+ inet_csk_reqsk_queue_drop(sk, exist_req, prev);
+ }
+ bh_unlock_sock(sk);
+
/* Allocate a request_sock */
req = inet_reqsk_alloc(&tcp_request_sock_ops);
if (!req) {
I'll post some V2 patches tomorrow, which integrates this changes in
patch 2/2.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists