* [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange
@ 2009-11-09 16:07 William Allen Simpson
2009-11-09 16:12 ` [net-next-2.6 PATCH v5 1/5 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK William Allen Simpson
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: William Allen Simpson @ 2009-11-09 16:07 UTC (permalink / raw)
To: Linux Kernel Network Developers
I'm torn at the moment, as the net-next tree modules don't compile today.
Although 'make vmlinux' compiles this successfully, it is not tested.
Keep in mind that earlier variants were extensively tested. But at least
this helps move the review comment process along....
Updated to patch cleanly with recent commits.
Changes from previous version:
* swapped conditional tests, requested by Eric and Joe.
* separate patch for constant cleanup, requested by Ilpo.
* added hash generation of Responder Cookie with RCU locking.
* shuffled a few bits to save another byte of tcp socket space.
^ permalink raw reply [flat|nested] 19+ messages in thread* [net-next-2.6 PATCH v5 1/5 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK 2009-11-09 16:07 [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange William Allen Simpson @ 2009-11-09 16:12 ` William Allen Simpson 2009-11-09 16:24 ` [net-next-2.6 PATCH v5 2/5 RFC] TCPCT part1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED William Allen Simpson ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: William Allen Simpson @ 2009-11-09 16:12 UTC (permalink / raw) To: Linux Kernel Network Developers [-- Attachment #1: Type: text/plain, Size: 1000 bytes --] Add optional function parameters associated with sending SYNACK. These parameters are not needed after sending SYNACK, and are not used for retransmission. Avoids extending struct tcp_request_sock, and avoids allocating kernel memory. Also affects DCCP as it uses common struct request_sock_ops, but this parameter is currently reserved for future use. Signed-off-by: William.Allen.Simpson@gmail.com Acked-by: Eric Dumazet <eric.dumazet@gmail.com> --- include/net/request_sock.h | 8 +++++++- include/net/tcp.h | 1 + net/dccp/ipv4.c | 5 +++-- net/dccp/ipv6.c | 5 +++-- net/dccp/minisocks.c | 2 +- net/ipv4/inet_connection_sock.c | 2 +- net/ipv4/tcp_ipv4.c | 17 ++++++++++------- net/ipv4/tcp_minisocks.c | 2 +- net/ipv4/tcp_output.c | 1 + net/ipv6/tcp_ipv6.c | 28 +++++++++++++--------------- 10 files changed, 41 insertions(+), 30 deletions(-) [-- Attachment #2: TCPCT+1a5.patch --] [-- Type: text/plain, Size: 8638 bytes --] diff --git a/include/net/request_sock.h b/include/net/request_sock.h index c719084..c9b50eb 100644 --- a/include/net/request_sock.h +++ b/include/net/request_sock.h @@ -27,13 +27,19 @@ struct sk_buff; struct dst_entry; struct proto; +/* empty to "strongly type" an otherwise void parameter. + */ +struct request_values { +}; + struct request_sock_ops { int family; int obj_size; struct kmem_cache *slab; char *slab_name; int (*rtx_syn_ack)(struct sock *sk, - struct request_sock *req); + struct request_sock *req, + struct request_values *rvp); void (*send_ack)(struct sock *sk, struct sk_buff *skb, struct request_sock *req); void (*send_reset)(struct sock *sk, diff --git a/include/net/tcp.h b/include/net/tcp.h index bf20f88..25bf3ba 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -443,6 +443,7 @@ extern int tcp_connect(struct sock *sk); extern struct sk_buff * tcp_make_synack(struct sock *sk, struct dst_entry *dst, + struct request_values *rvp, struct request_sock *req); extern int tcp_disconnect(struct sock *sk, int flags); diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c index 2423a08..efbcfdc 100644 --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c @@ -477,7 +477,8 @@ static struct dst_entry* dccp_v4_route_skb(struct net *net, struct sock *sk, return &rt->u.dst; } -static int dccp_v4_send_response(struct sock *sk, struct request_sock *req) +static int dccp_v4_send_response(struct sock *sk, struct request_sock *req, + struct request_values *rv_unused) { int err = -1; struct sk_buff *skb; @@ -626,7 +627,7 @@ int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb) dreq->dreq_iss = dccp_v4_init_sequence(skb); dreq->dreq_service = service; - if (dccp_v4_send_response(sk, req)) + if (dccp_v4_send_response(sk, req, NULL)) goto drop_and_free; inet_csk_reqsk_queue_hash_add(sk, req, DCCP_TIMEOUT_INIT); diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c index 50ea91a..6574215 100644 --- a/net/dccp/ipv6.c +++ b/net/dccp/ipv6.c @@ -241,7 +241,8 @@ out: } -static int dccp_v6_send_response(struct sock *sk, struct request_sock *req) +static int dccp_v6_send_response(struct sock *sk, struct request_sock *req, + struct request_values *rv_unused) { struct inet6_request_sock *ireq6 = inet6_rsk(req); struct ipv6_pinfo *np = inet6_sk(sk); @@ -468,7 +469,7 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb) dreq->dreq_iss = dccp_v6_init_sequence(skb); dreq->dreq_service = service; - if (dccp_v6_send_response(sk, req)) + if (dccp_v6_send_response(sk, req, NULL)) goto drop_and_free; inet6_csk_reqsk_queue_hash_add(sk, req, DCCP_TIMEOUT_INIT); diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c index 5ca49ce..af226a0 100644 --- a/net/dccp/minisocks.c +++ b/net/dccp/minisocks.c @@ -184,7 +184,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb, * counter (backoff, monitored by dccp_response_timer). */ req->retrans++; - req->rsk_ops->rtx_syn_ack(sk, req); + req->rsk_ops->rtx_syn_ack(sk, req, NULL); } /* Network Duplicate, discard packet */ return NULL; diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 26fb50e..ad098d6 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -531,7 +531,7 @@ void inet_csk_reqsk_queue_prune(struct sock *parent, &expire, &resend); if (!expire && (!resend || - !req->rsk_ops->rtx_syn_ack(parent, req) || + !req->rsk_ops->rtx_syn_ack(parent, req, NULL) || inet_rsk(req)->acked)) { unsigned long timeo; diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 657ae33..f83ac91 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -743,7 +743,8 @@ static void tcp_v4_reqsk_send_ack(struct sock *sk, struct sk_buff *skb, * socket. */ static int __tcp_v4_send_synack(struct sock *sk, struct request_sock *req, - struct dst_entry *dst) + struct dst_entry *dst, + struct request_values *rvp) { const struct inet_request_sock *ireq = inet_rsk(req); int err = -1; @@ -753,7 +754,7 @@ static int __tcp_v4_send_synack(struct sock *sk, struct request_sock *req, if (!dst && (dst = inet_csk_route_req(sk, req)) == NULL) return -1; - skb = tcp_make_synack(sk, dst, req); + skb = tcp_make_synack(sk, dst, rvp, req); if (skb) { struct tcphdr *th = tcp_hdr(skb); @@ -774,9 +775,10 @@ static int __tcp_v4_send_synack(struct sock *sk, struct request_sock *req, return err; } -static int tcp_v4_send_synack(struct sock *sk, struct request_sock *req) +static int tcp_v4_send_synack(struct sock *sk, struct request_sock *req, + struct request_values *rvp) { - return __tcp_v4_send_synack(sk, req, NULL); + return __tcp_v4_send_synack(sk, req, NULL, rvp); } /* @@ -1211,13 +1213,13 @@ static struct timewait_sock_ops tcp_timewait_sock_ops = { int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) { - struct inet_request_sock *ireq; struct tcp_options_received tmp_opt; struct request_sock *req; + struct inet_request_sock *ireq; + struct dst_entry *dst = NULL; __be32 saddr = ip_hdr(skb)->saddr; __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 @@ -1337,7 +1339,8 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) } tcp_rsk(req)->snt_isn = isn; - if (__tcp_v4_send_synack(sk, req, dst) || want_cookie) + if (__tcp_v4_send_synack(sk, req, dst, NULL) + || want_cookie) goto drop_and_free; inet_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT); diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c index a9d34e2..fb68bab 100644 --- a/net/ipv4/tcp_minisocks.c +++ b/net/ipv4/tcp_minisocks.c @@ -537,7 +537,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb, * Enforce "SYN-ACK" according to figure 8, figure 6 * of RFC793, fixed by RFC1122. */ - req->rsk_ops->rtx_syn_ack(sk, req); + req->rsk_ops->rtx_syn_ack(sk, req, NULL); return NULL; } diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 616c686..784e6ee 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -2224,6 +2224,7 @@ int tcp_send_synack(struct sock *sk) /* Prepare a SYN-ACK. */ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, + struct request_values *rvp, struct request_sock *req) { struct inet_request_sock *ireq = inet_rsk(req); diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 696a22f..6951827 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -461,7 +461,8 @@ out: } -static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req) +static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req, + struct request_values *rvp) { struct inet6_request_sock *treq = inet6_rsk(req); struct ipv6_pinfo *np = inet6_sk(sk); @@ -499,7 +500,7 @@ static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req) if ((err = xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0) goto done; - skb = tcp_make_synack(sk, dst, req); + skb = tcp_make_synack(sk, dst, rvp, req); if (skb) { struct tcphdr *th = tcp_hdr(skb); @@ -1161,13 +1162,13 @@ static struct sock *tcp_v6_hnd_req(struct sock *sk,struct sk_buff *skb) */ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) { + struct tcp_options_received tmp_opt; + struct request_sock *req; struct inet6_request_sock *treq; struct ipv6_pinfo *np = inet6_sk(sk); - struct tcp_options_received tmp_opt; struct tcp_sock *tp = tcp_sk(sk); - struct request_sock *req = NULL; - __u32 isn = TCP_SKB_CB(skb)->when; struct dst_entry *dst = __sk_dst_get(sk); + __u32 isn = TCP_SKB_CB(skb)->when; #ifdef CONFIG_SYN_COOKIES int want_cookie = 0; #else @@ -1239,23 +1240,20 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) isn = tcp_v6_init_sequence(skb); } - tcp_rsk(req)->snt_isn = isn; security_inet_conn_request(sk, skb, req); - if (tcp_v6_send_synack(sk, req)) - goto drop; + if (tcp_v6_send_synack(sk, req, NULL) + || want_cookie) + goto drop_and_free; - if (!want_cookie) { - inet6_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT); - return 0; - } + inet6_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT); + return 0; +drop_and_free: + reqsk_free(req); drop: - if (req) - reqsk_free(req); - return 0; /* don't send reset */ } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [net-next-2.6 PATCH v5 2/5 RFC] TCPCT part1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED 2009-11-09 16:07 [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange William Allen Simpson 2009-11-09 16:12 ` [net-next-2.6 PATCH v5 1/5 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK William Allen Simpson @ 2009-11-09 16:24 ` William Allen Simpson 2009-11-09 16:39 ` Eric Dumazet 2009-11-09 16:33 ` [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions William Allen Simpson ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: William Allen Simpson @ 2009-11-09 16:24 UTC (permalink / raw) To: Linux Kernel Network Developers [-- Attachment #1: Type: text/plain, Size: 556 bytes --] Define two symbols needed in both kernel and user space. Remove old (somewhat incorrect) variant that wasn't used in many cases. Default applies to both RMSS and SMSS. Replace numeric constants with defined symbols. Signed-off-by: William.Allen.Simpson@gmail.com --- include/linux/tcp.h | 6 ++++++ include/net/tcp.h | 3 --- net/ipv4/tcp_input.c | 4 ++-- net/ipv4/tcp_ipv4.c | 6 +++--- net/ipv4/tcp_minisocks.c | 2 +- net/ipv6/tcp_ipv6.c | 2 +- 6 files changed, 13 insertions(+), 10 deletions(-) [-- Attachment #2: TCPCT+1b5.patch --] [-- Type: text/plain, Size: 4017 bytes --] diff --git a/include/linux/tcp.h b/include/linux/tcp.h index eeecb85..32d7d77 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -81,6 +81,12 @@ enum { TCP_DATA_OFFSET = __cpu_to_be32(0xF0000000) }; +/* + * TCP general constants + */ +#define TCP_MSS_DEFAULT 536U /* IPv4 (RFC1122, RFC2581) */ +#define TCP_MSS_DESIRED 1220U /* IPv6 (tunneled), EDNS0 (RFC3226) */ + /* TCP socket options */ #define TCP_NODELAY 1 /* Turn off Nagle's algorithm. */ #define TCP_MAXSEG 2 /* Limit MSS */ diff --git a/include/net/tcp.h b/include/net/tcp.h index 25bf3ba..a413e9f 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -62,9 +62,6 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo); /* Minimal accepted MSS. It is (60+60+8) - (20+20). */ #define TCP_MIN_MSS 88U -/* Minimal RCV_MSS. */ -#define TCP_MIN_RCVMSS 536U - /* The least MTU to use for probing */ #define TCP_BASE_MSS 512 diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index be0c5bf..cc306ac 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -140,7 +140,7 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb) * "len" is invariant segment length, including TCP header. */ len += skb->data - skb_transport_header(skb); - if (len >= TCP_MIN_RCVMSS + sizeof(struct tcphdr) || + if (len >= TCP_MSS_DEFAULT + sizeof(struct tcphdr) || /* If PSH is not set, packet should be * full sized, provided peer TCP is not badly broken. * This observation (if it is correct 8)) allows @@ -411,7 +411,7 @@ void tcp_initialize_rcv_mss(struct sock *sk) unsigned int hint = min_t(unsigned int, tp->advmss, tp->mss_cache); hint = min(hint, tp->rcv_wnd / 2); - hint = min(hint, TCP_MIN_RCVMSS); + hint = min(hint, TCP_MSS_DEFAULT); hint = max(hint, TCP_MIN_MSS); inet_csk(sk)->icsk_ack.rcv_mss = hint; diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index f83ac91..0718fde 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -217,7 +217,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len) if (inet->opt) inet_csk(sk)->icsk_ext_hdr_len = inet->opt->optlen; - tp->rx_opt.mss_clamp = 536; + tp->rx_opt.mss_clamp = TCP_MSS_DEFAULT; /* Socket identity is still unknown (sport may be zero). * However we set state to SYN-SENT and not releasing socket @@ -1270,7 +1270,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) goto drop_and_free; tcp_clear_options(&tmp_opt); - tmp_opt.mss_clamp = 536; + tmp_opt.mss_clamp = TCP_MSS_DEFAULT; tmp_opt.user_mss = tcp_sk(sk)->rx_opt.user_mss; tcp_parse_options(skb, &tmp_opt, 0, dst); @@ -1818,7 +1818,7 @@ static int tcp_v4_init_sock(struct sock *sk) */ tp->snd_ssthresh = TCP_INFINITE_SSTHRESH; tp->snd_cwnd_clamp = ~0; - tp->mss_cache = 536; + tp->mss_cache = TCP_MSS_DEFAULT; tp->reordering = sysctl_tcp_reordering; icsk->icsk_ca_ops = &tcp_init_congestion_ops; diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c index fb68bab..7a42990 100644 --- a/net/ipv4/tcp_minisocks.c +++ b/net/ipv4/tcp_minisocks.c @@ -476,7 +476,7 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req, if (newtp->af_specific->md5_lookup(sk, newsk)) newtp->tcp_header_len += TCPOLEN_MD5SIG_ALIGNED; #endif - if (skb->len >= TCP_MIN_RCVMSS+newtp->tcp_header_len) + if (skb->len >= TCP_MSS_DEFAULT + newtp->tcp_header_len) newicsk->icsk_ack.last_seg_size = skb->len - newtp->tcp_header_len; newtp->rx_opt.mss_clamp = req->mss; TCP_ECN_openreq_child(newtp, req); diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 6951827..b528f75 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1849,7 +1849,7 @@ static int tcp_v6_init_sock(struct sock *sk) */ tp->snd_ssthresh = TCP_INFINITE_SSTHRESH; tp->snd_cwnd_clamp = ~0; - tp->mss_cache = 536; + tp->mss_cache = TCP_MSS_DEFAULT; tp->reordering = sysctl_tcp_reordering; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 2/5 RFC] TCPCT part1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED 2009-11-09 16:24 ` [net-next-2.6 PATCH v5 2/5 RFC] TCPCT part1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED William Allen Simpson @ 2009-11-09 16:39 ` Eric Dumazet 0 siblings, 0 replies; 19+ messages in thread From: Eric Dumazet @ 2009-11-09 16:39 UTC (permalink / raw) To: William Allen Simpson; +Cc: Linux Kernel Network Developers William Allen Simpson a écrit : > Define two symbols needed in both kernel and user space. > > Remove old (somewhat incorrect) variant that wasn't used in many cases. > Default applies to both RMSS and SMSS. > > Replace numeric constants with defined symbols. > > Signed-off-by: William.Allen.Simpson@gmail.com > --- > include/linux/tcp.h | 6 ++++++ > include/net/tcp.h | 3 --- > net/ipv4/tcp_input.c | 4 ++-- > net/ipv4/tcp_ipv4.c | 6 +++--- > net/ipv4/tcp_minisocks.c | 2 +- > net/ipv6/tcp_ipv6.c | 2 +- > 6 files changed, 13 insertions(+), 10 deletions(-) > Acked-by: Eric Dumazet <eric.dumazet@gmail.com> ^ permalink raw reply [flat|nested] 19+ messages in thread
* [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions 2009-11-09 16:07 [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange William Allen Simpson 2009-11-09 16:12 ` [net-next-2.6 PATCH v5 1/5 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK William Allen Simpson 2009-11-09 16:24 ` [net-next-2.6 PATCH v5 2/5 RFC] TCPCT part1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED William Allen Simpson @ 2009-11-09 16:33 ` William Allen Simpson 2009-11-10 5:31 ` Ilpo Järvinen 2009-11-09 16:50 ` [net-next-2.6 PATCH v5 4/5 RFC] TCPCT part1d: generate Responder Cookie William Allen Simpson 2009-11-09 17:05 ` [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data William Allen Simpson 4 siblings, 1 reply; 19+ messages in thread From: William Allen Simpson @ 2009-11-09 16:33 UTC (permalink / raw) To: Linux Kernel Network Developers [-- Attachment #1: Type: text/plain, Size: 1358 bytes --] Define sysctl (tcp_cookie_size) to turn on and off the cookie option default globally, instead of a compiled configuration option. Define per socket option (TCP_COOKIE_TRANSACTIONS) for setting constant data values, retrieving variable cookie values, and other facilities. Redefine two TCP header functions to accept TCP header pointer. When subtracting, return signed int to allow error checking. Move inline tcp_clear_options() unchanged from net/tcp.h to linux/tcp.h, near its corresponding struct tcp_options_received (prior to changes). This is a straightforward re-implementation of an earlier (year-old) patch that no longer applies cleanly, with permission of the original author (Adam Langley). The patch was previously reviewed: http://thread.gmane.org/gmane.linux.network/102586 These functions will also be used in subsequent patches that implement additional features. Requires: TCPCT part 1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED Signed-off-by: William.Allen.Simpson@gmail.com --- Documentation/networking/ip-sysctl.txt | 8 ++++++ include/linux/tcp.h | 44 +++++++++++++++++++++++++++++++- include/net/tcp.h | 6 +--- net/ipv4/sysctl_net_ipv4.c | 8 ++++++ net/ipv4/tcp_output.c | 8 ++++++ 5 files changed, 68 insertions(+), 6 deletions(-) [-- Attachment #2: TCPCT+1c5.patch --] [-- Type: text/plain, Size: 5589 bytes --] diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt index a0e134d..d47c000 100644 --- a/Documentation/networking/ip-sysctl.txt +++ b/Documentation/networking/ip-sysctl.txt @@ -164,6 +164,14 @@ tcp_congestion_control - STRING additional choices may be available based on kernel configuration. Default is set as part of kernel configuration. +tcp_cookie_size - INTEGER + Default size of TCP Cookie Transactions (TCPCT) option, that may be + overridden on a per socket basis by the TCPCT socket option. + Values greater than the maximum (16) are interpreted as the maximum. + Values greater than zero and less than the minimum (8) are interpreted + as the minimum. + Default: 0 (off). + tcp_dsack - BOOLEAN Allows TCP to send "duplicate" SACKs. diff --git a/include/linux/tcp.h b/include/linux/tcp.h index 32d7d77..57921c1 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -102,7 +102,9 @@ enum { #define TCP_QUICKACK 12 /* Block/reenable quick acks */ #define TCP_CONGESTION 13 /* Congestion control algorithm */ #define TCP_MD5SIG 14 /* TCP MD5 Signature (RFC2385) */ +#define TCP_COOKIE_TRANSACTIONS 15 /* TCP Cookie Transactions */ +/* for TCP_INFO socket option */ #define TCPI_OPT_TIMESTAMPS 1 #define TCPI_OPT_SACK 2 #define TCPI_OPT_WSCALE 4 @@ -174,6 +176,30 @@ struct tcp_md5sig { __u8 tcpm_key[TCP_MD5SIG_MAXKEYLEN]; /* key (binary) */ }; +/* for TCP_COOKIE_TRANSACTIONS (TCPCT) socket option */ +#define TCP_COOKIE_MAX 16 /* 128-bits */ +#define TCP_COOKIE_MIN 8 /* 64-bits */ +#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, + * 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 */ +struct tcp_cookie_transactions { + __u16 tcpct_flags; /* see above */ + __u8 __tcpct_pad1; /* zero */ + __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]; +}; + #ifdef __KERNEL__ #include <linux/skbuff.h> @@ -197,6 +223,17 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb) return (tcp_hdr(skb)->doff - 5) * 4; } +static inline unsigned int tcp_header_len_th(const struct tcphdr *th) +{ + return th->doff * 4; +} + +/* When doff is bad, this could be negative. */ +static inline int tcp_option_len_th(const struct tcphdr *th) +{ + return (int)(th->doff * 4) - sizeof(*th); +} + /* This defines a selective acknowledgement block. */ struct tcp_sack_block_wire { __be32 start_seq; @@ -227,6 +264,11 @@ struct tcp_options_received { u16 mss_clamp; /* Maximal mss, negotiated at connection setup */ }; +static inline void tcp_clear_options(struct tcp_options_received *rx_opt) +{ + rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0; +} + /* This is the max number of SACKS that we'll generate and process. It's safe * to increse this, although since: * size = TCPOLEN_SACK_BASE_ALIGNED (4) + n * TCPOLEN_SACK_PERBLOCK (8) @@ -435,6 +477,6 @@ static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk) return (struct tcp_timewait_sock *)sk; } -#endif +#endif /* __KERNEL__ */ #endif /* _LINUX_TCP_H */ diff --git a/include/net/tcp.h b/include/net/tcp.h index a413e9f..f15924b 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -234,6 +234,7 @@ extern int sysctl_tcp_base_mss; 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 atomic_t tcp_memory_allocated; extern struct percpu_counter tcp_sockets_allocated; @@ -340,11 +341,6 @@ static inline void tcp_dec_quickack_mode(struct sock *sk, extern void tcp_enter_quickack_mode(struct sock *sk); -static inline void tcp_clear_options(struct tcp_options_received *rx_opt) -{ - rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0; -} - #define TCP_ECN_OK 1 #define TCP_ECN_QUEUE_CWR 2 #define TCP_ECN_DEMAND_CWR 4 diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index 2dcf04d..3422c54 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -714,6 +714,14 @@ static struct ctl_table ipv4_table[] = { }, { .ctl_name = CTL_UNNUMBERED, + .procname = "tcp_cookie_size", + .data = &sysctl_tcp_cookie_size, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec + }, + { + .ctl_name = CTL_UNNUMBERED, .procname = "udp_mem", .data = &sysctl_udp_mem, .maxlen = sizeof(sysctl_udp_mem), diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 784e6ee..492ccdd 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -59,6 +59,14 @@ int sysctl_tcp_base_mss __read_mostly = 512; /* By default, RFC2861 behavior. */ int sysctl_tcp_slow_start_after_idle __read_mostly = 1; +#ifdef CONFIG_SYSCTL +/* By default, let the user enable it. */ +int sysctl_tcp_cookie_size __read_mostly = 0; +#else +int sysctl_tcp_cookie_size __read_mostly = TCP_COOKIE_MAX; +#endif + + /* 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) { -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions 2009-11-09 16:33 ` [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions William Allen Simpson @ 2009-11-10 5:31 ` Ilpo Järvinen 2009-11-10 14:43 ` William Allen Simpson 0 siblings, 1 reply; 19+ messages in thread From: Ilpo Järvinen @ 2009-11-10 5:31 UTC (permalink / raw) To: William Allen Simpson; +Cc: Linux Kernel Network Developers On Mon, 9 Nov 2009, William Allen Simpson wrote: > Define sysctl (tcp_cookie_size) to turn on and off the cookie option > default globally, instead of a compiled configuration option. > > Define per socket option (TCP_COOKIE_TRANSACTIONS) for setting constant > data values, retrieving variable cookie values, and other facilities. I guess most of the cookie stuff have nothing to do with the next, please make them separate: > Redefine two TCP header functions to accept TCP header pointer. > When subtracting, return signed int to allow error checking. Convert the users here, not in the fifth patch to avoid noise in the large fifth patch. And, I read more that fifth patch... seriously, please consider now to apply _all_ the coding style changes that have been brought to your attention by multiple people (you should know them now but for some reason again you choose to resent without complying -- I guess on purpose) into all upcoming patches you submit. -- i. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions 2009-11-10 5:31 ` Ilpo Järvinen @ 2009-11-10 14:43 ` William Allen Simpson 0 siblings, 0 replies; 19+ messages in thread From: William Allen Simpson @ 2009-11-10 14:43 UTC (permalink / raw) To: Ilpo Järvinen; +Cc: Linux Kernel Network Developers Ilpo Järvinen wrote: > I guess most of the cookie stuff have nothing to do with the next, > please make them separate: > >> Redefine two TCP header functions to accept TCP header pointer. >> When subtracting, return signed int to allow error checking. > > Convert the users here, not in the fifth patch to avoid noise in the large > fifth patch. > These comments are directly contrary to advice back on the 2nd or 3rd round, where I'd both converted and removed the old functions here, before going on to use them everywhere. I was told to *only* define the new ones here, use them in later patches as they applied, and then remove the old functions in a final cleanup patch. When I'm done (apparently very far in the distant future), there will be approximately 100 (maybe 150 or more) uses. > And, I read more that fifth patch... seriously, please consider now to > apply _all_ the coding style changes that have been brought to your > attention by multiple people (you should know them now but for some reason > again you choose to resent without complying -- I guess on purpose) into > all upcoming patches you submit. > Pardon me, but I'm having difficulty finding a substantive comment. Are you attempting humor or sarcasm? If so, it's not readily apparent. It seems more of an /ad hominem/ attack to me. There were many painstaking hours of coding style changes, every single one of them had to be individually examined side by side for correctness after I ran a series of regexp over the patches -- in 148 blocks (I just checked) of diff between the patch diffs, comprising eye-glazing trivial changes (although I used BBEdit's very nice GUI utility for the side-by-side comparison). If you don't like something else, please be specific (as others did privately). There may be places the regexp missed. But more importantly, I'd prefer actual analysis. Is there a better Linux function to use for something (like RCU that Eric recommended)? This is the *easy* patch series, based on code previously reviewed. The next series are much more conceptually difficult. Maybe that's the problem, this is too easy, and so the only things people find to comment about are trivia. Please, more thoughtful code review, less interpersonal sniping. TIA. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [net-next-2.6 PATCH v5 4/5 RFC] TCPCT part1d: generate Responder Cookie 2009-11-09 16:07 [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange William Allen Simpson ` (2 preceding siblings ...) 2009-11-09 16:33 ` [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions William Allen Simpson @ 2009-11-09 16:50 ` William Allen Simpson 2009-11-09 17:05 ` [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data William Allen Simpson 4 siblings, 0 replies; 19+ messages in thread From: William Allen Simpson @ 2009-11-09 16:50 UTC (permalink / raw) To: Linux Kernel Network Developers; +Cc: Eric Dumazet, Paul E. McKenney [-- Attachment #1: Type: text/plain, Size: 807 bytes --] Define (missing) hash message size for SHA1. Define hashing size constants specific to TCP cookies, and add new function. Maintain global secret values for tcp_cookie_generator(). This is a significantly revised implementation of earlier (15-year-old) Photuris [RFC-2522] code for the KA9Q cooperative multitasking platform. Linux RCU technique appears to be well-suited to this application, though neither of the circular queue items are freed. These functions will also be used in subsequent patches that implement additional features. Signed-off-by: William.Allen.Simpson@gmail.com --- include/linux/cryptohash.h | 1 + include/net/tcp.h | 8 +++ net/ipv4/tcp.c | 146 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 0 deletions(-) [-- Attachment #2: TCPCT+1d5.patch --] [-- Type: text/plain, Size: 6995 bytes --] diff --git a/include/linux/cryptohash.h b/include/linux/cryptohash.h index c118b2a..ec78a4b 100644 --- a/include/linux/cryptohash.h +++ b/include/linux/cryptohash.h @@ -2,6 +2,7 @@ #define __CRYPTOHASH_H #define SHA_DIGEST_WORDS 5 +#define SHA_MESSAGE_BYTES (512 /*bits*/ / 8) #define SHA_WORKSPACE_WORDS 80 void sha_init(__u32 *buf); diff --git a/include/net/tcp.h b/include/net/tcp.h index f15924b..702fd0a 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1474,6 +1474,14 @@ struct tcp_request_sock_ops { #endif }; +/* Using SHA1 for now, define some constants. + */ +#define COOKIE_DIGEST_WORDS (SHA_DIGEST_WORDS) +#define COOKIE_MESSAGE_WORDS (SHA_MESSAGE_BYTES / 4) +#define COOKIE_WORKSPACE_WORDS (COOKIE_DIGEST_WORDS + COOKIE_MESSAGE_WORDS) + +extern int tcp_cookie_generator(u32 *bakery); + extern void tcp_v4_init(void); extern void tcp_init(void); diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index e0cfa63..3ae01bf 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -264,6 +264,7 @@ #include <linux/cache.h> #include <linux/err.h> #include <linux/crypto.h> +#include <linux/time.h> #include <net/icmp.h> #include <net/tcp.h> @@ -2842,6 +2843,141 @@ EXPORT_SYMBOL(tcp_md5_hash_key); #endif +/** + * Each Responder maintains up to two secret values concurrently for + * efficient secret rollover. Each secret value has 4 states: + * + * Generating. (tcp_secret_generating != tcp_secret_primary) + * Generates new Responder-Cookies, but not yet used for primary + * verification. This is a short-term state, typically lasting only + * one round trip time (RTT). + * + * Primary. (tcp_secret_generating == tcp_secret_primary) + * Used both for generation and primary verification. + * + * Retiring. (tcp_secret_retiring != tcp_secret_secondary) + * Used for verification, until the first failure that can be + * verified by the newer Generating secret. At that time, this + * cookie's state is changed to Secondary, and the Generating + * cookie's state is changed to Primary. This is a short-term state, + * typically lasting only one round trip time (RTT). + * + * Secondary. (tcp_secret_retiring == tcp_secret_secondary) + * Used for secondary verification, after primary verification + * failures. This state lasts no more than twice the Maximum Segment + * Lifetime (2MSL). Then, the secret is discarded. + */ +struct tcp_cookie_secret { + /* The secret is divided into two parts. The digest part is the + * equivalent of previously hashing a secret and saving the state, + * and serves as an initialization vector (IV). The message part + * serves as the trailing secret. + */ + u32 secrets[COOKIE_WORKSPACE_WORDS]; + unsigned long expires; +}; + +#define TCP_SECRET_1MSL (HZ * TCP_PAWS_MSL) +#define TCP_SECRET_2MSL (HZ * TCP_PAWS_MSL * 2) +#define TCP_SECRET_LIFE (HZ * 600) + +static struct tcp_cookie_secret tcp_secret_one; +static struct tcp_cookie_secret tcp_secret_two; + +/* Essentially a circular list, without dynamic allocation. */ +static struct tcp_cookie_secret *tcp_secret_generating; +static struct tcp_cookie_secret *tcp_secret_primary; +static struct tcp_cookie_secret *tcp_secret_retiring; +static struct tcp_cookie_secret *tcp_secret_secondary; + +static DEFINE_SPINLOCK(tcp_secret_locker); + +/* Select a random word in the cookie workspace. + */ +static inline u32 tcp_cookie_work(const u32 *ws, const int n) +{ + return ws[COOKIE_DIGEST_WORDS + ((COOKIE_MESSAGE_WORDS-1) & ws[n])]; +} + +/* Fill bakery[COOKIE_WORKSPACE_WORDS] with generator, updating as needed. + * Called in softirq context. + * Returns: 0 for success. + */ +int tcp_cookie_generator(u32 *bakery) +{ + unsigned long jiffy = jiffies; + + if (unlikely(time_after_eq(jiffy, tcp_secret_generating->expires))) { + spin_lock_bh(&tcp_secret_locker); + if (!time_after_eq(jiffy, tcp_secret_generating->expires)) { + /* refreshed by another */ + spin_unlock_bh(&tcp_secret_locker); + memcpy(bakery, + &tcp_secret_generating->secrets[0], + sizeof(tcp_secret_generating->secrets)); + } else { + u32 secrets[COOKIE_WORKSPACE_WORDS]; + + /* still needs refreshing */ + get_random_bytes(secrets, sizeof(secrets)); + + /* The first time, paranoia assumes that the + * randomization function isn't as strong. But, + * this secret initialization is delayed until + * the last possible moment (packet arrival). + * Although that time is observable, it is + * unpredictably variable. Mash in the most + * volatile clock bits available, and expire the + * secret extra quickly. + */ + if (unlikely(tcp_secret_primary->expires == + tcp_secret_secondary->expires)) { + struct timespec tv; + + getnstimeofday(&tv); + secrets[COOKIE_DIGEST_WORDS+0] ^= + (u32)tv.tv_nsec; + + tcp_secret_secondary->expires = jiffy + + TCP_SECRET_1MSL + + (0x0f & tcp_cookie_work(secrets, 0)); + } else { + tcp_secret_secondary->expires = jiffy + + TCP_SECRET_LIFE + + (0xff & tcp_cookie_work(secrets, 1)); + tcp_secret_primary->expires = jiffy + + TCP_SECRET_2MSL + + (0x1f & tcp_cookie_work(secrets, 2)); + } + memcpy(&tcp_secret_secondary->secrets[0], + &secrets[0], + sizeof(secrets)); + + rcu_assign_pointer(tcp_secret_generating, + tcp_secret_secondary); + rcu_assign_pointer(tcp_secret_retiring, + tcp_secret_primary); + spin_unlock_bh(&tcp_secret_locker); + /* Neither call_rcu() or synchronize_rcu() are needed. + * Retiring data is not freed. It is replaced after + * further (locked) pointer updates, and a quiet time + * (minimum 1MSL, maximum LIFE - 2MSL). + */ + memcpy(bakery, + &secrets[0], + sizeof(secrets)); + } + } else { + rcu_read_lock_bh(); + memcpy(bakery, + &rcu_dereference(tcp_secret_generating)->secrets[0], + sizeof(tcp_secret_generating->secrets)); + rcu_read_unlock_bh(); + } + return 0; +} +EXPORT_SYMBOL(tcp_cookie_generator); + void tcp_done(struct sock *sk) { if (sk->sk_state == TCP_SYN_SENT || sk->sk_state == TCP_SYN_RECV) @@ -2876,6 +3012,7 @@ void __init tcp_init(void) struct sk_buff *skb = NULL; unsigned long nr_pages, limit; int order, i, max_share; + unsigned long jiffy = jiffies; BUILD_BUG_ON(sizeof(struct tcp_skb_cb) > sizeof(skb->cb)); @@ -2969,6 +3106,15 @@ void __init tcp_init(void) tcp_hashinfo.ehash_mask + 1, tcp_hashinfo.bhash_size); tcp_register_congestion_control(&tcp_reno); + + memset(&tcp_secret_one.secrets[0], 0, sizeof(tcp_secret_one.secrets)); + memset(&tcp_secret_two.secrets[0], 0, sizeof(tcp_secret_two.secrets)); + tcp_secret_one.expires = jiffy; /* past due */ + tcp_secret_two.expires = jiffy; /* past due */ + tcp_secret_generating = &tcp_secret_one; + tcp_secret_primary = &tcp_secret_one; + tcp_secret_retiring = &tcp_secret_two; + tcp_secret_secondary = &tcp_secret_two; } EXPORT_SYMBOL(tcp_close); -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-09 16:07 [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange William Allen Simpson ` (3 preceding siblings ...) 2009-11-09 16:50 ` [net-next-2.6 PATCH v5 4/5 RFC] TCPCT part1d: generate Responder Cookie William Allen Simpson @ 2009-11-09 17:05 ` William Allen Simpson 2009-11-10 5:05 ` Ilpo Järvinen 2009-11-10 14:29 ` Eric Dumazet 4 siblings, 2 replies; 19+ messages in thread From: William Allen Simpson @ 2009-11-09 17:05 UTC (permalink / raw) To: Linux Kernel Network Developers Cc: Eric Dumazet, Ilpo Järvinen, Joe Perches [-- Attachment #1: Type: text/plain, Size: 2346 bytes --] This is a significantly revised implementation of an earlier (year-old) patch that no longer applies cleanly, with permission of the original author (Adam Langley). That patch was previously reviewed: http://thread.gmane.org/gmane.linux.network/102586 The principle difference is using a TCP option to carry the cookie nonce, instead of a user configured offset in the data. This is more flexible and less subject to user configuration error. Such a cookie option has been suggested for many years, and is also useful without SYN data, allowing several related concepts to use the same extension option. "Re: SYN floods (was: does history repeat itself?)", September 9, 1996. http://www.merit.net/mail.archives/nanog/1996-09/msg00235.html "Re: what a new TCP header might look like", May 12, 1998. ftp://ftp.isi.edu/end2end/end2end-interest-1998.mail Data structures are carefully composed to require minimal additions. For example, the struct tcp_options_received cookie_plus variable fits between existing 16-bit and 8-bit variables, requiring no additional space (taking alignment into consideration). There are no additions to tcp_request_sock, and only 1 pointer in tcp_sock. Allocations have been rearranged to avoid requiring GFP_ATOMIC, with only one unavoidable exception in tcp_create_openreq_child(), where the tcp_sock itself is created GFP_ATOMIC. These functions will also be used in subsequent patches that implement additional features. Requires: TCPCT part 1a: add request_values parameter for sending SYNACK TCPCT part 1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED TCPCT part 1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions TCPCT part 1d: generate Responder Cookie Signed-off-by: William.Allen.Simpson@gmail.com --- include/linux/tcp.h | 29 ++++- include/net/tcp.h | 72 +++++++++++++ net/ipv4/syncookies.c | 5 +- net/ipv4/tcp.c | 127 ++++++++++++++++++++++- net/ipv4/tcp_input.c | 86 +++++++++++++-- net/ipv4/tcp_ipv4.c | 69 ++++++++++++- net/ipv4/tcp_minisocks.c | 59 ++++++++--- net/ipv4/tcp_output.c | 255 +++++++++++++++++++++++++++++++++++++++++----- net/ipv6/syncookies.c | 5 +- net/ipv6/tcp_ipv6.c | 65 +++++++++++- 10 files changed, 701 insertions(+), 71 deletions(-) [-- Attachment #2: TCPCT+1e5.patch --] [-- Type: text/plain, Size: 41725 bytes --] diff --git a/include/linux/tcp.h b/include/linux/tcp.h index 57921c1..80ebb52 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -258,31 +258,38 @@ struct tcp_options_received { sack_ok : 4, /* SACK seen on SYN packet */ snd_wscale : 4, /* Window scaling received from sender */ rcv_wscale : 4; /* Window scaling to send to receiver */ -/* SACKs data */ + u8 cookie_plus:6, /* bytes in authenticator/cookie option */ + cookie_out_never:1, + cookie_in_always:1; u8 num_sacks; /* Number of SACK blocks */ - u16 user_mss; /* mss requested by user in ioctl */ + u16 user_mss; /* mss requested by user in ioctl */ u16 mss_clamp; /* Maximal mss, negotiated at connection setup */ }; static inline void tcp_clear_options(struct tcp_options_received *rx_opt) { - rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0; + rx_opt->tstamp_ok = rx_opt->sack_ok = 0; + rx_opt->wscale_ok = rx_opt->snd_wscale = 0; + rx_opt->cookie_plus = 0; } /* This is the max number of SACKS that we'll generate and process. It's safe - * to increse this, although since: + * to increase this, although since: * size = TCPOLEN_SACK_BASE_ALIGNED (4) + n * TCPOLEN_SACK_PERBLOCK (8) * only four options will fit in a standard TCP header */ #define TCP_NUM_SACKS 4 +struct tcp_cookie_values; +struct tcp_request_sock_ops; + struct tcp_request_sock { struct inet_request_sock req; #ifdef CONFIG_TCP_MD5SIG /* Only used by TCP MD5 Signature so far. */ const struct tcp_request_sock_ops *af_specific; #endif - u32 rcv_isn; - u32 snt_isn; + u32 rcv_isn; + u32 snt_isn; }; static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req) @@ -452,6 +459,12 @@ struct tcp_sock { /* TCP MD5 Signature Option information */ struct tcp_md5sig_info *md5sig_info; #endif + + /* When the cookie options are generated and exchanged, then this + * object holds a reference to them (cookie_values->kref). Also + * contains related tcp_cookie_transactions fields. + */ + struct tcp_cookie_values *cookie_values; }; static inline struct tcp_sock *tcp_sk(const struct sock *sk) @@ -470,6 +483,10 @@ struct tcp_timewait_sock { u16 tw_md5_keylen; u8 tw_md5_key[TCP_MD5SIG_MAXKEYLEN]; #endif + /* Few sockets in timewait have cookies; in that case, then this + * object holds a reference to it (tw_cookie_values->kref) + */ + struct tcp_cookie_values *tw_cookie_values; }; static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk) diff --git a/include/net/tcp.h b/include/net/tcp.h index 702fd0a..d992c35 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -30,6 +30,7 @@ #include <linux/dmaengine.h> #include <linux/crypto.h> #include <linux/cryptohash.h> +#include <linux/kref.h> #include <net/inet_connection_sock.h> #include <net/inet_timewait_sock.h> @@ -164,6 +165,7 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo); #define TCPOPT_SACK 5 /* SACK Block */ #define TCPOPT_TIMESTAMP 8 /* Better RTT estimations/PAWS */ #define TCPOPT_MD5SIG 19 /* MD5 Signature (RFC2385) */ +#define TCPOPT_COOKIE 253 /* Cookie extension (experimental) */ /* * TCP option lengths @@ -174,6 +176,10 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo); #define TCPOLEN_SACK_PERM 2 #define TCPOLEN_TIMESTAMP 10 #define TCPOLEN_MD5SIG 18 +#define TCPOLEN_COOKIE_BASE 2 /* Cookie-less header extension */ +#define TCPOLEN_COOKIE_PAIR 3 /* Cookie pair header extension */ +#define TCPOLEN_COOKIE_MAX (TCPOLEN_COOKIE_BASE+TCP_COOKIE_MAX) +#define TCPOLEN_COOKIE_MIN (TCPOLEN_COOKIE_BASE+TCP_COOKIE_MIN) /* But this is what stacks really send out. */ #define TCPOLEN_TSTAMP_ALIGNED 12 @@ -401,6 +407,7 @@ extern int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, extern void tcp_parse_options(struct sk_buff *skb, struct tcp_options_received *opt_rx, + u8 **cryptic, int estab, struct dst_entry *dst); @@ -1482,6 +1489,71 @@ struct tcp_request_sock_ops { extern int tcp_cookie_generator(u32 *bakery); +/** + * A tcp_sock contains a pointer to the current value, and this is cloned to + * the tcp_timewait_sock. + * + * @cookie_pair: variable data from the option exchange. + * + * @cookie_desired: user specified tcpct_cookie_desired. Zero + * indicates default (sysctl_tcp_cookie_size). + * After cookie sent, remembers size of cookie. + * Range 0, TCP_COOKIE_MIN to TCP_COOKIE_MAX. + * + * @s_data_desired: user specified tcpct_s_data_desired. When the + * constant payload is specified (@s_data_constant), + * holds its length instead. + * Range 0 to TCP_MSS_DESIRED. + * + * @s_data_payload: constant data that is to be included in the + * payload of SYN or SYNACK segments when the + * cookie option is present. + */ +struct tcp_cookie_values { + struct kref kref; + u8 cookie_pair[TCP_COOKIE_PAIR_SIZE]; + u8 cookie_pair_size; + u8 cookie_desired; + u16 s_data_desired:11, + s_data_constant:1, + s_data_in:1, + s_data_out:1; + u8 s_data_payload[0]; +}; + +static inline void tcp_cookie_values_release(struct kref *kref) +{ + kfree(container_of(kref, struct tcp_cookie_values, kref)); +} + +/* The length of constant payload data. Note that s_data_desired is + * overloaded, depending on s_data_constant: either the length of constant + * data (returned here) or the limit on variable data. + */ +static inline int tcp_s_data_size(const struct tcp_sock *tp) +{ + return (tp->cookie_values != NULL && tp->cookie_values->s_data_constant) + ? tp->cookie_values->s_data_desired + : 0; +} + +/* As tcp_request_sock has already been extended in other places, the + * only remaining method is to pass stack values along as function + * parameters. These parameters are not needed after sending SYNACK. + */ +struct tcp_extend_values { + struct request_values rv; + u32 cookie_bakery[COOKIE_WORKSPACE_WORDS]; + u8 cookie_plus:6, + cookie_out_never:1, + cookie_in_always:1; +}; + +static inline struct tcp_extend_values *tcp_xv(struct request_values *rvp) +{ + return (struct tcp_extend_values *)rvp; +} + extern void tcp_v4_init(void); extern void tcp_init(void); diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 3146cc4..3503762 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -253,6 +253,8 @@ EXPORT_SYMBOL(cookie_check_timestamp); struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb, struct ip_options *opt) { + struct tcp_options_received tcp_opt; + u8 *cryptic_value; struct inet_request_sock *ireq; struct tcp_request_sock *treq; struct tcp_sock *tp = tcp_sk(sk); @@ -263,7 +265,6 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb, int mss; struct rtable *rt; __u8 rcv_wscale; - struct tcp_options_received tcp_opt; if (!sysctl_tcp_syncookies || !th->ack) goto out; @@ -341,7 +342,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb, /* check for timestamp cookie support */ memset(&tcp_opt, 0, sizeof(tcp_opt)); - tcp_parse_options(skb, &tcp_opt, 0, &rt->u.dst); + tcp_parse_options(skb, &tcp_opt, &cryptic_value, 0, &rt->u.dst); if (tcp_opt.saw_tstamp) cookie_check_timestamp(&tcp_opt); diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 3ae01bf..b650e67 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -2079,8 +2079,8 @@ static int do_tcp_setsockopt(struct sock *sk, int level, int val; int err = 0; - /* This is a string value all the others are int's */ - if (optname == TCP_CONGESTION) { + /* These are data/string values, all the others are ints */ + if (TCP_CONGESTION == optname) { char name[TCP_CA_NAME_MAX]; if (optlen < 1) @@ -2096,6 +2096,88 @@ static int do_tcp_setsockopt(struct sock *sk, int level, err = tcp_set_congestion_control(sk, name); release_sock(sk); return err; + } else if (TCP_COOKIE_TRANSACTIONS == optname) { + struct tcp_cookie_transactions ctd; + struct tcp_cookie_values *cvp = NULL; + + if (sizeof(ctd) > optlen) + return -EINVAL; + if (copy_from_user(&ctd, optval, sizeof(ctd))) + return -EFAULT; + + if (sizeof(ctd.tcpct_value) < ctd.tcpct_used + || TCP_MSS_DESIRED < ctd.tcpct_s_data_desired) + return -EINVAL; + + if (ctd.tcpct_cookie_desired == 0) { + /* default to global value */ + } else if ((0x1 & ctd.tcpct_cookie_desired) + || TCP_COOKIE_MAX < ctd.tcpct_cookie_desired + || TCP_COOKIE_MIN > ctd.tcpct_cookie_desired) { + return -EINVAL; + } + + if (TCP_COOKIE_OUT_NEVER & ctd.tcpct_flags) { + /* Supercedes all other values */ + lock_sock(sk); + if (tp->cookie_values != NULL) { + kref_put(&tp->cookie_values->kref, + tcp_cookie_values_release); + tp->cookie_values = NULL; + } + tp->rx_opt.cookie_in_always = 0; /* false */ + tp->rx_opt.cookie_out_never = 1; /* true */ + release_sock(sk); + return err; + } + + /* Allocate ancillary memory before locking. + */ + if (ctd.tcpct_used > 0 + || (tp->cookie_values == NULL + && (sysctl_tcp_cookie_size > 0 + || ctd.tcpct_cookie_desired > 0 + || ctd.tcpct_s_data_desired > 0))) { + cvp = kzalloc(sizeof(*cvp) + ctd.tcpct_used, + GFP_KERNEL); + if (cvp == NULL) + return -ENOMEM; + } + lock_sock(sk); + tp->rx_opt.cookie_in_always = + (TCP_COOKIE_IN_ALWAYS & ctd.tcpct_flags); + tp->rx_opt.cookie_out_never = 0; /* false */ + + if (tp->cookie_values != NULL) { + if (cvp != NULL) { + /* Changed values are recorded by a changed + * pointer, ensuring the cookie will differ, + * without separately hashing each value later. + */ + kref_put(&tp->cookie_values->kref, + tcp_cookie_values_release); + kref_init(&cvp->kref); + tp->cookie_values = cvp; + } else { + cvp = tp->cookie_values; + } + } + if (cvp != NULL) { + cvp->cookie_desired = ctd.tcpct_cookie_desired; + + if (ctd.tcpct_used > 0) { + memcpy(cvp->s_data_payload, ctd.tcpct_value, + ctd.tcpct_used); + cvp->s_data_desired = ctd.tcpct_used; + cvp->s_data_constant = 1; /* true */ + } else { + /* No constant payload data. */ + cvp->s_data_desired = ctd.tcpct_s_data_desired; + cvp->s_data_constant = 0; /* false */ + } + } + release_sock(sk); + return err; } if (optlen < sizeof(int)) @@ -2421,6 +2503,47 @@ static int do_tcp_getsockopt(struct sock *sk, int level, if (copy_to_user(optval, icsk->icsk_ca_ops->name, len)) return -EFAULT; return 0; + + case TCP_COOKIE_TRANSACTIONS: { + struct tcp_cookie_transactions ctd; + struct tcp_cookie_values *cvp = tp->cookie_values; + + if (get_user(len, optlen)) + return -EFAULT; + if (len < sizeof(ctd)) + return -EINVAL; + + memset(&ctd, 0, sizeof(ctd)); + ctd.tcpct_flags = (tp->rx_opt.cookie_in_always + ? TCP_COOKIE_IN_ALWAYS : 0) + + (tp->rx_opt.cookie_out_never + ? TCP_COOKIE_OUT_NEVER : 0); + + if (cvp != NULL) { + ctd.tcpct_flags += (cvp->s_data_in + ? TCP_S_DATA_IN : 0) + + (cvp->s_data_out + ? TCP_S_DATA_OUT : 0); + + ctd.tcpct_cookie_desired = cvp->cookie_desired; + ctd.tcpct_s_data_desired = cvp->s_data_desired; + + /* Cookie(s) saved, return as nonce */ + if (sizeof(ctd.tcpct_value) < cvp->cookie_pair_size) { + /* impossible? */ + return -EINVAL; + } + memcpy(&ctd.tcpct_value[0], &cvp->cookie_pair[0], + cvp->cookie_pair_size); + ctd.tcpct_used = cvp->cookie_pair_size; + } + + if (put_user(sizeof(ctd), optlen)) + return -EFAULT; + if (copy_to_user(optval, &ctd, sizeof(ctd))) + return -EFAULT; + return 0; + } default: return -ENOPROTOOPT; } diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index cc306ac..8fb50f0 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -3698,11 +3698,11 @@ old_ack: * the fast version below fails. */ void tcp_parse_options(struct sk_buff *skb, struct tcp_options_received *opt_rx, - int estab, struct dst_entry *dst) + u8 **cryptic, int estab, struct dst_entry *dst) { unsigned char *ptr; struct tcphdr *th = tcp_hdr(skb); - int length = (th->doff * 4) - sizeof(struct tcphdr); + int length = tcp_option_len_th(th); ptr = (unsigned char *)(th + 1); opt_rx->saw_tstamp = 0; @@ -3785,6 +3785,19 @@ void tcp_parse_options(struct sk_buff *skb, struct tcp_options_received *opt_rx, */ break; #endif + case TCPOPT_COOKIE: + /* This option carries 3 different lengths. + */ + if (TCPOLEN_COOKIE_MAX >= opsize + && TCPOLEN_COOKIE_MIN <= opsize) { + opt_rx->cookie_plus = opsize; + *cryptic = ptr; + } else if (TCPOLEN_COOKIE_PAIR == opsize) { + /* not yet implemented */ + } else if (TCPOLEN_COOKIE_BASE == opsize) { + /* not yet implemented */ + } + break; } ptr += opsize-2; @@ -3813,17 +3826,21 @@ static int tcp_parse_aligned_timestamp(struct tcp_sock *tp, struct tcphdr *th) * If it is wrong it falls back on tcp_parse_options(). */ static int tcp_fast_parse_options(struct sk_buff *skb, struct tcphdr *th, - struct tcp_sock *tp) + struct tcp_sock *tp, u8 **cryptic) { - if (th->doff == sizeof(struct tcphdr) >> 2) { + /* In the spirit of fast parsing, compare doff directly to shifted + * constant values. Because equality is used, short doff can be + * ignored here, and checked later. + */ + if (th->doff == (sizeof(*th) >> 2)) { tp->rx_opt.saw_tstamp = 0; return 0; } else if (tp->rx_opt.tstamp_ok && - th->doff == (sizeof(struct tcphdr)>>2)+(TCPOLEN_TSTAMP_ALIGNED>>2)) { + th->doff == ((sizeof(*th)+TCPOLEN_TSTAMP_ALIGNED)>>2)) { if (tcp_parse_aligned_timestamp(tp, th)) return 1; } - tcp_parse_options(skb, &tp->rx_opt, 1, NULL); + tcp_parse_options(skb, &tp->rx_opt, cryptic, 1, NULL); return 1; } @@ -3833,7 +3850,7 @@ static int tcp_fast_parse_options(struct sk_buff *skb, struct tcphdr *th, */ u8 *tcp_parse_md5sig_option(struct tcphdr *th) { - int length = (th->doff << 2) - sizeof (*th); + int length = tcp_option_len_th(th); u8 *ptr = (u8*)(th + 1); /* If the TCP option is too short, we can short cut */ @@ -5077,10 +5094,11 @@ out: static int tcp_validate_incoming(struct sock *sk, struct sk_buff *skb, struct tcphdr *th, int syn_inerr) { + u8 *cv; struct tcp_sock *tp = tcp_sk(sk); /* RFC1323: H1. Apply PAWS check first. */ - if (tcp_fast_parse_options(skb, th, tp) && tp->rx_opt.saw_tstamp && + if (tcp_fast_parse_options(skb, th, tp, &cv) && tp->rx_opt.saw_tstamp && tcp_paws_discard(sk, skb)) { if (!th->rst) { NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED); @@ -5368,12 +5386,15 @@ discard: static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb, struct tcphdr *th, unsigned len) { - struct tcp_sock *tp = tcp_sk(sk); + u8 *cryptic_value; struct inet_connection_sock *icsk = inet_csk(sk); - int saved_clamp = tp->rx_opt.mss_clamp; + struct tcp_sock *tp = tcp_sk(sk); struct dst_entry *dst = __sk_dst_get(sk); + struct tcp_cookie_values *cvp = tp->cookie_values; + int saved_clamp = tp->rx_opt.mss_clamp; + int queued = 0; - tcp_parse_options(skb, &tp->rx_opt, 0, dst); + tcp_parse_options(skb, &tp->rx_opt, &cryptic_value, 0, dst); if (th->ack) { /* rfc793: @@ -5470,6 +5491,44 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb, * Change state from SYN-SENT only after copied_seq * is initialized. */ tp->copied_seq = tp->rcv_nxt; + + if (cvp != NULL + && cvp->cookie_pair_size > 0 + && tp->rx_opt.cookie_plus > 0) { + int cookie_size = tp->rx_opt.cookie_plus + - TCPOLEN_COOKIE_BASE; + int cookie_pair_size = cookie_size + + cvp->cookie_desired; + + /* A cookie extension option was sent and returned. + * Note that each incoming SYNACK replaces the + * Responder cookie. The initial exchange is most + * fragile, as protection against spoofing relies + * entirely upon the sequence and timestamp (above). + * This replacement strategy allows the correct pair to + * pass through, while any others will be filtered via + * Responder verification later. + */ + if (sizeof(cvp->cookie_pair) >= cookie_pair_size) { + memcpy(&cvp->cookie_pair[cvp->cookie_desired], + cryptic_value, cookie_size); + cvp->cookie_pair_size = cookie_pair_size; + } + + if (tcp_header_len_th(th) < skb->len) { + /* Queue incoming transaction data. */ + __skb_pull(skb, tcp_header_len_th(th)); + __skb_queue_tail(&sk->sk_receive_queue, skb); + skb_set_owner_r(skb, sk); + sk->sk_data_ready(sk, 0); + cvp->s_data_in = 1; /* true */ + queued = 1; /* should be amount? */ + tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq; + tp->rcv_wup = TCP_SKB_CB(skb)->end_seq; + tp->copied_seq = TCP_SKB_CB(skb)->seq + 1; + } + } + smp_mb(); tcp_set_state(sk, TCP_ESTABLISHED); @@ -5521,11 +5580,14 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb, TCP_DELACK_MAX, TCP_RTO_MAX); discard: - __kfree_skb(skb); + if (queued == 0) + __kfree_skb(skb); return 0; } else { tcp_send_ack(sk); } + if (queued > 0) + return 0; /* amount queued? */ return -1; } diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 0718fde..5992d0b 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1213,9 +1213,12 @@ static struct timewait_sock_ops tcp_timewait_sock_ops = { int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) { + struct tcp_extend_values tmp_ext; struct tcp_options_received tmp_opt; + u8 *cryptic_value; struct request_sock *req; struct inet_request_sock *ireq; + struct tcp_sock *tp = tcp_sk(sk); struct dst_entry *dst = NULL; __be32 saddr = ip_hdr(skb)->saddr; __be32 daddr = ip_hdr(skb)->daddr; @@ -1271,15 +1274,50 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) tcp_clear_options(&tmp_opt); tmp_opt.mss_clamp = TCP_MSS_DEFAULT; - tmp_opt.user_mss = tcp_sk(sk)->rx_opt.user_mss; + tmp_opt.user_mss = tp->rx_opt.user_mss; + tcp_parse_options(skb, &tmp_opt, &cryptic_value, 0, dst); + + if (tmp_opt.cookie_plus > 0 + && tmp_opt.saw_tstamp + && !tp->rx_opt.cookie_out_never + && (sysctl_tcp_cookie_size > 0 + || (tp->cookie_values != NULL + && tp->cookie_values->cookie_desired > 0))) { + u8 *c; + u32 *mess = &tmp_ext.cookie_bakery[COOKIE_DIGEST_WORDS]; + int l = tmp_opt.cookie_plus - TCPOLEN_COOKIE_BASE; + + if (tcp_cookie_generator(&tmp_ext.cookie_bakery[0]) != 0) + goto drop_and_release; + + /* Secret recipe starts with IP addresses */ + *mess++ ^= daddr; + *mess++ ^= saddr; + + /* plus variable length Initiator Cookie */ + c = (u8 *)mess; + while (l-- > 0) { + *c++ ^= *cryptic_value++; + } - tcp_parse_options(skb, &tmp_opt, 0, dst); +#ifdef CONFIG_SYN_COOKIES + want_cookie = 0; /* not our kind of cookie */ +#endif + tmp_ext.cookie_out_never = 0; /* false */ + tmp_ext.cookie_plus = tmp_opt.cookie_plus; + } else if (!tp->rx_opt.cookie_in_always) { + /* redundant indications, but ensure initialization. */ + tmp_ext.cookie_out_never = 1; /* true */ + tmp_ext.cookie_plus = 0; + } else { + goto drop_and_release; + } + tmp_ext.cookie_in_always = tp->rx_opt.cookie_in_always; if (want_cookie && !tmp_opt.saw_tstamp) tcp_clear_options(&tmp_opt); tmp_opt.tstamp_ok = tmp_opt.saw_tstamp; - tcp_openreq_init(req, &tmp_opt, skb); if (security_inet_conn_request(sk, skb, req)) @@ -1339,7 +1377,8 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) } tcp_rsk(req)->snt_isn = isn; - if (__tcp_v4_send_synack(sk, req, dst, NULL) + if (__tcp_v4_send_synack(sk, req, dst, + (struct request_values *)&tmp_ext) || want_cookie) goto drop_and_free; @@ -1834,6 +1873,19 @@ static int tcp_v4_init_sock(struct sock *sk) tp->af_specific = &tcp_sock_ipv4_specific; #endif + /* TCP Cookie Transactions */ + if (sysctl_tcp_cookie_size > 0) { + /* Default, cookies without s_data. */ + tp->cookie_values = + kzalloc(sizeof(*tp->cookie_values), + sk->sk_allocation); + if (tp->cookie_values != NULL) + kref_init(&tp->cookie_values->kref); + } + /* Presumed zeroed, in order of appearance: + * cookie_in_always, cookie_out_never, extend_timestamp, + * s_data_constant, s_data_in, s_data_out + */ sk->sk_sndbuf = sysctl_tcp_wmem[1]; sk->sk_rcvbuf = sysctl_tcp_rmem[1]; @@ -1887,6 +1939,15 @@ void tcp_v4_destroy_sock(struct sock *sk) sk->sk_sndmsg_page = NULL; } + /* + * If cookie or s_data exists, remove it. + */ + if (tp->cookie_values != NULL) { + kref_put(&tp->cookie_values->kref, + tcp_cookie_values_release); + tp->cookie_values = NULL; + } + percpu_counter_dec(&tcp_sockets_allocated); } diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c index 7a42990..db46797 100644 --- a/net/ipv4/tcp_minisocks.c +++ b/net/ipv4/tcp_minisocks.c @@ -96,13 +96,14 @@ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb, const struct tcphdr *th) { - struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw); struct tcp_options_received tmp_opt; + u8 *cryptic_value; + struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw); int paws_reject = 0; if (th->doff > (sizeof(*th) >> 2) && tcptw->tw_ts_recent_stamp) { tmp_opt.tstamp_ok = 1; - tcp_parse_options(skb, &tmp_opt, 1, NULL); + tcp_parse_options(skb, &tmp_opt, &cryptic_value, 1, NULL); if (tmp_opt.saw_tstamp) { tmp_opt.ts_recent = tcptw->tw_ts_recent; @@ -389,14 +390,42 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req, const struct inet_request_sock *ireq = inet_rsk(req); struct tcp_request_sock *treq = tcp_rsk(req); struct inet_connection_sock *newicsk = inet_csk(newsk); - struct tcp_sock *newtp; + struct tcp_sock *newtp = tcp_sk(newsk); + struct tcp_sock *oldtp = tcp_sk(sk); + struct tcp_cookie_values *oldcvp = oldtp->cookie_values; + + /* TCP Cookie Transactions require space for the cookie pair, + * as it differs for each connection. There is no need to + * copy any s_data stored at the original socket. Failure + * will prevent resuming the connection. + * + * Presumed copied, in order of appearance: + * cookie_in_always, cookie_out_never + */ + if (oldcvp != NULL) { + struct tcp_cookie_values *newcvp = + kzalloc(sizeof(*newtp->cookie_values), + GFP_ATOMIC); + + if (newcvp != NULL) { + kref_init(&newcvp->kref); + newcvp->cookie_desired = + oldcvp->cookie_desired; + newtp->cookie_values = newcvp; + } else { + /* Not Yet Implemented */ + newtp->cookie_values = NULL; + } + } /* Now setup tcp_sock */ - newtp = tcp_sk(newsk); newtp->pred_flags = 0; - newtp->rcv_wup = newtp->copied_seq = newtp->rcv_nxt = treq->rcv_isn + 1; - newtp->snd_sml = newtp->snd_una = newtp->snd_nxt = treq->snt_isn + 1; - newtp->snd_up = treq->snt_isn + 1; + + newtp->rcv_wup = newtp->copied_seq = + newtp->rcv_nxt = treq->rcv_isn + 1; + + newtp->snd_sml = newtp->snd_una = newtp->snd_nxt = + newtp->snd_up = treq->snt_isn + 1 + tcp_s_data_size(oldtp); tcp_prequeue_init(newtp); @@ -429,8 +458,8 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req, tcp_set_ca_state(newsk, TCP_CA_Open); tcp_init_xmit_timers(newsk); skb_queue_head_init(&newtp->out_of_order_queue); - newtp->write_seq = treq->snt_isn + 1; - newtp->pushed_seq = newtp->write_seq; + newtp->write_seq = newtp->pushed_seq = + treq->snt_isn + 1 + tcp_s_data_size(oldtp); newtp->rx_opt.saw_tstamp = 0; @@ -495,15 +524,16 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb, struct request_sock *req, struct request_sock **prev) { + struct tcp_options_received tmp_opt; + u8 *cryptic_value; + struct sock *child; const struct tcphdr *th = tcp_hdr(skb); __be32 flg = tcp_flag_word(th) & (TCP_FLAG_RST|TCP_FLAG_SYN|TCP_FLAG_ACK); int paws_reject = 0; - struct tcp_options_received tmp_opt; - struct sock *child; - if ((th->doff > (sizeof(struct tcphdr)>>2)) && (req->ts_recent)) { + if ((th->doff > (sizeof(*th) >> 2)) && (req->ts_recent)) { tmp_opt.tstamp_ok = 1; - tcp_parse_options(skb, &tmp_opt, 1, NULL); + tcp_parse_options(skb, &tmp_opt, &cryptic_value, 1, NULL); if (tmp_opt.saw_tstamp) { tmp_opt.ts_recent = req->ts_recent; @@ -596,7 +626,8 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb, * Invalid ACK: reset will be sent by listening socket */ if ((flg & TCP_FLAG_ACK) && - (TCP_SKB_CB(skb)->ack_seq != tcp_rsk(req)->snt_isn + 1)) + (TCP_SKB_CB(skb)->ack_seq != tcp_rsk(req)->snt_isn + 1 + + tcp_s_data_size(tcp_sk(sk)))) return sk; /* Also, it would be not so bad idea to check rcv_tsecr, which diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 492ccdd..08dc649 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -370,15 +370,45 @@ static inline int tcp_urg_mode(const struct tcp_sock *tp) #define OPTION_TS (1 << 1) #define OPTION_MD5 (1 << 2) #define OPTION_WSCALE (1 << 3) +#define OPTION_COOKIE_EXTENSION (1 << 4) struct tcp_out_options { u8 options; /* bit field of OPTION_* */ u8 ws; /* window scale, 0 to disable */ + u8 hash_size; /* bytes in hash */ u8 num_sack_blocks; /* number of SACK blocks to include */ u16 mss; /* 0 to disable */ __u32 tsval, tsecr; /* need to include OPTION_TS */ + __u8 *hash_location; /* temporary pointer, overloaded */ }; +/* The sysctl int routines are generic, so check consistency here. + */ +static u8 tcp_cookie_size_check(u8 desired) +{ + if (desired > 0) { + /* previously specified */ + return desired; + } + if (sysctl_tcp_cookie_size <= 0) { + /* no default specified */ + return 0; + } + if (sysctl_tcp_cookie_size < TCP_COOKIE_MIN) { + /* value too small, increase to minimum */ + return TCP_COOKIE_MIN; + } + if (sysctl_tcp_cookie_size > TCP_COOKIE_MAX) { + /* value too large, decrease to maximum */ + return TCP_COOKIE_MAX; + } + if (0x1 & sysctl_tcp_cookie_size) { + /* 8-bit multiple, illegal, fix it */ + return (u8)(sysctl_tcp_cookie_size + 0x1); + } + return (u8)sysctl_tcp_cookie_size; +} + /* Write previously computed TCP options to the packet. * * Beware: Something in the Internet is very sensitive to the ordering of @@ -393,17 +423,34 @@ struct tcp_out_options { * (but it may well be that other scenarios fail similarly). */ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, - const struct tcp_out_options *opts, - __u8 **md5_hash) { - if (unlikely(OPTION_MD5 & opts->options)) { - *ptr++ = htonl((TCPOPT_NOP << 24) | - (TCPOPT_NOP << 16) | - (TCPOPT_MD5SIG << 8) | - TCPOLEN_MD5SIG); - *md5_hash = (__u8 *)ptr; + struct tcp_out_options *opts) +{ + u8 options = opts->options; /* mungable copy */ + + /* Having both authentication and cookies for security is redundant, + * and there's certainly not enough room. Instead, the cookie-less + * extension variant is proposed. + * + * Consider the pessimal case with authentication. The options + * could look like: + * COOKIE|MD5(20) + MSS(4) + SACK|TS(12) + WSCALE(4) == 40 + */ + if (unlikely(OPTION_MD5 & options)) { + if (unlikely(OPTION_COOKIE_EXTENSION & options)) { + *ptr++ = htonl((TCPOPT_COOKIE << 24) | + (TCPOLEN_COOKIE_BASE << 16) | + (TCPOPT_MD5SIG << 8) | + TCPOLEN_MD5SIG); + } else { + *ptr++ = htonl((TCPOPT_NOP << 24) | + (TCPOPT_NOP << 16) | + (TCPOPT_MD5SIG << 8) | + TCPOLEN_MD5SIG); + } + options &= ~OPTION_COOKIE_EXTENSION; + /* overload cookie hash location */ + opts->hash_location = (__u8 *)ptr; ptr += 4; - } else { - *md5_hash = NULL; } if (unlikely(opts->mss)) { @@ -412,12 +459,13 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, opts->mss); } - if (likely(OPTION_TS & opts->options)) { - if (unlikely(OPTION_SACK_ADVERTISE & opts->options)) { + if (likely(OPTION_TS & options)) { + if (unlikely(OPTION_SACK_ADVERTISE & options)) { *ptr++ = htonl((TCPOPT_SACK_PERM << 24) | (TCPOLEN_SACK_PERM << 16) | (TCPOPT_TIMESTAMP << 8) | TCPOLEN_TIMESTAMP); + options &= ~OPTION_SACK_ADVERTISE; } else { *ptr++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) | @@ -428,15 +476,52 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, *ptr++ = htonl(opts->tsecr); } - if (unlikely(OPTION_SACK_ADVERTISE & opts->options && - !(OPTION_TS & opts->options))) { + /* Specification requires after timestamp, so do it now. + * + * Consider the pessimal case without authentication. The options + * could look like: + * MSS(4) + SACK|TS(12) + COOKIE(20) + WSCALE(4) == 40 + */ + if (unlikely(OPTION_COOKIE_EXTENSION & options)) { + __u8 *cookie_copy = opts->hash_location; + u8 cookie_size = opts->hash_size; + + if (unlikely(0x1 & cookie_size)) { + /* 8-bit multiple, illegal, ignore */ + cookie_size = 0; + } else if (likely(0x2 & cookie_size)) { + __u8 *p = (__u8 *)ptr; + + /* 16-bit multiple */ + *p++ = TCPOPT_COOKIE; + *p++ = TCPOLEN_COOKIE_BASE + cookie_size; + *p++ = *cookie_copy++; + *p++ = *cookie_copy++; + ptr++; + cookie_size -= 2; + } else { + /* 32-bit multiple */ + *ptr++ = htonl(((TCPOPT_NOP << 24) | + (TCPOPT_NOP << 16) | + (TCPOPT_COOKIE << 8) | + TCPOLEN_COOKIE_BASE) + + cookie_size); + } + + if (cookie_size > 0) { + memcpy(ptr, cookie_copy, cookie_size); + ptr += (cookie_size >> 2); + } + } + + if (unlikely(OPTION_SACK_ADVERTISE & options)) { *ptr++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) | (TCPOPT_SACK_PERM << 8) | TCPOLEN_SACK_PERM); } - if (unlikely(OPTION_WSCALE & opts->options)) { + if (unlikely(OPTION_WSCALE & options)) { *ptr++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_WINDOW << 16) | (TCPOLEN_WINDOW << 8) | @@ -471,8 +556,12 @@ static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb, struct tcp_out_options *opts, struct tcp_md5sig_key **md5) { struct tcp_sock *tp = tcp_sk(sk); - unsigned size = 0; + struct tcp_cookie_values *cvp = tp->cookie_values; struct dst_entry *dst = __sk_dst_get(sk); + unsigned size = 0; + u8 cookie_size = (!tp->rx_opt.cookie_out_never && cvp != NULL) + ? tcp_cookie_size_check(cvp->cookie_desired) + : 0; #ifdef CONFIG_TCP_MD5SIG *md5 = tp->af_specific->md5_lookup(sk, sk); @@ -517,6 +606,53 @@ static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb, size += TCPOLEN_SACKPERM_ALIGNED; } + /* Note that timestamps are required by the specification. + * + * Odd numbers of bytes are prohibited by the specification, ensuring + * that the cookie is 16-bit aligned, and the resulting cookie pair is + * 32-bit aligned. + */ + if (*md5 == NULL + && (OPTION_TS & opts->options) + && cookie_size > 0) { + int need = TCPOLEN_COOKIE_BASE + cookie_size; + int remaining = MAX_TCP_OPTION_SPACE - size; + + if (0x2 & need) { + /* 32-bit multiple */ + need += 2; /* NOPs */ + + if (need > remaining) { + /* try shrinking cookie to fit */ + cookie_size -= 2; + need -= 4; + } + } + while (need > remaining && TCP_COOKIE_MIN <= cookie_size) { + cookie_size -= 4; + need -= 4; + } + if (TCP_COOKIE_MIN <= cookie_size) { + opts->options |= OPTION_COOKIE_EXTENSION; + opts->hash_location = (__u8 *)&cvp->cookie_pair[0]; + opts->hash_size = cookie_size; + + /* Remember for future incarnations. */ + cvp->cookie_desired = cookie_size; + + if (cvp->cookie_desired != cvp->cookie_pair_size) { + /* Currently use random bytes as a nonce, + * assuming these are completely unpredictable + * by hostile users of the same system. + */ + get_random_bytes(&cvp->cookie_pair[0], + cookie_size); + cvp->cookie_pair_size = cookie_size; + } + + size += need; + } + } return size; } @@ -525,9 +661,14 @@ static unsigned tcp_synack_options(struct sock *sk, struct request_sock *req, unsigned mss, struct sk_buff *skb, struct tcp_out_options *opts, - struct tcp_md5sig_key **md5) { - unsigned size = 0; + struct tcp_md5sig_key **md5, + struct tcp_extend_values *xvp) +{ struct inet_request_sock *ireq = inet_rsk(req); + unsigned size = 0; + u8 cookie_plus = (xvp != NULL && !xvp->cookie_out_never) + ? xvp->cookie_plus + : 0; char doing_ts; #ifdef CONFIG_TCP_MD5SIG @@ -566,6 +707,28 @@ static unsigned tcp_synack_options(struct sock *sk, size += TCPOLEN_SACKPERM_ALIGNED; } + /* Similar rationale to tcp_syn_options() applies here, too. + * If the <SYN> options fit, the same options should fit now! + */ + if (*md5 == NULL + && doing_ts + && cookie_plus > TCPOLEN_COOKIE_BASE) { + int need = cookie_plus; /* has TCPOLEN_COOKIE_BASE */ + int remaining = MAX_TCP_OPTION_SPACE - size; + + if (0x2 & need) { + /* 32-bit multiple */ + need += 2; /* NOPs */ + } + if (need <= remaining) { + opts->options |= OPTION_COOKIE_EXTENSION; + opts->hash_size = cookie_plus - TCPOLEN_COOKIE_BASE; + size += need; + } else { + /* There's no error return, so flag it. */ + xvp->cookie_out_never = 1; /* true */ + } + } return size; } @@ -632,7 +795,6 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, struct tcp_out_options opts; unsigned tcp_options_size, tcp_header_size; struct tcp_md5sig_key *md5; - __u8 *md5_hash_location; struct tcphdr *th; int err; @@ -703,7 +865,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, } } - tcp_options_write((__be32 *)(th + 1), tp, &opts, &md5_hash_location); + tcp_options_write((__be32 *)(th + 1), tp, &opts); if (likely((tcb->flags & TCPCB_FLAG_SYN) == 0)) TCP_ECN_send(sk, skb, tcp_header_size); @@ -711,7 +873,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, /* Calculate the MD5 hash, as we have all we need now */ if (md5) { sk->sk_route_caps &= ~NETIF_F_GSO_MASK; - tp->af_specific->calc_md5_hash(md5_hash_location, + tp->af_specific->calc_md5_hash(opts.hash_location, md5, sk, NULL, skb); } #endif @@ -2235,14 +2397,14 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, struct request_values *rvp, struct request_sock *req) { + struct tcp_out_options opts; + struct tcp_extend_values *xvp = tcp_xv(rvp); struct inet_request_sock *ireq = inet_rsk(req); struct tcp_sock *tp = tcp_sk(sk); struct tcphdr *th; - int tcp_header_size; - struct tcp_out_options opts; struct sk_buff *skb; struct tcp_md5sig_key *md5; - __u8 *md5_hash_location; + int tcp_header_size; int mss; skb = sock_wmalloc(sk, MAX_TCP_HEADER + 15, 1, GFP_ATOMIC); @@ -2280,7 +2442,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, #endif TCP_SKB_CB(skb)->when = tcp_time_stamp; tcp_header_size = tcp_synack_options(sk, req, mss, - skb, &opts, &md5) + + skb, &opts, &md5, xvp) + sizeof(struct tcphdr); skb_push(skb, tcp_header_size); @@ -2298,19 +2460,58 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, */ tcp_init_nondata_skb(skb, tcp_rsk(req)->snt_isn, TCPCB_FLAG_SYN | TCPCB_FLAG_ACK); + + if (OPTION_COOKIE_EXTENSION & opts.options) { + const struct tcp_cookie_values *cvp = tp->cookie_values; + + if (cvp != NULL + && cvp->s_data_constant + && cvp->s_data_desired > 0) { + u8 *buf = skb_put(skb, cvp->s_data_desired); + + /* copy data directly from the listening socket. */ + memcpy(buf, cvp->s_data_payload, cvp->s_data_desired); + TCP_SKB_CB(skb)->end_seq += cvp->s_data_desired; + } + + if (opts.hash_size > 0) { + __u32 workspace[SHA_WORKSPACE_WORDS]; + u32 *mess = &xvp->cookie_bakery[COOKIE_DIGEST_WORDS]; + u32 *tail = &mess[COOKIE_MESSAGE_WORDS-1]; + + /* Secret recipe depends on the Timestamp, (future) + * Sequence and Acknowledgment Numbers, Initiator + * Cookie, and others handled by IP variant caller. + */ + *tail-- ^= opts.tsval; + *tail-- ^= tcp_rsk(req)->rcv_isn + 1; + *tail-- ^= TCP_SKB_CB(skb)->seq + 1; + + /* recommended */ + *tail-- ^= ((th->dest << 16) | th->source); + *tail-- ^= (u32)cvp; /* per sockopt */ + + sha_transform((__u32 *)&xvp->cookie_bakery[0], + (char *)mess, + &workspace[0]); + opts.hash_location = + (__u8 *)&xvp->cookie_bakery[0]; + } + } + th->seq = htonl(TCP_SKB_CB(skb)->seq); th->ack_seq = htonl(tcp_rsk(req)->rcv_isn + 1); /* RFC1323: The window in SYN & SYN/ACK segments is never scaled. */ th->window = htons(min(req->rcv_wnd, 65535U)); - tcp_options_write((__be32 *)(th + 1), tp, &opts, &md5_hash_location); + tcp_options_write((__be32 *)(th + 1), tp, &opts); th->doff = (tcp_header_size >> 2); TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTSEGS); #ifdef CONFIG_TCP_MD5SIG /* Okay, we have all we need - do the md5 hash if needed */ if (md5) { - tcp_rsk(req)->af_specific->calc_md5_hash(md5_hash_location, + tcp_rsk(req)->af_specific->calc_md5_hash(opts.hash_location, md5, NULL, req, skb); } #endif diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c index 612fc53..a60b934 100644 --- a/net/ipv6/syncookies.c +++ b/net/ipv6/syncookies.c @@ -159,6 +159,8 @@ static inline int cookie_check(struct sk_buff *skb, __u32 cookie) struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) { + struct tcp_options_received tcp_opt; + u8 *cryptic_value; struct inet_request_sock *ireq; struct inet6_request_sock *ireq6; struct tcp_request_sock *treq; @@ -171,7 +173,6 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) int mss; struct dst_entry *dst; __u8 rcv_wscale; - struct tcp_options_received tcp_opt; if (!sysctl_tcp_syncookies || !th->ack) goto out; @@ -254,7 +255,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) /* check for timestamp cookie support */ memset(&tcp_opt, 0, sizeof(tcp_opt)); - tcp_parse_options(skb, &tcp_opt, 0, dst); + tcp_parse_options(skb, &tcp_opt, &cryptic_value, 0, dst); if (tcp_opt.saw_tstamp) cookie_check_timestamp(&tcp_opt); diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index b528f75..14b6a32 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1162,7 +1162,9 @@ static struct sock *tcp_v6_hnd_req(struct sock *sk,struct sk_buff *skb) */ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) { + struct tcp_extend_values tmp_ext; struct tcp_options_received tmp_opt; + u8 *cryptic_value; struct request_sock *req; struct inet6_request_sock *treq; struct ipv6_pinfo *np = inet6_sk(sk); @@ -1206,8 +1208,53 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) tcp_clear_options(&tmp_opt); tmp_opt.mss_clamp = IPV6_MIN_MTU - sizeof(struct tcphdr) - sizeof(struct ipv6hdr); tmp_opt.user_mss = tp->rx_opt.user_mss; + tcp_parse_options(skb, &tmp_opt, &cryptic_value, 0, dst); + + if (tmp_opt.cookie_plus > 0 + && tmp_opt.saw_tstamp + && !tp->rx_opt.cookie_out_never + && (sysctl_tcp_cookie_size > 0 + || (tp->cookie_values != NULL + && tp->cookie_values->cookie_desired > 0))) { + u8 *c; + u32 *d; + u32 *mess = &tmp_ext.cookie_bakery[COOKIE_DIGEST_WORDS]; + int l = tmp_opt.cookie_plus - TCPOLEN_COOKIE_BASE; + + if (tcp_cookie_generator(&tmp_ext.cookie_bakery[0]) != 0) + goto drop_and_free; + + /* Secret recipe starts with IP addresses */ + d = &ipv6_hdr(skb)->daddr.s6_addr32[0]; + *mess++ ^= *d++; + *mess++ ^= *d++; + *mess++ ^= *d++; + *mess++ ^= *d++; + d = &ipv6_hdr(skb)->saddr.s6_addr32[0]; + *mess++ ^= *d++; + *mess++ ^= *d++; + *mess++ ^= *d++; + *mess++ ^= *d++; + + /* plus variable length Initiator Cookie */ + c = (u8 *)mess; + while (l-- > 0) { + *c++ ^= *cryptic_value++; + } - tcp_parse_options(skb, &tmp_opt, 0, dst); +#ifdef CONFIG_SYN_COOKIES + want_cookie = 0; /* not our kind of cookie */ +#endif + tmp_ext.cookie_out_never = 0; /* false */ + tmp_ext.cookie_plus = tmp_opt.cookie_plus; + } else if (!tp->rx_opt.cookie_in_always) { + /* redundant indications, but ensure initialization. */ + tmp_ext.cookie_out_never = 1; /* true */ + tmp_ext.cookie_plus = 0; + } else { + goto drop_and_free; + } + tmp_ext.cookie_in_always = tp->rx_opt.cookie_in_always; if (want_cookie && !tmp_opt.saw_tstamp) tcp_clear_options(&tmp_opt); @@ -1244,7 +1291,8 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) security_inet_conn_request(sk, skb, req); - if (tcp_v6_send_synack(sk, req, NULL) + if (tcp_v6_send_synack(sk, req, + (struct request_values *)&tmp_ext) || want_cookie) goto drop_and_free; @@ -1865,6 +1913,19 @@ static int tcp_v6_init_sock(struct sock *sk) tp->af_specific = &tcp_sock_ipv6_specific; #endif + /* TCP Cookie Transactions */ + if (sysctl_tcp_cookie_size > 0) { + /* Default, cookies without s_data. */ + tp->cookie_values = + kzalloc(sizeof(*tp->cookie_values), + sk->sk_allocation); + if (tp->cookie_values != NULL) + kref_init(&tp->cookie_values->kref); + } + /* Presumed zeroed, in order of appearance: + * cookie_in_always, cookie_out_never, extend_timestamp, + * s_data_constant, s_data_in, s_data_out + */ sk->sk_sndbuf = sysctl_tcp_wmem[1]; sk->sk_rcvbuf = sysctl_tcp_rmem[1]; -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-09 17:05 ` [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data William Allen Simpson @ 2009-11-10 5:05 ` Ilpo Järvinen 2009-11-10 13:41 ` William Allen Simpson 2009-11-10 14:29 ` Eric Dumazet 1 sibling, 1 reply; 19+ messages in thread From: Ilpo Järvinen @ 2009-11-10 5:05 UTC (permalink / raw) To: William Allen Simpson Cc: Linux Kernel Network Developers, Eric Dumazet, Joe Perches On Mon, 9 Nov 2009, William Allen Simpson wrote: > This is a significantly revised implementation of an earlier (year-old) > patch that no longer applies cleanly, with permission of the original > author (Adam Langley). That patch was previously reviewed: > > http://thread.gmane.org/gmane.linux.network/102586 > > The principle difference is using a TCP option to carry the cookie nonce, > instead of a user configured offset in the data. This is more flexible and > less subject to user configuration error. Such a cookie option has been > suggested for many years, and is also useful without SYN data, allowing > several related concepts to use the same extension option. > > "Re: SYN floods (was: does history repeat itself?)", September 9, 1996. > http://www.merit.net/mail.archives/nanog/1996-09/msg00235.html > > "Re: what a new TCP header might look like", May 12, 1998. > ftp://ftp.isi.edu/end2end/end2end-interest-1998.mail > > Data structures are carefully composed to require minimal additions. > For example, the struct tcp_options_received cookie_plus variable fits > between existing 16-bit and 8-bit variables, requiring no additional > space (taking alignment into consideration). There are no additions to > tcp_request_sock, and only 1 pointer in tcp_sock. > > Allocations have been rearranged to avoid requiring GFP_ATOMIC, with > only one unavoidable exception in tcp_create_openreq_child(), where the > tcp_sock itself is created GFP_ATOMIC. > > These functions will also be used in subsequent patches that implement > additional features. > > Requires: > TCPCT part 1a: add request_values parameter for sending SYNACK > TCPCT part 1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED > TCPCT part 1c: sysctl_tcp_cookie_size, socket option > TCP_COOKIE_TRANSACTIONS, functions > TCPCT part 1d: generate Responder Cookie > > Signed-off-by: William.Allen.Simpson@gmail.com > --- > include/linux/tcp.h | 29 ++++- > include/net/tcp.h | 72 +++++++++++++ > net/ipv4/syncookies.c | 5 +- > net/ipv4/tcp.c | 127 ++++++++++++++++++++++- > net/ipv4/tcp_input.c | 86 +++++++++++++-- > net/ipv4/tcp_ipv4.c | 69 ++++++++++++- > net/ipv4/tcp_minisocks.c | 59 ++++++++--- > net/ipv4/tcp_output.c | 255 > +++++++++++++++++++++++++++++++++++++++++----- > net/ipv6/syncookies.c | 5 +- > net/ipv6/tcp_ipv6.c | 65 +++++++++++- > 10 files changed, 701 insertions(+), 71 deletions(-) > One general comment. ...This particular patch still has lots of noise which does not belong to the context of this change. ...Please try to minimize. Eg., if you don't like sizeof(struct tcphdr) but prefer sizeof(*th), you certainly don't have to do it in this particular patch! ...Also some comment changes which certainly are not mandatory nor even related. -- i. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 5:05 ` Ilpo Järvinen @ 2009-11-10 13:41 ` William Allen Simpson 2009-11-10 14:00 ` Ilpo Järvinen 2009-11-10 14:30 ` Ilpo Järvinen 0 siblings, 2 replies; 19+ messages in thread From: William Allen Simpson @ 2009-11-10 13:41 UTC (permalink / raw) To: Ilpo Järvinen Cc: Linux Kernel Network Developers, Eric Dumazet, Joe Perches Ilpo Järvinen wrote: > On Mon, 9 Nov 2009, William Allen Simpson wrote: >>... >> Data structures are carefully composed to require minimal additions. >> For example, the struct tcp_options_received cookie_plus variable fits >> between existing 16-bit and 8-bit variables, requiring no additional >> space (taking alignment into consideration). There are no additions to >> tcp_request_sock, and only 1 pointer in tcp_sock. >>... > > One general comment. ...This particular patch still has lots of noise > which does not belong to the context of this change. ...Please try to > minimize. Eg., if you don't like sizeof(struct tcphdr) but prefer > sizeof(*th), you certainly don't have to do it in this particular patch! This *is* actually in CodingStyle (line 679), and I'm trying to conform: <blockquote> The preferred form for passing a size of a struct is the following: p = kmalloc(sizeof(*p), ...); The alternative form where struct name is spelled out hurts readability and introduces an opportunity for a bug when the pointer variable type is changed but the corresponding sizeof that is passed to a memory allocator is not. </blockquote> Maybe some of these anticipate part 2, so I can defer them to later. I've already coded much of part 2, so things are bleeding back and forth. > ...Also some comment changes which certainly are not mandatory nor even > related. > Hmmm.... 1) The first is a hole left by the removal of the data fields some time ago, but they left the old (now incorrect) comment: -/* SACKs data */ + u8 cookie_plus:6, /* bytes in authenticator/cookie option */ + cookie_out_never:1, + cookie_in_always:1; u8 num_sacks; /* Number of SACK blocks */ Although it's not evident from the diff -u, I'm *filling* that hole, putting everything on a nice 32-bit boundary, thus taking *NO* space (already mentioned in my patch description above): u16 saw_tstamp : 1, /* Saw TIMESTAMP on last packet */ tstamp_ok : 1, /* TIMESTAMP seen on SYN packet */ dsack : 1, /* D-SACK is scheduled */ wscale_ok : 1, /* Wscale seen on SYN packet */ sack_ok : 4, /* SACK seen on SYN packet */ snd_wscale : 4, /* Window scaling received from sender */ rcv_wscale : 4; /* Window scaling to send to receiver */ u8 cookie_plus:6, /* bytes in authenticator/cookie option */ cookie_out_never:1, cookie_in_always:1; u8 num_sacks; /* Number of SACK blocks */ u16 user_mss; /* mss requested by user in ioctl */ u16 mss_clamp; /* Maximal mss, negotiated at connection setup */ The only reason that it's done this way was Miller's imperative that required cramming this into as small a space as possible. "Store the data either somewhere else or in an extremely compact form." "Make your state take up less space in tcp_sock without making it cost more in some other form." Of course, it costs more, and I have to keep copying it from place to place, adding to the code complexity. But I was feeling rather clever to have found that hole! 2) The second is a spelling error that I noticed in passing. It's within the same diff -u area (close to other insertions both before and after), so fixing it was trivial: - * to increse this, although since: + * to increase this, although since: Are we not supposed to fix spelling errors in comments? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 13:41 ` William Allen Simpson @ 2009-11-10 14:00 ` Ilpo Järvinen 2009-11-10 14:30 ` Ilpo Järvinen 1 sibling, 0 replies; 19+ messages in thread From: Ilpo Järvinen @ 2009-11-10 14:00 UTC (permalink / raw) To: William Allen Simpson Cc: Linux Kernel Network Developers, Eric Dumazet, Joe Perches [-- Attachment #1: Type: TEXT/PLAIN, Size: 966 bytes --] On Tue, 10 Nov 2009, William Allen Simpson wrote: > Ilpo Järvinen wrote: > > On Mon, 9 Nov 2009, William Allen Simpson wrote: > > > ... > > > Data structures are carefully composed to require minimal additions. > > > For example, the struct tcp_options_received cookie_plus variable fits > > > between existing 16-bit and 8-bit variables, requiring no additional > > > space (taking alignment into consideration). There are no additions to > > > tcp_request_sock, and only 1 pointer in tcp_sock. > > > ... > > > > One general comment. ...This particular patch still has lots of noise which > > does not belong to the context of this change. ...Please try to minimize. > > Eg., if you don't like sizeof(struct tcphdr) but prefer sizeof(*th), you > > certainly don't have to do it in this particular patch! > > This *is* actually in CodingStyle (line 679), and I'm trying to conform: I skipped reading after this statement.... Please re-read what I said. -- i. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 13:41 ` William Allen Simpson 2009-11-10 14:00 ` Ilpo Järvinen @ 2009-11-10 14:30 ` Ilpo Järvinen 2009-11-10 16:49 ` William Allen Simpson 1 sibling, 1 reply; 19+ messages in thread From: Ilpo Järvinen @ 2009-11-10 14:30 UTC (permalink / raw) To: William Allen Simpson Cc: Linux Kernel Network Developers, Eric Dumazet, Joe Perches [-- Attachment #1: Type: TEXT/PLAIN, Size: 5192 bytes --] On Tue, 10 Nov 2009, William Allen Simpson wrote: > Ilpo Järvinen wrote: > > On Mon, 9 Nov 2009, William Allen Simpson wrote: > > > ... > > > Data structures are carefully composed to require minimal additions. > > > For example, the struct tcp_options_received cookie_plus variable fits > > > between existing 16-bit and 8-bit variables, requiring no additional > > > space (taking alignment into consideration). There are no additions to > > > tcp_request_sock, and only 1 pointer in tcp_sock. > > > ... > > > > One general comment. ...This particular patch still has lots of noise which > > does not belong to the context of this change. ...Please try to minimize. > > Eg., if you don't like sizeof(struct tcphdr) but prefer sizeof(*th), you > > certainly don't have to do it in this particular patch! After some time I returned to it. My apologies for my previous harsh response. My point was to say that you must separate such changes because they add to noise which makes it rather annoying to review. At least my brains are rather limited still in keeping track of different things. > This *is* actually in CodingStyle (line 679), and I'm trying to conform: Sure, fell free to but not in some random places like you now do in this patch. If you are not touching a line because of a real change and want do a syntax cleanup, please do it in another patch. Especially when your actual change is as complicated as this is. > > ...Also some comment changes which certainly are not mandatory nor even > > related. > > > Hmmm.... > > 1) The first is a hole left by the removal of the data fields some time > ago, but they left the old (now incorrect) comment: > > -/* SACKs data */ > + u8 cookie_plus:6, /* bytes in authenticator/cookie option */ > + cookie_out_never:1, > + cookie_in_always:1; > u8 num_sacks; /* Number of SACK blocks */ > > Although it's not evident from the diff -u, I'm *filling* that hole, > putting everything on a nice 32-bit boundary, thus taking *NO* space > (already mentioned in my patch description above): > > u16 saw_tstamp : 1, /* Saw TIMESTAMP on last packet */ > tstamp_ok : 1, /* TIMESTAMP seen on SYN packet */ > dsack : 1, /* D-SACK is scheduled */ > wscale_ok : 1, /* Wscale seen on SYN packet */ > sack_ok : 4, /* SACK seen on SYN packet */ > snd_wscale : 4, /* Window scaling received from sender */ > rcv_wscale : 4; /* Window scaling to send to receiver */ > u8 cookie_plus:6, /* bytes in authenticator/cookie option */ > cookie_out_never:1, > cookie_in_always:1; > u8 num_sacks; /* Number of SACK blocks */ > u16 user_mss; /* mss requested by user in ioctl */ > u16 mss_clamp; /* Maximal mss, negotiated at connection setup > */ > > The only reason that it's done this way was Miller's imperative that > required cramming this into as small a space as possible. > > "Store the data either somewhere else or in an extremely compact form." > > "Make your state take up less space in tcp_sock without making it cost > more in some other form." > > Of course, it costs more, and I have to keep copying it from place to place, > adding to the code complexity. But I was feeling rather clever to have > found that hole! I'm sorry but this response tells me that you don't seem to know even yourself what you were submitting. Does this ring a bell: - if (th->doff == sizeof(struct tcphdr) >> 2) { + /* In the spirit of fast parsing, compare doff directly to shifted + * constant values. Because equality is used, short doff can be + * ignored here, and checked later. + */ + if (th->doff == (sizeof(*th) >> 2)) { Besides, I don't understand even what you're trying to say. ...I think we already checked the length in tcp_v[46]_rcv against underflow. Why you add such a non-sense comment here (plus the sizeof change I covered elsewhere)? This is just a noise to annoy reviewer, nothing else. > 2) The second is a spelling error that I noticed in passing. It's within > the same diff -u area (close to other insertions both before and after), so > fixing it was trivial: > > - * to increse this, although since: > + * to increase this, although since: > > Are we not supposed to fix spelling errors in comments? How many extra lines a reviewer has to read because of that? Why not do it in a separate patch. git add -i / git stash would help you there. ...To be honest, I'd rather have typos in comments which like this are not trouble for the reader than clutter feature patches with all this random junk. ...I really wish that next time when you submit you get rid of all these showstoppers and annoyances (ie., at least honestly try to do your best) that we could finally concentrate to the actual change and reviewing that :-). My recommendation is that before hitting the send button you review (read through) your own patches, and if you find something to correct instead of submitting, postpone that until the problem is solved. Like DaveM once noted somebody, you'll not be timed :-). ...I usually do that (read my own patches) and end up rather often to not submit (and find even real bugs that way quite often). -- i. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 14:30 ` Ilpo Järvinen @ 2009-11-10 16:49 ` William Allen Simpson 2009-11-11 1:48 ` Ilpo Järvinen 0 siblings, 1 reply; 19+ messages in thread From: William Allen Simpson @ 2009-11-10 16:49 UTC (permalink / raw) To: Ilpo Järvinen Cc: Linux Kernel Network Developers, Eric Dumazet, Joe Perches Ilpo Järvinen wrote: > After some time I returned to it. My apologies for my previous harsh > response. My point was to say that you must separate such changes because > they add to noise which makes it rather annoying to review. At least my > brains are rather limited still in keeping track of different things. > Apology accepted. I'd think we agree, and that's why I wouldn't go around fixing all the misspelled comments -- just those in the code I'm actually revising. >> This *is* actually in CodingStyle (line 679), and I'm trying to conform: > > Sure, fell free to but not in some random places like you now do in this > patch. If you are not touching a line because of a real change and want do > a syntax cleanup, please do it in another patch. Especially when your > actual change is as complicated as this is. > Not a single one is in a random place. >>> ...Also some comment changes which certainly are not mandatory nor even >>> related. >>> >> 1) The first is a hole left by the removal of the data fields some time >> ago, but they left the old (now incorrect) comment: >> >> -/* SACKs data */ >> + u8 cookie_plus:6, /* bytes in authenticator/cookie option */ >> + cookie_out_never:1, >> + cookie_in_always:1; >> u8 num_sacks; /* Number of SACK blocks */ >>... > I'm sorry but this response tells me that you don't seem to know even > yourself what you were submitting. Does this ring a bell: > > - if (th->doff == sizeof(struct tcphdr) >> 2) { > + /* In the spirit of fast parsing, compare doff directly to shifted > + * constant values. Because equality is used, short doff can be > + * ignored here, and checked later. > + */ > + if (th->doff == (sizeof(*th) >> 2)) { > Ah, it's become apparent that you didn't click through to any of the references, internet-drafts, or mailing list discussion, so you don't know what this patch series is implementing. One of the great difficulties in tracking down all several hundred uses of doff -- and functions that use doff -- is the serious deficiency of comments (compared to other code bases). Oh yeah -- Miller says I'm anal. This is TCP. Strict scrutiny is much better than the alternative. > Besides, I don't understand even what you're trying to say. ...I think we > already checked the length in tcp_v[46]_rcv against underflow. Why you add > such a non-sense comment here (plus the sizeof change I covered > elsewhere)? This is just a noise to annoy reviewer, nothing else. > You merely think? You don't know? (I didn't find one on the code walk through that got me to this point in the code, but perhaps I missed something.) If that's the case, there should at least be a comment that underflow was already checked, and *where* it was checked! The lack of pertinent comments and error checking annoys the next coder that comes down the pike. >> 2) The second is a spelling error that I noticed in passing. It's within >> the same diff -u area (close to other insertions both before and after), so >> fixing it was trivial: >> >> - * to increse this, although since: >> + * to increase this, although since: >> >> Are we not supposed to fix spelling errors in comments? > > How many extra lines a reviewer has to read because of that? Why not do it > in a separate patch. git add -i / git stash would help you there. ...To be > honest, I'd rather have typos in comments which like this are not > trouble for the reader than clutter feature patches with all this random > junk. > I'll have to look up the usage of git add -i / git stash, but in any case, I'm not likely to go around fixing comments later. Each patch requires a great deal of time. I'd rather combine such things, or not do them at all. > ...I really wish that next time when you submit you get rid of all these > showstoppers and annoyances (ie., at least honestly try to do your best) > that we could finally concentrate to the actual change and reviewing > that :-). My recommendation is that before hitting the send button you > review (read through) your own patches, and if you find something to > correct instead of submitting, postpone that until the problem is solved. > Like DaveM once noted somebody, you'll not be timed :-). ...I usually do > that (read my own patches) and end up rather often to not submit (and find > even real bugs that way quite often). > Maybe you've been dealing too much with some folks that submitted a patch that was accepted (over my expressed reservations), and then had to send out 2 emergency patches to handle crashing bugs last week.... Before I send any patch, it's been compared against my last patch sent, *and* tested for compilation individually, *and* _usually_ tested pretty thoroughly -- although not this time, because the net-next-2.6 modules don't compile, as I warned at the head of this patch series. Because the compile blew up, I actually spent my Sunday evening re-reviewing the patches again! I wanted to ensure it wasn't something that I'd done, so I recompiled everything again incrementally. It took me more than a day to make the syntax changes requested, and another 6 hours to prepare and send these 5 patches. I cannot promise there will never be any bugs, this patch series will be much too complicated for that, but the very idea that I'd send without reviewing.... At this point, I'm rather personally insulted. And now I'm done with email for the day. I'll do the split that Eric suggested, and send it out whenever the damn modules compile again. In the meantime, I'd appreciate that you "finally concentrate".... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 16:49 ` William Allen Simpson @ 2009-11-11 1:48 ` Ilpo Järvinen 0 siblings, 0 replies; 19+ messages in thread From: Ilpo Järvinen @ 2009-11-11 1:48 UTC (permalink / raw) To: William Allen Simpson Cc: Ilpo Järvinen, Linux Kernel Network Developers, Eric Dumazet, Joe Perches [-- Attachment #1: Type: TEXT/PLAIN, Size: 4477 bytes --] On Tue, 10 Nov 2009, William Allen Simpson wrote: > Ilpo Järvinen wrote: > > > > This *is* actually in CodingStyle (line 679), and I'm trying to conform: > > > > Sure, fell free to but not in some random places like you now do in this > > patch. If you are not touching a line because of a real change and want do a > > syntax cleanup, please do it in another patch. Especially when your actual > > change is as complicated as this is. > > > Not a single one is in a random place. I disagree. > > > > ...Also some comment changes which certainly are not mandatory nor even > > > > related. > > > > > > > 1) The first is a hole left by the removal of the data fields some time > > > ago, but they left the old (now incorrect) comment: > > > > > > -/* SACKs data */ > > > + u8 cookie_plus:6, /* bytes in authenticator/cookie option */ > > > + cookie_out_never:1, > > > + cookie_in_always:1; > > > u8 num_sacks; /* Number of SACK blocks */ > > > ... > > I'm sorry but this response tells me that you don't seem to know even > > yourself what you were submitting. Does this ring a bell: > > > > - if (th->doff == sizeof(struct tcphdr) >> 2) { > > + /* In the spirit of fast parsing, compare doff directly to shifted > > + * constant values. Because equality is used, short doff can be > > + * ignored here, and checked later. > > + */ > > + if (th->doff == (sizeof(*th) >> 2)) { > > > Ah, it's become apparent that you didn't click through to any of the > references, internet-drafts, or mailing list discussion, so you don't > know what this patch series is implementing. Sure, I'll find that out from your patches instead, and it will be more definitive than what the specs say ;-). > One of the great difficulties in tracking down all several hundred uses > of doff -- and functions that use doff -- is the serious deficiency of > comments (compared to other code bases). > > Oh yeah -- Miller says I'm anal. This is TCP. Strict scrutiny is much > better than the alternative. > > > > Besides, I don't understand even what you're trying to say. ...I think we > > already checked the length in tcp_v[46]_rcv against underflow. Why you add > > such a non-sense comment here (plus the sizeof change I covered elsewhere)? > > This is just a noise to annoy reviewer, nothing else. > > > You merely think? You don't know? Let me say it out loud then. I know that among the very first things we check is this underflow check. I quoted is below. > (I didn't find one on the code walk > through that got me to this point in the code, but perhaps I missed > something.) If that's the case, there should at least be a comment that > underflow was already checked, and *where* it was checked! You added a comment which says neither but something else? :-D I wonder how trustworthy such comment then would be to that poor "next coder" that comes along ;-). > The lack of pertinent comments and error checking annoys the next coder > that comes down the pike. This is something I can hardly understand... these are among the first things we check before we even begin processing a TCP segment: if (!pskb_may_pull(skb, sizeof(struct tcphdr))) goto discard_it; th = tcp_hdr(skb); if (th->doff < sizeof(struct tcphdr) / 4) goto bad_packet; if (!pskb_may_pull(skb, th->doff * 4)) goto discard_it; > > > 2) The second is a spelling error that I noticed in passing. It's within > > > the same diff -u area (close to other insertions both before and after), > > > so > > > fixing it was trivial: > > > > > > - * to increse this, although since: > > > + * to increase this, although since: > > > > > > Are we not supposed to fix spelling errors in comments? > > > > How many extra lines a reviewer has to read because of that? Why not do it > > in a separate patch. git add -i / git stash would help you there. ...To be > > honest, I'd rather have typos in comments which like this are not trouble > > for the reader than clutter feature patches with all this random junk. > > > I'll have to look up the usage of git add -i / git stash, but in any case, > I'm not likely to go around fixing comments later. With git add -i you can change them while you go but easily split them on before commit time to multiple patches. Rather easy unless except if you you're also modifying the line right next or before to the comment it. -- i. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-09 17:05 ` [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data William Allen Simpson 2009-11-10 5:05 ` Ilpo Järvinen @ 2009-11-10 14:29 ` Eric Dumazet 2009-11-10 15:45 ` William Allen Simpson 1 sibling, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2009-11-10 14:29 UTC (permalink / raw) To: William Allen Simpson Cc: Linux Kernel Network Developers, Ilpo Järvinen, Joe Perches William Allen Simpson a écrit : > This is a significantly revised implementation of an earlier (year-old) > patch that no longer applies cleanly, with permission of the original > author (Adam Langley). That patch was previously reviewed: > > http://thread.gmane.org/gmane.linux.network/102586 > > The principle difference is using a TCP option to carry the cookie nonce, > instead of a user configured offset in the data. This is more flexible and > less subject to user configuration error. Such a cookie option has been > suggested for many years, and is also useful without SYN data, allowing > several related concepts to use the same extension option. > > "Re: SYN floods (was: does history repeat itself?)", September 9, 1996. > http://www.merit.net/mail.archives/nanog/1996-09/msg00235.html > > "Re: what a new TCP header might look like", May 12, 1998. > ftp://ftp.isi.edu/end2end/end2end-interest-1998.mail > > Data structures are carefully composed to require minimal additions. > For example, the struct tcp_options_received cookie_plus variable fits > between existing 16-bit and 8-bit variables, requiring no additional > space (taking alignment into consideration). There are no additions to > tcp_request_sock, and only 1 pointer in tcp_sock. > > Allocations have been rearranged to avoid requiring GFP_ATOMIC, with > only one unavoidable exception in tcp_create_openreq_child(), where the > tcp_sock itself is created GFP_ATOMIC. > > These functions will also be used in subsequent patches that implement > additional features. > > Requires: > TCPCT part 1a: add request_values parameter for sending SYNACK > TCPCT part 1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED > TCPCT part 1c: sysctl_tcp_cookie_size, socket option > TCP_COOKIE_TRANSACTIONS, functions > TCPCT part 1d: generate Responder Cookie > > Signed-off-by: William.Allen.Simpson@gmail.com > --- > include/linux/tcp.h | 29 ++++- > include/net/tcp.h | 72 +++++++++++++ > net/ipv4/syncookies.c | 5 +- > net/ipv4/tcp.c | 127 ++++++++++++++++++++++- > net/ipv4/tcp_input.c | 86 +++++++++++++-- > net/ipv4/tcp_ipv4.c | 69 ++++++++++++- > net/ipv4/tcp_minisocks.c | 59 ++++++++--- > net/ipv4/tcp_output.c | 255 > +++++++++++++++++++++++++++++++++++++++++----- > net/ipv6/syncookies.c | 5 +- > net/ipv6/tcp_ipv6.c | 65 +++++++++++- > 10 files changed, 701 insertions(+), 71 deletions(-) > I really tried hard to understand what was going on, and failed, because I dont have much time these days... Lack of documentation maybe ? Some DATA flow could help... Please please, cook up elementatry patches to perform one action at a time, even if they are not fully functionnal ? One patch to be able to send SYN + COOKIE (if we are the client) One patch to be able to receive this SYN + COOKIE and answer a SYNACK + cookies (we are the server) One patch to ... Receive the ACK from client (if we are the server) and check cookies One patch to ... Send the ACK (if we are the client) Patches to receive FIN + cookies One patch to ... enable the whole thing and setsockopt() bits That way we could review your patches step by step, and not 770 lines in one block. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 14:29 ` Eric Dumazet @ 2009-11-10 15:45 ` William Allen Simpson 2009-11-11 1:32 ` Ilpo Järvinen 0 siblings, 1 reply; 19+ messages in thread From: William Allen Simpson @ 2009-11-10 15:45 UTC (permalink / raw) To: Eric Dumazet Cc: Linux Kernel Network Developers, Ilpo Järvinen, Joe Perches Eric Dumazet wrote: > William Allen Simpson a écrit : >> This is a significantly revised implementation of an earlier (year-old) >> patch that no longer applies cleanly, with permission of the original >> author (Adam Langley). That patch was previously reviewed: >> >> http://thread.gmane.org/gmane.linux.network/102586 > I really tried hard to understand what was going on, and failed, because I dont > have much time these days... > Thanks very much for your time. As I just wrote in a previous message, this is the easy part. It's going to get much more complicated -- as you know, since I sent you the papers with 30+ references.... > Lack of documentation maybe ? Some DATA flow could help... > Where should that be? In the commit messages? > Please please, cook up elementatry patches to perform one action at a time, > even if they are not fully functionnal ? > Well, that's what I did with the (now) parts 1b, 1c, and 1d, but somebody complained that the callers didn't exist yet. > One patch to be able to send SYN + COOKIE (if we are the client) > > One patch to be able to receive this SYN + COOKIE and answer a SYNACK + cookies (we are the server) > I'll split them. Remember, I started with a code base previously submitted, and everything was in one *single* large patch. I thought that's what you (Linux folks in general) wanted. > One patch to ... Receive the ACK from client (if we are the server) and check cookies > > One patch to ... Send the ACK (if we are the client) > > Patches to receive FIN + cookies > Maybe that's what you are missing, and why you're have difficulty wrapping your mind around it. None of that is there yet! Receive the Ack options from the client is currently part 2. I've also lately divided part 2 into 4 sub-patches, although it all has to be committed at one time: 2a) extending the TCP option space, 2b) receiving 64-bit timestamps, 2c) receiving extended SACKs, 2d) receiving the cookie pair. Perhaps that could be split even further, although the order has to stay the same. A lot is written, although I gave up updating regularly as the part 1 process has taken so long.... Send the Ack is currently part 3. Handling FIN is currently part 4, although it might be split, too. Handling RST is probably part 5. Handling transactions was part 4 or 5, but could be a future part 6 instead (although it's rather tied up with both 4 and 5). > One patch to ... enable the whole thing and setsockopt() bits > Ummm, then there'd be no way to test. This is TCP we're talking about. It has to be tested extensively. This stuff needs to work. I need the bits to initiate all the tests.... That's why I like to get all my data structures nailed down first, and then incrementally add code and test. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-10 15:45 ` William Allen Simpson @ 2009-11-11 1:32 ` Ilpo Järvinen 2009-11-11 17:35 ` William Allen Simpson 0 siblings, 1 reply; 19+ messages in thread From: Ilpo Järvinen @ 2009-11-11 1:32 UTC (permalink / raw) To: William Allen Simpson Cc: Eric Dumazet, Linux Kernel Network Developers, Joe Perches [-- Attachment #1: Type: TEXT/PLAIN, Size: 3927 bytes --] On Tue, 10 Nov 2009, William Allen Simpson wrote: > Eric Dumazet wrote: > > William Allen Simpson a écrit : > > > This is a significantly revised implementation of an earlier (year-old) > > > patch that no longer applies cleanly, with permission of the original > > > author (Adam Langley). That patch was previously reviewed: > > > > > > http://thread.gmane.org/gmane.linux.network/102586 > > > I really tried hard to understand what was going on, and failed, because I > > dont > > have much time these days... > > > Thanks very much for your time. > > As I just wrote in a previous message, this is the easy part. It's going > to get much more complicated -- as you know, since I sent you the papers > with 30+ references.... > > > > Lack of documentation maybe ? Some DATA flow could help... > > > Where should that be? In the commit messages? > > > > Please please, cook up elementatry patches to perform one action at a time, > > even if they are not fully functionnal ? > > > Well, that's what I did with the (now) parts 1b, 1c, and 1d, but somebody > complained that the callers didn't exist yet. I guess there's misunderstanding here. Yes, if you bring in new helpers that solely are used with your feature, then it that feature patch (and I found DaveM commenting specifically this point I think), or right beforehand with a clear note that it will be used in the subsequent patch. But if there are uses in the existing code without any of your changes, add the helpers and convert existing callers in a single go without doing some other things (preferrably convert all callers, not just those you intend to touch -- there is nothing extraordinary in this, other people do the same kind of cleaning up work while they go, so please don't find that this is something we try to force specifically for you :-)). ...Therefore each change becomes rather self-sufficient to understand and review. There was serious problems in your series in this as I constantly had to jump between 3 patches to see what you said (changed) in the other. But also, please don't make this an over-statement, I'd still occassionally have to do that "patch hopping" even if nicely split (so this is not something to be turned into all in a single patch statement :-)). On positive side, there certainly has been clear progress already, as patches 1 and 2 were quite nice already (except for some weird local initializer order change which I could sort of "tolerate" but I didn't find any particular reason why those had to be switched around. And, one thing to not be afraid is making many short patches (nor long one either). So if a self-contained change is a small one do it such and no more. ...It is often so that you will be doing many relatively small changes first to support the actual large one, and those small changes might actually not end up being that small in lines if there are e.g. multiple callsites converted but nevertheless they often are rather simple, so that a quick browsing will already be enough to reveal that it is ok (like in the MSS define change you made). > > One patch to be able to send SYN + COOKIE (if we are the client) > > > > One patch to be able to receive this SYN + COOKIE and answer a SYNACK + > > cookies (we are the server) > > > I'll split them. Remember, I started with a code base previously > submitted, and everything was in one *single* large patch. I thought > that's what you (Linux folks in general) wanted. Only if the large patch is only for a single logical change. A syntax cleanup, literal cleanup, cleanup to use helpers and a feature patch are not logically same changes, and thus should be separated. It's like you have already being doing for a part of the changes. Many times the cleanup things are even such that they would be beneficial even without any feature addition (but nobody has just done them so far). -- i. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data 2009-11-11 1:32 ` Ilpo Järvinen @ 2009-11-11 17:35 ` William Allen Simpson 0 siblings, 0 replies; 19+ messages in thread From: William Allen Simpson @ 2009-11-11 17:35 UTC (permalink / raw) To: Ilpo Järvinen Cc: Eric Dumazet, Linux Kernel Network Developers, Joe Perches Ilpo Järvinen wrote: > On Tue, 10 Nov 2009, William Allen Simpson wrote: >> Well, that's what I did with the (now) parts 1b, 1c, and 1d, but somebody >> complained that the callers didn't exist yet. > > I guess there's misunderstanding here. Yes, if you bring in new helpers > that solely are used with your feature, then it that feature patch (and > I found DaveM commenting specifically this point I think), or right > beforehand with a clear note that it will be used in the subsequent patch. If I understand correctly, in that case I look forward to your Ack for part 1d, a self-contained "helper" feature patch "with a clear note". ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2009-11-11 17:35 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-09 16:07 [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange William Allen Simpson 2009-11-09 16:12 ` [net-next-2.6 PATCH v5 1/5 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK William Allen Simpson 2009-11-09 16:24 ` [net-next-2.6 PATCH v5 2/5 RFC] TCPCT part1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED William Allen Simpson 2009-11-09 16:39 ` Eric Dumazet 2009-11-09 16:33 ` [net-next-2.6 PATCH v5 3/5 RFC] TCPCT part1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions William Allen Simpson 2009-11-10 5:31 ` Ilpo Järvinen 2009-11-10 14:43 ` William Allen Simpson 2009-11-09 16:50 ` [net-next-2.6 PATCH v5 4/5 RFC] TCPCT part1d: generate Responder Cookie William Allen Simpson 2009-11-09 17:05 ` [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange with SYNACK data William Allen Simpson 2009-11-10 5:05 ` Ilpo Järvinen 2009-11-10 13:41 ` William Allen Simpson 2009-11-10 14:00 ` Ilpo Järvinen 2009-11-10 14:30 ` Ilpo Järvinen 2009-11-10 16:49 ` William Allen Simpson 2009-11-11 1:48 ` Ilpo Järvinen 2009-11-10 14:29 ` Eric Dumazet 2009-11-10 15:45 ` William Allen Simpson 2009-11-11 1:32 ` Ilpo Järvinen 2009-11-11 17:35 ` William Allen Simpson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).