diff options
Diffstat (limited to 'cpp/src')
-rw-r--r-- | cpp/src/Ice/Security.h | 21 | ||||
-rw-r--r-- | cpp/src/Ice/SslConnectionOpenSSL.cpp | 314 | ||||
-rw-r--r-- | cpp/src/Ice/SslConnectionOpenSSL.h | 90 | ||||
-rw-r--r-- | cpp/src/Ice/SslConnectionOpenSSLClient.cpp | 312 | ||||
-rw-r--r-- | cpp/src/Ice/SslConnectionOpenSSLServer.cpp | 279 | ||||
-rw-r--r-- | cpp/src/Ice/SslSystemOpenSSL.cpp | 2 | ||||
-rw-r--r-- | cpp/src/Ice/SslTransceiver.cpp | 20 | ||||
-rw-r--r-- | cpp/src/Ice/SslTransceiver.h | 1 |
8 files changed, 592 insertions, 447 deletions
diff --git a/cpp/src/Ice/Security.h b/cpp/src/Ice/Security.h index 29005412f03..10232ec0a15 100644 --- a/cpp/src/Ice/Security.h +++ b/cpp/src/Ice/Security.h @@ -16,6 +16,7 @@ #endif
#include <openssl/ssl.h>
+#include <time.h>
namespace IceSecurity
{
@@ -28,6 +29,7 @@ typedef enum SECURITY_METHODS,
SECURITY_EXCEPTIONS,
SECURITY_PROTOCOL,
+ SECURITY_DEV_DEBUG,
SECURITY_PROTOCOL_DEBUG
} SecurityTraceLevel;
@@ -39,10 +41,18 @@ typedef enum #define GETTHREADID getpid()
#endif
+#define ICE_SECURITY_DISPLAYTHREADS
+
+/*
+ time_t ltime; \
+ time(<ime); \
+ thread << " " << dec << ltime << " " << hex << (void *)this << " Thread(" << dec << GETTHREADID << ") "; \
+*/
+
#ifdef ICE_SECURITY_DISPLAYTHREADS
#define ICE_SECURITY_LOGGER(s) \
ostringstream thread; \
- thread << "Thread(" << dec << GETTHREADID << ") "; \
+ thread << hex << (void *)this << " Thread(" << dec << GETTHREADID << ") "; \
_logger->trace(_traceLevels->securityCat, thread.str() + s);
#else
#define ICE_SECURITY_LOGGER(s) _logger->trace(_traceLevels->securityCat, s);
@@ -57,6 +67,7 @@ typedef enum #define ICE_SECURITY_LEVEL_EXCEPTIONS (_traceLevels->security >= IceSecurity::SECURITY_EXCEPTIONS)
#define ICE_SECURITY_LEVEL_PROTOCOL (_traceLevels->security >= IceSecurity::SECURITY_PROTOCOL)
#define ICE_SECURITY_LEVEL_PROTOCOL_DEBUG (_traceLevels->security >= IceSecurity::SECURITY_PROTOCOL_DEBUG)
+#define ICE_SECURITY_LEVEL_DEV_DEBUG (_traceLevels->security >= IceSecurity::SECURITY_DEV_DEBUG)
#define ICE_SECURITY_LEVEL_PROTOCOL_GLOBAL \
(IceSecurity::Ssl::OpenSSL::System::_globalTraceLevels->security >= IceSecurity::SECURITY_PROTOCOL)
@@ -106,6 +117,12 @@ typedef enum #define ICE_PROTOCOL_DEBUG(s) \
if (ICE_SECURITY_LEVEL_PROTOCOL_DEBUG) \
{ \
+ ICE_SECURITY_LOGGER("PDB " + string(s)); \
+ }
+
+#define ICE_DEV_DEBUG(s) \
+ if (ICE_SECURITY_LEVEL_DEV_DEBUG) \
+ { \
ICE_SECURITY_LOGGER("DBG " + string(s)); \
}
@@ -118,6 +135,7 @@ typedef enum #define ICE_SECURITY_LEVEL_PROTOCOL false
#define ICE_SECURITY_LEVEL_PROTOCOL_DEBUG false
#define ICE_SECURITY_LEVEL_PROTOCOL_GLOBAL false
+#define ICE_SECURITY_LEVEL_DEV_DEBUG false
#define ICE_METHOD_INV(s)
#define ICE_METHOD_INS(s)
@@ -138,6 +156,7 @@ typedef enum #define ICE_EXCEPTION(s)
#define ICE_PROTOCOL(s)
#define ICE_PROTOCOL_DEBUG(s)
+#define ICE_DEV_DEBUG(s)
#endif
diff --git a/cpp/src/Ice/SslConnectionOpenSSL.cpp b/cpp/src/Ice/SslConnectionOpenSSL.cpp index 9e982281e15..5eae3270133 100644 --- a/cpp/src/Ice/SslConnectionOpenSSL.cpp +++ b/cpp/src/Ice/SslConnectionOpenSSL.cpp @@ -47,18 +47,22 @@ IceSecurity::Ssl::OpenSSL::Connection::Connection(SSL* sslConnection, string& sy _initWantRead = 0; _initWantWrite = 0; -
- _timeoutEncountered = false;
-
- // None configured, default to indicated timeout
- _handshakeReadTimeout = 0;
+ + _timeoutEncountered = false; + + // None configured, default to indicated timeout + _handshakeReadTimeout = 0; } IceSecurity::Ssl::OpenSSL::Connection::~Connection() { ICE_METHOD_INV("OpenSSL::Connection::~Connection()"); - shutdown(); + if (_sslConnection != 0) + { + SSL_free(_sslConnection); + _sslConnection = 0; + } IceSecurity::Ssl::Factory::releaseSystem(_system); @@ -73,9 +77,23 @@ IceSecurity::Ssl::OpenSSL::Connection::shutdown() if (_sslConnection != 0) { ICE_WARNING(string("shutting down SSL connection\n") + fdToString(SSL_get_fd(_sslConnection))); + int shutdown = 0; + int retries = 100; - SSL_free(_sslConnection); - _sslConnection = 0; + do + { + shutdown = SSL_shutdown(_sslConnection); + retries--; + } + while ((shutdown == 0) && (retries > 0)); + + if (shutdown <= 0) + { + ostringstream s; + s << "SSL shutdown failure encountered: code[" << shutdown << "] retries["; + s << retries << "]\n" << fdToString(SSL_get_fd(_sslConnection)); + ICE_PROTOCOL_DEBUG(s.str()); + } } ICE_METHOD_RET("OpenSSL::Connection::shutdown()"); @@ -122,6 +140,30 @@ IceSecurity::Ssl::OpenSSL::Connection::renegotiate() } int +IceSecurity::Ssl::OpenSSL::Connection::initialize(int timeout) +{ + HandshakeSentinel handshakeSentinel(_handshakeFlag); + + ICE_METHOD_INV("OpenSSL::Connection::initialize()"); + + int retCode; + + if (!handshakeSentinel.ownHandshake()) + { + // We don't own the handshake, we can't go any further. + retCode = -1; + } + else + { + retCode = init(timeout); + } + + ICE_METHOD_RET("OpenSSL::Connection::initialize()"); + + return retCode; +} + +int IceSecurity::Ssl::OpenSSL::Connection::sslRead(char* buffer, int bufferSize) { ICE_METHOD_INV("OpenSSL::Connection::sslRead()"); @@ -322,13 +364,15 @@ IceSecurity::Ssl::OpenSSL::Connection::readSelect(int timeout) if (ret == SOCKET_ERROR) { + ICE_DEV_DEBUG("Connection::readSelect(): Throwing SocketException... SslConnectionOpenSSL.cpp, 325"); SocketException ex(__FILE__, __LINE__); ex.error = getSocketErrno(); throw ex; } if (ret == 0) - {
+ { + ICE_DEV_DEBUG("Connection::readSelect(): Throwing TimeoutException... SslConnectionOpenSSL.cpp, 333"); _timeoutEncountered = true; throw TimeoutException(__FILE__, __LINE__); } @@ -373,6 +417,7 @@ IceSecurity::Ssl::OpenSSL::Connection::writeSelect(int timeout) if (ret == SOCKET_ERROR) { + ICE_DEV_DEBUG("Connection::writeSelect(): Throwing SocketException... SslConnectionOpenSSL.cpp, 378"); SocketException ex(__FILE__, __LINE__); ex.error = getSocketErrno(); throw ex; @@ -380,6 +425,7 @@ IceSecurity::Ssl::OpenSSL::Connection::writeSelect(int timeout) if (ret == 0) { + ICE_DEV_DEBUG("Connection::writeSelect(): Throwing TimeoutException... SslConnectionOpenSSL.cpp, 386"); throw TimeoutException(__FILE__, __LINE__); } @@ -402,156 +448,178 @@ IceSecurity::Ssl::OpenSSL::Connection::readSSL(Buffer& buf, int timeout) while (buf.i != buf.b.end()) { // Ensure we're initialized. - if (init(timeout)) + int initReturn = initialize(timeout); + + if (initReturn == -1) + { + // Handshake underway, we should just return with what we've got (even if that's nothing). + break; + } + + if (initReturn == 0) + { + // Retry the initialize call + continue; + } + + // initReturn must be > 0, so we're okay to try a write + + bytesPending = pending(); + + if (!bytesPending && readSelect(_readTimeout)) { - bytesPending = pending(); -
- if (!bytesPending && readSelect(_readTimeout))
- {
- bytesPending = 1;
- }
-
- _readTimeout = timeout;
-
- if (!bytesPending)
- {
- // We're done here.
- break;
- }
-
- bytesRead = sslRead((char *)buf.i, packetSize); - - switch (getLastError()) + bytesPending = 1; + } + + _readTimeout = timeout; + + if (!bytesPending) + { + ICE_PROTOCOL("No pending application-level bytes."); + + // We're done here. + break; + } + + bytesRead = sslRead((char *)buf.i, packetSize); + + switch (getLastError()) + { + case SSL_ERROR_NONE: { - case SSL_ERROR_NONE: + if (bytesRead > 0) { - if (bytesRead > 0) + if (_traceLevels->network >= 3) { - if (_traceLevels->network >= 3)
- {
- ostringstream s; - s << "received " << bytesRead << " of " << packetSize; - s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection)); - _logger->trace(_traceLevels->networkCat, s.str());
- }
- - totalBytesRead += bytesRead; - - buf.i += bytesRead; - - if (packetSize > buf.b.end() - buf.i) - { - packetSize = buf.b.end() - buf.i; - } + ostringstream s; + s << "received " << bytesRead << " of " << packetSize; + s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection)); + _logger->trace(_traceLevels->networkCat, s.str()); } - else - { - // TODO: The client application performs a cleanup at this point, - // not even shutting down SSL - it just frees the SSL - // structure. The server does nothing. I'm ignoring this, - // at the moment, I'm sure it will come back at me. - ICE_PROTOCOL("Error SSL_ERROR_NONE: Repeating as per protocol."); + totalBytesRead += bytesRead; + + buf.i += bytesRead; + + if (packetSize > buf.b.end() - buf.i) + { + packetSize = buf.b.end() - buf.i; } - continue; } - - case SSL_ERROR_WANT_WRITE: + else { - // If we get this error here, it HAS to be because the protocol wants - // to do something handshake related. As such, We're going to call - // write with an empty buffer. I've seen this done in the demo - // programs, so this should be valid. No actual application data - // will be sent, just protocol packets. + // TODO: The client application performs a cleanup at this point, + // not even shutting down SSL - it just frees the SSL + // structure. The server does nothing. I'm ignoring this, + // at the moment, I'm sure it will come back at me. + + ICE_PROTOCOL("Error SSL_ERROR_NONE: Repeating as per protocol."); + } + continue; + } - ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE."); + case SSL_ERROR_WANT_WRITE: + { + // If we get this error here, it HAS to be because the protocol wants + // to do something handshake related. As such, We're going to call + // write with an empty buffer. I've seen this done in the demo + // programs, so this should be valid. No actual application data + // will be sent, just protocol packets. - protocolWrite(); + ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE."); - continue; - } + protocolWrite(); - case SSL_ERROR_WANT_READ: - { - // Repeat with the same arguments! (as in the OpenSSL documentation) - // Whatever happened, the last read didn't actually read anything for - // us. This is effectively a retry. + continue; + } - ICE_PROTOCOL("Error SSL_ERROR_WANT_READ: Repeating as per protocol."); + case SSL_ERROR_WANT_READ: + { + // Repeat with the same arguments! (as in the OpenSSL documentation) + // Whatever happened, the last read didn't actually read anything for + // us. This is effectively a retry. - continue; - } + ICE_PROTOCOL("Error SSL_ERROR_WANT_READ: Repeating as per protocol."); - case SSL_ERROR_WANT_X509_LOOKUP: - { - // Perform another read. The read should take care of this. + continue; + } - ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol."); + case SSL_ERROR_WANT_X509_LOOKUP: + { + // Perform another read. The read should take care of this. - continue; - } + ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol."); + + continue; + } - case SSL_ERROR_SYSCALL: + case SSL_ERROR_SYSCALL: + { + if(bytesRead == -1) { - if(bytesRead == -1) + // IO Error in underlying BIO + + if (interrupted()) { - // IO Error in underlying BIO - - if (interrupted()) - { - break; - } - - if (wouldBlock()) - { - break; - } - - if (connectionLost()) - { - ConnectionLostException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } - else - { - SocketException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } + break; } - else // (bytesRead == 0) - { - ProtocolException protocolEx(__FILE__, __LINE__); - - // Protocol Error: Unexpected EOF - protocolEx._message = "Encountered an EOF that violates the SSL Protocol."; - ICE_SSLERRORS(protocolEx._message); - ICE_EXCEPTION(protocolEx._message); + if (wouldBlock()) + { + break; + } - throw protocolEx; + if (connectionLost()) + { + ICE_DEV_DEBUG("Connection::readSSL(): Throwing ConnectionLostException... SslConnectionOpenSSL.cpp, 518"); + ConnectionLostException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; + } + else + { + ICE_DEV_DEBUG("Connection::readSSL(): Throwing SocketException...SslConnectionOpenSSL.cpp, 525"); + SocketException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; } } - - case SSL_ERROR_SSL: + else // (bytesRead == 0) { ProtocolException protocolEx(__FILE__, __LINE__); - protocolEx._message = "Encountered a violation of the SSL Protocol."; + // Protocol Error: Unexpected EOF + protocolEx._message = "Encountered an EOF that violates the SSL Protocol."; ICE_SSLERRORS(protocolEx._message); ICE_EXCEPTION(protocolEx._message); throw protocolEx; } + } - case SSL_ERROR_ZERO_RETURN: - { - ConnectionLostException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } + case SSL_ERROR_SSL: + { + ProtocolException protocolEx(__FILE__, __LINE__); + + protocolEx._message = "Encountered a violation of the SSL Protocol."; + + ICE_SSLERRORS(protocolEx._message); + ICE_EXCEPTION(protocolEx._message); + + throw protocolEx; + } + + case SSL_ERROR_ZERO_RETURN: + { + // Indicates that that the SSL Connection has been closed. + // But does not necessarily indicate that the underlying transport + // has been closed (in the case of Ice, it definitely hasn't yet). + + ICE_DEV_DEBUG("Connection::readSSL(): Throwing ConnectionLostException... SslConnectionOpenSSL.cpp, 559"); + ConnectionLostException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; } } } diff --git a/cpp/src/Ice/SslConnectionOpenSSL.h b/cpp/src/Ice/SslConnectionOpenSSL.h index 9a89277262f..716da8cd2c3 100644 --- a/cpp/src/Ice/SslConnectionOpenSSL.h +++ b/cpp/src/Ice/SslConnectionOpenSSL.h @@ -29,6 +29,85 @@ namespace OpenSSL using namespace Ice; +class SafeFlag +{ +public: + SafeFlag(bool flagVal = false) + { + _flag = flagVal; + } + + ~SafeFlag() + { + } + + bool checkAndSet() + { + JTCSyncT<JTCMutex> sync(_mutex); + + if (_flag) + { + return false; + } + else + { + _flag = true; + return true; + } + } + + bool check() + { + JTCSyncT<JTCMutex> sync(_mutex); + return _flag; + } + + void set() + { + JTCSyncT<JTCMutex> sync(_mutex); + _flag = true; + } + + void unset() + { + JTCSyncT<JTCMutex> sync(_mutex); + _flag = false; + } + +private: + JTCMutex _mutex; + bool _flag; +}; + + +class HandshakeSentinel +{ + +public: + HandshakeSentinel(SafeFlag& handshakeFlag) : + _flag(handshakeFlag) + { + _ownHandshake = _flag.checkAndSet(); + } + + ~HandshakeSentinel() + { + if (_ownHandshake) + { + _flag.unset(); + } + } + + bool ownHandshake() + { + return _ownHandshake; + } + +private: + bool _ownHandshake; + SafeFlag& _flag; +}; + class Connection : public IceSecurity::Ssl::Connection { @@ -47,13 +126,14 @@ public: void setTrace(TraceLevelsPtr traceLevels) { _traceLevels = traceLevels; }; void setLogger(LoggerPtr traceLevels) { _logger = traceLevels; }; - void setHandshakeReadTimeout(int timeout) { _handshakeReadTimeout = timeout; };
+ void setHandshakeReadTimeout(int timeout) { _handshakeReadTimeout = timeout; }; protected: int connect(); int accept(); int renegotiate(); + int initialize(int timeout); inline int pending() { return SSL_pending(_sslConnection); }; inline int getLastError() const { return SSL_get_error(_sslConnection, _lastError); }; @@ -103,12 +183,12 @@ protected: System* _system; - JTCMutex _initMutex; + SafeFlag _handshakeFlag; int _initWantRead; int _initWantWrite; - bool _timeoutEncountered;
- int _handshakeReadTimeout;
- int _readTimeout;
+ bool _timeoutEncountered; + int _handshakeReadTimeout; + int _readTimeout; }; } diff --git a/cpp/src/Ice/SslConnectionOpenSSLClient.cpp b/cpp/src/Ice/SslConnectionOpenSSLClient.cpp index acd736de782..e13759f3fbd 100644 --- a/cpp/src/Ice/SslConnectionOpenSSLClient.cpp +++ b/cpp/src/Ice/SslConnectionOpenSSLClient.cpp @@ -36,8 +36,6 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::~ClientConnection() { ICE_METHOD_INV("OpenSSL::ClientConnection::~ClientConnection()"); - shutdown(); - ICE_METHOD_RET("OpenSSL::ClientConnection::~ClientConnection()"); } @@ -46,27 +44,6 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::shutdown() { ICE_METHOD_INV("OpenSSL::ClientConnection::shutdown()"); - if (_sslConnection != 0) - { - int shutdown = 0; - int retries = 100; - - do - { - shutdown = SSL_shutdown(_sslConnection); - retries--; - } - while ((shutdown == 0) && (retries > 0)); - - if (shutdown <= 0) - { - ostringstream s; - s << "SSL shutdown failure encountered: code[" << shutdown << "] retries["; - s << retries << "]\n" << fdToString(SSL_get_fd(_sslConnection)); - ICE_PROTOCOL_DEBUG(s.str()); - } - } - Connection::shutdown(); ICE_METHOD_RET("OpenSSL::ClientConnection::shutdown()"); @@ -75,25 +52,23 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::shutdown() int IceSecurity::Ssl::OpenSSL::ClientConnection::init(int timeout) { - JTCSyncT<JTCMutex> sync(_initMutex); - - ICE_METHOD_INV("OpenSSL::ClientConnection::init()");
-
- if (_timeoutEncountered)
- {
- throw TimeoutException(__FILE__, __LINE__);
- }
-
+ ICE_METHOD_INV("OpenSSL::ClientConnection::init()"); + + if (_timeoutEncountered) + { + throw TimeoutException(__FILE__, __LINE__); + } + int retCode = SSL_is_init_finished(_sslConnection); while (!retCode) { - int i = 0;
-
- _readTimeout = timeout > _handshakeReadTimeout ? timeout : _handshakeReadTimeout;
-
- try
- {
+ int i = 0; + + _readTimeout = timeout > _handshakeReadTimeout ? timeout : _handshakeReadTimeout; + + try + { if (_initWantRead) { i = readSelect(_readTimeout); @@ -101,12 +76,12 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::init(int timeout) else if (_initWantWrite) { i = writeSelect(timeout); - }
- }
- catch (const TimeoutException&)
- {
- _timeoutEncountered = true;
- throw;
+ } + } + catch (const TimeoutException&) + { + _timeoutEncountered = true; + throw; } if (_initWantRead && i == 0) @@ -174,12 +149,14 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::init(int timeout) if (connectionLost()) { + ICE_DEV_DEBUG("ClientConnection::init(): Throwing ConnectionLostException... SslConnectionOpenSSLClient.cpp, 177"); ConnectionLostException ex(__FILE__, __LINE__); ex.error = getSocketErrno(); throw ex; } else { + ICE_DEV_DEBUG("ClientConnection::init(): Throwing SocketException... SslConnectionOpenSSLClient.cpp, 184"); SocketException ex(__FILE__, __LINE__); ex.error = getSocketErrno(); throw ex; @@ -266,8 +243,8 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::write(Buffer& buf, int timeout) int totalBytesWritten = 0; int bytesWritten = 0; - int packetSize = buf.b.end() - buf.i;
-
+ int packetSize = buf.b.end() - buf.i; + #ifdef WIN32 // // Limit packet size to avoid performance problems on WIN32. @@ -283,162 +260,177 @@ IceSecurity::Ssl::OpenSSL::ClientConnection::write(Buffer& buf, int timeout) while (buf.i != buf.b.end()) { // Ensure we're initialized. - if (init(timeout)) + int initReturn = initialize(timeout); + + if (initReturn == -1) { - // Perform a select on the socket. - if (!writeSelect(timeout)) - { - // We're done here. - break; - } + // Handshake underway, we should just return with what we've got (even if that's nothing). + break; + } + + if (initReturn == 0) + { + // Retry the initialize call + continue; + } - bytesWritten = sslWrite((char *)buf.i, packetSize); + // initReturn must be > 0, so we're okay to try a write - switch (getLastError()) + // Perform a select on the socket. + if (!writeSelect(timeout)) + { + // We're done here. + break; + } + + bytesWritten = sslWrite((char *)buf.i, packetSize); + + switch (getLastError()) + { + case SSL_ERROR_NONE: { - case SSL_ERROR_NONE: + if (bytesWritten > 0) { - if (bytesWritten > 0) + if (_traceLevels->network >= 3) { - if (_traceLevels->network >= 3)
- {
- ostringstream s;
- s << "sent " << bytesWritten << " of " << packetSize;
- s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection));
- _logger->trace(_traceLevels->networkCat, s.str());
- }
-
- totalBytesWritten += bytesWritten; - - buf.i += bytesWritten; - - if (packetSize > buf.b.end() - buf.i) - { - packetSize = buf.b.end() - buf.i; - } + ostringstream s; + s << "sent " << bytesWritten << " of " << packetSize; + s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection)); + _logger->trace(_traceLevels->networkCat, s.str()); } - else - { - // TODO: The client application performs a cleanup at this point, - // not even shutting down SSL - it just frees the SSL - // structure. I'm ignoring this, at the moment, as I'm sure - // the demo is handling it in an artificial manner. - ICE_PROTOCOL("Error SSL_ERROR_NONE: Repeating as per protocol."); + totalBytesWritten += bytesWritten; + + buf.i += bytesWritten; + + if (packetSize > buf.b.end() - buf.i) + { + packetSize = buf.b.end() - buf.i; } - continue; } - - case SSL_ERROR_WANT_WRITE: + else { - // Repeat with the same arguments! (as in the OpenSSL documentation) - // Whatever happened, the last write didn't actually write anything - // for us. This is effectively a retry. + // TODO: The client application performs a cleanup at this point, + // not even shutting down SSL - it just frees the SSL + // structure. I'm ignoring this, at the moment, as I'm sure + // the demo is handling it in an artificial manner. - ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE: Repeating as per protocol."); - - continue; + ICE_PROTOCOL("Error SSL_ERROR_NONE: Repeating as per protocol."); } + continue; + } - case SSL_ERROR_WANT_READ: - { - // If we get this error here, it HAS to be because - // the protocol wants to do something handshake related. - // In the case that we might actually get some application data, - // we will use the base SSL read method, using the _inBuffer. + case SSL_ERROR_WANT_WRITE: + { + // Repeat with the same arguments! (as in the OpenSSL documentation) + // Whatever happened, the last write didn't actually write anything + // for us. This is effectively a retry. - ICE_PROTOCOL("Error SSL_ERROR_WANT_READ."); + ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE: Repeating as per protocol."); - readSSL(_inBuffer, timeout); + continue; + } - continue; - } + case SSL_ERROR_WANT_READ: + { + // If we get this error here, it HAS to be because + // the protocol wants to do something handshake related. + // In the case that we might actually get some application data, + // we will use the base SSL read method, using the _inBuffer. - case SSL_ERROR_WANT_X509_LOOKUP: - { - // Perform another read. The read should take care of this. + ICE_PROTOCOL("Error SSL_ERROR_WANT_READ."); - ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol."); + readSSL(_inBuffer, timeout); - continue; - } + continue; + } - case SSL_ERROR_SYSCALL: - { - // NOTE: The demo client only throws an exception if there were actually bytes - // written. This is considered to be an error status requiring shutdown. - // If nothing was written, the demo client stops writing - we continue. - // This is potentially something wierd to watch out for. - if (bytesWritten == -1) - { - // IO Error in underlying BIO - - if (interrupted()) - { - break; - } - - if (wouldBlock()) - { - break; - } - - if (connectionLost()) - { - ConnectionLostException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } - else - { - SocketException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } - } - else if (bytesWritten > 0) - { - ProtocolException protocolEx(__FILE__, __LINE__); + case SSL_ERROR_WANT_X509_LOOKUP: + { + // Perform another read. The read should take care of this. - // Protocol Error: Unexpected EOF - protocolEx._message = "Encountered an EOF that violates the SSL Protocol.\n"; + ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol."); - ICE_SSLERRORS(protocolEx._message); - ICE_EXCEPTION(protocolEx._message); + continue; + } - throw protocolEx; - } - else // bytesWritten == 0 - { - // Didn't write anything, continue, should be fine. + case SSL_ERROR_SYSCALL: + { + // NOTE: The demo client only throws an exception if there were actually bytes + // written. This is considered to be an error status requiring shutdown. + // If nothing was written, the demo client stops writing - we continue. + // This is potentially something wierd to watch out for. + if (bytesWritten == -1) + { + // IO Error in underlying BIO - ICE_PROTOCOL("Error SSL_ERROR_SYSCALL: Repeating as per protocol."); + if (interrupted()) + { + break; + } + if (wouldBlock()) + { break; } - } - case SSL_ERROR_SSL: + if (connectionLost()) + { + ICE_DEV_DEBUG("ClientConnection::write(): Throwing ConnectionLostException... SslConnectionOpenSSLClient.cpp, 390"); + ConnectionLostException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; + } + else + { + ICE_DEV_DEBUG("ClientConnection::write(): Throwing SocketException... SslConnectionOpenSSLClient.cpp, 397"); + SocketException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; + } + } + else if (bytesWritten > 0) { ProtocolException protocolEx(__FILE__, __LINE__); - protocolEx._message = "Encountered a violation of the SSL Protocol.\n"; + // Protocol Error: Unexpected EOF + protocolEx._message = "Encountered an EOF that violates the SSL Protocol.\n"; ICE_SSLERRORS(protocolEx._message); ICE_EXCEPTION(protocolEx._message); throw protocolEx; } - - case SSL_ERROR_ZERO_RETURN: + else // bytesWritten == 0 { - ICE_EXCEPTION("SSL_ERROR_ZERO_RETURN"); + // Didn't write anything, continue, should be fine. + + ICE_PROTOCOL("Error SSL_ERROR_SYSCALL: Repeating as per protocol."); - ConnectionLostException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; + break; } } + + case SSL_ERROR_SSL: + { + ProtocolException protocolEx(__FILE__, __LINE__); + + protocolEx._message = "Encountered a violation of the SSL Protocol.\n"; + + ICE_SSLERRORS(protocolEx._message); + ICE_EXCEPTION(protocolEx._message); + + throw protocolEx; + } + + case SSL_ERROR_ZERO_RETURN: + { + ICE_EXCEPTION("SSL_ERROR_ZERO_RETURN"); + + ConnectionLostException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; + } } } diff --git a/cpp/src/Ice/SslConnectionOpenSSLServer.cpp b/cpp/src/Ice/SslConnectionOpenSSLServer.cpp index 40991e654ae..415cdafaae1 100644 --- a/cpp/src/Ice/SslConnectionOpenSSLServer.cpp +++ b/cpp/src/Ice/SslConnectionOpenSSLServer.cpp @@ -37,8 +37,6 @@ IceSecurity::Ssl::OpenSSL::ServerConnection::~ServerConnection() { ICE_METHOD_INV("OpenSSL::ServerConnection::~ServerConnection()"); - shutdown(); - ICE_METHOD_RET("OpenSSL::ServerConnection::~ServerConnection()"); } @@ -47,31 +45,6 @@ IceSecurity::Ssl::OpenSSL::ServerConnection::shutdown() { ICE_METHOD_INV("OpenSSL::ServerConnection::shutdown()"); - if (_sslConnection != 0) - { - // NOTE: This call is how the server application shuts down, but they are - // also using SSL_CTX_set_quiet_shutdown(). - // SSL_set_shutdown(_sslConnection,SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN); - - int shutdown = 0; - int retries = 100; - - do - { - shutdown = SSL_shutdown(_sslConnection); - retries--; - } - while ((shutdown == 0) && (retries > 0)); - - if (shutdown <= 0) - { - ostringstream s; - s << "SSL shutdown failure encountered: code[" << shutdown << "] retries["; - s << retries << "]\n" << fdToString(SSL_get_fd(_sslConnection)); - ICE_PROTOCOL_DEBUG(s.str()); - } - } - Connection::shutdown(); ICE_METHOD_RET("OpenSSL::ServerConnection::shutdown()"); @@ -80,40 +53,38 @@ IceSecurity::Ssl::OpenSSL::ServerConnection::shutdown() int IceSecurity::Ssl::OpenSSL::ServerConnection::init(int timeout) { - JTCSyncT<JTCMutex> sync(_initMutex); - - ICE_METHOD_INV("OpenSSL::ServerConnection::init()");
-
- if (_timeoutEncountered)
- {
- throw TimeoutException(__FILE__, __LINE__);
- }
-
+ ICE_METHOD_INV("OpenSSL::ServerConnection::init()"); + + if (_timeoutEncountered) + { + throw TimeoutException(__FILE__, __LINE__); + } + int retCode = SSL_is_init_finished(_sslConnection); while (!retCode) { int i = 0; - _readTimeout = timeout > _handshakeReadTimeout ? timeout : _handshakeReadTimeout;
-
- try
- {
- if (_initWantRead)
- {
- i = readSelect(_readTimeout);
- }
- else if (_initWantWrite)
- {
- i = writeSelect(timeout);
- }
- }
- catch (const TimeoutException&)
- {
- _timeoutEncountered = true;
- throw;
- }
-
+ _readTimeout = timeout > _handshakeReadTimeout ? timeout : _handshakeReadTimeout; + + try + { + if (_initWantRead) + { + i = readSelect(_readTimeout); + } + else if (_initWantWrite) + { + i = writeSelect(timeout); + } + } + catch (const TimeoutException&) + { + _timeoutEncountered = true; + throw; + } + if (_initWantRead && i == 0) { return 0; @@ -204,12 +175,14 @@ IceSecurity::Ssl::OpenSSL::ServerConnection::init(int timeout) if (connectionLost()) { + ICE_DEV_DEBUG("ServerConnection::init(): Throwing ConnectionLostException... SslConnectionOpenSSLServer.cpp, 207"); ConnectionLostException ex(__FILE__, __LINE__); ex.error = getSocketErrno(); throw ex; } else { + ICE_DEV_DEBUG("ServerConnection::init(): Throwing SocketException... SslConnectionOpenSSLServer.cpp, 214"); SocketException ex(__FILE__, __LINE__); ex.error = getSocketErrno(); throw ex; @@ -294,7 +267,7 @@ IceSecurity::Ssl::OpenSSL::ServerConnection::write(Buffer& buf, int timeout) int totalBytesWritten = 0; int bytesWritten = 0; - int packetSize = buf.b.end() - buf.i;
+ int packetSize = buf.b.end() - buf.i; #ifdef WIN32 // @@ -310,124 +283,140 @@ IceSecurity::Ssl::OpenSSL::ServerConnection::write(Buffer& buf, int timeout) while (buf.i != buf.b.end()) { // Ensure we're initialized. - if (init(timeout)) + int initReturn = initialize(timeout); + + if (initReturn == -1) { - // Perform a select on the socket. - if (!writeSelect(timeout)) - { - // We're done here. - break; - } + // Handshake underway, we should just return with what we've got (even if that's nothing). + break; + } + + if (initReturn == 0) + { + // Retry the initialize call + continue; + } + + // initReturn must be > 0, so we're okay to try a write - bytesWritten = sslWrite((char *)buf.i, packetSize); + // Perform a select on the socket. + if (!writeSelect(timeout)) + { + // We're done here. + break; + } + + bytesWritten = sslWrite((char *)buf.i, packetSize); - switch (getLastError()) + switch (getLastError()) + { + case SSL_ERROR_NONE: { - case SSL_ERROR_NONE: - { - if (_traceLevels->network >= 3)
- {
- ostringstream s;
- s << "sent " << bytesWritten << " of " << packetSize;
- s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection));
- _logger->trace(_traceLevels->networkCat, s.str());
- }
-
- totalBytesWritten += bytesWritten; - - buf.i += bytesWritten; - - if (packetSize > buf.b.end() - buf.i) - { - packetSize = buf.b.end() - buf.i; - } - continue; + if (_traceLevels->network >= 3) + { + ostringstream s; + s << "sent " << bytesWritten << " of " << packetSize; + s << " bytes via ssl\n" << fdToString(SSL_get_fd(_sslConnection)); + _logger->trace(_traceLevels->networkCat, s.str()); } - case SSL_ERROR_WANT_WRITE: // Retry... - { - ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE: Repeating as per protocol."); + totalBytesWritten += bytesWritten; - continue; - } + buf.i += bytesWritten; - case SSL_ERROR_WANT_READ: // The demo server ignores this error. + if (packetSize > buf.b.end() - buf.i) { - ICE_PROTOCOL("Error SSL_ERROR_WANT_READ: Ignoring as per protocol."); - - continue; + packetSize = buf.b.end() - buf.i; } + continue; + } - case SSL_ERROR_WANT_X509_LOOKUP: // The demo server ignores this error. - { - ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol."); + case SSL_ERROR_WANT_WRITE: // Retry... + { + ICE_PROTOCOL("Error SSL_ERROR_WANT_WRITE: Repeating as per protocol."); - continue; - } + continue; + } - case SSL_ERROR_SYSCALL: - { - if (bytesWritten == -1) - { - // IO Error in underlying BIO - - if (interrupted()) - { - break; - } - - if (wouldBlock()) - { - break; - } - - if (connectionLost()) - { - ConnectionLostException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } - else - { - SocketException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } - } - else - { - ProtocolException protocolEx(__FILE__, __LINE__); + case SSL_ERROR_WANT_READ: // The demo server ignores this error. + { + ICE_PROTOCOL("Error SSL_ERROR_WANT_READ: Ignoring as per protocol."); - // Protocol Error: Unexpected EOF - protocolEx._message = "Encountered an EOF that violates the SSL Protocol."; + continue; + } + + case SSL_ERROR_WANT_X509_LOOKUP: // The demo server ignores this error. + { + ICE_PROTOCOL("Error SSL_ERROR_WANT_X509_LOOKUP: Repeating as per protocol."); - ICE_SSLERRORS(protocolEx._message); - ICE_EXCEPTION(protocolEx._message); + continue; + } - throw protocolEx; + case SSL_ERROR_SYSCALL: + { + if (bytesWritten == -1) + { + // IO Error in underlying BIO + + if (interrupted()) + { + break; + } + + if (wouldBlock()) + { + break; } - } - case SSL_ERROR_SSL: + if (connectionLost()) + { + ICE_DEV_DEBUG("ServerConnection::write(): Throwing ConnectionLostException... SslConnectionOpenSSLServer.cpp, 388"); + ConnectionLostException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; + } + else + { + ICE_DEV_DEBUG("ServerConnection::write(): Throwing SocketException... SslConnectionOpenSSLServer.cpp, 395"); + SocketException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; + } + } + else { ProtocolException protocolEx(__FILE__, __LINE__); - protocolEx._message = "Encountered a violation of the SSL Protocol."; + // Protocol Error: Unexpected EOF + protocolEx._message = "Encountered an EOF that violates the SSL Protocol."; ICE_SSLERRORS(protocolEx._message); ICE_EXCEPTION(protocolEx._message); throw protocolEx; } + } - case SSL_ERROR_ZERO_RETURN: - { - ICE_EXCEPTION("SSL_ERROR_ZERO_RETURN"); + case SSL_ERROR_SSL: + { + ProtocolException protocolEx(__FILE__, __LINE__); - ConnectionLostException ex(__FILE__, __LINE__); - ex.error = getSocketErrno(); - throw ex; - } + protocolEx._message = "Encountered a violation of the SSL Protocol."; + + ICE_SSLERRORS(protocolEx._message); + ICE_EXCEPTION(protocolEx._message); + + throw protocolEx; + } + + case SSL_ERROR_ZERO_RETURN: + { + ICE_EXCEPTION("SSL_ERROR_ZERO_RETURN"); + ICE_DEV_DEBUG("ServerConnection::write(): Throwing ConnectionLostException... SslConnectionOpenSSLServer.cpp, 430"); + + ConnectionLostException ex(__FILE__, __LINE__); + ex.error = getSocketErrno(); + throw ex; } } } diff --git a/cpp/src/Ice/SslSystemOpenSSL.cpp b/cpp/src/Ice/SslSystemOpenSSL.cpp index 5da6270aa8f..aada4c7bff9 100644 --- a/cpp/src/Ice/SslSystemOpenSSL.cpp +++ b/cpp/src/Ice/SslSystemOpenSSL.cpp @@ -1207,7 +1207,7 @@ IceSecurity::Ssl::OpenSSL::System::commonConnectionSetup(Connection* connection) }
else
{
- handshakeReadTimeout = 10000;
+ handshakeReadTimeout = 5000;
}
connection->setHandshakeReadTimeout(handshakeReadTimeout);
diff --git a/cpp/src/Ice/SslTransceiver.cpp b/cpp/src/Ice/SslTransceiver.cpp index 31d9e854612..cac54001969 100644 --- a/cpp/src/Ice/SslTransceiver.cpp +++ b/cpp/src/Ice/SslTransceiver.cpp @@ -42,8 +42,8 @@ IceInternal::SslTransceiver::close() } SOCKET fd = _fd; - cleanUpSSL(); _fd = INVALID_SOCKET; + _sslConnection->shutdown(); ::shutdown(fd, SHUT_RDWR); // helps to unblock threads in recv() closeSocket(fd); @@ -62,6 +62,7 @@ IceInternal::SslTransceiver::shutdown() _logger->trace(_traceLevels->networkCat, s.str()); } + _sslConnection->shutdown(); ::shutdown(_fd, SHUT_WR); // Shutdown socket for writing ICE_METHOD_RET("SslTransceiver::shutdown()"); @@ -82,9 +83,13 @@ IceInternal::SslTransceiver::read(Buffer& buf, int timeout) if (!_sslConnection->read(buf, timeout)) { - ConnectionLostException clEx(__FILE__, __LINE__); - clEx.error = 0; - throw clEx; + ICE_WARNING("Connection::read() returning no bytes read."); + + // TODO: Perhaps this should be a NoApplicationDataException ??? + // ICE_WARNING("Throwing ConnectionLostException."); + // ConnectionLostException clEx(__FILE__, __LINE__); + // clEx.error = 0; + // throw clEx; } ICE_METHOD_RET("SslTransceiver::read()"); @@ -113,15 +118,8 @@ IceInternal::SslTransceiver::~SslTransceiver() { assert(_fd == INVALID_SOCKET); - cleanUpSSL(); -} - -void -IceInternal::SslTransceiver::cleanUpSSL() -{ if (_sslConnection != 0) { - _sslConnection->shutdown(); delete _sslConnection; _sslConnection = 0; } diff --git a/cpp/src/Ice/SslTransceiver.h b/cpp/src/Ice/SslTransceiver.h index 502e46946d5..86c297c1b0b 100644 --- a/cpp/src/Ice/SslTransceiver.h +++ b/cpp/src/Ice/SslTransceiver.h @@ -41,7 +41,6 @@ private: SslTransceiver(const InstancePtr&, SOCKET, Connection*); virtual ~SslTransceiver(); - void cleanUpSSL(); friend class SslConnector; friend class SslAcceptor; |