diff --git a/postfix/HISTORY b/postfix/HISTORY index ccdbaa296..41e3eeb05 100644 --- a/postfix/HISTORY +++ b/postfix/HISTORY @@ -16879,5 +16879,35 @@ Apologies for any names omitted. Workaround: report a {client_connections} Milter macro value of zero instead of garbage, when the remote SMTP client is - excluded from connection count limits. Problem reported by - Christian Roessner. File: smtpd/smtpd_state,c, + not subject to any smtpd_client_* limits. Problem reported + by Christian Roessner. Files: smtpd/smtpd_state.c, + proto/MILTER_README.html. + +20110817 + + Cleanup: avoid misleading error messages after future code + change. The tls_bio_ops(3) module now returns non-zero errno + values only when requests fail due to a system-call error. + File: tls/tls_bio_ops.c. + + Cleanup: TLS handshake error messages. The SMTP client and + server now report STARTTLS network errors as "connection + timed out", "connection reset by peer", etc., instead of + reporting TLS error number 0. Files: tls/tls_bio_ops.c, + tls/tls_server.c, tls/tls_client.c. + +20110818 + + Cleanup: VSTREAM-over-TLS error return values, for robustness + against future change. For consistency with VSTREAM internal + interfaces, the tls_stream(3) read/write routines now return + -1 instead of unspecified negative OpenSSL results. File: + tls/tls_stream.c. + +20110819 + + Cleanup: further TLS code cleanups, for robustness against + future change. Unexpected TLS errors are no longer silently + treated as ordinary errors, and one corner-case error in TLS + timeout handling was fixed before it could cause trouble. + File: tls/tls_bio_ops.c. diff --git a/postfix/README_FILES/MILTER_README b/postfix/README_FILES/MILTER_README index b31be4eca..8e2a68759 100644 --- a/postfix/README_FILES/MILTER_README +++ b/postfix/README_FILES/MILTER_README @@ -347,8 +347,11 @@ Sendmail. See the workarounds section below for solutions. |_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ | |{client_addr} |Always |Client IP address | |_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ | - |{client_connections}|CONNECT |Connection concurrency for| - | | |this client | + | | |Connection concurrency for| + | | |this client (zero if the | + |{client_connections}|CONNECT |client is excluded from | + | | |all smtpd_client_* | + | | |limits). | |_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ | | | |Client hostname | | | |When address -> name | diff --git a/postfix/html/MILTER_README.html b/postfix/html/MILTER_README.html index 63a61a70f..d0dc9e81a 100644 --- a/postfix/html/MILTER_README.html +++ b/postfix/html/MILTER_README.html @@ -573,7 +573,8 @@ login method address {client_connections} CONNECT -Connection concurrency for this client +Connection concurrency for this client (zero if the client is +excluded from all smtpd_client_* limits). {client_name} Always Client hostname
When address → name lookup or name → address diff --git a/postfix/html/smtpd.8.html b/postfix/html/smtpd.8.html index 8bcd08697..fe412be52 100644 --- a/postfix/html/smtpd.8.html +++ b/postfix/html/smtpd.8.html @@ -47,16 +47,17 @@ SMTPD(8) SMTPD(8) RFC 1123 (Host requirements) RFC 1652 (8bit-MIME transport) RFC 1869 (SMTP service extensions) - RFC 1870 (Message Size Declaration) + RFC 1870 (Message size declaration) RFC 1985 (ETRN command) - RFC 2034 (SMTP Enhanced Status Codes) + RFC 2034 (SMTP enhanced status codes) RFC 2554 (AUTH command) RFC 2821 (SMTP protocol) - RFC 2920 (SMTP Pipelining) + RFC 2920 (SMTP pipelining) RFC 3207 (STARTTLS command) - RFC 3461 (SMTP DSN Extension) - RFC 3463 (Enhanced Status Codes) - RFC 3848 (ESMTP Transmission Types) + RFC 3461 (SMTP DSN extension) + RFC 3463 (Enhanced status codes) + RFC 3848 (ESMTP transmission types) + RFC 4409 (Message submission) RFC 4954 (AUTH command) DIAGNOSTICS @@ -158,7 +159,7 @@ SMTPD(8) SMTPD(8) smtpd_per_record_deadline (normal: no, overload: yes) Change the behavior of the smtpd_timeout time limit, from a time limit per read or write system - call, to a time limit to read or write a complete + call, to a time limit to send or receive a complete record (an SMTP command line, SMTP response line, SMTP message content line, or TLS protocol mes- sage). @@ -857,7 +858,7 @@ SMTPD(8) SMTPD(8) smtpd_per_record_deadline (normal: no, overload: yes) Change the behavior of the smtpd_timeout time limit, from a time limit per read or write system - call, to a time limit to read or write a complete + call, to a time limit to send or receive a complete record (an SMTP command line, SMTP response line, SMTP message content line, or TLS protocol mes- sage). diff --git a/postfix/man/man8/smtpd.8 b/postfix/man/man8/smtpd.8 index c8355b7f1..9150ec351 100644 --- a/postfix/man/man8/smtpd.8 +++ b/postfix/man/man8/smtpd.8 @@ -50,16 +50,17 @@ RFC 821 (SMTP protocol) RFC 1123 (Host requirements) RFC 1652 (8bit-MIME transport) RFC 1869 (SMTP service extensions) -RFC 1870 (Message Size Declaration) +RFC 1870 (Message size declaration) RFC 1985 (ETRN command) -RFC 2034 (SMTP Enhanced Status Codes) +RFC 2034 (SMTP enhanced status codes) RFC 2554 (AUTH command) RFC 2821 (SMTP protocol) -RFC 2920 (SMTP Pipelining) +RFC 2920 (SMTP pipelining) RFC 3207 (STARTTLS command) -RFC 3461 (SMTP DSN Extension) -RFC 3463 (Enhanced Status Codes) -RFC 3848 (ESMTP Transmission Types) +RFC 3461 (SMTP DSN extension) +RFC 3463 (Enhanced status codes) +RFC 3848 (ESMTP transmission types) +RFC 4409 (Message submission) RFC 4954 (AUTH command) .SH DIAGNOSTICS .ad diff --git a/postfix/proto/MILTER_README.html b/postfix/proto/MILTER_README.html index 3c8d95418..ba30ad050 100644 --- a/postfix/proto/MILTER_README.html +++ b/postfix/proto/MILTER_README.html @@ -573,7 +573,8 @@ login method address {client_connections} CONNECT -Connection concurrency for this client +Connection concurrency for this client (zero if the client is +excluded from all smtpd_client_* limits). {client_name} Always Client hostname
When address → name lookup or name → address diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h index fc1b0e90c..013b14363 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,7 +20,7 @@ * Patches change both the patchlevel and the release date. Snapshots have no * patchlevel; they change the release date only. */ -#define MAIL_RELEASE_DATE "20110814" +#define MAIL_RELEASE_DATE "20110820" #define MAIL_VERSION_NUMBER "2.9" #ifdef SNAPSHOT diff --git a/postfix/src/smtpd/smtpd.c b/postfix/src/smtpd/smtpd.c index f813884b5..6bc2a68bb 100644 --- a/postfix/src/smtpd/smtpd.c +++ b/postfix/src/smtpd/smtpd.c @@ -40,16 +40,17 @@ /* RFC 1123 (Host requirements) /* RFC 1652 (8bit-MIME transport) /* RFC 1869 (SMTP service extensions) -/* RFC 1870 (Message Size Declaration) +/* RFC 1870 (Message size declaration) /* RFC 1985 (ETRN command) -/* RFC 2034 (SMTP Enhanced Status Codes) +/* RFC 2034 (SMTP enhanced status codes) /* RFC 2554 (AUTH command) /* RFC 2821 (SMTP protocol) -/* RFC 2920 (SMTP Pipelining) +/* RFC 2920 (SMTP pipelining) /* RFC 3207 (STARTTLS command) -/* RFC 3461 (SMTP DSN Extension) -/* RFC 3463 (Enhanced Status Codes) -/* RFC 3848 (ESMTP Transmission Types) +/* RFC 3461 (SMTP DSN extension) +/* RFC 3463 (Enhanced status codes) +/* RFC 3848 (ESMTP transmission types) +/* RFC 4409 (Message submission) /* RFC 4954 (AUTH command) /* DIAGNOSTICS /* Problems and transactions are logged to \fBsyslogd\fR(8). @@ -131,8 +132,8 @@ /* Available in Postfix version 2.9 and later: /* .IP "\fBsmtpd_per_record_deadline (normal: no, overload: yes)\fR" /* Change the behavior of the smtpd_timeout time limit, from a -/* time limit per read or write system call, to a time limit to read -/* or write a complete record (an SMTP command line, SMTP response +/* time limit per read or write system call, to a time limit to send +/* or receive a complete record (an SMTP command line, SMTP response /* line, SMTP message content line, or TLS protocol message). /* ADDRESS REWRITING CONTROLS /* .ad @@ -642,8 +643,8 @@ /* Available in Postfix version 2.9 and later: /* .IP "\fBsmtpd_per_record_deadline (normal: no, overload: yes)\fR" /* Change the behavior of the smtpd_timeout time limit, from a -/* time limit per read or write system call, to a time limit to read -/* or write a complete record (an SMTP command line, SMTP response +/* time limit per read or write system call, to a time limit to send +/* or receive a complete record (an SMTP command line, SMTP response /* line, SMTP message content line, or TLS protocol message). /* TARPIT CONTROLS /* .ad diff --git a/postfix/src/tls/tls_bio_ops.c b/postfix/src/tls/tls_bio_ops.c index e6a1dc3f6..4d1bf1a41 100644 --- a/postfix/src/tls/tls_bio_ops.c +++ b/postfix/src/tls/tls_bio_ops.c @@ -36,8 +36,8 @@ /* int timeout; /* TLS_SESS_STATE *context; /* DESCRIPTION -/* This module enforces timeouts on non-blocking I/O while -/* performing TLS handshake or input/output operations. +/* This module enforces VSTREAM-style timeouts on non-blocking +/* I/O while performing TLS handshake or input/output operations. /* /* The Postfix VSTREAM read/write routines invoke the /* tls_bio_read/write routines to send and receive plain-text @@ -74,8 +74,26 @@ /* .IP TLScontext /* TLS session state. /* DIAGNOSTICS -/* The result value is -1 in case of a network read/write -/* error, otherwise it is the result value of the TLS operation. +/* A result value > 0 means successful completion. +/* +/* A result value < 0 means that the requested operation did +/* not complete due to TLS protocol failure, system call +/* failure, or for any reason described under "in addition" +/* below. +/* +/* A result value of 0 from tls_bio_shutdown() means that the +/* operation is in progress. A result value of 0 from other +/* tls_bio_ops(3) operations means that the remote party either +/* closed the network connection or that it sent a TLS shutdown +/* request. +/* +/* Upon return from the tls_bio_ops(3) routines the global +/* errno value is non-zero when the requested operation did not +/* complete due to system call failure. +/* +/* In addition, the result value is set to -1, and the global +/* errno value is set to ETIMEDOUT, when some network read/write +/* operation did not complete within the time limit. /* LICENSE /* .ad /* .fi @@ -140,8 +158,6 @@ int tls_bio(int fd, int timeout, TLS_SESS_STATE *TLScontext, const char *myname = "tls_bio"; int status; int err; - int retval = 0; - int done; int enable_deadline; struct timeval time_limit; /* initial time limit */ struct timeval time_left; /* amount of time left */ @@ -149,23 +165,38 @@ int tls_bio(int fd, int timeout, TLS_SESS_STATE *TLScontext, struct timeval time_now; /* time after SSL_mumble() call */ struct timeval time_elapsed; /* total elapsed time */ + /* + * Compensation for interface mis-match: With VSTREAMs, timeout <= 0 + * means wait forever; with the read/write_wait() calls below, we need + * to specify timeout < 0 instead. + * + * Safety: no time limit means no deadline. + */ + if (timeout <= 0) { + timeout = -1; + enable_deadline = 0; + } + /* * Deadline management is simpler than with VSTREAMs, because we don't * need to decrement a per-stream time limit. We just work within the * budget that is available for this tls_bio() call. */ - enable_deadline = vstream_fstat(TLScontext->stream, VSTREAM_FLAG_DEADLINE); - if (enable_deadline) { - time_limit.tv_sec = timeout; - time_limit.tv_usec = 0; - GETTIMEOFDAY(&time_entry); + else { + enable_deadline = + vstream_fstat(TLScontext->stream, VSTREAM_FLAG_DEADLINE); + if (enable_deadline) { + time_limit.tv_sec = timeout; + time_limit.tv_usec = 0; + GETTIMEOFDAY(&time_entry); + } } /* * If necessary, retry the SSL handshake or read/write operation after * handling any pending network I/O. */ - for (done = 0; done == 0; /* void */ ) { + for (;;) { if (hsfunc) status = hsfunc(TLScontext->con); else if (rfunc) @@ -212,6 +243,27 @@ int tls_bio(int fd, int timeout, TLS_SESS_STATE *TLScontext, #endif /* + * Correspondence between SSL_ERROR_* error codes and tls_bio_(read, + * write, accept, connect, shutdown) return values (for brevity: + * retval). + * + * SSL_ERROR_NONE corresponds with retval > 0. With SSL_(read, write) + * this is the number of plaintext bytes sent or received. With + * SSL_(accept, connect, shutdown) this means that the operation was + * completed successfully. + * + * SSL_ERROR_WANT_(WRITE, READ) start a new loop iteration, or force + * (retval = -1, errno = ETIMEDOUT) when the time limit is exceeded. + * + * All other SSL_ERROR_* cases correspond with retval <= 0. With + * SSL_(read, write, accept, connect) retval == 0 means that the + * remote party either closed the network connection or that it + * requested TLS shutdown; with SSL_shutdown() retval == 0 means that + * our own shutdown request is in progress. With all operations + * retval < 0 means that there was an error. In the latter case, + * SSL_ERROR_SYSCALL means that error details are returned via the + * errno value. + * * Find out if we must retry the operation and/or if there is pending * network I/O. * @@ -220,10 +272,6 @@ int tls_bio(int fd, int timeout, TLS_SESS_STATE *TLScontext, * anomaly here and repeat the call. */ switch (err) { - case SSL_ERROR_NONE: /* success */ - retval = status; - done = 1; - break; case SSL_ERROR_WANT_WRITE: case SSL_ERROR_WANT_READ: if (enable_deadline) { @@ -245,6 +293,16 @@ int tls_bio(int fd, int timeout, TLS_SESS_STATE *TLScontext, } break; + /* + * Unhandled cases: SSL_ERROR_WANT_(ACCEPT, CONNECT, X509_LOOKUP) + * etc. Historically, Postfix silently treated these as ordinary + * I/O errors so we don't really know how common they are. For + * now, we just log a warning. + */ + default: + msg_warn("%s: unexpected SSL_ERROR code %d", myname, err); + /* FALLTHROUGH */ + /* * With tls_timed_read() and tls_timed_write() the caller is the * VSTREAM library module which is unaware of TLS, so we log the @@ -257,13 +315,14 @@ int tls_bio(int fd, int timeout, TLS_SESS_STATE *TLScontext, if (rfunc || wfunc) tls_print_errors(); /* FALLTHROUGH */ - default: - retval = status; - done = 1; - break; + case SSL_ERROR_ZERO_RETURN: + case SSL_ERROR_NONE: + errno = 0; /* avoid bogus warnings */ + /* FALLTHROUGH */ + case SSL_ERROR_SYSCALL: + return (status); } } - return (retval); } #endif diff --git a/postfix/src/tls/tls_client.c b/postfix/src/tls/tls_client.c index 795ba7d70..750fb5a83 100644 --- a/postfix/src/tls/tls_client.c +++ b/postfix/src/tls/tls_client.c @@ -900,8 +900,15 @@ TLS_SESS_STATE *tls_client_start(const TLS_CLIENT_START_PROPS *props) sts = tls_bio_connect(vstream_fileno(props->stream), props->timeout, TLScontext); if (sts <= 0) { - msg_info("SSL_connect error to %s: %d", props->namaddr, sts); - tls_print_errors(); + if (ERR_peek_error() != 0) { + msg_info("SSL_connect error to %s: %d", props->namaddr, sts); + tls_print_errors(); + } else if (errno != 0) { + msg_info("SSL_connect error to %s: %m", props->namaddr); + } else { + msg_info("SSL_connect error to %s: lost connection", + props->namaddr); + } uncache_session(app_ctx->ssl_ctx, TLScontext); tls_free_context(TLScontext); return (0); diff --git a/postfix/src/tls/tls_server.c b/postfix/src/tls/tls_server.c index f764a5a39..19b0fb2ae 100644 --- a/postfix/src/tls/tls_server.c +++ b/postfix/src/tls/tls_server.c @@ -704,8 +704,15 @@ TLS_SESS_STATE *tls_server_start(const TLS_SERVER_START_PROPS *props) sts = tls_bio_accept(vstream_fileno(props->stream), props->timeout, TLScontext); if (sts <= 0) { - msg_info("SSL_accept error from %s: %d", props->namaddr, sts); - tls_print_errors(); + if (ERR_peek_error() != 0) { + msg_info("SSL_accept error from %s: %d", props->namaddr, sts); + tls_print_errors(); + } else if (errno != 0) { + msg_info("SSL_accept error from %s: %m", props->namaddr); + } else { + msg_info("SSL_accept error from %s: lost connection", + props->namaddr); + } tls_free_context(TLScontext); return (0); } diff --git a/postfix/src/tls/tls_stream.c b/postfix/src/tls/tls_stream.c index ca73a1136..73def41b9 100644 --- a/postfix/src/tls/tls_stream.c +++ b/postfix/src/tls/tls_stream.c @@ -24,6 +24,21 @@ /* tls_stream_stop() replaces the VSTREAM read/write routines /* by dummies that have no side effects, and deletes the /* VSTREAM's reference to the TLS context. +/* DIAGNOSTICS +/* The tls_stream(3) read/write routines return the non-zero +/* number of plaintext bytes read/written if successful; -1 +/* after TLS protocol failure, system-call failure, or for any +/* reason described under "in addition" below; and zero when +/* the remote party closed the connection or sent a TLS shutdown +/* request. +/* +/* Upon return from the tls_stream(3) read/write routines the +/* global errno value is non-zero when the requested operation +/* did not complete due to system call failure. +/* +/* In addition, the result value is set to -1, and the global +/* errno value is set to ETIMEDOUT, when a network read/write +/* request did not complete within the time limit. /* SEE ALSO /* dummy_read(3), placebo read routine /* dummy_write(3), placebo write routine @@ -65,6 +80,15 @@ #define TLS_INTERNAL #include + /* + * Interface mis-match compensation. The OpenSSL read/write routines return + * unspecified negative values when an operation fails, while the vstream(3) + * plaintext timed_read/write() functions follow the convention of UNIX + * system calls, and return -1 upon error. The macro below makes OpenSSL + * read/write results consistent with the UNIX system-call convention. + */ +#define NORMALIZED_VSTREAM_RETURN(retval) ((retval) < 0 ? -1 : (retval)) + /* tls_timed_read - read content from stream, then TLS decapsulate */ static ssize_t tls_timed_read(int fd, void *buf, size_t len, int timeout, @@ -82,7 +106,7 @@ static ssize_t tls_timed_read(int fd, void *buf, size_t len, int timeout, if (ret > 0 && TLScontext->log_level >= 4) msg_info("Read %ld chars: %.*s", (long) ret, (int) (ret > 40 ? 40 : ret), (char *) buf); - return (ret); + return (NORMALIZED_VSTREAM_RETURN(ret)); } /* tls_timed_write - TLS encapsulate content, then write to stream */ @@ -91,6 +115,7 @@ static ssize_t tls_timed_write(int fd, void *buf, size_t len, int timeout, void *context) { const char *myname = "tls_timed_write"; + ssize_t ret; TLS_SESS_STATE *TLScontext; TLScontext = (TLS_SESS_STATE *) context; @@ -100,7 +125,8 @@ static ssize_t tls_timed_write(int fd, void *buf, size_t len, int timeout, if (TLScontext->log_level >= 4) msg_info("Write %ld chars: %.*s", (long) len, (int) (len > 40 ? 40 : len), (char *) buf); - return (tls_bio_write(fd, buf, len, timeout, TLScontext)); + ret = tls_bio_write(fd, buf, len, timeout, TLScontext); + return (NORMALIZED_VSTREAM_RETURN(ret)); } /* tls_stream_start - start VSTREAM over TLS */ diff --git a/postfix/src/util/vstream.c b/postfix/src/util/vstream.c index fdb1b2101..5da2f1598 100644 --- a/postfix/src/util/vstream.c +++ b/postfix/src/util/vstream.c @@ -266,9 +266,20 @@ /* .IP "VSTREAM_CTL_READ_FN (ssize_t (*)(int, void *, size_t, int, void *))" /* The argument specifies an alternative for the timed_read(3) function, /* for example, a read function that performs decryption. +/* This function receives as arguments a file descriptor, buffer pointer, +/* buffer length, timeout value, and the VSTREAM's context value. +/* A timeout value <= 0 disables the time limit. +/* This function should return the positive number of bytes transferred, +/* 0 upon EOF, and -1 upon error with errno set appropriately. /* .IP "VSTREAM_CTL_WRITE_FN (ssize_t (*)(int, void *, size_t, int, void *))" /* The argument specifies an alternative for the timed_write(3) function, /* for example, a write function that performs encryption. +/* This function receives as arguments a file descriptor, buffer pointer, +/* buffer length, timeout value, and the VSTREAM's context value. +/* A timeout value <= 0 disables the time limit. +/* This function should return the positive number of bytes transferred, +/* and -1 upon error with errno set appropriately. Instead of -1 it may +/* also return 0, e.g., upon remote party-initiated protocol shutdown. /* .IP "VSTREAM_CTL_CONTEXT (char *)" /* The argument specifies application context that is passed on to /* the application-specified read/write routines. No copy is made.