From 5dfe4aa0b515ab453e430442ac8e7c88586c83d9 Mon Sep 17 00:00:00 2001 From: Xinyu Hou Date: Wed, 1 Apr 2015 16:04:10 +0100 Subject: [PATCH 01/15] Added verification of server certificate fingerprint #4522 Conflicts: src/lib/mt/Thread.cpp src/lib/plugin/ns/SecureSocket.cpp src/lib/synergy/ClientArgs.cpp --- src/lib/mt/Thread.cpp | 1 - src/lib/plugin/ns/SecureSocket.cpp | 79 +++++++++++++++++++++++++++--- src/lib/plugin/ns/SecureSocket.h | 10 ++++ src/lib/synergy/ArgParser.cpp | 4 ++ src/lib/synergy/ClientArgs.cpp | 5 +- src/lib/synergy/ClientArgs.h | 3 +- 6 files changed, 92 insertions(+), 10 deletions(-) diff --git a/src/lib/mt/Thread.cpp b/src/lib/mt/Thread.cpp index dd45d872..d77d0705 100644 --- a/src/lib/mt/Thread.cpp +++ b/src/lib/mt/Thread.cpp @@ -160,7 +160,6 @@ Thread::threadFunc(void* vjob) } catch (XSocket& e) { - // client called cancel() LOG((CLOG_DEBUG "%s", e.what())); } catch (XThreadCancel&) { diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index cb6db76f..facf8337 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -23,11 +23,13 @@ #include "arch/XArch.h" #include "base/Log.h" -#include #include #include #include #include +#include +#include +#include // // SecureSocket @@ -40,11 +42,14 @@ struct Ssl { SSL* m_ssl; }; -SecureSocket::SecureSocket( +bool CSecureSocket::s_verifyFingerprintFailed = false; + +CSecureSocket::CSecureSocket( IEventQueue* events, - SocketMultiplexer* socketMultiplexer) : - TCPSocket(events, socketMultiplexer), - m_secureReady(false) + CSocketMultiplexer* socketMultiplexer) : + CTCPSocket(events, socketMultiplexer), + m_secureReady(false), + m_certFingerprint() { } @@ -169,6 +174,8 @@ SecureSocket::loadCertificates(const char* filename) } } +const char test[] = "/Users/xinyu/serverCertificateFingerprint.txt"; + void SecureSocket::initContext(bool server) { @@ -197,6 +204,11 @@ SecureSocket::initContext(bool server) if (m_ssl->m_context == NULL) { showError(); } + + if (!server) { + //void* p = reinterpret_cast(const_cast(m_certFingerprint.c_str())); + SSL_CTX_set_cert_verify_callback(m_ssl->m_context, CSecureSocket::verifyCertFingerprint, (void*)test); + } } void @@ -261,6 +273,10 @@ SecureSocket::secureConnect(int socket) m_secureReady = !retry; + if (s_verifyFingerprintFailed) { + throwError("failed to verify server certificate fingerprint"); + } + if (m_secureReady) { LOG((CLOG_INFO "connected to secure socket")); showCertificate(); @@ -366,7 +382,7 @@ SecureSocket::throwError(const char* reason) String error = getError(); if (!error.empty()) { throw XSocket(synergy::string::sprintf( - "%s: %s", reason, error.c_str())); + "%s: %s", reason, error.c_str())); } else { throw XSocket(reason); @@ -419,3 +435,54 @@ SecureSocket::serviceAccept(ISocketMultiplexerJob* job, return retry ? job : newJob(); } + +int +CSecureSocket::verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg) +{ + X509 *cert = ctx->cert; + + EVP_MD* tempDigest; + unsigned char tempFingerprint[EVP_MAX_MD_SIZE]; + unsigned int tempFingerprintLen; + tempDigest = (EVP_MD*)EVP_sha1(); + if (X509_digest(cert, tempDigest, tempFingerprint, &tempFingerprintLen) <= 0) { + s_verifyFingerprintFailed = true; + return 0; + } + + std::stringstream ss; + ss << std::hex; + for (int i = 0; i < tempFingerprintLen; i++) { + ss << std::setw(2) << std::setfill('0') << (int)tempFingerprint[i]; + } + CString fingerprint = ss.str(); + std::transform(fingerprint.begin(), fingerprint.end(), fingerprint.begin(), ::toupper); + + CString fileLine; + CString certificateFingerprint; + char* certFingerprintFilename = reinterpret_cast(arg); + std::ifstream file; + file.open(certFingerprintFilename); + + while (!file.eof()) { + getline(file,fileLine); + size_t found = fileLine.find('='); + if (found != CString::npos) { + certificateFingerprint = fileLine.substr(found + 1); + + if (!certificateFingerprint.empty()) { + certificateFingerprint.erase(std::remove(certificateFingerprint.begin(), certificateFingerprint.end(), ':'), certificateFingerprint.end()); + + if(certificateFingerprint.compare(fingerprint) == 0) { + file.close(); + return 1; + } + } + } + } + + file.close(); + + s_verifyFingerprintFailed = true; + return 0; +} diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index d703d2ca..d184814c 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -20,6 +20,8 @@ #include "net/TCPSocket.h" #include "net/XSocket.h" +#include + class IEventQueue; class SocketMultiplexer; class ISocketMultiplexerJob; @@ -71,7 +73,15 @@ private: serviceAccept(ISocketMultiplexerJob*, bool, bool, bool); +private: +static int verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg); + + private: Ssl* m_ssl; bool m_secureReady; + CString m_certFingerprint; + +private: + static bool s_verifyFingerprintFailed; }; diff --git a/src/lib/synergy/ArgParser.cpp b/src/lib/synergy/ArgParser.cpp index bec60632..7ba85752 100644 --- a/src/lib/synergy/ArgParser.cpp +++ b/src/lib/synergy/ArgParser.cpp @@ -89,6 +89,10 @@ ArgParser::parseClientArgs(ClientArgs& args, int argc, const char* const* argv) // define scroll args.m_yscroll = atoi(argv[++i]); } + else if (isArg(i, argc, argv, NULL, "--certificate-fingerprint", 1)) { + // define scroll + args.m_certFingerprint = argv[++i]; + } else { if (i + 1 == argc) { args.m_synergyAddress = argv[i]; diff --git a/src/lib/synergy/ClientArgs.cpp b/src/lib/synergy/ClientArgs.cpp index c997f3cc..efbbd90b 100644 --- a/src/lib/synergy/ClientArgs.cpp +++ b/src/lib/synergy/ClientArgs.cpp @@ -17,7 +17,8 @@ #include "synergy/ClientArgs.h" -ClientArgs::ClientArgs() : - m_yscroll(0) +CClientArgs::CClientArgs() : + m_yscroll(0), + m_certFingerprint() { } diff --git a/src/lib/synergy/ClientArgs.h b/src/lib/synergy/ClientArgs.h index db749b3e..f4024618 100644 --- a/src/lib/synergy/ClientArgs.h +++ b/src/lib/synergy/ClientArgs.h @@ -26,5 +26,6 @@ public: ClientArgs(); public: - int m_yscroll; + int m_yscroll; + CString m_certFingerprint; }; From b8ba37b4f4687a4bdd6160095ee10abecca7e841 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Wed, 1 Apr 2015 16:32:04 +0100 Subject: [PATCH 02/15] Made callback function global #4522 --- src/lib/plugin/ns/SecureSocket.cpp | 14 +++++++++----- src/lib/plugin/ns/SecureSocket.h | 8 +------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index facf8337..bf4d4c3b 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -23,6 +23,7 @@ #include "arch/XArch.h" #include "base/Log.h" +#include #include #include #include @@ -30,6 +31,9 @@ #include #include #include +#include + +int verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg); // // SecureSocket @@ -207,7 +211,7 @@ SecureSocket::initContext(bool server) if (!server) { //void* p = reinterpret_cast(const_cast(m_certFingerprint.c_str())); - SSL_CTX_set_cert_verify_callback(m_ssl->m_context, CSecureSocket::verifyCertFingerprint, (void*)test); + SSL_CTX_set_cert_verify_callback(m_ssl->m_context, verifyCertFingerprint, (void*)test); } } @@ -437,7 +441,7 @@ SecureSocket::serviceAccept(ISocketMultiplexerJob* job, } int -CSecureSocket::verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg) +verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg) { X509 *cert = ctx->cert; @@ -446,13 +450,13 @@ CSecureSocket::verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg) unsigned int tempFingerprintLen; tempDigest = (EVP_MD*)EVP_sha1(); if (X509_digest(cert, tempDigest, tempFingerprint, &tempFingerprintLen) <= 0) { - s_verifyFingerprintFailed = true; + CSecureSocket::s_verifyFingerprintFailed = true; return 0; } std::stringstream ss; ss << std::hex; - for (int i = 0; i < tempFingerprintLen; i++) { + for (unsigned int i = 0; i < tempFingerprintLen; i++) { ss << std::setw(2) << std::setfill('0') << (int)tempFingerprint[i]; } CString fingerprint = ss.str(); @@ -483,6 +487,6 @@ CSecureSocket::verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg) file.close(); - s_verifyFingerprintFailed = true; + CSecureSocket::s_verifyFingerprintFailed = true; return 0; } diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index d184814c..507566e8 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -20,8 +20,6 @@ #include "net/TCPSocket.h" #include "net/XSocket.h" -#include - class IEventQueue; class SocketMultiplexer; class ISocketMultiplexerJob; @@ -73,15 +71,11 @@ private: serviceAccept(ISocketMultiplexerJob*, bool, bool, bool); -private: -static int verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg); - - private: Ssl* m_ssl; bool m_secureReady; CString m_certFingerprint; -private: +public: static bool s_verifyFingerprintFailed; }; From 71dc472a64a81390294b53a4c803eac50adb18d3 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Wed, 1 Apr 2015 16:35:03 +0100 Subject: [PATCH 03/15] Fixed code style --- src/lib/plugin/ns/SecureSocket.cpp | 4 ++-- src/lib/synergy/ClientArgs.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index bf4d4c3b..6b38ec3c 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -33,8 +33,6 @@ #include #include -int verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg); - // // SecureSocket // @@ -46,6 +44,8 @@ struct Ssl { SSL* m_ssl; }; +int verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg); + bool CSecureSocket::s_verifyFingerprintFailed = false; CSecureSocket::CSecureSocket( diff --git a/src/lib/synergy/ClientArgs.h b/src/lib/synergy/ClientArgs.h index f4024618..c9ddb9e9 100644 --- a/src/lib/synergy/ClientArgs.h +++ b/src/lib/synergy/ClientArgs.h @@ -26,6 +26,6 @@ public: ClientArgs(); public: - int m_yscroll; - CString m_certFingerprint; + int m_yscroll; + CString m_certFingerprint; }; From b24eb2b724fde5f406a3d4ec5ab7aa235ea31518 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Wed, 1 Apr 2015 16:39:40 +0100 Subject: [PATCH 04/15] Removed test string #4522 --- src/lib/plugin/ns/SecureSocket.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 6b38ec3c..ecda101d 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -178,8 +178,6 @@ SecureSocket::loadCertificates(const char* filename) } } -const char test[] = "/Users/xinyu/serverCertificateFingerprint.txt"; - void SecureSocket::initContext(bool server) { @@ -210,8 +208,11 @@ SecureSocket::initContext(bool server) } if (!server) { - //void* p = reinterpret_cast(const_cast(m_certFingerprint.c_str())); - SSL_CTX_set_cert_verify_callback(m_ssl->m_context, verifyCertFingerprint, (void*)test); + void* p = reinterpret_cast(const_cast(m_certFingerprint.c_str())); + SSL_CTX_set_cert_verify_callback( + m_ssl->m_context, + verifyCertFingerprint, + p); } } From cb0f0dd06dba84617585fe18e01bcdaffedce4f5 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Thu, 2 Apr 2015 11:56:51 +0100 Subject: [PATCH 05/15] Improved fingerprint verification #4522 Conflicts: src/lib/plugin/ns/SecureSocket.cpp src/lib/plugin/ns/SecureSocket.h src/lib/synergy/ClientApp.cpp --- src/lib/plugin/ns/SecureSocket.cpp | 150 ++++++++++++++--------------- src/lib/plugin/ns/SecureSocket.h | 7 +- 2 files changed, 78 insertions(+), 79 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index ecda101d..56900461 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -44,10 +44,6 @@ struct Ssl { SSL* m_ssl; }; -int verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg); - -bool CSecureSocket::s_verifyFingerprintFailed = false; - CSecureSocket::CSecureSocket( IEventQueue* events, CSocketMultiplexer* socketMultiplexer) : @@ -206,14 +202,6 @@ SecureSocket::initContext(bool server) if (m_ssl->m_context == NULL) { showError(); } - - if (!server) { - void* p = reinterpret_cast(const_cast(m_certFingerprint.c_str())); - SSL_CTX_set_cert_verify_callback( - m_ssl->m_context, - verifyCertFingerprint, - p); - } } void @@ -278,13 +266,15 @@ SecureSocket::secureConnect(int socket) m_secureReady = !retry; - if (s_verifyFingerprintFailed) { - throwError("failed to verify server certificate fingerprint"); - } - if (m_secureReady) { - LOG((CLOG_INFO "connected to secure socket")); - showCertificate(); + if (verifyCertFingerprint()) { + LOG((CLOG_INFO "connected to secure socket")); + showCertificate(); + } + else { + LOG((CLOG_ERR "failed to verity server certificate fingerprint")); + disconnect(); + } } return retry; @@ -367,8 +357,7 @@ SecureSocket::checkResult(int n, bool& fatal, bool& retry) if (fatal) { showError(); - sendEvent(getEvents()->forISocket().disconnected()); - sendEvent(getEvents()->forIStream().inputShutdown()); + disconnect(); } } @@ -409,11 +398,73 @@ SecureSocket::getError() } } +void +CSecureSocket::disconnect() +{ + sendEvent(getEvents()->forISocket().disconnected()); + sendEvent(getEvents()->forIStream().inputShutdown()); +} + +bool +CSecureSocket::verifyCertFingerprint() +{ + // calculate received certificate fingerprint + X509 *cert = cert = SSL_get_peer_certificate(m_ssl->m_ssl); + EVP_MD* tempDigest; + unsigned char tempFingerprint[EVP_MAX_MD_SIZE]; + unsigned int tempFingerprintLen; + tempDigest = (EVP_MD*)EVP_sha1(); + if (X509_digest(cert, tempDigest, tempFingerprint, &tempFingerprintLen) <= 0) { + return false; + } + + // convert fingerprint into hexdecimal format + std::stringstream ss; + ss << std::hex; + for (unsigned int i = 0; i < tempFingerprintLen; i++) { + ss << std::setw(2) << std::setfill('0') << (int)tempFingerprint[i]; + } + + // all uppercase + CString fingerprint = ss.str(); + std::transform(fingerprint.begin(), fingerprint.end(), fingerprint.begin(), ::toupper); + + // check if this fingerprint exist + CString fileLine; + CString certificateFingerprint; + std::ifstream file; + file.open(m_certFingerprint.c_str()); + + while (!file.eof()) { + getline(file,fileLine); + // example of a fingerprint: + // SHA1 Fingerprint=6E:41:1A:21:53:2E:A3:EF:4D:A6:F2:A6:BA:0E:27:09:8A:F3:A1:10 + size_t found = fileLine.find('='); + if (found != CString::npos) { + certificateFingerprint = fileLine.substr(found + 1); + + if (!certificateFingerprint.empty()) { + // remove colons + certificateFingerprint.erase(std::remove(certificateFingerprint.begin(), certificateFingerprint.end(), ':'), certificateFingerprint.end()); + + if(certificateFingerprint.compare(fingerprint) == 0) { + file.close(); + return true; + } + } + } + } + + file.close(); + + return false; +} + ISocketMultiplexerJob* -SecureSocket::serviceConnect(ISocketMultiplexerJob* job, +CSecureSocket::serviceConnect(ISocketMultiplexerJob* job, bool, bool write, bool error) { - Lock lock(&getMutex()); + CLock lock(&getMutex()); bool retry = true; #ifdef SYSAPI_WIN32 @@ -426,10 +477,10 @@ SecureSocket::serviceConnect(ISocketMultiplexerJob* job, } ISocketMultiplexerJob* -SecureSocket::serviceAccept(ISocketMultiplexerJob* job, +CSecureSocket::serviceAccept(ISocketMultiplexerJob* job, bool, bool write, bool error) { - Lock lock(&getMutex()); + CLock lock(&getMutex()); bool retry = true; #ifdef SYSAPI_WIN32 @@ -440,54 +491,3 @@ SecureSocket::serviceAccept(ISocketMultiplexerJob* job, return retry ? job : newJob(); } - -int -verifyCertFingerprint(X509_STORE_CTX* ctx, void* arg) -{ - X509 *cert = ctx->cert; - - EVP_MD* tempDigest; - unsigned char tempFingerprint[EVP_MAX_MD_SIZE]; - unsigned int tempFingerprintLen; - tempDigest = (EVP_MD*)EVP_sha1(); - if (X509_digest(cert, tempDigest, tempFingerprint, &tempFingerprintLen) <= 0) { - CSecureSocket::s_verifyFingerprintFailed = true; - return 0; - } - - std::stringstream ss; - ss << std::hex; - for (unsigned int i = 0; i < tempFingerprintLen; i++) { - ss << std::setw(2) << std::setfill('0') << (int)tempFingerprint[i]; - } - CString fingerprint = ss.str(); - std::transform(fingerprint.begin(), fingerprint.end(), fingerprint.begin(), ::toupper); - - CString fileLine; - CString certificateFingerprint; - char* certFingerprintFilename = reinterpret_cast(arg); - std::ifstream file; - file.open(certFingerprintFilename); - - while (!file.eof()) { - getline(file,fileLine); - size_t found = fileLine.find('='); - if (found != CString::npos) { - certificateFingerprint = fileLine.substr(found + 1); - - if (!certificateFingerprint.empty()) { - certificateFingerprint.erase(std::remove(certificateFingerprint.begin(), certificateFingerprint.end(), ':'), certificateFingerprint.end()); - - if(certificateFingerprint.compare(fingerprint) == 0) { - file.close(); - return 1; - } - } - } - } - - file.close(); - - CSecureSocket::s_verifyFingerprintFailed = true; - return 0; -} diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 507566e8..725d5da9 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -61,7 +61,9 @@ private: void checkResult(int n, bool& fatal, bool& retry); void showError(); void throwError(const char* reason); - String getError(); + CString getError(); + void disconnect(); + bool verifyCertFingerprint(); ISocketMultiplexerJob* serviceConnect(ISocketMultiplexerJob*, @@ -75,7 +77,4 @@ private: Ssl* m_ssl; bool m_secureReady; CString m_certFingerprint; - -public: - static bool s_verifyFingerprintFailed; }; From 39e183da3efd87950a980f8770133d3b631298b4 Mon Sep 17 00:00:00 2001 From: Xinyu Hou Date: Thu, 2 Apr 2015 14:01:50 +0100 Subject: [PATCH 06/15] Refactored string operations Conflicts: src/lib/base/String.cpp --- src/lib/base/String.cpp | 27 +++++++++++++++++++++++++++ src/lib/base/String.h | 19 +++++++++++++++++++ src/lib/plugin/ns/SecureSocket.cpp | 15 ++++----------- 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/lib/base/String.cpp b/src/lib/base/String.cpp index 4ce38899..32531df8 100644 --- a/src/lib/base/String.cpp +++ b/src/lib/base/String.cpp @@ -27,6 +27,9 @@ #include #include #include +#include +#include +#include namespace synergy { namespace string { @@ -180,6 +183,30 @@ removeFileExt(String filename) return filename.substr(0, dot); } +void +toHex(CString& subject, int width, const char fill) +{ + std::stringstream ss; + ss << std::hex; + for (unsigned int i = 0; i < subject.length(); i++) { + ss << std::setw(width) << std::setfill(fill) << (int)(unsigned char)subject[i]; + } + + subject = ss.str(); +} + +void +uppercase(CString& subject) +{ + std::transform(subject.begin(), subject.end(), subject.begin(), ::toupper); +} + +void +removeChar(CString& subject, const char c) +{ + subject.erase(std::remove(subject.begin(), subject.end(), c), subject.end()); +} + // // CaselessCmp // diff --git a/src/lib/base/String.h b/src/lib/base/String.h index 9d137143..57f3c69c 100644 --- a/src/lib/base/String.h +++ b/src/lib/base/String.h @@ -70,6 +70,25 @@ Finds the last dot and remove all characters from the dot to the end */ String removeFileExt(String filename); +//! Convert into hexdecimal +/*! +Convert each character in \c subject into hexdecimal form with \c width +*/ +void toHex(CString& subject, int width, const char fill = '0'); + +//! Convert to all uppercase +/*! +Convert each character in \c subject to uppercase +*/ +void uppercase(CString& subject); + +//! Remove all specific char in suject +/*! +Remove all specific \c char in \c suject +*/ +void removeChar(CString& subject, const char c); + + //! Case-insensitive comparisons /*! This class provides case-insensitve comparison functions. diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 56900461..8821a3e6 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -28,10 +28,7 @@ #include #include #include -#include -#include #include -#include // // SecureSocket @@ -419,15 +416,11 @@ CSecureSocket::verifyCertFingerprint() } // convert fingerprint into hexdecimal format - std::stringstream ss; - ss << std::hex; - for (unsigned int i = 0; i < tempFingerprintLen; i++) { - ss << std::setw(2) << std::setfill('0') << (int)tempFingerprint[i]; - } + CString fingerprint(reinterpret_cast(tempFingerprint), tempFingerprintLen); + synergy::string::toHex(fingerprint, 2); // all uppercase - CString fingerprint = ss.str(); - std::transform(fingerprint.begin(), fingerprint.end(), fingerprint.begin(), ::toupper); + synergy::string::uppercase(fingerprint); // check if this fingerprint exist CString fileLine; @@ -445,7 +438,7 @@ CSecureSocket::verifyCertFingerprint() if (!certificateFingerprint.empty()) { // remove colons - certificateFingerprint.erase(std::remove(certificateFingerprint.begin(), certificateFingerprint.end(), ':'), certificateFingerprint.end()); + synergy::string::removeChar(certificateFingerprint, ':'); if(certificateFingerprint.compare(fingerprint) == 0) { file.close(); From b4665b9cd514a9c1ebbf78ac691fbeabefec3b23 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Thu, 2 Apr 2015 14:43:23 +0100 Subject: [PATCH 07/15] Passed args into client and socket Conflicts: src/lib/client/Client.cpp src/lib/client/Client.h src/lib/synergy/ClientApp.cpp src/test/integtests/net/NetworkTests.cpp --- src/lib/client/Client.cpp | 11 ++++++----- src/lib/client/Client.h | 5 +++-- src/lib/net/TCPSocket.h | 1 + src/lib/plugin/ns/SecureSocket.cpp | 8 ++++++-- src/lib/plugin/ns/SecureSocket.h | 3 ++- src/lib/synergy/ArgParser.cpp | 2 +- src/lib/synergy/ClientApp.cpp | 3 +-- src/lib/synergy/ClientArgs.cpp | 2 +- src/lib/synergy/ClientArgs.h | 2 +- src/test/integtests/net/NetworkTests.cpp | 24 +++++++++++++++++++----- 10 files changed, 41 insertions(+), 20 deletions(-) diff --git a/src/lib/client/Client.cpp b/src/lib/client/Client.cpp index 456260f0..43717134 100644 --- a/src/lib/client/Client.cpp +++ b/src/lib/client/Client.cpp @@ -60,8 +60,7 @@ Client::Client( const String& name, const NetworkAddress& address, ISocketFactory* socketFactory, synergy::Screen* screen, - bool enableDragDrop, - bool enableCrypto) : + ClientArgs args) : m_mock(false), m_name(name), m_serverAddress(address), @@ -77,9 +76,9 @@ Client::Client( m_events(events), m_sendFileThread(NULL), m_writeToDropDirThread(NULL), - m_enableDragDrop(enableDragDrop), m_socket(NULL), m_useSecureNetwork(false) + m_args(args) { assert(m_socketFactory != NULL); assert(m_screen != NULL); @@ -94,7 +93,7 @@ Client::Client( new TMethodEventJob(this, &Client::handleResume)); - if (m_enableDragDrop) { + if (m_args.m_enableDragDrop) { m_events->adoptHandler(m_events->forIScreen().fileChunkSending(), this, new TMethodEventJob(this, @@ -162,6 +161,7 @@ Client::connect() // create the socket IDataSocket* socket = m_socketFactory->create(m_useSecureNetwork); m_socket = dynamic_cast(socket); + m_socket->setFingerprintFilename(m_args.m_certFingerprintFilename); // filter socket messages, including a packetizing filter m_stream = socket; @@ -780,7 +780,8 @@ Client::fileChunkReceived(String data) void Client::dragInfoReceived(UInt32 fileNum, String data) { - if (!m_enableDragDrop) { + // TODO: fix duplicate function from CServer + if (!m_args.m_enableDragDrop) { LOG((CLOG_DEBUG "drag drop not enabled, ignoring drag info.")); return; } diff --git a/src/lib/client/Client.h b/src/lib/client/Client.h index f869ef64..12baadb4 100644 --- a/src/lib/client/Client.h +++ b/src/lib/client/Client.h @@ -23,6 +23,7 @@ #include "synergy/IClipboard.h" #include "synergy/DragInformation.h" #include "synergy/INode.h" +#include "synergy/ClientArgs.h" #include "net/NetworkAddress.h" #include "base/EventTypes.h" @@ -59,8 +60,7 @@ public: const String& name, const NetworkAddress& address, ISocketFactory* socketFactory, synergy::Screen* screen, - bool enableDragDrop, - bool enableCrypto); + ClientArgs args); ~Client(); #ifdef TEST_ENV @@ -227,4 +227,5 @@ private: bool m_enableDragDrop; TCPSocket* m_socket; bool m_useSecureNetwork; + ClientArgs m_args; }; diff --git a/src/lib/net/TCPSocket.h b/src/lib/net/TCPSocket.h index 832cd128..57bb0169 100644 --- a/src/lib/net/TCPSocket.h +++ b/src/lib/net/TCPSocket.h @@ -59,6 +59,7 @@ public: virtual void secureConnect() {} virtual void secureAccept() {} + virtual void setFingerprintFilename(CString& f) {} protected: ArchSocket getSocket() { return m_socket; } diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 8821a3e6..de63e333 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -46,7 +46,7 @@ CSecureSocket::CSecureSocket( CSocketMultiplexer* socketMultiplexer) : CTCPSocket(events, socketMultiplexer), m_secureReady(false), - m_certFingerprint() + m_certFingerprintFilename() { } @@ -405,6 +405,10 @@ CSecureSocket::disconnect() bool CSecureSocket::verifyCertFingerprint() { + if (m_certFingerprintFilename.empty()) { + return false; + } + // calculate received certificate fingerprint X509 *cert = cert = SSL_get_peer_certificate(m_ssl->m_ssl); EVP_MD* tempDigest; @@ -426,7 +430,7 @@ CSecureSocket::verifyCertFingerprint() CString fileLine; CString certificateFingerprint; std::ifstream file; - file.open(m_certFingerprint.c_str()); + file.open(m_certFingerprintFilename.c_str()); while (!file.eof()) { getline(file,fileLine); diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 725d5da9..7f3f0028 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -43,6 +43,7 @@ public: void secureConnect(); void secureAccept(); + void setFingerprintFilename(CString& f) { m_certFingerprintFilename = f; } bool isReady() const { return m_secureReady; } bool isSecureReady(); bool isSecure() { return true; } @@ -76,5 +77,5 @@ private: private: Ssl* m_ssl; bool m_secureReady; - CString m_certFingerprint; + CString m_certFingerprintFilename; }; diff --git a/src/lib/synergy/ArgParser.cpp b/src/lib/synergy/ArgParser.cpp index 7ba85752..82186b3a 100644 --- a/src/lib/synergy/ArgParser.cpp +++ b/src/lib/synergy/ArgParser.cpp @@ -91,7 +91,7 @@ ArgParser::parseClientArgs(ClientArgs& args, int argc, const char* const* argv) } else if (isArg(i, argc, argv, NULL, "--certificate-fingerprint", 1)) { // define scroll - args.m_certFingerprint = argv[++i]; + args.m_certFingerprintFilename = argv[++i]; } else { if (i + 1 == argc) { diff --git a/src/lib/synergy/ClientApp.cpp b/src/lib/synergy/ClientApp.cpp index b9c00414..f27a8331 100644 --- a/src/lib/synergy/ClientApp.cpp +++ b/src/lib/synergy/ClientApp.cpp @@ -342,8 +342,7 @@ ClientApp::openClient(const String& name, const NetworkAddress& address, address, new TCPSocketFactory(m_events, getSocketMultiplexer()), screen, - args().m_enableDragDrop, - args().m_enableCrypto); + args()); try { m_events->adoptHandler( diff --git a/src/lib/synergy/ClientArgs.cpp b/src/lib/synergy/ClientArgs.cpp index efbbd90b..133d5423 100644 --- a/src/lib/synergy/ClientArgs.cpp +++ b/src/lib/synergy/ClientArgs.cpp @@ -19,6 +19,6 @@ CClientArgs::CClientArgs() : m_yscroll(0), - m_certFingerprint() + m_certFingerprintFilename() { } diff --git a/src/lib/synergy/ClientArgs.h b/src/lib/synergy/ClientArgs.h index c9ddb9e9..947ca619 100644 --- a/src/lib/synergy/ClientArgs.h +++ b/src/lib/synergy/ClientArgs.h @@ -27,5 +27,5 @@ public: public: int m_yscroll; - CString m_certFingerprint; + CString m_certFingerprintFilename; }; diff --git a/src/test/integtests/net/NetworkTests.cpp b/src/test/integtests/net/NetworkTests.cpp index 7e3d3bd5..b307ff51 100644 --- a/src/test/integtests/net/NetworkTests.cpp +++ b/src/test/integtests/net/NetworkTests.cpp @@ -140,7 +140,11 @@ TEST_F(NetworkTests, sendToClient_mockData) ON_CALL(clientScreen, getShape(_, _, _, _)).WillByDefault(Invoke(getScreenShape)); ON_CALL(clientScreen, getCursorPos(_, _)).WillByDefault(Invoke(getCursorPos)); - Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, true, false); + + ClientArgs args; + args.m_enableDragDrop = true; + args.m_enableCrypto = false; + Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, args); m_events.adoptHandler( m_events.forIScreen().fileRecieveCompleted(), &client, @@ -192,7 +196,11 @@ TEST_F(NetworkTests, sendToClient_mockFile) ON_CALL(clientScreen, getShape(_, _, _, _)).WillByDefault(Invoke(getScreenShape)); ON_CALL(clientScreen, getCursorPos(_, _)).WillByDefault(Invoke(getCursorPos)); - Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, true, false); + + ClientArgs args; + args.m_enableDragDrop = true; + args.m_enableCrypto = false; + Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, args); m_events.adoptHandler( m_events.forIScreen().fileRecieveCompleted(), &client, @@ -238,7 +246,10 @@ TEST_F(NetworkTests, sendToServer_mockData) ON_CALL(clientScreen, getShape(_, _, _, _)).WillByDefault(Invoke(getScreenShape)); ON_CALL(clientScreen, getCursorPos(_, _)).WillByDefault(Invoke(getCursorPos)); - Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, true, false); + ClientArgs args; + args.m_enableDragDrop = true; + args.m_enableCrypto = false; + Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, args); m_events.adoptHandler( m_events.forClientListener().connected(), &listener, @@ -290,8 +301,11 @@ TEST_F(NetworkTests, sendToServer_mockFile) ON_CALL(clientScreen, getShape(_, _, _, _)).WillByDefault(Invoke(getScreenShape)); ON_CALL(clientScreen, getCursorPos(_, _)).WillByDefault(Invoke(getCursorPos)); - Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, true, false); - + ClientArgs args; + args.m_enableDragDrop = true; + args.m_enableCrypto = false; + Client client(&m_events, "stub", serverAddress, clientSocketFactory, &clientScreen, args); + m_events.adoptHandler( m_events.forClientListener().connected(), &listener, new TMethodEventJob( From 28eb85660f01163b044d1d37ce82adc62a40e383 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Tue, 14 Apr 2015 13:44:10 +0100 Subject: [PATCH 08/15] Fixed error from merge --- src/lib/base/String.cpp | 6 +++--- src/lib/base/String.h | 6 +++--- src/lib/client/Client.cpp | 4 ++-- src/lib/net/TCPSocket.h | 2 +- src/lib/plugin/ns/SecureSocket.cpp | 28 ++++++++++++++-------------- src/lib/plugin/ns/SecureSocket.h | 6 +++--- src/lib/synergy/ClientArgs.cpp | 2 +- src/lib/synergy/ClientArgs.h | 2 +- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/lib/base/String.cpp b/src/lib/base/String.cpp index 32531df8..4674deff 100644 --- a/src/lib/base/String.cpp +++ b/src/lib/base/String.cpp @@ -184,7 +184,7 @@ removeFileExt(String filename) } void -toHex(CString& subject, int width, const char fill) +toHex(String& subject, int width, const char fill) { std::stringstream ss; ss << std::hex; @@ -196,13 +196,13 @@ toHex(CString& subject, int width, const char fill) } void -uppercase(CString& subject) +uppercase(String& subject) { std::transform(subject.begin(), subject.end(), subject.begin(), ::toupper); } void -removeChar(CString& subject, const char c) +removeChar(String& subject, const char c) { subject.erase(std::remove(subject.begin(), subject.end(), c), subject.end()); } diff --git a/src/lib/base/String.h b/src/lib/base/String.h index 57f3c69c..dbe5c21b 100644 --- a/src/lib/base/String.h +++ b/src/lib/base/String.h @@ -74,19 +74,19 @@ String removeFileExt(String filename); /*! Convert each character in \c subject into hexdecimal form with \c width */ -void toHex(CString& subject, int width, const char fill = '0'); +void toHex(String& subject, int width, const char fill = '0'); //! Convert to all uppercase /*! Convert each character in \c subject to uppercase */ -void uppercase(CString& subject); +void uppercase(String& subject); //! Remove all specific char in suject /*! Remove all specific \c char in \c suject */ -void removeChar(CString& subject, const char c); +void removeChar(String& subject, const char c); //! Case-insensitive comparisons diff --git a/src/lib/client/Client.cpp b/src/lib/client/Client.cpp index 43717134..329733d4 100644 --- a/src/lib/client/Client.cpp +++ b/src/lib/client/Client.cpp @@ -77,7 +77,7 @@ Client::Client( m_sendFileThread(NULL), m_writeToDropDirThread(NULL), m_socket(NULL), - m_useSecureNetwork(false) + m_useSecureNetwork(false), m_args(args) { assert(m_socketFactory != NULL); @@ -104,7 +104,7 @@ Client::Client( &Client::handleFileRecieveCompleted)); } - if (enableCrypto) { + if (m_args.m_enableCrypto) { m_useSecureNetwork = ARCH->plugin().exists(s_networkSecurity); if (m_useSecureNetwork == false) { LOG((CLOG_NOTE "crypto disabled because of ns plugin not available")); diff --git a/src/lib/net/TCPSocket.h b/src/lib/net/TCPSocket.h index 57bb0169..2e75f357 100644 --- a/src/lib/net/TCPSocket.h +++ b/src/lib/net/TCPSocket.h @@ -59,7 +59,7 @@ public: virtual void secureConnect() {} virtual void secureAccept() {} - virtual void setFingerprintFilename(CString& f) {} + virtual void setFingerprintFilename(String& f) {} protected: ArchSocket getSocket() { return m_socket; } diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index de63e333..d1af5b85 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -41,10 +41,10 @@ struct Ssl { SSL* m_ssl; }; -CSecureSocket::CSecureSocket( +SecureSocket::SecureSocket( IEventQueue* events, - CSocketMultiplexer* socketMultiplexer) : - CTCPSocket(events, socketMultiplexer), + SocketMultiplexer* socketMultiplexer) : + TCPSocket(events, socketMultiplexer), m_secureReady(false), m_certFingerprintFilename() { @@ -396,14 +396,14 @@ SecureSocket::getError() } void -CSecureSocket::disconnect() +SecureSocket::disconnect() { sendEvent(getEvents()->forISocket().disconnected()); sendEvent(getEvents()->forIStream().inputShutdown()); } bool -CSecureSocket::verifyCertFingerprint() +SecureSocket::verifyCertFingerprint() { if (m_certFingerprintFilename.empty()) { return false; @@ -420,15 +420,15 @@ CSecureSocket::verifyCertFingerprint() } // convert fingerprint into hexdecimal format - CString fingerprint(reinterpret_cast(tempFingerprint), tempFingerprintLen); + String fingerprint(reinterpret_cast(tempFingerprint), tempFingerprintLen); synergy::string::toHex(fingerprint, 2); // all uppercase synergy::string::uppercase(fingerprint); // check if this fingerprint exist - CString fileLine; - CString certificateFingerprint; + String fileLine; + String certificateFingerprint; std::ifstream file; file.open(m_certFingerprintFilename.c_str()); @@ -437,14 +437,14 @@ CSecureSocket::verifyCertFingerprint() // example of a fingerprint: // SHA1 Fingerprint=6E:41:1A:21:53:2E:A3:EF:4D:A6:F2:A6:BA:0E:27:09:8A:F3:A1:10 size_t found = fileLine.find('='); - if (found != CString::npos) { + if (found != String::npos) { certificateFingerprint = fileLine.substr(found + 1); if (!certificateFingerprint.empty()) { // remove colons synergy::string::removeChar(certificateFingerprint, ':'); - if(certificateFingerprint.compare(fingerprint) == 0) { + if (certificateFingerprint.compare(fingerprint) == 0) { file.close(); return true; } @@ -458,10 +458,10 @@ CSecureSocket::verifyCertFingerprint() } ISocketMultiplexerJob* -CSecureSocket::serviceConnect(ISocketMultiplexerJob* job, +SecureSocket::serviceConnect(ISocketMultiplexerJob* job, bool, bool write, bool error) { - CLock lock(&getMutex()); + Lock lock(&getMutex()); bool retry = true; #ifdef SYSAPI_WIN32 @@ -474,10 +474,10 @@ CSecureSocket::serviceConnect(ISocketMultiplexerJob* job, } ISocketMultiplexerJob* -CSecureSocket::serviceAccept(ISocketMultiplexerJob* job, +SecureSocket::serviceAccept(ISocketMultiplexerJob* job, bool, bool write, bool error) { - CLock lock(&getMutex()); + Lock lock(&getMutex()); bool retry = true; #ifdef SYSAPI_WIN32 diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 7f3f0028..50ca17b6 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -43,7 +43,7 @@ public: void secureConnect(); void secureAccept(); - void setFingerprintFilename(CString& f) { m_certFingerprintFilename = f; } + void setFingerprintFilename(String& f) { m_certFingerprintFilename = f; } bool isReady() const { return m_secureReady; } bool isSecureReady(); bool isSecure() { return true; } @@ -62,7 +62,7 @@ private: void checkResult(int n, bool& fatal, bool& retry); void showError(); void throwError(const char* reason); - CString getError(); + String getError(); void disconnect(); bool verifyCertFingerprint(); @@ -77,5 +77,5 @@ private: private: Ssl* m_ssl; bool m_secureReady; - CString m_certFingerprintFilename; + String m_certFingerprintFilename; }; diff --git a/src/lib/synergy/ClientArgs.cpp b/src/lib/synergy/ClientArgs.cpp index 133d5423..fff34817 100644 --- a/src/lib/synergy/ClientArgs.cpp +++ b/src/lib/synergy/ClientArgs.cpp @@ -17,7 +17,7 @@ #include "synergy/ClientArgs.h" -CClientArgs::CClientArgs() : +ClientArgs::ClientArgs() : m_yscroll(0), m_certFingerprintFilename() { diff --git a/src/lib/synergy/ClientArgs.h b/src/lib/synergy/ClientArgs.h index 947ca619..093b0ccf 100644 --- a/src/lib/synergy/ClientArgs.h +++ b/src/lib/synergy/ClientArgs.h @@ -27,5 +27,5 @@ public: public: int m_yscroll; - CString m_certFingerprintFilename; + String m_certFingerprintFilename; }; From dd574c4f2ca1f505ff02106f88fe375c537ae1f4 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Tue, 14 Apr 2015 13:47:34 +0100 Subject: [PATCH 09/15] Added unit tests for string operations --- src/test/unittests/base/StringTests.cpp | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/test/unittests/base/StringTests.cpp b/src/test/unittests/base/StringTests.cpp index e0108413..2a461f05 100644 --- a/src/test/unittests/base/StringTests.cpp +++ b/src/test/unittests/base/StringTests.cpp @@ -53,3 +53,32 @@ TEST(StringTests, sprintf) EXPECT_EQ("answer=42", result); } + +TEST(StringTests, toHex) +{ + String subject = "foobar"; + int width = 2; + + string::toHex(subject, width); + + EXPECT_EQ("666f6f626172", subject); +} + +TEST(StringTests, uppercase) +{ + String subject = "12foo3BaR"; + + string::uppercase(subject); + + EXPECT_EQ("12FOO3BAR", subject); +} + +TEST(StringTests, removeChar) +{ + String subject = "foobar"; + const char c = 'o'; + + string::removeChar(subject, c); + + EXPECT_EQ("fbar", subject); +} From 916a4c75af19f9d6031c7236d63b308f943250fc Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Fri, 10 Apr 2015 14:07:53 +0100 Subject: [PATCH 10/15] 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(); From f60e98c8cd1c6810acfb749f86cf1d53fdd502ca Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Tue, 14 Apr 2015 14:03:43 +0100 Subject: [PATCH 11/15] Fixed error from merge --- src/lib/net/TCPListenSocket.cpp | 6 +++--- src/lib/plugin/ns/SecureListenSocket.cpp | 15 +-------------- src/lib/plugin/ns/SecureSocket.cpp | 10 +++++----- src/lib/plugin/ns/SecureSocket.h | 2 +- 4 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/lib/net/TCPListenSocket.cpp b/src/lib/net/TCPListenSocket.cpp index 38d4dd8c..f0f06ee3 100644 --- a/src/lib/net/TCPListenSocket.cpp +++ b/src/lib/net/TCPListenSocket.cpp @@ -133,11 +133,11 @@ TCPListenSocket::accept() } void -CTCPListenSocket::setListeningJob() +TCPListenSocket::setListeningJob() { m_socketMultiplexer->addSocket(this, - new TSocketMultiplexerMethodJob( - this, &CTCPListenSocket::serviceListening, + new TSocketMultiplexerMethodJob( + this, &TCPListenSocket::serviceListening, m_socket, true, false)); } diff --git a/src/lib/plugin/ns/SecureListenSocket.cpp b/src/lib/plugin/ns/SecureListenSocket.cpp index bee4b29d..562a6899 100644 --- a/src/lib/plugin/ns/SecureListenSocket.cpp +++ b/src/lib/plugin/ns/SecureListenSocket.cpp @@ -54,17 +54,13 @@ SecureListenSocket::accept() m_events, m_socketMultiplexer, ARCH->acceptSocket(m_socket, NULL)); - -<<<<<<< HEAD:src/lib/plugin/ns/SecureListenSocket.cpp + socket->initSsl(true); 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 @@ -82,15 +78,6 @@ SecureListenSocket::accept() 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&) { diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 1be5f184..64d013d1 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -152,7 +152,7 @@ SecureSocket::initSsl(bool server) } bool -CSecureSocket::loadCertificates(CString& filename) +SecureSocket::loadCertificates(String& filename) { if (filename.empty()) { showError("ssl certificate is not specified"); @@ -164,7 +164,7 @@ CSecureSocket::loadCertificates(CString& filename) file.close(); if (!exist) { - CString errorMsg("ssl certificate doesn't exist: "); + String errorMsg("ssl certificate doesn't exist: "); errorMsg.append(filename); showError(errorMsg.c_str()); return false; @@ -303,7 +303,7 @@ SecureSocket::secureConnect(int socket) } bool -CSecureSocket::showCertificate() +SecureSocket::showCertificate() { X509* cert; char* line; @@ -387,13 +387,13 @@ SecureSocket::checkResult(int n, bool& fatal, bool& retry) } void -CSecureSocket::showError(const char* reason) +SecureSocket::showError(const char* reason) { if (reason != NULL) { LOG((CLOG_ERR "%s", reason)); } - CString error = getError(); + String error = getError(); if (!error.empty()) { LOG((CLOG_ERR "%s", error.c_str())); } diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 4f5e5d70..cff4c34f 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); - bool loadCertificates(CString& CertFile); + bool loadCertificates(String& CertFile); private: // SSL From fa1ea0022b1d61b919d1a50e9893f9a950132125 Mon Sep 17 00:00:00 2001 From: XinyuHou Date: Tue, 14 Apr 2015 14:04:54 +0100 Subject: [PATCH 12/15] Fixed code style --- src/lib/synergy/ArgsBase.h | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/lib/synergy/ArgsBase.h b/src/lib/synergy/ArgsBase.h index 125f9b51..743202e8 100644 --- a/src/lib/synergy/ArgsBase.h +++ b/src/lib/synergy/ArgsBase.h @@ -24,25 +24,27 @@ class ArgsBase { public: ArgsBase(); virtual ~ArgsBase(); - bool m_daemon; - bool m_backend; - bool m_restartable; - bool m_noHooks; - const char* m_pname; - const char* m_logFilter; - const char* m_logFile; - const char* m_display; - String m_name; - bool m_disableTray; - bool m_enableIpc; - bool m_enableDragDrop; + +public: + bool m_daemon; + bool m_backend; + bool m_restartable; + bool m_noHooks; + const char* m_pname; + const char* m_logFilter; + const char* m_logFile; + const char* m_display; + String m_name; + bool m_disableTray; + bool m_enableIpc; + bool m_enableDragDrop; #if SYSAPI_WIN32 - bool m_debugServiceWait; - bool m_pauseOnExit; - bool m_stopOnDeskSwitch; + bool m_debugServiceWait; + bool m_pauseOnExit; + bool m_stopOnDeskSwitch; #endif #if WINAPI_XWINDOWS - bool m_disableXInitThreads; + bool m_disableXInitThreads; #endif bool m_shouldExit; String m_synergyAddress; From 1e2b822226a5c88c31ce742065a4c764e575630e Mon Sep 17 00:00:00 2001 From: Xinyu Hou Date: Tue, 14 Apr 2015 14:28:32 +0100 Subject: [PATCH 13/15] Removed unused variable --- src/lib/client/Client.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/client/Client.h b/src/lib/client/Client.h index 12baadb4..e50b567b 100644 --- a/src/lib/client/Client.h +++ b/src/lib/client/Client.h @@ -224,7 +224,6 @@ private: String m_dragFileExt; Thread* m_sendFileThread; Thread* m_writeToDropDirThread; - bool m_enableDragDrop; TCPSocket* m_socket; bool m_useSecureNetwork; ClientArgs m_args; From 52d9b1beedd24ebaeaa39455253593e8e267ae6e Mon Sep 17 00:00:00 2001 From: Xinyu Hou Date: Wed, 15 Apr 2015 13:06:49 +0100 Subject: [PATCH 14/15] Logged out new fingerprint when not match #4522 --- src/lib/plugin/ns/SecureSocket.cpp | 50 ++++++++++++++++++------------ src/lib/plugin/ns/SecureSocket.h | 3 ++ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 64d013d1..22815cce 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -421,6 +421,26 @@ SecureSocket::disconnect() sendEvent(getEvents()->forIStream().inputShutdown()); } +void +SecureSocket::formatFingerprint(String& fingerprint, bool hex, bool separator) +{ + if (hex) { + // to hexidecimal + synergy::string::toHex(fingerprint, 2); + } + + // all uppercase + synergy::string::uppercase(fingerprint); + + if (separator) { + // add colon to separate each 2 charactors + size_t separators = fingerprint.size() / 2; + for (size_t i = 1; i < separators; i++) { + fingerprint.insert(i * 3 - 1, ":"); + } + } +} + bool SecureSocket::verifyCertFingerprint() { @@ -438,41 +458,31 @@ SecureSocket::verifyCertFingerprint() return false; } - // convert fingerprint into hexdecimal format + // format fingerprint into hexdecimal format with colon separator String fingerprint(reinterpret_cast(tempFingerprint), tempFingerprintLen); - synergy::string::toHex(fingerprint, 2); - - // all uppercase - synergy::string::uppercase(fingerprint); + formatFingerprint(fingerprint); // check if this fingerprint exist String fileLine; - String certificateFingerprint; std::ifstream file; file.open(m_certFingerprintFilename.c_str()); while (!file.eof()) { getline(file,fileLine); - // example of a fingerprint: - // SHA1 Fingerprint=6E:41:1A:21:53:2E:A3:EF:4D:A6:F2:A6:BA:0E:27:09:8A:F3:A1:10 - size_t found = fileLine.find('='); - if (found != String::npos) { - certificateFingerprint = fileLine.substr(found + 1); - - if (!certificateFingerprint.empty()) { - // remove colons - synergy::string::removeChar(certificateFingerprint, ':'); - - if (certificateFingerprint.compare(fingerprint) == 0) { - file.close(); - return true; - } + // example of a fingerprint:A1:B2:C3 + if (!fileLine.empty()) { + if (fileLine.compare(fingerprint) == 0) { + file.close(); + return true; } } } file.close(); + LOG((CLOG_NOTE "new fingerprint from a server")); + LOG((CLOG_NOTE "server fingerprint: %s", fingerprint.c_str())); + return false; } diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index cff4c34f..1bc64628 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -63,6 +63,9 @@ private: void showError(const char* reason = NULL); String getError(); void disconnect(); + void formatFingerprint(String& fingerprint, + bool hex = true, + bool separator = true); bool verifyCertFingerprint(); ISocketMultiplexerJob* From e405ec25e30b2187bb6ada867b87f16e9454ef0b Mon Sep 17 00:00:00 2001 From: Xinyu Hou Date: Wed, 15 Apr 2015 13:25:18 +0100 Subject: [PATCH 15/15] Always log out server fingerprint #4522 --- src/lib/plugin/ns/SecureSocket.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 22815cce..7f78554c 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -461,29 +461,27 @@ SecureSocket::verifyCertFingerprint() // format fingerprint into hexdecimal format with colon separator String fingerprint(reinterpret_cast(tempFingerprint), tempFingerprintLen); formatFingerprint(fingerprint); + LOG((CLOG_NOTE "server fingerprint: %s", fingerprint.c_str())); // check if this fingerprint exist String fileLine; std::ifstream file; file.open(m_certFingerprintFilename.c_str()); + bool isValid = false; while (!file.eof()) { getline(file,fileLine); // example of a fingerprint:A1:B2:C3 if (!fileLine.empty()) { if (fileLine.compare(fingerprint) == 0) { - file.close(); - return true; + isValid = true; + break; } } } file.close(); - - LOG((CLOG_NOTE "new fingerprint from a server")); - LOG((CLOG_NOTE "server fingerprint: %s", fingerprint.c_str())); - - return false; + return isValid; } ISocketMultiplexerJob*