TCPCT API update for 2.6.37

PATCH In review William Allen Simpson 4 participants 10 messages Jan 12, 2011 – Jan 14, 2011

Subsystems (4)

Networking Network drivers Documentation TCP

lore

Share & tools

lore

William Allen Simpson

author

2011-01-12 17:43 UTC #0 lore raw

With the recent flurry of messages related to TCPCT, I devoted a nice snowy Saturday afternoon to updating the socket option code. Linux is rather behind on TCPCT implementation, so this will help with future software compatibility.

Currently, any userland programs written to RFC-to-be-6013 will not interoperate with Linux 2.6.33 and beyond. I've made these patches for 2.6.37 -- they'll also need to be ported to earlier releases.

I'm not on this list, so anybody with comments should CC me. Thanks.

William Allen Simpson

author

2011-01-12 17:52 UTC #1 lore raw

PATCH 1/2 TCPCT API sysctl update to draft -03

Use most recently specified symbols of RFC-to-be-6013.

Allows different global s_data limits for SYN and SYN_ACK.

CC "Eric W. Biederman" <[email protected]>

CC Stephen Hemminger <[email protected]>

CC Andi Kleen <[email protected]>

Signed-off-by [email protected]

---
 Documentation/networking/ip-sysctl.txt |   10 ++++++++++
 include/net/tcp.h                      |    2 ++
 net/ipv4/sysctl_net_ipv4.c             |   25 ++++++++++++++++++++++++-
 net/ipv4/tcp_output.c                  |   19 +++++++++++++++++--
 4 files changed, 53 insertions(+), 3 deletions(-)

Documentation/networking/ip-sysctl.txt +10 -0

index d99940d..4e14b3a 100644
@@ -184,6 +184,16 @@ tcp_cookie_size - INTEGER
 	as the minimum.  Odd values are interpreted as the next even value.
 	Default: 0 (off).
 
+tcp_syn_data_limit - INTEGER
+	Limit for TCP Cookie Transactions (TCPCT) data transmitted with
+	the <SYN>. Default: 496. Maximum: 496.
+
+tcp_syn_ack_data_limit - INTEGER
+	Limit for TCP Cookie Transactions (TCPCT) data transmitted with
+	the <SYN,ACK(SYN)>. As a matter of security policy, keep the
+	setting small to avoid amplification denial of service attacks.
+	Default: 80. Maximum: 1220.
+
 tcp_dsack - BOOLEAN
 	Allows TCP to send "duplicate" SACKs.

include/net/tcp.h +2 -0

index 38509f0..3ac2bca 100644
@@ -241,6 +241,8 @@ extern int sysctl_tcp_workaround_signed_windows;
 extern int sysctl_tcp_slow_start_after_idle;
 extern int sysctl_tcp_max_ssthresh;
 extern int sysctl_tcp_cookie_size;
+extern int sysctl_tcp_syn_data_limit;
+extern int sysctl_tcp_syn_ack_data_limit;
 extern int sysctl_tcp_thin_linear_timeouts;
 extern int sysctl_tcp_thin_dupack;

net/ipv4/sysctl_net_ipv4.c +24 -1

index 1a45665..629f90b 100644
@@ -30,6 +30,9 @@ static int tcp_adv_win_scale_min = -31;
 static int tcp_adv_win_scale_max = 31;
 static int ip_ttl_min = 1;
 static int ip_ttl_max = 255;
+static int tcp_cookie_max = TCP_COOKIE_MAX;
+static int tcp_syn_data_max = TCP_MSS_DEFAULT - 40;
+static int tcp_syn_ack_data_max = TCP_MSS_DESIRED;
 
 /* Update system visible IP port range */
 static void set_local_port_range(int range[2])

