diff options
author | Steve Huston <shuston@riverace.com> | 2012-09-05 22:22:19 +0000 |
---|---|---|
committer | Steve Huston <shuston@riverace.com> | 2012-09-05 22:22:19 +0000 |
commit | cb99a5559e9e17e321239728de1e8cb69d1e783b (patch) | |
tree | f0406f0db3a1551b06f4241abfc89f046a664cf9 | |
parent | c4bd4a2781acc18a6ea7c5671241ace49d0603d5 (diff) | |
download | ATCD-cb99a5559e9e17e321239728de1e8cb69d1e783b.tar.gz |
ChangeLogTag:Wed Sep 5 20:09:09 UTC 2012 Steve Huston <shuston@riverace.com>
-rw-r--r-- | ChangeLog | 12 | ||||
-rw-r--r-- | ace/SSL/SSL_SOCK_Stream.cpp | 52 | ||||
-rw-r--r-- | ace/SSL/SSL_SOCK_Stream.inl | 157 |
3 files changed, 106 insertions, 115 deletions
diff --git a/ChangeLog b/ChangeLog index d5da2cdf64e..9744619817c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +Wed Sep 5 20:09:09 UTC 2012 Steve Huston <shuston@riverace.com> + + * ace/SSL/SSL_SOCK_Stream.inl (recv_i): Added back in logic to wait + for data available to receive when called with a timeout. Thanks + to Ossama Othman for helping to sort this out. For reference, the + issues involved here are well-explained in "SSL and TLS" by + Eric Rescorla, section 8.9. + + * ace/SSL/SSL_SOCK_Stream.cpp (recv_n): Removed the hack-y retry on + EWOULDBLOCK - the proper handling of timeouts is now in its proper + place in recv_i() - see above. + Thu May 24 14:35:04 UTC 2012 Steve Huston <shuston@riverace.com> * ace/Cache_Map_Manager_T.cpp (find): Remove extraneous () from diff --git a/ace/SSL/SSL_SOCK_Stream.cpp b/ace/SSL/SSL_SOCK_Stream.cpp index d5e2a51e3a4..4bccbb833a5 100644 --- a/ace/SSL/SSL_SOCK_Stream.cpp +++ b/ace/SSL/SSL_SOCK_Stream.cpp @@ -409,22 +409,9 @@ ACE_SSL_SOCK_Stream::recv_n (void *buf, timeout); if (n < 0) - { - if (errno == EWOULDBLOCK) - { - // If blocked, try again. - n = 0; - continue; - } - else - { - return -1; - } - } + return -1; else if (n == 0) - { - break; - } + break; } return ACE_Utils::truncate_cast<ssize_t> (bytes_transferred); @@ -455,22 +442,9 @@ ACE_SSL_SOCK_Stream::recv_n (void *buf, int len, int flags) const flags); if (n < 0) - { - if (errno == EWOULDBLOCK) - { - // If blocked, try again. - n = 0; - continue; - } - else - { - return -1; - } - } + return -1; else if (n == 0) - { - break; - } + break; } return ACE_Utils::truncate_cast<ssize_t> (bytes_transferred); @@ -498,24 +472,10 @@ ACE_SSL_SOCK_Stream::send_n (const void *buf, int len, int flags) const n = this->send ((const char*) buf + bytes_transferred, len - bytes_transferred, flags); - if (n < 0) - { - if (errno == EWOULDBLOCK) - { - // If blocked, try again. - n = 0; - continue; - } - else - { - return -1; - } - } + return -1; else if (n == 0) - { - break; - } + break; } return ACE_Utils::truncate_cast<ssize_t> (bytes_transferred); diff --git a/ace/SSL/SSL_SOCK_Stream.inl b/ace/SSL/SSL_SOCK_Stream.inl index ca4e48ee22f..a720f07ebc6 100644 --- a/ace/SSL/SSL_SOCK_Stream.inl +++ b/ace/SSL/SSL_SOCK_Stream.inl @@ -115,89 +115,108 @@ ACE_SSL_SOCK_Stream::recv_i (void *buf, ACE::record_and_set_non_blocking_mode (handle, val); - // Only block on select() with a timeout if no data in the - // internal OpenSSL buffer is pending read completion for - // the same reasons stated above, i.e. all data must be read - // before blocking on select(). - if (timeout != 0 - && !::SSL_pending (this->ssl_)) - { - if (ACE::enter_recv_timedwait (handle, - timeout, - val) == -1) - return -1; - } - + // Only block to wait for I/O ready with a timeout if all on-hand data + // has been consumed. If there is remaining data in the SSL buffers + // the socket won't "select" even though SSL_read would complete. + // See "SSL and TLS" by Rescorla section 8.9 for more info. + bool peeking = false; + bool retry = false; if (flags) { if (ACE_BIT_ENABLED (flags, MSG_PEEK)) { - bytes_read = ::SSL_peek (this->ssl_, - static_cast<char *> (buf), - ACE_Utils::truncate_cast<int> (n)); + peeking = true; } else { ACE_NOTSUP_RETURN (-1); } } - else - { - bytes_read = ::SSL_read (this->ssl_, - static_cast<char *> (buf), - ACE_Utils::truncate_cast<int> (n)); - } - int const status = ::SSL_get_error (this->ssl_, bytes_read); - switch (status) + do { - case SSL_ERROR_NONE: - if (timeout != 0) - ACE::restore_non_blocking_mode (handle, val); - - return bytes_read; - - case SSL_ERROR_WANT_READ: - case SSL_ERROR_WANT_WRITE: - errno = EWOULDBLOCK; - - return -1; - - case SSL_ERROR_ZERO_RETURN: - if (timeout != 0) - ACE::restore_non_blocking_mode (handle, val); - - // The peer has notified us that it is shutting down via the SSL - // "close_notify" message so we need to shutdown, too. - (void) ::SSL_shutdown (this->ssl_); - - return bytes_read; - - case SSL_ERROR_SYSCALL: - if (bytes_read == 0) - // An EOF occured but the SSL "close_notify" message was not - // sent. This is a protocol error, but we ignore it. - return 0; - - // If not an EOF, then fall through to "default" case. - - // On some platforms (e.g. MS Windows) OpenSSL does not store - // the last error in errno so explicitly do so. - ACE_OS::set_errno_to_last_error (); - - break; - - default: - // Reset errno to prevent previous values (e.g. EWOULDBLOCK) - // from being associated with a fatal SSL error. - errno = 0; - - ACE_SSL_Context::report_error (); + retry = false; + if (peeking) + { + bytes_read = ::SSL_peek (this->ssl_, + static_cast<char *> (buf), + ACE_Utils::truncate_cast<int> (n)); + } + else + { + bytes_read = ::SSL_read (this->ssl_, + static_cast<char *> (buf), + ACE_Utils::truncate_cast<int> (n)); + } - break; - } + int const status = ::SSL_get_error (this->ssl_, bytes_read); + int substat = 0; + switch (status) + { + case SSL_ERROR_NONE: + break; + + case SSL_ERROR_WANT_READ: + case SSL_ERROR_WANT_WRITE: + if (timeout == 0) + { + errno = EWOULDBLOCK; + bytes_read = -1; + break; + } + substat = ACE::handle_ready (handle, + timeout, + status == SSL_ERROR_WANT_READ, + status == SSL_ERROR_WANT_WRITE, + 0); + if (substat == 1) // Now ready to proceed + { + retry = true; + break; + } + bytes_read = -1; + if (substat == 0) + errno = ETIME; + break; + + case SSL_ERROR_ZERO_RETURN: + bytes_read = 0; + + // The peer has notified us that it is shutting down via the SSL + // "close_notify" message so we need to shutdown, too. + (void) ::SSL_shutdown (this->ssl_); + + break; + + case SSL_ERROR_SYSCALL: + if (bytes_read == 0) + // An EOF occured but the SSL "close_notify" message was not + // sent. This is a protocol error, but we ignore it. + break; + + // If not an EOF, then fall through to "default" case. + + // On some platforms (e.g. MS Windows) OpenSSL does not store + // the last error in errno so explicitly do so. + ACE_OS::set_errno_to_last_error (); + + break; + + default: + // Reset errno to prevent previous values (e.g. EWOULDBLOCK) + // from being associated with a fatal SSL error. + bytes_read = -1; + errno = 0; + + ACE_SSL_Context::report_error (); + + break; + } + } while (retry); - return -1; + if (timeout != 0) + ACE::restore_non_blocking_mode (handle, val); + return bytes_read; } ACE_INLINE ssize_t |