Re: Enable syn cookies by default

Previous thread: [Patch] sctp: remove deprecated SCTP_GET_*_OLD stuffs by Amerigo Wang on Thursday, October 15, 2009 - 1:26 am. (8 messages)

Next thread: [patch 4/5] [PATCH] lcs: Recognize return codes of ccw_device_set_online(). by Ursula Braun on Thursday, October 15, 2009 - 1:54 am. (1 message)
From: Olaf van der Spek
Date: Thursday, October 15, 2009 - 1:59 am

Somebody?
--

From: Florian Westphal
Date: Friday, October 16, 2009 - 12:49 pm

Always print a warning if the syn queue is full, just like
the tcp/ipv6 code does.

The "want_cookie" define is no longer needed -- gcc
removes the relevant branches in the CONFIG_SYN_COOKIES=n case.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/tcp_ipv4.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7cda24b..93b02a3 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -786,19 +786,19 @@ static void tcp_v4_reqsk_destructor(struct request_sock *req)
 	kfree(inet_rsk(req)->opt);
 }
 
-#ifdef CONFIG_SYN_COOKIES
 static void syn_flood_warning(struct sk_buff *skb)
 {
-	static unsigned long warntime;
-
-	if (time_after(jiffies, (warntime + HZ * 60))) {
-		warntime = jiffies;
+#ifdef CONFIG_SYN_COOKIES
+	if (sysctl_tcp_syncookies)
 		printk(KERN_INFO
-		       "possible SYN flooding on port %d. Sending cookies.\n",
-		       ntohs(tcp_hdr(skb)->dest));
-	}
-}
+		       "Possible SYN flooding on port %d. "
+		       "Sending cookies.\n", ntohs(tcp_hdr(skb)->dest));
+	else
 #endif