@@ -588,7 +591,27 @@ static struct ctl_table ipv4_table[] = {
 		.data		= &sysctl_tcp_cookie_size,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &tcp_cookie_max,
+	},
+	{
+		.procname	= "tcp_syn_data_limit",
+		.data		= &sysctl_tcp_syn_data_limit,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &tcp_syn_data_max,
+	},
+	{
+		.procname	= "tcp_syn_ack_data_limit",
+		.data		= &sysctl_tcp_syn_ack_data_limit,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &tcp_syn_ack_data_max,
 	},
 	{
 		.procname       = "tcp_thin_linear_timeouts",

net/ipv4/tcp_output.c +17 -2

index dc7c096..16a9e40 100644
@@ -63,6 +63,15 @@ int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
 int sysctl_tcp_cookie_size __read_mostly = 0; /* TCP_COOKIE_MAX */
 EXPORT_SYMBOL_GPL(sysctl_tcp_cookie_size);
 
+int sysctl_tcp_syn_data_limit __read_mostly = TCP_MSS_DEFAULT - 40;
+EXPORT_SYMBOL_GPL(sysctl_tcp_syn_data_limit);
+
+/* As a matter of security policy, keep the setting small to avoid
+ * amplification denial of service attacks.
+ */
+int sysctl_tcp_syn_ack_data_limit __read_mostly = 80; /* TCP_MSS_DESIRED */
+EXPORT_SYMBOL_GPL(sysctl_tcp_syn_ack_data_limit);
+
 
 /* Account for new data that has been sent to the network. */
 static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)

@@ -2418,10 +2427,16 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 	struct tcp_md5sig_key *md5;
 	int tcp_header_size;
 	int mss;
-	int s_data_desired = 0;
+	int s_data_desired;
 
-	if (cvp != NULL && cvp->s_data_constant && cvp->s_data_desired)
+	if (cvp != NULL &&
+	    cvp->s_data_constant &&
+	    cvp->s_data_desired > 0 &&
+	    cvp->s_data_desired <= sysctl_tcp_syn_ack_data_limit)
 		s_data_desired = cvp->s_data_desired;
+	else
+		s_data_desired = 0;
+
 	skb = sock_wmalloc(sk, MAX_TCP_HEADER + 15 + s_data_desired, 1, GFP_ATOMIC);
 	if (skb == NULL)
 		return NULL;

Signature · 1 line

1.7.1

Eric Dumazet

2011-01-12 18:05 UTC #2 lore raw

Re: [PATCH v1 1/2] TCPCT API sysctl update to draft -03

Le mercredi 12 janvier 2011 à 12:52 -0500, William Allen Simpson a écrit :

Quoted 10 lines

Use most recently specified symbols of RFC-to-be-6013.
Allows different global s_data limits for SYN and SYN_ACK.
CC "Eric W. Biederman" <[email protected]>
CC Stephen Hemminger <[email protected]>
CC Andi Kleen <[email protected]>
Signed-off-by [email protected]

Should be :

Signed-off-by William Allen Simpson <[email protected]>

Quoted 6 lines

---
  Documentation/networking/ip-sysctl.txt |   10 ++++++++++
  include/net/tcp.h                      |    2 ++
  net/ipv4/sysctl_net_ipv4.c             |   25 ++++++++++++++++++++++++-
  net/ipv4/tcp_output.c                  |   19 +++++++++++++++++--
  4 files changed, 53 insertions(+), 3 deletions(-)

Hmm, patch is not inlined, so I have to copy/paste

+static int tcp_cookie_max = TCP_COOKIE_MAX;

...

/* Update system visible IP port range */
static void set_local_port_range(int range[2])
@@ -588,7 +591,27 @@ static struct ctl_table ipv4_table[] = {

                .data           = &sysctl_tcp_cookie_size,
                .maxlen         = sizeof(int),
                .mode           = 0644,
-               .proc_handler   = proc_dointvec
+               .proc_handler   = proc_dointvec_minmax,
+               .extra1         = &zero,
+               .extra2         = &tcp_cookie_max,

+ },

Now sysctl_tcp_cookie_size has a max limit of TCP_COOKIE_MAX, you can remove the now uneeded check in :

static u8 tcp_cookie_size_check(u8 desired)
...
if (cookie_size >= TCP_COOKIE_MAX)

	/* value too large, specify maximum */
	return TCP_COOKIE_MAX;

William Allen Simpson

author

2011-01-12 17:59 UTC #3 lore raw

PATCH 2/2 TCPCT API sockopt update to draft -03

In reply to William Allen Simpson

Use most recently specified symbols of RFC-to-be-6013.

Permit setting either cookie or s_data (alternatively).

Split the data value from socket option header, saving more than 1K of stack space in the handler by copying long data values directly from user space into the kref block.

Signed-off-by [email protected]

---
 include/linux/tcp.h |   35 ++++++++++++-----
 net/ipv4/tcp.c      |  102 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 96 insertions(+), 41 deletions(-)

