From 1ccd5f670a432f98e94b384dd169a1a760dced9a Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 1 Oct 2017 18:11:36 +0100 Subject: [PATCH] TFO: better detection of client fast-open connections (again) --- src/OS/os.h-Linux | 1 + src/src/deliver.c | 8 ++-- src/src/globals.c | 2 +- src/src/globals.h | 4 +- src/src/ip.c | 20 +++++---- src/src/smtp_in.c | 2 +- src/src/smtp_out.c | 61 +++++++++++++++++++++++++++- src/src/structs.h | 3 +- src/src/transports/smtp.c | 5 ++- src/src/transports/smtp_socks.c | 14 +++++-- test/scripts/1990-TCP-Fast-Open/1990 | 9 ++++ 11 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/OS/os.h-Linux b/src/OS/os.h-Linux index cc1f3cab2..f6d35772b 100644 --- a/src/OS/os.h-Linux +++ b/src/OS/os.h-Linux @@ -79,6 +79,7 @@ then change the 0 to 1 in the next block. */ #if defined(TCP_FASTOPEN) && !defined(MSG_FASTOPEN) # define MSG_FASTOPEN 0x20000000 #endif +#define EXIM_HAVE_TCPI_UNACKED /* End */ diff --git a/src/src/deliver.c b/src/src/deliver.c index 73921980e..648c63d69 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -3535,7 +3535,8 @@ while (!done) break; case 'T': - setflag(addr, af_tcp_fastopen); + setflag(addr, af_tcp_fastopen_conn); + if (*subid > '0') setflag(addr, af_tcp_fastopen); break; case 'D': @@ -4837,8 +4838,9 @@ all pipes, so I do not see a reason to use non-blocking IO here if (testflag(addr, af_chunking_used)) rmt_dlv_checked_write(fd, 'K', '0', NULL, 0); - if (testflag(addr, af_tcp_fastopen)) - rmt_dlv_checked_write(fd, 'T', '0', NULL, 0); + if (testflag(addr, af_tcp_fastopen_conn)) + rmt_dlv_checked_write(fd, 'T', + testflag(addr, af_tcp_fastopen) ? '1' : '0', NULL, 0); memcpy(big_buffer, &addr->dsn_aware, sizeof(addr->dsn_aware)); rmt_dlv_checked_write(fd, 'D', '0', big_buffer, sizeof(addr->dsn_aware)); diff --git a/src/src/globals.c b/src/src/globals.c index 57041fc4e..2a53835e9 100644 --- a/src/src/globals.c +++ b/src/src/globals.c @@ -1421,7 +1421,7 @@ blob tcp_fastopen_nodata = { .data = NULL, .len = 0 }; BOOL tcp_in_fastopen = FALSE; BOOL tcp_in_fastopen_logged = FALSE; BOOL tcp_nodelay = TRUE; -BOOL tcp_out_fastopen = FALSE; +int tcp_out_fastopen = 0; BOOL tcp_out_fastopen_logged= FALSE; #ifdef USE_TCP_WRAPPERS uschar *tcp_wrappers_daemon_name = US TCP_WRAPPERS_DAEMON_NAME; diff --git a/src/src/globals.h b/src/src/globals.h index 2957587b0..62336c275 100644 --- a/src/src/globals.h +++ b/src/src/globals.h @@ -923,10 +923,10 @@ extern BOOL system_filtering; /* TRUE when running system filter */ extern BOOL tcp_fastopen_ok; /* appears to be supported by kernel */ extern blob tcp_fastopen_nodata; /* for zero-data TFO connect requests */ -extern BOOL tcp_in_fastopen; /* conn used fastopen */ +extern BOOL tcp_in_fastopen; /* conn usefully used fastopen */ extern BOOL tcp_in_fastopen_logged; /* one-time logging */ extern BOOL tcp_nodelay; /* Controls TCP_NODELAY on daemon */ -extern BOOL tcp_out_fastopen; /* conn used fastopen */ +extern int tcp_out_fastopen; /* 0: no 1: conn used 2: useful */ extern BOOL tcp_out_fastopen_logged; /* one-time logging */ #ifdef USE_TCP_WRAPPERS extern uschar *tcp_wrappers_daemon_name; /* tcpwrappers daemon lookup name */ diff --git a/src/src/ip.c b/src/src/ip.c index 266eaf414..eb01229e6 100644 --- a/src/src/ip.c +++ b/src/src/ip.c @@ -236,22 +236,26 @@ if (fastopen) { if ((rc = sendto(sock, fastopen->data, fastopen->len, MSG_FASTOPEN | MSG_DONTWAIT, s_ptr, s_len)) >= 0) + /* seen for with-data, experimental TFO option, with-cookie case */ { - DEBUG(D_transport|D_v) - debug_printf("TCP_FASTOPEN mode connection, with data\n"); - tcp_out_fastopen = TRUE; + DEBUG(D_transport|D_v) debug_printf("TFO mode connection attempt, %s data\n", + fastopen->len > 0 ? "with" : "no"); + tcp_out_fastopen = fastopen->len > 0 ? 2 : 1; } else if (errno == EINPROGRESS) /* expected for nonready peer */ - { + /* seen for no-data, proper TFO option, both cookie-request and with-cookie cases */ + /* apparently no visibility of the diffference at this point */ + /* with netwk delay, post-conn tcp_info sees unacked 1 for R, 2 for C; code in smtp_out.c */ + /* ? older Experimental TFO option behaviour ? */ + { /* queue unsent data */ if (!fastopen->data) { - DEBUG(D_transport|D_v) - debug_printf("TCP_FASTOPEN mode connection, no data\n"); - tcp_out_fastopen = TRUE; + tcp_out_fastopen = 1; /* we tried; unknown if useful yet */ rc = 0; } else if ( (rc = send(sock, fastopen->data, fastopen->len, 0)) < 0 - && errno == EINPROGRESS) /* expected for nonready peer */ + && errno == EINPROGRESS /* expected for nonready peer */ + ) rc = 0; } else if(errno == EOPNOTSUPP) diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 36f685677..0f8d5599b 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -2346,7 +2346,7 @@ if ( getsockopt(fileno(smtp_out), IPPROTO_TCP, TCP_INFO, &tinfo, &len) == 0 && tinfo.tcpi_state == TCP_SYN_RECV ) { - DEBUG(D_receive) debug_printf("TCP_FASTOPEN mode connection\n"); + DEBUG(D_receive) debug_printf("TCP_FASTOPEN mode connection (state TCP_SYN_RECV)\n"); tcp_in_fastopen = TRUE; } # endif diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c index db33ac66e..786f8b592 100644 --- a/src/src/smtp_out.c +++ b/src/src/smtp_out.c @@ -140,6 +140,60 @@ return TRUE; +#ifdef TCP_FASTOPEN +static void +tfo_out_check(int sock) +{ +# if defined(TCP_INFO) && defined(EXIM_HAVE_TCPI_UNACKED) +struct tcp_info tinfo; +socklen_t len = sizeof(tinfo); + +if (getsockopt(sock, IPPROTO_TCP, TCP_INFO, &tinfo, &len) == 0) + { + switch (tcp_out_fastopen) + { + /* This is a somewhat dubious detection method; totally undocumented so likely + to fail in future kernels. There seems to be no documented way. What we really + want to know is if the server sent smtp-banner data before our ACK of his SYN,ACK + hit him. What this (possibly?) detects is whether we sent a TFO cookie with our + SYN, as distinct from a TFO request. This gets a false-positive when the server + key is rotated; we send the old one (which this test sees) but the server returns + the new one and does not send its SMTP banner before we ACK his SYN,ACK. + To force that rotation case: + '# echo -n "00000000-00000000-00000000-0000000" >/proc/sys/net/ipv4/tcp_fastopen_key' + The kernel seems to be counting unack'd packets. */ + + case 1: + if (tinfo.tcpi_unacked > 1) + { + DEBUG(D_transport|D_v) + debug_printf("TCP_FASTOPEN tcpi_unacked %d\n", tinfo.tcpi_unacked); + tcp_out_fastopen = 2; + } + break; + +#ifdef notdef /* This seems to always fire, meaning that we cannot tell + whether the server accepted data we sent. For now assume + that it did. */ + + /* If there was data-on-SYN but we had to retrasnmit it, declare no TFO */ + + case 2: + if (!(tinfo.tcpi_options & TCPI_OPT_SYN_DATA)) + { + DEBUG(D_transport|D_v) debug_printf("TFO: had to retransmit\n"); + tcp_out_fastopen = 0; + } + break; +#endif + } + + } +# endif +} +#endif + + /* Arguments as for smtp_connect(), plus early_data if non-NULL, data to be sent - preferably in the TCP SYN segment @@ -158,6 +212,8 @@ int dscp_level; int dscp_option; int sock; int save_errno = 0; +const blob * fastopen = NULL; + #ifndef DISABLE_EVENT deliver_host_address = host->address; @@ -207,8 +263,6 @@ requested some early-data then include that in the TFO request. */ else { - const blob * fastopen = NULL; - #ifdef TCP_FASTOPEN if (verify_check_given_host(&ob->hosts_try_fastopen, host) == OK) fastopen = early_data ? early_data : &tcp_fastopen_nodata; @@ -254,6 +308,9 @@ else return -1; } if (ob->keepalive) ip_keepalive(sock, host->address, TRUE); +#ifdef TCP_FASTOPEN + if (fastopen) tfo_out_check(sock); +#endif return sock; } } diff --git a/src/src/structs.h b/src/src/structs.h index d8ac19ab8..c16899a0c 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -610,7 +610,8 @@ typedef struct address_item { BOOL af_cert_verified:1; /* delivered with verified TLS cert */ BOOL af_pass_message:1; /* pass message in bounces */ BOOL af_bad_reply:1; /* filter could not generate autoreply */ - BOOL af_tcp_fastopen:1; /* delivery used TCP Fast Open */ + BOOL af_tcp_fastopen_conn:1; /* delivery connection used TCP Fast Open */ + BOOL af_tcp_fastopen:1; /* delivery usefuly used TCP Fast Open */ #ifndef DISABLE_PRDR BOOL af_prdr_used:1; /* delivery used SMTP PRDR */ #endif diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 461b26c4a..14cfde72a 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -2511,7 +2511,10 @@ for (addr = sx->first_addr, address_count = 0; uschar * rcpt_addr; if (tcp_out_fastopen && !tcp_out_fastopen_logged) - setflag(addr, af_tcp_fastopen); + { + setflag(addr, af_tcp_fastopen_conn); + if (tcp_out_fastopen > 1) setflag(addr, af_tcp_fastopen); + } addr->dsn_aware = sx->peer_offered & OPTION_DSN ? dsn_support_yes : dsn_support_no; diff --git a/src/src/transports/smtp_socks.c b/src/src/transports/smtp_socks.c index 1368849d6..c9907dd54 100644 --- a/src/src/transports/smtp_socks.c +++ b/src/src/transports/smtp_socks.c @@ -126,10 +126,12 @@ switch(method) for (i = 0; i> 05 01 %02x\n", sob- /* expect method response */ +#ifdef TCP_QUICKACK +(void) setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, US &off, sizeof(off)); +#endif + if ( !fd_ready(fd, tmo-time(NULL)) || read(fd, buf, 2) != 2 ) diff --git a/test/scripts/1990-TCP-Fast-Open/1990 b/test/scripts/1990-TCP-Fast-Open/1990 index cbedd3622..4f5758f5a 100644 --- a/test/scripts/1990-TCP-Fast-Open/1990 +++ b/test/scripts/1990-TCP-Fast-Open/1990 @@ -8,6 +8,9 @@ # option on the SYN, but the fast-output SMTP banner will not # be seen unless you also deliberately emulate a long path: # 'sudo tc qdisc add dev lo root netem delay 100ms' +# You'll need kernel-modules-extra installed, or you get +# an unhelpful error from RTNETLINK. +# To tidy up: 'sudo tc qdisc delete dev lo root' # # First time runs will see a TFO request option only; subsequent # ones should see the TFO cookie and fast-output SMTP banner @@ -21,6 +24,12 @@ # this will only be obtained when the above delay is inserted into the # loopback net path. # +# this attempt to tidy up does not work +#sudo perl +#open (my $fh, "/proc/sys/net/ipv4/tcp_fastopen_key"); +#print $fh "00000000-00000000-00000000-00000000"; +#close $fh; +#**** # # # FreeBSD: it looks like you have to compile a custom kernel, with -- 2.30.2