From cc9397414a655cab4c47de6559e69ecda06530cc Mon Sep 17 00:00:00 2001 From: Matthias Andree Date: Thu, 26 May 2011 01:29:34 +0200 Subject: Revert SO_???TIMEO-based STARTTLS timeout handling. This reverts commits 47c05b10018f5ec7493e4bd9f521aaa18d96f1e2 and 72ce8bce8dd655b6aefa33d0a74e883dad5202b5, the code isn't portable, for instance, Solaris does not support SO_RCVTIMEO/SO_SNDTIMEO. These socket-level options are known, but Solaris returns EAFNOSUPPORT. Reported by Jonathan Buschmann. --- NEWS | 8 -------- TODO-6.3.20 | 11 +++++++++++ smtp.c | 4 +--- socket.c | 29 +---------------------------- socket.h | 3 --- transact.c | 6 ------ 6 files changed, 13 insertions(+), 48 deletions(-) create mode 100644 TODO-6.3.20 diff --git a/NEWS b/NEWS index 4a4e0c39..bcf2537b 100644 --- a/NEWS +++ b/NEWS @@ -58,14 +58,6 @@ removed from a 6.4.0 or newer release.) fetchmail-6.3.20 (not yet released): -# SECURITY FIXES -* Fetchmail's socket timeout handling was incomplete. Network outages in the - wrong phase of a communication, combined with unlucky operating systems and - their defaults, could cause fetchmail to hang for extended amounts of time. - Freezes for beyond a week were reported by Thomas Jarosch. Fetchmail sets - UNIX- and Internet-domain socket send and receive timeouts now. - This fixes a hang during STARTTLS negotiation reported by Thomas Jarosch. - # CHANGES * fetchmail now always uses its own MD5 implementation. The library and header variants are too diverse, and we've been bitten before -- and configure diff --git a/TODO-6.3.20 b/TODO-6.3.20 new file mode 100644 index 00000000..d9d79977 --- /dev/null +++ b/TODO-6.3.20 @@ -0,0 +1,11 @@ +- fix STARTTLS timeouts by setting socket timings + possibly using a different structure than an int to save the fd + and SSL context -- and then also timeout? + Or just make set_timeout take an optional fd, which, when != -1, + also sets the socket timeouts? + +- make SSLv2 removal dependent on openssl configuration + (see Debian FTBFS bug for how to detect that in configure) + +- make --with-ssl default? + diff --git a/smtp.c b/smtp.c index 90c35173..1c99c696 100644 --- a/smtp.c +++ b/smtp.c @@ -313,12 +313,10 @@ int SMTP_ok(int sock, char smtp_mode, int mintimeout) { SIGHANDLERTYPE alrmsave; char reply[MSGBUFSIZE], *i; - int tmo = (mytimeout >= mintimeout) ? mytimeout : mintimeout; /* set an alarm for smtp ok */ alrmsave = set_signal_handler(SIGALRM, null_signal_handler); - set_timeout(tmo); - SockTimeout(sock, tmo); + set_timeout(mytimeout >= mintimeout ? mytimeout : mintimeout); smtp_response[0] = '\0'; diff --git a/socket.c b/socket.c index e0f0f857..e338207a 100644 --- a/socket.c +++ b/socket.c @@ -200,31 +200,6 @@ static int handle_plugin(const char *host, } #endif /* HAVE_SOCKETPAIR */ -static int setsocktimeout(int sock, int which, int timeout) { - struct timeval tv; - int rc; - - tv.tv_sec = timeout; - tv.tv_usec = 0; - rc = setsockopt(sock, SOL_SOCKET, which, &tv, sizeof(tv)); - if (rc) { - report(stderr, GT_("setsockopt(%d, SOL_SOCKET) failed: %s\n"), sock, strerror(errno)); - } - return rc; -} - -/** Configure socket options such as send/receive timeout at the socket - * level, to avoid network-induced stalls. - */ -int SockTimeout(int sock, int timeout) -{ - int err = 0; - - if (setsocktimeout(sock, SO_RCVTIMEO, timeout)) err = 1; - if (setsocktimeout(sock, SO_SNDTIMEO, timeout)) err = 1; - return err; -} - /** Set socket to SO_KEEPALIVE. \return 0 for success. */ int SockKeepalive(int sock) { int keepalive = 1; @@ -251,7 +226,6 @@ int UnixOpen(const char *path) */ mailserver_socket_temp = sock; - SockTimeout(sock, mytimeout); if (connect(sock, (struct sockaddr *) &ad, sizeof(ad)) < 0) { int olderr = errno; @@ -326,7 +300,6 @@ int SockOpen(const char *host, const char *service, continue; } - SockTimeout(i, mytimeout); SockKeepalive(i); /* Save socket descriptor. @@ -391,8 +364,8 @@ va_dcl { #endif vsnprintf(buf, sizeof(buf), format, ap); va_end(ap); - SockTimeout(sock, mytimeout); return SockWrite(sock, buf, strlen(buf)); + } #ifdef SSL_ENABLE diff --git a/socket.h b/socket.h index cbdeec06..0c4ac001 100644 --- a/socket.h +++ b/socket.h @@ -20,9 +20,6 @@ struct addrinfo; /** Create a new client socket; returns -1 on error */ int SockOpen(const char *host, const char *service, const char *plugin, struct addrinfo **); -/** Sets the send/receive timeouts for socket \a sock to \a timeout - * seconds. \return zero on success. */ -int SockTimeout(int sock, int timeout); /** Get a string terminated by an '\n' (matches interface of fgets). diff --git a/transact.c b/transact.c index 9667bd25..d1e4f6a9 100644 --- a/transact.c +++ b/transact.c @@ -487,7 +487,6 @@ int readheaders(int sock, char *sp, *tp; set_timeout(mytimeout); - SockTimeout(sock, mytimeout); if ((n = SockRead(sock, buf, sizeof(buf)-1)) == -1) { set_timeout(0); free(line); @@ -1409,7 +1408,6 @@ int readbody(int sock, struct query *ctl, flag forward, int len) while (protocol->delimited || len > 0) { set_timeout(mytimeout); - SockTimeout(sock, mytimeout); /* XXX FIXME: for undelimited protocols that ship the size, such * as IMAP, we might want to use the count of remaining characters * instead of the buffer size -- not for fetchmail 6.3.X though */ @@ -1553,7 +1551,6 @@ va_dcl va_end(ap); snprintf(buf+strlen(buf), sizeof(buf)-strlen(buf), "\r\n"); - SockTimeout(sock, mytimeout); SockWrite(sock, buf, strlen(buf)); if (outlevel >= O_MONITOR) @@ -1574,7 +1571,6 @@ int gen_recv(int sock /** socket to which server is connected */, phase = SERVER_WAIT; set_timeout(mytimeout); - SockTimeout(sock, mytimeout); if (SockRead(sock, buf, size) == -1) { set_timeout(0); @@ -1688,7 +1684,6 @@ int gen_recv_split(int sock /** socket to which server is connected */, phase = SERVER_WAIT; set_timeout(mytimeout); - SockTimeout(sock, mytimeout); rr = SockRead(sock, buf + n, size - n); set_timeout(0); phase = oldphase; @@ -1760,7 +1755,6 @@ va_dcl va_end(ap); snprintf(buf+strlen(buf), sizeof(buf)-strlen(buf), "\r\n"); - SockTimeout(sock, mytimeout); ok = SockWrite(sock, buf, strlen(buf)); if (ok == -1 || (size_t)ok != strlen(buf)) { /* short write, bail out */ -- cgit v1.2.3