From 916a4c75af19f9d6031c7236d63b308f943250fc Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Fri, 10 Apr 2015 14:07:53 +0100 Subject: [PATCH] Refactored no or wrong ssl certificate error handling #4410 Conflicts: src/lib/net/TCPListenSocket.cpp src/lib/plugin/ns/SecureListenSocket.cpp src/lib/plugin/ns/SecureSocket.cpp src/lib/plugin/ns/SecureSocket.h --- src/lib/mt/Thread.cpp | 5 -- src/lib/net/TCPListenSocket.cpp | 16 +++-- src/lib/net/TCPListenSocket.h | 3 + src/lib/plugin/ns/SecureListenSocket.cpp | 19 +++++- src/lib/plugin/ns/SecureSocket.cpp | 75 +++++++++++++++--------- src/lib/plugin/ns/SecureSocket.h | 7 +-- 6 files changed, 83 insertions(+), 42 deletions(-) diff --git a/src/lib/mt/Thread.cpp b/src/lib/mt/Thread.cpp index d77d0705..f28bb6b9 100644 --- a/src/lib/mt/Thread.cpp +++ b/src/lib/mt/Thread.cpp @@ -18,7 +18,6 @@ #include "mt/Thread.h" -#include "net/XSocket.h" #include "mt/XMT.h" #include "mt/XThread.h" #include "arch/Arch.h" @@ -158,10 +157,6 @@ Thread::threadFunc(void* vjob) job->run(); LOG((CLOG_DEBUG1 "thread 0x%08x exit", id)); } - - catch (XSocket& e) { - LOG((CLOG_DEBUG "%s", e.what())); - } catch (XThreadCancel&) { // client called cancel() LOG((CLOG_DEBUG1 "caught cancel on thread 0x%08x", id)); diff --git a/src/lib/net/TCPListenSocket.cpp b/src/lib/net/TCPListenSocket.cpp index cdbd73c4..38d4dd8c 100644 --- a/src/lib/net/TCPListenSocket.cpp +++ b/src/lib/net/TCPListenSocket.cpp @@ -112,27 +112,35 @@ TCPListenSocket::accept() try { socket = new TCPSocket(m_events, m_socketMultiplexer, ARCH->acceptSocket(m_socket, NULL)); if (socket != NULL) { - m_socketMultiplexer->addSocket(this, - new TSocketMultiplexerMethodJob( - this, &TCPListenSocket::serviceListening, - m_socket, true, false)); + setListeningJob(); } return socket; } catch (XArchNetwork&) { if (socket != NULL) { delete socket; + setListeningJob(); } return NULL; } catch (std::exception &ex) { if (socket != NULL) { delete socket; + setListeningJob(); } throw ex; } } +void +CTCPListenSocket::setListeningJob() +{ + m_socketMultiplexer->addSocket(this, + new TSocketMultiplexerMethodJob( + this, &CTCPListenSocket::serviceListening, + m_socket, true, false)); +} + ISocketMultiplexerJob* TCPListenSocket::serviceListening(ISocketMultiplexerJob* job, bool read, bool, bool error) diff --git a/src/lib/net/TCPListenSocket.h b/src/lib/net/TCPListenSocket.h index ef2687db..c769e170 100644 --- a/src/lib/net/TCPListenSocket.h +++ b/src/lib/net/TCPListenSocket.h @@ -45,6 +45,9 @@ public: accept(); virtual void deleteSocket(void*) { } +protected: + void setListeningJob(); + public: ISocketMultiplexerJob* serviceListening(ISocketMultiplexerJob*, diff --git a/src/lib/plugin/ns/SecureListenSocket.cpp b/src/lib/plugin/ns/SecureListenSocket.cpp index a0df2519..bee4b29d 100644 --- a/src/lib/plugin/ns/SecureListenSocket.cpp +++ b/src/lib/plugin/ns/SecureListenSocket.cpp @@ -55,9 +55,16 @@ SecureListenSocket::accept() m_socketMultiplexer, ARCH->acceptSocket(m_socket, NULL)); +<<<<<<< HEAD:src/lib/plugin/ns/SecureListenSocket.cpp m_secureSocketSet.insert(socket); socket->initSsl(true); +======= + if (socket != NULL) { + setListeningJob(); + } + +>>>>>>> 79b9c52... Refactored no or wrong ssl certificate error handling #30:src/lib/net/SecureListenSocket.cpp // TODO: customized certificate path String certificateFilename = ARCH->getProfileDirectory(); #if SYSAPI_WIN32 @@ -67,26 +74,36 @@ SecureListenSocket::accept() #endif certificateFilename.append(s_certificateFilename); - socket->loadCertificates(certificateFilename.c_str()); + bool loaded = socket->loadCertificates(certificateFilename); + if (!loaded) { + delete socket; + return NULL; + } + socket->secureAccept(); +<<<<<<< HEAD:src/lib/plugin/ns/SecureListenSocket.cpp if (socket != NULL) { m_socketMultiplexer->addSocket(this, new TSocketMultiplexerMethodJob( this, &TCPListenSocket::serviceListening, m_socket, true, false)); } +======= +>>>>>>> 79b9c52... Refactored no or wrong ssl certificate error handling #30:src/lib/net/SecureListenSocket.cpp return dynamic_cast(socket); } catch (XArchNetwork&) { if (socket != NULL) { delete socket; + setListeningJob(); } return NULL; } catch (std::exception &ex) { if (socket != NULL) { delete socket; + setListeningJob(); } throw ex; } diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index d1af5b85..1be5f184 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -151,24 +151,46 @@ SecureSocket::initSsl(bool server) initContext(server); } -void -SecureSocket::loadCertificates(const char* filename) +bool +CSecureSocket::loadCertificates(CString& filename) { - int r = 0; - r = SSL_CTX_use_certificate_file(m_ssl->m_context, filename, SSL_FILETYPE_PEM); - if (r <= 0) { - throwError("could not use ssl certificate"); + if (filename.empty()) { + showError("ssl certificate is not specified"); + return false; + } + else { + std::ifstream file(filename.c_str()); + bool exist = file.good(); + file.close(); + + if (!exist) { + CString errorMsg("ssl certificate doesn't exist: "); + errorMsg.append(filename); + showError(errorMsg.c_str()); + return false; + } } - r = SSL_CTX_use_PrivateKey_file(m_ssl->m_context, filename, SSL_FILETYPE_PEM); + int r = 0; + r = SSL_CTX_use_certificate_file(m_ssl->m_context, filename.c_str(), SSL_FILETYPE_PEM); if (r <= 0) { - throwError("could not use ssl private key"); + showError("could not use ssl certificate"); + return false; + } + + r = SSL_CTX_use_PrivateKey_file(m_ssl->m_context, filename.c_str(), SSL_FILETYPE_PEM); + if (r <= 0) { + showError("could not use ssl private key"); + return false; } r = SSL_CTX_check_private_key(m_ssl->m_context); if (!r) { - throwError("could not verify ssl private key"); + showError("could not verify ssl private key"); + return false; } + + return true; } void @@ -258,7 +280,8 @@ SecureSocket::secureConnect(int socket) // tell user and sleep so the socket isn't hammered. LOG((CLOG_ERR "failed to connect secure socket")); LOG((CLOG_INFO "server connection may not be secure")); - ARCH->sleep(1); + disconnect(); + return false; } m_secureReady = !retry; @@ -266,7 +289,9 @@ SecureSocket::secureConnect(int socket) if (m_secureReady) { if (verifyCertFingerprint()) { LOG((CLOG_INFO "connected to secure socket")); - showCertificate(); + if (!showCertificate()) { + disconnect(); + } } else { LOG((CLOG_ERR "failed to verity server certificate fingerprint")); @@ -277,8 +302,8 @@ SecureSocket::secureConnect(int socket) return retry; } -void -SecureSocket::showCertificate() +bool +CSecureSocket::showCertificate() { X509* cert; char* line; @@ -292,8 +317,11 @@ SecureSocket::showCertificate() X509_free(cert); } else { - throwError("server has no ssl certificate"); + showError("server has no ssl certificate"); + return false; } + + return true; } void @@ -359,24 +387,15 @@ SecureSocket::checkResult(int n, bool& fatal, bool& retry) } void -SecureSocket::showError() +CSecureSocket::showError(const char* reason) { - String error = getError(); - if (!error.empty()) { - LOG((CLOG_ERR "secure socket error: %s", error.c_str())); + if (reason != NULL) { + LOG((CLOG_ERR "%s", reason)); } -} -void -SecureSocket::throwError(const char* reason) -{ - String error = getError(); + CString error = getError(); if (!error.empty()) { - throw XSocket(synergy::string::sprintf( - "%s: %s", reason, error.c_str())); - } - else { - throw XSocket(reason); + LOG((CLOG_ERR "%s", error.c_str())); } } diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 50ca17b6..4f5e5d70 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -50,7 +50,7 @@ public: UInt32 secureRead(void* buffer, UInt32 n); UInt32 secureWrite(const void* buffer, UInt32 n); void initSsl(bool server); - void loadCertificates(const char* CertFile); + bool loadCertificates(CString& CertFile); private: // SSL @@ -58,10 +58,9 @@ private: void createSSL(); bool secureAccept(int s); bool secureConnect(int s); - void showCertificate(); + bool showCertificate(); void checkResult(int n, bool& fatal, bool& retry); - void showError(); - void throwError(const char* reason); + void showError(const char* reason = NULL); String getError(); void disconnect(); bool verifyCertFingerprint();