include/linux/tcp.h +25 ~~-10~~

index e64f4c6..c8f4017 100644
@@ -185,22 +185,37 @@ struct tcp_md5sig {
 #define TCP_COOKIE_PAIR_SIZE	(2*TCP_COOKIE_MAX)
 
 /* Flags for both getsockopt and setsockopt */
-#define TCP_COOKIE_IN_ALWAYS	(1 << 0)	/* Discard SYN without cookie */
-#define TCP_COOKIE_OUT_NEVER	(1 << 1)	/* Prohibit outgoing cookies,
+#define TCPCT_IN_ALWAYS		(1 << 0)	/* Discard SYN without cookie */
+#define TCPCT_OUT_NEVER		(1 << 1)	/* Prohibit outgoing cookies,
 						 * supercedes everything. */
-
-/* Flags for getsockopt */
-#define TCP_S_DATA_IN		(1 << 2)	/* Was data received? */
-#define TCP_S_DATA_OUT		(1 << 3)	/* Was data sent? */
-
-/* TCP_COOKIE_TRANSACTIONS data */
+#define TCPCT_IN_DATA		(1 << 2)	/* Was data received? */
+#define TCPCT_OUT_DATA		(1 << 3)	/* Was data sent? */
+/* reserved for future use: bits 4 .. 6 */
+#define TCPCT_EXTEND		(1 << 7)
+
+/* Extended Option flags for both getsockopt and setsockopt */
+#define TCPCT_EXTEND_SIZE	(0x7)		/* mask */
+#define TCPCT_EXTEND_TS32	(0x1)		/* default */
+#define TCPCT_EXTEND_TS64	(0x2)
+#define TCPCT_EXTEND_TS128	(0x4)
+
+/* TCP_COOKIE_TRANSACTIONS socket option header */
 struct tcp_cookie_transactions {
 	__u16	tcpct_flags;			/* see above */
-	__u8	__tcpct_pad1;			/* zero */
+	__u8	tcpct_extended;
 	__u8	tcpct_cookie_desired;		/* bytes */
 	__u16	tcpct_s_data_desired;		/* bytes of variable data */
 	__u16	tcpct_used;			/* bytes in value */
-	__u8	tcpct_value[TCP_MSS_DEFAULT];
+};
+
+struct tcpct_full {
+	struct tcp_cookie_transactions soh;
+	__u8	tcpct_value[TCP_COOKIE_PAIR_SIZE];
+};
+
+struct tcpct_half {
+	struct tcp_cookie_transactions soh;
+	__u8	tcpct_value[TCP_COOKIE_MAX];
 };
 
 #ifdef __KERNEL__

net/ipv4/tcp.c +71 ~~-31~~

157 lines not on this page · raw message

Signature · 1 line

1.7.1

Stephen Hemminger

2011-01-12 18:56 UTC #4 lore raw

Re: [PATCH v1 2/2] TCPCT API sockopt update to draft -03

include/linux/tcp.h struct tcp_md5sig { · line 189

Quoted 12 lines · On Wed, 12 Jan 2011 12:59:38 -0500 William Allen Simpson…

include/linux/tcp.h +2 -2

index e64f4c6..c8f4017 100644
@@ -185,22 +185,37 @@ struct tcp_md5sig {
 #define TCP_COOKIE_PAIR_SIZE	(2*TCP_COOKIE_MAX)
 
 /* Flags for both getsockopt and setsockopt */
-#define TCP_COOKIE_IN_ALWAYS	(1 << 0)	/* Discard SYN without cookie */
-#define TCP_COOKIE_OUT_NEVER	(1 << 1)	/* Prohibit outgoing cookies,
+#define TCPCT_IN_ALWAYS		(1 << 0)	/* Discard SYN without cookie */
+#define TCPCT_OUT_NEVER		(1 << 1)	/* Prohibit outgoing cookies,

You end up changing values in kernel userspace API in a way that is incompatible with older applications. This is not acceptable.

William Allen Simpson

author

2011-01-13 17:32 UTC #5 lore raw

Re: [PATCH v1 2/2] TCPCT API sockopt update to draft -03

Quoted 10 lines · Stephen Hemminger

Quoted 7 lines · On Wed, 12 Jan 2011 12:59:38 -0500 William Allen…
include/linux/tcp.h +0 -0
index e64f4c6..c8f4017 100644
@@ -185,22 +185,37 @@ struct tcp_md5sig {
  #define TCP_COOKIE_PAIR_SIZE	(2*TCP_COOKIE_MAX)
 

Quoted 9 lines

include/linux/tcp.h struct tcp_md5sig { · line 189
Quoted 5 lines
include/linux/tcp.h +2 -2
  /* Flags for both getsockopt and setsockopt */
-#define TCP_COOKIE_IN_ALWAYS	(1<<  0)	/* Discard SYN without cookie */
-#define TCP_COOKIE_OUT_NEVER	(1<<  1)	/* Prohibit outgoing cookies,
+#define TCPCT_IN_ALWAYS		(1<<  0)	/* Discard SYN without cookie */
+#define TCPCT_OUT_NEVER		(1<<  1)	/* Prohibit outgoing cookies,
You end up changing values in kernel userspace API in a way that is incompatible with older applications. This is not acceptable.

While I agree in principle and argued strongly against it, other members of the research group (particularly the original project sponsor) have over-ridden my concerns. I'm sorry to inform you that many/most participants don't care much about Linux.

Note that the *bits* are the same, and previously compiled programs (that don't access more advanced features) should continue to run as they have in the past.

Even though I'm not paid to work on Linux, I'm doing my best to give you folks a quick heads up and provide code to rectify the very recent changes that can be propagated back through the stable tree (to 2.6.33).

As always, what you actually do with my code is up to you....

Arnaud Lacombe

2011-01-13 17:53 UTC #6 lore raw

Re: [PATCH v1 2/2] TCPCT API sockopt update to draft -03

Hi,

Quoted 34 lines · William Allen Simpson

Quoted 11 lines · Stephen Hemminger
Quoted 7 lines · On Wed, 12 Jan 2011 12:59:38 -0500 William Allen…
include/linux/tcp.h +0 -0
index e64f4c6..c8f4017 100644
@@ -185,22 +185,37 @@ struct tcp_md5sig {
  #define TCP_COOKIE_PAIR_SIZE  (2*TCP_COOKIE_MAX)
 
Quoted 13 lines
Quoted 9 lines
include/linux/tcp.h +0 -1
  /* Flags for both getsockopt and setsockopt */
-#define TCP_COOKIE_IN_ALWAYS   (1<<  0)        /* Discard SYN without
cookie */
-#define TCP_COOKIE_OUT_NEVER (1<< 1) /* Prohibit outgoing cookies,
+#define TCPCT_IN_ALWAYS (1<< 0) /* Discard SYN without cookie */
+#define TCPCT_OUT_NEVER (1<< 1) /* Prohibit outgoing cookies,
You end up changing values in kernel userspace API in a way that is incompatible with older applications. This is not acceptable.
While I agree in principle and argued strongly against it, other members of the research group (particularly the original project sponsor) have over-ridden my concerns. I'm sorry to inform you that many/most participants don't care much about Linux.
Note that the *bits* are the same, and previously compiled programs (that don't access more advanced features) should continue to run as they have in the past.

Quoted 6 lines

Even though I'm not paid to work on Linux, I'm doing my best to give you folks a quick heads up and provide code to rectify the very recent changes that can be propagated back through the stable tree (to 2.6.33).
As always, what you actually do with my code is up to you....

FWIW, what is the basis of this hunk ? The RFC text[0] seems to use the TCP_COOKIE_* naming, not TCPCT_.

Thanks,
- Arnaud

[0]: http://www.rfc-editor.org/authors/rfc6013.txt

Quoted 5 lines

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

William Allen Simpson

author

2011-01-14 03:00 UTC #7 lore raw

Re: [PATCH v1 2/2] TCPCT API sockopt update to draft -03

Quoted 8 lines · Arnaud Lacombe

Quoted 6 lines · William Allen Simpson
Even though I'm not paid to work on Linux, I'm doing my best to give you folks a quick heads up and provide code to rectify the very recent changes that can be propagated back through the stable tree (to 2.6.33).
As always, what you actually do with my code is up to you....

Quoted 8 lines

FWIW, what is the basis of this hunk ? The RFC text[0] seems to use the TCP_COOKIE_* naming, not TCPCT_.
Thanks,
- Arnaud
[0]: http://www.rfc-editor.org/authors/rfc6013.txt

Is this supposed to be humorous? Maybe folks here find it amusing that somebody thinks they know more than the *author* about the contents of the document? Did you note the words above? That is, "very recent changes"?

Perhaps you are viewing an older cached version. Please check for the current month on every page: "January 2011".

We discussed -- and ultimately decided -- these changes in private email during the independent review process before making them available to the general public. That's how the RFC publication procedure works.

I tried to be helpful to the Linux community in advance of publication, so you would be prepared. I'm sorry that the community here is so lacking in appreciation for my efforts on your behalf.

As always, what you actually do with my code is up to you....

Eric Dumazet

2011-01-14 03:08 UTC #8 lore raw

Re: [PATCH v1 2/2] TCPCT API sockopt update to draft -03

Quoted 11 lines · William Allen Simpson

Is this supposed to be humorous? Maybe folks here find it amusing that somebody thinks they know more than the *author* about the contents of the document? Did you note the words above? That is, "very recent changes"?
Perhaps you are viewing an older cached version. Please check for the current month on every page: "January 2011".
We discussed -- and ultimately decided -- these changes in private email during the independent review process before making them available to the general public. That's how the RFC publication procedure works.

Quoted 6 lines

I tried to be helpful to the Linux community in advance of publication, so you would be prepared. I'm sorry that the community here is so lacking in appreciation for my efforts on your behalf.
As always, what you actually do with my code is up to you.... --

Next time you come here, provide an up2date link for us mere mortals, so that we can check your code against your claims. We dont trust you anymore, we had to fix several bugs.

This is getting ridiculous.

As I said, we are going to wait for official RFC, because its time consuming to review your patches, and nobody asked for early TCPCT coding in linux kernel (you already said your buddies dont care at all)

Eric Dumazet

2011-01-13 18:00 UTC #9 lore raw

Re: [PATCH v1 2/2] TCPCT API sockopt update to draft -03

In reply to William Allen Simpson

Quoted 20 lines · William Allen Simpson

Quoted 10 lines · Stephen Hemminger
Quoted 7 lines · On Wed, 12 Jan 2011 12:59:38 -0500 William Allen…
include/linux/tcp.h +0 -0
index e64f4c6..c8f4017 100644
@@ -185,22 +185,37 @@ struct tcp_md5sig {
  #define TCP_COOKIE_PAIR_SIZE	(2*TCP_COOKIE_MAX)
 
Quoted 9 lines
include/linux/tcp.h struct tcp_md5sig { · line 189
Quoted 5 lines
include/linux/tcp.h +2 -2
  /* Flags for both getsockopt and setsockopt */
-#define TCP_COOKIE_IN_ALWAYS	(1<<  0)	/* Discard SYN without cookie */
-#define TCP_COOKIE_OUT_NEVER	(1<<  1)	/* Prohibit outgoing cookies,
+#define TCPCT_IN_ALWAYS		(1<<  0)	/* Discard SYN without cookie */
+#define TCPCT_OUT_NEVER		(1<<  1)	/* Prohibit outgoing cookies,
You end up changing values in kernel userspace API in a way that is incompatible with older applications. This is not acceptable.

Quoted 5 lines

While I agree in principle and argued strongly against it, other members of the research group (particularly the original project sponsor) have over-ridden my concerns. I'm sorry to inform you that many/most participants don't care much about Linux.

How leaving TCP_COOKIE_IN_ALWAYS and TCP_COOKIE_OUT_NEVER definitions so that user space programs compiles can be a problem to "research group" ?

AFAIK, TCPCT_IN_ALWAYS / TCPCT_OUT_NEVER are not mentioned in
http://www.rfc-editor.org/authors/rfc6013.txt

But TCP_COOKIE_IN_ALWAYS and TCP_COOKIE_OUT_NEVER are ...

Isnt it a bit confusing ?

Quoted 9 lines

Note that the *bits* are the same, and previously compiled programs (that don't access more advanced features) should continue to run as they have in the past.
Even though I'm not paid to work on Linux, I'm doing my best to give you folks a quick heads up and provide code to rectify the very recent changes that can be propagated back through the stable tree (to 2.6.33).
As always, what you actually do with my code is up to you....

Maybe its too early, and we should wait for an official RFC, especially if you insist breaking API in 6 months.