+		printk(KERN_INFO
+		       "Possible SYN flooding on port %d. "
+		       "Dropping request.\n", ntohs(tcp_hdr(skb)->dest));
+}
 
 /*
  * Save and compile IPv4 options into the request_sock if needed.
@@ -1217,11 +1217,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 	__be32 daddr = ip_hdr(skb)->daddr;
 	__u32 isn = TCP_SKB_CB(skb)->when;
 	struct dst_entry *dst = NULL;
-#ifdef CONFIG_SYN_COOKIES
 	int want_cookie = 0;
-#else
-#define want_cookie 0 /* Argh, why doesn't gcc optimize this :( */
-#endif
 
 	/* Never answer to SYNs send to broadcast or multicast */
 	if (skb_rtable(skb)->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
@@ -1232,6 +1228,8 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 	 * evidently real one.
 	 */
 	if (inet_csk_reqsk_queue_is_full(sk) && !isn) {
+		if ...
From: Florian Westphal
Date: Friday, October 16, 2009 - 12:51 pm

change syncookie sysctl initialization to 1.

Syn cookies have no effect under normal conditions; cookies are
only sent if a sockets syn queue is exhausted (and the connection
request would be dropped with cookies disabled).

sysctl_tcp_syncookies needs to be set to 0 in the CONFIG_SYN_COOKIES=n
case, as tcp_v4_conn_request() evaluates the variable in a conditional
expression (which then would always be false).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/Kconfig         |    7 +++----
 net/ipv4/tcp_minisocks.c |    6 +++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 70491d9..86e5bc8 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -289,7 +289,7 @@ config ARPD
 	  If unsure, say N.
 
 config SYN_COOKIES
-	bool "IP: TCP syncookie support (disabled per default)"
+	bool "IP: TCP syncookie support"
 	---help---
 	  Normal TCP/IP networking is open to an attack known as "SYN
 	  flooding". This denial-of-service attack prevents legitimate remote
@@ -314,11 +314,10 @@ config SYN_COOKIES
 	  server is really overloaded. If this happens frequently better turn
 	  them off.
 
-	  If you say Y here, note that SYN cookies aren't enabled by default;
-	  you can enable them by saying Y to "/proc file system support" and
+	  You can disable them by saying Y to "/proc file system support" and
 	  "Sysctl support" below and executing the command
 
-	  echo 1 >/proc/sys/net/ipv4/tcp_syncookies
+	  echo 0 >/proc/sys/net/ipv4/tcp_syncookies
 
 	  at boot time after the /proc file system has been mounted.
 
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 624c3c9..2b0ddc2 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -26,10 +26,10 @@
 #include <net/inet_common.h>
 #include <net/xfrm.h>
 
-#ifdef CONFIG_SYSCTL
-#define SYNC_INIT 0 /* let the user enable it */
-#else
+#ifdef CONFIG_SYN_COOKIES
 #define SYNC_INIT 1
+#else
+#define SYNC_INIT 0 ...
From: Olaf van der Spek
Date: Tuesday, December 8, 2009 - 7:47 am

Any comments?

Olaf
--

From: David Miller
Date: Tuesday, December 8, 2009 - 2:09 pm

From: Olaf van der Spek <olafvdspek@gmail.com>

You patch isn't even in patchwork any more, so for one thing
it's definitely not in my queue any more.
--

From: Olaf van der Spek
Date: Wednesday, January 27, 2010 - 10:01 am

Florian?
--

From: Olaf van der Spek
Date: Wednesday, October 21, 2009 - 12:17 am

On Thu, Oct 15, 2009 at 10:59 AM, Olaf van der Spek

Anybody?
--

From: Eric Dumazet
Date: Wednesday, October 21, 2009 - 12:25 am

This is a user selectable setting. What's wrong with /etc/sysctl.conf ?


--

From: Olaf van der Spek
Date: Wednesday, October 21, 2009 - 12:48 am

It requires user action...
Often you notice cookies are disabled only after a service becomes unreachable.
What's wrong with improving defaults?
Don't forget the missing log entries.

Olaf
--

From: William Allen Simpson
Date: Wednesday, October 21, 2009 - 2:16 am

I've not been a regular contributor here, so I'm not sure that my view has
much weight, but I'm *against* changing the coded default.

Keep in mind that I'm busy trying to replace syncookies with real cookies,
so I'm biased.  The syncookies interfere with new options; although in
Linux, they interfere less than other systems.

For Ubuntu, the practice is complicated.  In /etc/sysctl.conf, the text
assumes that the default is off:

# Uncomment the next line to enable TCP/IP SYN cookies
# This disables TCP Window Scaling (http://lkml.org/lkml/2008/2/5/167),
# and is not recommended.
#net.ipv4.tcp_syncookies=1

But in the default installed /etc/sysctl.d/10-network-security.conf, it
is explicitly on in any case:

# Turn on SYN-flood protections.  Starting with 2.6.26, there is no loss
# of TCP functionality/features under normal conditions.  When flood
# protections kick in under high unanswered-SYN load, the system
# should remain more stable, with a trade off of some loss of TCP
# functionality/features (e.g. TCP Window scaling).
net.ipv4.tcp_syncookies=1

On this I agree.  I'd like the system to syslog it's under attack,
especially whenever syncookies are off.
--

From: Olaf van der Spek
Date: Wednesday, October 21, 2009 - 3:10 am

On Wed, Oct 21, 2009 at 11:16 AM, William Allen Simpson

How and when do they interfere?
If syn cookies are enabled and the queue isn't full, they're not used
so they don't interfere.
If the queue is full, they do interfere, but the alternative would be
no connection at all.

Actually changing the value isn't the problem, but the Debian
maintainer isn't sure it's a good idea (but he doesn't know why).

Olaf
--

From: William Allen Simpson
Date: Wednesday, October 21, 2009 - 11:36 am

On systems with long delay paths, it represents turning back the clock
more than a decade or so.  A better solution is usually a firewall/IDS.
The best solution: I'm working on it.

Well, that depends.  For a client, it's a good idea, as the defense is
mostly local and rare.  For a server run by a small underfunded ISP, it's
still a good idea as a last ditch defense.  But for a full-fledged ISP,
especially running in a satellite environment or with a lot of dial-up
customers, it's terrible!

That's a reason the Ubuntu configuration approach works for me.

A caveat: I've not run debian directly in many, many years (IIRC, since
Red Hat Colgate), and more recently via Unbuntu (since Badger).  I don't
know whether debian has evolved different installation procedures for
different environments.

My comments are based on fairly extensive experience with deployment of
Yellow Dog Linux servers at an ISP (as a co-founder), and Ubuntu clients
for the past 2 (US) election cycles.

--

From: Olaf van der Spek
Date: Wednesday, October 21, 2009 - 11:45 am

On Wed, Oct 21, 2009 at 8:36 PM, William Allen Simpson


How's that? Are you saying no connection is better than a connection
with timestamps and SACK?
I don't believe you.

Wasn't there recently a patch to enable these things even when syn






Olaf
--

From: David Miller
Date: Wednesday, October 21, 2009 - 6:04 am

From: Olaf van der Spek <olafvdspek@gmail.com>

Would please you be patient?

In case you haven't fucking noticed, all of the major kernel
developers are in Japan at the annual kernel summit and the Japan
Linux Symposium since late last week.

So nobody has the time to look into anything requiring real
long thinking like this issue does.
--

From: William Allen Simpson
Date: Wednesday, October 21, 2009 - 11:04 am

Wow, that's way over the top!  I'd noticed your recent rudeness to many
folks in my perusal of this list, and carelessness about reading their
documentation (such as confusing "interdependent" with independent), but
I'd ascribed that to the Peter Principle and overwork....

Thanks for the information.  Too bad it conflicts with the NANOG and ARIN
conferences hosted here in Michigan this week.
--

From: Olaf van der Spek
Date: Friday, November 13, 2009 - 5:42 am

Hi David,

Have you had a chance to do some real long thinking? ;)

Greetings,

Olaf
--

Previous thread: [Patch] sctp: remove deprecated SCTP_GET_*_OLD stuffs by Amerigo Wang on Thursday, October 15, 2009 - 1:26 am. (8 messages)

Next thread: [patch 4/5] [PATCH] lcs: Recognize return codes of ccw_device_set_online(). by Ursula Braun on Thursday, October 15, 2009 - 1:54 am. (1 message)