From 1a794b3b00bd4b2f720f3426a386d5c86cc65ea8 Mon Sep 17 00:00:00 2001 From: Matthias Andree Date: Tue, 17 May 2011 17:59:53 +0200 Subject: [PATCH] Use SO_???TIMEO, to fix hangs during STARTTLS negotiation. The hangs were reported by Thomas Jarosch, and using BSD socket timeouts should fix the problem in-depth, and will also cover cases where no explicit SIGALRM timeout is set. --- NEWS | 2 ++ driver.c | 10 ++-------- imap.c | 1 - sink.c | 3 ++- smtp.c | 32 +++++++------------------------- socket.c | 10 ++++++++-- transact.c | 29 ++++++++++++++--------------- 7 files changed, 35 insertions(+), 52 deletions(-) diff --git a/NEWS b/NEWS index 9f1bbf98..464fd894 100644 --- a/NEWS +++ b/NEWS @@ -82,6 +82,8 @@ fetchmail-6.3.20 (not yet released): * Do not print "skipping message" for old messages even in verbose mode. If there are too many old messages, the logs just get filled without any real activity. (Sunil Shetye) (suggested by Yunfan Jiang) +* Set socket timeouts, to fix hangs during STARTTLS negotiation. + Reported by Thomas Jarosch. # TRANSLATION UPDATES [ja] Japanese (Takeshi Hamasaki) diff --git a/driver.c b/driver.c index cfc4af0f..500f9eb2 100644 --- a/driver.c +++ b/driver.c @@ -130,14 +130,8 @@ static RETSIGTYPE timeout_handler (int signal) static int cleanupSockClose (int fd) /* close sockets in maximum CLEANUP_TIMEOUT seconds during cleanup */ { - int scerror; - SIGHANDLERTYPE alrmsave; - alrmsave = set_signal_handler(SIGALRM, null_signal_handler); - set_timeout(CLEANUP_TIMEOUT); - scerror = SockClose(fd); - set_timeout(0); - set_signal_handler(SIGALRM, alrmsave); - return (scerror); + (void)SockTimeout(fd, CLEANUP_TIMEOUT); /* ignore errors */ + return SockClose(fd); } #ifdef KERBEROS_V4 diff --git a/imap.c b/imap.c index 0bbf06db..bdda1656 100644 --- a/imap.c +++ b/imap.c @@ -751,7 +751,6 @@ static int imap_idle(int sock) } /* restore normal timeout value */ - set_timeout(0); mytimeout = saved_timeout; stage = STAGE_GETRANGE; diff --git a/sink.c b/sink.c index 5d92556f..04bbbd24 100644 --- a/sink.c +++ b/sink.c @@ -127,12 +127,12 @@ int smtp_setup(struct query *ctl) oldphase = phase; phase = LISTENER_WAIT; - set_timeout(ctl->server.timeout); for (idp = ctl->smtphunt; idp; idp = idp->next) { char *cp; const char *portnum = SMTP_PORT; + set_timeout(ctl->server.timeout); ctl->smtphost = idp->id; /* remember last host tried. */ if (ctl->smtphost[0]=='/') { @@ -180,6 +180,7 @@ int smtp_setup(struct query *ctl) * so it's safest not to assume the socket will still be good. */ smtp_close(ctl, 0); + set_timeout(ctl->server.timeout); /* if opening for ESMTP failed, try SMTP */ if (ctl->smtphost[0]=='/') diff --git a/smtp.c b/smtp.c index 1c99c696..603c4014 100644 --- a/smtp.c +++ b/smtp.c @@ -171,24 +171,22 @@ int SMTP_ehlo(int sock, char smtp_mode, const char *host, char *name, char *pass { struct opt *hp; char auth_response[511]; - SIGHANDLERTYPE alrmsave; const int tmout = (mytimeout >= TIMEOUT_HELO ? mytimeout : TIMEOUT_HELO); + if (SockTimeout(sock, tmout) != 0) + return SM_UNRECOVERABLE; + SockPrintf(sock,"%cHLO %s\r\n", (smtp_mode == 'S') ? 'E' : smtp_mode, host); if (outlevel >= O_MONITOR) report(stdout, "%cMTP> %cHLO %s\n", smtp_mode, (smtp_mode == 'S') ? 'E' : smtp_mode, host); - alrmsave = set_signal_handler(SIGALRM, null_signal_handler); - set_timeout(tmout); - *opt = 0; while ((SockRead(sock, smtp_response, sizeof(smtp_response)-1)) != -1) { size_t n; set_timeout(0); - (void)set_signal_handler(SIGALRM, alrmsave); n = strlen(smtp_response); if (n > 0 && smtp_response[n-1] == '\n') @@ -214,9 +212,6 @@ int SMTP_ehlo(int sock, char smtp_mode, const char *host, char *name, char *pass } else if (smtp_response[3] != '-') return SM_ERROR; - - alrmsave = set_signal_handler(SIGALRM, null_signal_handler); - set_timeout(tmout); } return SM_UNRECOVERABLE; } @@ -311,23 +306,18 @@ int SMTP_ok(int sock, char smtp_mode, int mintimeout) * smtp_response, without trailing [CR]LF, but with normalized CRLF * between multiple lines of multi-line replies */ { - SIGHANDLERTYPE alrmsave; char reply[MSGBUFSIZE], *i; - - /* set an alarm for smtp ok */ - alrmsave = set_signal_handler(SIGALRM, null_signal_handler); - set_timeout(mytimeout >= mintimeout ? mytimeout : mintimeout); + int tmo = mytimeout >= mintimeout ? mytimeout : mintimeout; smtp_response[0] = '\0'; + if (SockTimeout(sock, tmo)) + return SM_UNRECOVERABLE; + while ((SockRead(sock, reply, sizeof(reply)-1)) != -1) { size_t n; - /* restore alarm */ - set_timeout(0); - set_signal_handler(SIGALRM, alrmsave); - n = strlen(reply); if (n > 0 && reply[n-1] == '\n') reply[--n] = '\0'; @@ -363,16 +353,8 @@ int SMTP_ok(int sock, char smtp_mode, int mintimeout) return SM_ERROR; strlcat(smtp_response, "\r\n", sizeof(smtp_response)); - - /* set an alarm for smtp ok */ - set_signal_handler(SIGALRM, null_signal_handler); - set_timeout(mytimeout); } - /* restore alarm */ - set_timeout(0); - set_signal_handler(SIGALRM, alrmsave); - if (outlevel >= O_MONITOR) report(stderr, GT_("smtp listener protocol error\n")); return SM_UNRECOVERABLE; diff --git a/socket.c b/socket.c index 26e37de8..4088e74f 100644 --- a/socket.c +++ b/socket.c @@ -168,6 +168,10 @@ static int handle_plugin(const char *host, report(stderr, GT_("fetchmail: socketpair failed\n")); return -1; } + + if (SockTimeout(fds[0], mytimeout)) return -1; + if (SockTimeout(fds[1], mytimeout)) return -1; + switch (fork()) { case -1: /* error */ @@ -214,7 +218,8 @@ static int setsocktimeout(int sock, int which, int timeout) { } /** Configure socket options such as send/receive timeout at the socket - * level, to avoid network-induced stalls. + * level, to avoid network-induced stalls. \return 0 for success, 1 for + * error. */ int SockTimeout(int sock, int timeout) { @@ -246,6 +251,8 @@ int UnixOpen(const char *path) return -1; } + SockTimeout(sock, mytimeout); + /* Socket opened saved. Usefull if connect timeout * because it can be closed. */ @@ -369,7 +376,6 @@ int SockOpen(const char *host, const char *service, return i; } - #if defined(HAVE_STDARG_H) int SockPrintf(int sock, const char* format, ...) { diff --git a/transact.c b/transact.c index d1e4f6a9..f55bdee2 100644 --- a/transact.c +++ b/transact.c @@ -486,13 +486,13 @@ int readheaders(int sock, do { char *sp, *tp; - set_timeout(mytimeout); - if ((n = SockRead(sock, buf, sizeof(buf)-1)) == -1) { - set_timeout(0); + SockTimeout(sock, mytimeout); + n = SockRead(sock, buf, sizeof(buf) - 1); + + if (n == -1) { free(line); return(PS_SOCKET); } - set_timeout(0); /* * Smash out any NULs, they could wreak havoc later on. @@ -617,9 +617,8 @@ eoh: } /* check for RFC822 continuations */ - set_timeout(mytimeout); + SockTimeout(sock, mytimeout); ch = SockPeek(sock); - set_timeout(0); } while (ch == ' ' || ch == '\t'); /* continuation to next line? */ @@ -1407,17 +1406,16 @@ 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 */ - if ((linelen = SockRead(sock, inbufp, sizeof(buf)-4-(inbufp-buf)))==-1) + linelen = SockRead(sock, inbufp, sizeof(buf)-4-(inbufp-buf)); + if (linelen == -1) { - set_timeout(0); release_sink(ctl); return(PS_SOCKET); } - set_timeout(0); /* write the message size dots */ if (linelen > 0) @@ -1568,13 +1566,15 @@ int gen_recv(int sock /** socket to which server is connected */, { size_t n; int oldphase = phase; /* we don't have to be re-entrant */ + int retval; phase = SERVER_WAIT; - set_timeout(mytimeout); - if (SockRead(sock, buf, size) == -1) + SockTimeout(sock, mytimeout); + retval = SockRead(sock, buf, size); + phase = oldphase; + + if (retval == -1) { - set_timeout(0); - phase = oldphase; if(is_idletimeout()) { resetidletimeout(); @@ -1593,7 +1593,6 @@ int gen_recv(int sock /** socket to which server is connected */, buf[--n] = '\0'; if (outlevel >= O_MONITOR) report(stdout, "%s< %s\n", protocol->name, buf); - phase = oldphase; return(PS_SUCCESS); } } -- 2.43.2