diff --git a/src/lib/net/IDataSocket.h b/src/lib/net/IDataSocket.h index ebd31049..08970657 100644 --- a/src/lib/net/IDataSocket.h +++ b/src/lib/net/IDataSocket.h @@ -68,5 +68,6 @@ public: virtual void shutdownInput() = 0; virtual void shutdownOutput() = 0; virtual bool isReady() const = 0; + virtual bool isFatal() const = 0; virtual UInt32 getSize() const = 0; }; diff --git a/src/lib/net/TCPSocket.cpp b/src/lib/net/TCPSocket.cpp index ad4fbd40..620e26ec 100644 --- a/src/lib/net/TCPSocket.cpp +++ b/src/lib/net/TCPSocket.cpp @@ -253,6 +253,14 @@ TCPSocket::isReady() const return (m_inputBuffer.getSize() > 0); } +bool +TCPSocket::isFatal() const +{ + // TCP sockets aren't ever left in a fatal state. + LOG((CLOG_ERR "isFatal() not valid for non-secure connections")); + return false; +} + UInt32 TCPSocket::getSize() const { @@ -481,6 +489,9 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, if (n > 0) { s_retryOutputBufferSize = 0; } + else if (n < 0) { + return NULL; + } } else { return job; @@ -537,6 +548,9 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, if (isSecure()) { if (isSecureReady()) { n = secureRead(buffer, sizeof(buffer)); + if (n < 0) { + return NULL; + } } else { return job; @@ -555,6 +569,9 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, if (isSecure() && isSecureReady()) { n = secureRead(buffer, sizeof(buffer)); + if (n < 0) { + return NULL; + } } else { n = ARCH->readSocket(m_socket, buffer, sizeof(buffer)); diff --git a/src/lib/net/TCPSocket.h b/src/lib/net/TCPSocket.h index 60866b3e..831d4729 100644 --- a/src/lib/net/TCPSocket.h +++ b/src/lib/net/TCPSocket.h @@ -52,6 +52,7 @@ public: virtual void shutdownInput(); virtual void shutdownOutput(); virtual bool isReady() const; + virtual bool isFatal() const; virtual UInt32 getSize() const; // IDataSocket overrides diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 6a34fce3..d9e47d73 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -52,7 +52,8 @@ SecureSocket::SecureSocket( SocketMultiplexer* socketMultiplexer) : TCPSocket(events, socketMultiplexer), m_secureReady(false), - m_maxRetry(MAX_RETRY_COUNT) + m_maxRetry(MAX_RETRY_COUNT), + m_fatal(false) { } @@ -62,12 +63,14 @@ SecureSocket::SecureSocket( ArchSocket socket) : TCPSocket(events, socketMultiplexer, socket), m_secureReady(false), - m_maxRetry(MAX_RETRY_COUNT) + m_maxRetry(MAX_RETRY_COUNT), + m_fatal(false) { } SecureSocket::~SecureSocket() { + isFatal(true); if (m_ssl->m_ssl != NULL) { SSL_shutdown(m_ssl->m_ssl); @@ -78,13 +81,15 @@ SecureSocket::~SecureSocket() SSL_CTX_free(m_ssl->m_context); m_ssl->m_context = NULL; } - + ARCH->sleep(1); delete m_ssl; } void SecureSocket::close() { + isFatal(true); + SSL_shutdown(m_ssl->m_ssl); TCPSocket::close(); @@ -114,17 +119,23 @@ SecureSocket::secureRead(void* buffer, UInt32 n) LOG((CLOG_DEBUG2 "reading secure socket")); r = SSL_read(m_ssl->m_ssl, buffer, n); - bool fatal; static int retry; - checkResult(r, fatal, retry); + // Check result will cleanup the connection in the case of a fatal + checkResult(r, retry); if (retry) { - r = 0; + return 0; + } + + if (isFatal()) { + return -1; } } - - return r > 0 ? (UInt32)r : 0; + // According to SSL spec, r must not be negative and not have an error code + // from SSL_get_error(). If this happens, it is itself an error. Let the + // parent handle the case + return (UInt32)r; } UInt32 @@ -132,20 +143,27 @@ SecureSocket::secureWrite(const void* buffer, UInt32 n) { int r = 0; if (m_ssl->m_ssl != NULL) { - LOG((CLOG_DEBUG2 "writing secure socket")); + LOG((CLOG_DEBUG2 "writing secure socket:%p", this)); + r = SSL_write(m_ssl->m_ssl, buffer, n); - bool fatal; static int retry; - checkResult(r, fatal, retry); + // Check result will cleanup the connection in the case of a fatal + checkResult(r, retry); if (retry) { - r = 0; + return 0; + } + + if (isFatal()) { + return -1; } } - - return r > 0 ? (UInt32)r : 0; + // According to SSL spec, r must not be negative and not have an error code + // from SSL_get_error(). If this happens, it is itself an error. Let the + // parent handle the case + return (UInt32)r; } bool @@ -260,12 +278,11 @@ SecureSocket::secureAccept(int socket) LOG((CLOG_DEBUG2 "accepting secure socket")); int r = SSL_accept(m_ssl->m_ssl); - bool fatal; static int retry; - checkResult(r, fatal, retry); + checkResult(r, retry); - if (fatal) { + if (isFatal()) { // tell user and sleep so the socket isn't hammered. LOG((CLOG_ERR "failed to accept secure socket")); LOG((CLOG_INFO "client connection may not be secure")); @@ -304,12 +321,11 @@ SecureSocket::secureConnect(int socket) LOG((CLOG_DEBUG2 "connecting secure socket")); int r = SSL_connect(m_ssl->m_ssl); - bool fatal; static int retry; - checkResult(r, fatal, retry); + checkResult(r, retry); - if (fatal) { + if (isFatal()) { LOG((CLOG_ERR "failed to connect secure socket")); return -1; } @@ -362,13 +378,11 @@ SecureSocket::showCertificate() } void -SecureSocket::checkResult(int n, bool& fatal, int& retry) +SecureSocket::checkResult(int n, int& retry) { // ssl errors are a little quirky. the "want" errors are normal and // should result in a retry. - fatal = false; - int errorCode = SSL_get_error(m_ssl->m_ssl, n); switch (errorCode) { case SSL_ERROR_NONE: @@ -378,8 +392,8 @@ SecureSocket::checkResult(int n, bool& fatal, int& retry) case SSL_ERROR_ZERO_RETURN: // connection closed - retry = 0; - LOG((CLOG_DEBUG2 "SSL connection has been closed")); + isFatal(true); + LOG((CLOG_DEBUG "SSL connection has been closed")); break; case SSL_ERROR_WANT_READ: @@ -389,7 +403,7 @@ SecureSocket::checkResult(int n, bool& fatal, int& retry) retry += 1; // If there are a lot of retrys, it's worth warning about if ( retry % 5 == 0 ) { - LOG((CLOG_INFO "need to retry the same SSL function(%d) retry:%d", errorCode, retry)); + LOG((CLOG_DEBUG "need to retry the same SSL function(%d) retry:%d", errorCode, retry)); } else if ( retry == (maxRetry() / 2) ) { LOG((CLOG_WARN "need to retry the same SSL function(%d) retry:%d", errorCode, retry)); @@ -416,27 +430,27 @@ SecureSocket::checkResult(int n, bool& fatal, int& retry) } } - fatal = true; + isFatal(true); break; case SSL_ERROR_SSL: LOG((CLOG_ERR "a failure in the SSL library occurred")); - fatal = true; + isFatal(true); break; default: LOG((CLOG_ERR "unknown secure socket error")); - fatal = true; + isFatal(true); break; } // If the retry max would exceed the allowed, treat it as a fatal error if (retry > maxRetry()) { LOG((CLOG_ERR "Maximum retry count exceeded:%d",retry)); - fatal = true; + isFatal(true); } - if (fatal) { + if (isFatal()) { retry = 0; showError(); disconnect(); diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 3ea13c59..81a6c7f3 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -44,6 +44,8 @@ public: void secureConnect(); void secureAccept(); bool isReady() const { return m_secureReady; } + bool isFatal() const { return m_fatal; } + void isFatal(bool b) { m_fatal = b; } bool isSecureReady(); bool isSecure() { return true; } UInt32 secureRead(void* buffer, UInt32 n); @@ -60,7 +62,7 @@ private: int secureAccept(int s); int secureConnect(int s); bool showCertificate(); - void checkResult(int n, bool& fatal, int& retry); + void checkResult(int n, int& retry); void showError(const char* reason = NULL); String getError(); void disconnect(); @@ -80,5 +82,6 @@ private: private: Ssl* m_ssl; bool m_secureReady; + bool m_fatal; int m_maxRetry; }; diff --git a/src/lib/server/ClientListener.cpp b/src/lib/server/ClientListener.cpp index 4383ac04..efcb736c 100644 --- a/src/lib/server/ClientListener.cpp +++ b/src/lib/server/ClientListener.cpp @@ -142,26 +142,34 @@ ClientListener::handleClientConnecting(const Event&, void*) { // accept client connection IDataSocket* socket = m_listen->accept(); - synergy::IStream* stream = socket; - if (stream == NULL) { + if (socket == NULL) { return; } LOG((CLOG_NOTE "accepted client connection")); + if (m_useSecureNetwork) { + LOG((CLOG_DEBUG2 "attempting sercure Connection")); + while(!socket->isReady()) { + if(socket->isFatal()) { + m_listen->deleteSocket(socket); + return; + } + LOG((CLOG_DEBUG2 "retrying sercure Connection")); + ARCH->sleep(.5f); + } + } + + LOG((CLOG_DEBUG2 "sercure Connection established:%d")); + + synergy::IStream* stream = socket; // filter socket messages, including a packetizing filter bool adopt = !m_useSecureNetwork; stream = new PacketStreamFilter(m_events, stream, adopt); assert(m_server != NULL); - if (m_useSecureNetwork) { - while(!socket->isReady()) { - ARCH->sleep(.5f); - } - } - // create proxy for unknown client ClientProxyUnknown* client = new ClientProxyUnknown(stream, 30.0, m_server, m_events); m_newClients.insert(client);