From 6017757bc079f4446aa77bc5c0855c52741280f4 Mon Sep 17 00:00:00 2001 From: Reinhard Tartler Date: Tue, 28 May 2019 07:20:50 -0400 Subject: New upstream version 0.13~~git20190527.g039c4a1 --- lib/server/ConnectionException.txt | 1 + lib/server/Daemon.cpp | 4 + lib/server/Daemon.h | 2 + lib/server/ServerControl.cpp | 171 ++++++++++++++++++++++++++++++++++++- lib/server/ServerControl.h | 16 ++-- lib/server/ServerException.txt | 1 + lib/server/ServerStream.h | 11 ++- lib/server/ServerTLS.h | 9 +- lib/server/SocketStream.cpp | 4 + lib/server/SocketStream.h | 1 + lib/server/SocketStreamTLS.cpp | 51 ++++++++--- lib/server/SocketStreamTLS.h | 3 +- lib/server/TLSContext.cpp | 72 ++++++++++++---- lib/server/TLSContext.h | 3 +- 14 files changed, 311 insertions(+), 38 deletions(-) (limited to 'lib/server') diff --git a/lib/server/ConnectionException.txt b/lib/server/ConnectionException.txt index 7dcaadeb..74390853 100644 --- a/lib/server/ConnectionException.txt +++ b/lib/server/ConnectionException.txt @@ -15,6 +15,7 @@ TLSNoPeerCertificate 36 TLSPeerCertificateInvalid 37 Check certification process TLSClosedWhenWriting 38 TLSHandshakeTimedOut 39 +TLSPeerWeakCertificate 40 The peer's certificate is too weak for the current SSL Security Level, see https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates Protocol_Timeout 41 Probably a network issue between client and server. Protocol_ObjTooBig 42 Protocol_BadCommandRecieved 44 diff --git a/lib/server/Daemon.cpp b/lib/server/Daemon.cpp index d3c8441f..76141b6f 100644 --- a/lib/server/Daemon.cpp +++ b/lib/server/Daemon.cpp @@ -42,6 +42,7 @@ #include "autogen_ConnectionException.h" #include "autogen_ServerException.h" +#include "BoxPortsAndFiles.h" #include "Configuration.h" #include "Daemon.h" #include "FileModificationTime.h" @@ -52,6 +53,9 @@ #include "MemLeakFindOn.h" +const ConfigurationVerifyKey ssl_security_level_key("SSLSecurityLevel", + ConfigTest_IsInt | ConfigTest_LastEntry, BOX_DEFAULT_SSL_SECURITY_LEVEL); + Daemon *Daemon::spDaemon = 0; diff --git a/lib/server/Daemon.h b/lib/server/Daemon.h index b5384918..60ce4507 100644 --- a/lib/server/Daemon.h +++ b/lib/server/Daemon.h @@ -121,5 +121,7 @@ private: ConfigurationVerifyKey("LogFacility", 0), \ ConfigurationVerifyKey("User", ConfigTest_LastEntry) +extern const ConfigurationVerifyKey ssl_security_level_key; + #endif // DAEMON__H diff --git a/lib/server/ServerControl.cpp b/lib/server/ServerControl.cpp index f1a718df..b6cadab3 100644 --- a/lib/server/ServerControl.cpp +++ b/lib/server/ServerControl.cpp @@ -18,7 +18,9 @@ #include "BoxTime.h" #include "IOStreamGetLine.h" #include "ServerControl.h" +#include "SocketStream.h" #include "Test.h" +#include "autogen_ServerException.h" #ifdef WIN32 @@ -227,7 +229,7 @@ bool KillServer(int pid, bool WaitForProcess) return !ServerIsAlive(pid); } -bool KillServer(std::string pid_file, bool WaitForProcess) +bool KillServer(const std::string& pid_file, bool WaitForProcess) { FileStream fs(pid_file); IOStreamGetLine getline(fs); @@ -251,11 +253,174 @@ bool KillServer(std::string pid_file, bool WaitForProcess) return status; } -int StartDaemon(int current_pid, const std::string& cmd_line, const char* pid_file) +int LaunchServer(const std::string& rCommandLine, const char *pidFile, int port, + const std::string& socket_path) +{ + BOX_INFO("Starting server: " << rCommandLine); + +#ifdef WIN32 + + // Use a Windows "Job Object" as a container for all our child + // processes. The test runner will create this job object when + // it starts, and close the handle (killing any running daemons) + // when it exits. This is the best way to avoid daemons hanging + // around and causing subsequent tests to fail, and/or the test + // runner to hang waiting for a daemon that will never terminate. + + PROCESS_INFORMATION procInfo; + + STARTUPINFO startInfo; + startInfo.cb = sizeof(startInfo); + startInfo.lpReserved = NULL; + startInfo.lpDesktop = NULL; + startInfo.lpTitle = NULL; + startInfo.dwFlags = 0; + startInfo.cbReserved2 = 0; + startInfo.lpReserved2 = NULL; + + std::string cmd = ConvertPaths(rCommandLine); + CHAR* tempCmd = strdup(cmd.c_str()); + + DWORD result = CreateProcess + ( + NULL, // lpApplicationName, naughty! + tempCmd, // lpCommandLine + NULL, // lpProcessAttributes + NULL, // lpThreadAttributes + false, // bInheritHandles + 0, // dwCreationFlags + NULL, // lpEnvironment + NULL, // lpCurrentDirectory + &startInfo, // lpStartupInfo + &procInfo // lpProcessInformation + ); + + free(tempCmd); + + TEST_THAT_OR(result != 0, + BOX_LOG_WIN_ERROR("Failed to CreateProcess: " << rCommandLine); + return -1; + ); + + CloseHandle(procInfo.hProcess); + CloseHandle(procInfo.hThread); + + return WaitForServerStartup(pidFile, (int)procInfo.dwProcessId, port, socket_path); + +#else // !WIN32 + + TEST_THAT_OR(RunCommand(rCommandLine) == 0, + TEST_FAIL_WITH_MESSAGE("Failed to start server: " << rCommandLine); + return -1; + ) + + return WaitForServerStartup(pidFile, 0, port, socket_path); + +#endif // WIN32 +} + +int WaitForServerStartup(const char *pidFile, int pidIfKnown, int port, + const std::string& socket_path) +{ +#ifdef WIN32 + if(pidFile == NULL && port == 0 && socket_path == "") + { + return pidIfKnown; + } +#else + // On other platforms there is no other way to get the PID, so a NULL pidFile doesn't + // make sense. + ASSERT(pidFile != NULL); +#endif + + // time for it to start up + BOX_TRACE("Waiting for server to start"); + + for (int i = 150; i >= 0; i--) + { + if(i == 0) + { + // ran out of time waiting + TEST_FAIL_WITH_MESSAGE("Server didn't start within expected time"); + return -1; + } + + ShortSleep(MilliSecondsToBoxTime(100), false); + + if(!TestFileNotEmpty(pidFile)) + { + // Hasn't written a complete PID file yet, go round again + continue; + } + + // Once we know what PID the process has/had, we can check if it has died during or + // shortly after startup: + if (pidIfKnown && !ServerIsAlive(pidIfKnown)) + { + TEST_FAIL_WITH_MESSAGE("Server died!"); + return -1; + } + + if(port != 0 || socket_path != "") + { + try + { + if(port != 0) + { + SocketStream conn; + conn.Open(Socket::TypeINET, "localhost", port); + } + + if(socket_path != "") + { + SocketStream conn; + conn.Open(Socket::TypeUNIX, socket_path); + } + } + catch(ServerException &e) + { + if(EXCEPTION_IS_TYPE(e, ServerException, SocketOpenError)) + { + // not listening on port, go round again + continue; + } + else + { + // something bad happened, break + throw; + } + } + } + + // All tests that we can do have passed, looks good! + break; + } + + BOX_TRACE("Server started"); + + // read pid file + int pid = ReadPidFile(pidFile); + + // On Win32 we can check whether the PID in the pidFile matches + // the one returned by the system, which it always should. + if (pidIfKnown && pid != pidIfKnown) + { + BOX_ERROR("Server wrote wrong pid to file (" << pidFile << + "): expected " << pidIfKnown << " but found " << + pid); + TEST_FAIL_WITH_MESSAGE("Server wrote wrong pid to file"); + return -1; + } + + return pid; +} + +int StartDaemon(int current_pid, const std::string& cmd_line, const char* pid_file, int port, + const std::string& socket_path) { TEST_THAT_OR(current_pid == 0, return 0); - int new_pid = LaunchServer(cmd_line, pid_file); + int new_pid = LaunchServer(cmd_line, pid_file, port, socket_path); TEST_THAT_OR(new_pid != -1 && new_pid != 0, return 0); ::sleep(1); diff --git a/lib/server/ServerControl.h b/lib/server/ServerControl.h index be2464c1..28320491 100644 --- a/lib/server/ServerControl.h +++ b/lib/server/ServerControl.h @@ -4,11 +4,17 @@ #include "Test.h" bool HUPServer(int pid); -bool KillServer(int pid, bool WaitForProcess = false); -bool KillServer(std::string pid_file, bool WaitForProcess = false); -int StartDaemon(int current_pid, const std::string& cmd_line, const char* pid_file); -bool StopDaemon(int current_pid, const std::string& pid_file, - const std::string& memleaks_file, bool wait_for_process); +bool KillServer(int pid, bool wait_for_process = false); +bool KillServer(const std::string& pid_file, bool wait_for_process = false); +bool KillServerInternal(int pid); +int StartDaemon(int current_pid, const std::string& cmd_line, const char* pid_file, int port = 0, + const std::string& socket_path = ""); +bool StopDaemon(int current_pid, const std::string& pid_file, const std::string& memleaks_file, + bool wait_for_process); +int LaunchServer(const std::string& rCommandLine, const char *pidFile, int port = 0, + const std::string& socket_path = ""); +int WaitForServerStartup(const char *pidFile, int pidIfKnown, int port = 0, + const std::string& socket_path = ""); #ifdef WIN32 #include "WinNamedPipeStream.h" diff --git a/lib/server/ServerException.txt b/lib/server/ServerException.txt index 474b4067..02fb379c 100644 --- a/lib/server/ServerException.txt +++ b/lib/server/ServerException.txt @@ -29,6 +29,7 @@ TLSSetCiphersFailed 28 SSLLibraryInitialisationError 29 TLSNoSSLObject 31 TLSAlreadyHandshaked 35 +TLSServerWeakCertificate 36 Our SSL certificate is too weak for the current SSL Security Level, see https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates SocketSetNonBlockingFailed 40 Protocol_BadUsage 43 Protocol_UnsuitableStreamTypeForSending 51 diff --git a/lib/server/ServerStream.h b/lib/server/ServerStream.h index 3f6eed7e..db8beaf2 100644 --- a/lib/server/ServerStream.h +++ b/lib/server/ServerStream.h @@ -306,7 +306,16 @@ public: #endif // !WIN32 // Just handle in this process SetProcessTitle("handling"); - HandleConnection(connection); + try + { + HandleConnection(connection); + } + catch(BoxException &e) + { + BOX_ERROR("Error in child handler, terminating connection: " + "exception " << e.what() << "(" << e.GetType() << "/" << + e.GetSubType() << ")"); + } SetProcessTitle("idle"); #ifndef WIN32 } diff --git a/lib/server/ServerTLS.h b/lib/server/ServerTLS.h index f748f4b2..6b53e860 100644 --- a/lib/server/ServerTLS.h +++ b/lib/server/ServerTLS.h @@ -10,6 +10,7 @@ #ifndef SERVERTLS__H #define SERVERTLS__H +#include "BoxPortsAndFiles.h" #include "ServerStream.h" #include "SocketStreamTLS.h" #include "SSLLib.h" @@ -52,8 +53,12 @@ public: std::string certFile(serverconf.GetKeyValue("CertificateFile")); std::string keyFile(serverconf.GetKeyValue("PrivateKeyFile")); std::string caFile(serverconf.GetKeyValue("TrustedCAsFile")); + + int ssl_security_level(serverconf.GetKeyValueInt("SSLSecurityLevel", + BOX_DEFAULT_SSL_SECURITY_LEVEL)); + mContext.Initialise(true /* as server */, certFile.c_str(), - keyFile.c_str(), caFile.c_str()); + keyFile.c_str(), caFile.c_str(), ssl_security_level); // Then do normal stream server stuff ServerStream -#include + #include #include @@ -19,6 +18,10 @@ #include #endif +#include +#include +#include + #include "autogen_ConnectionException.h" #include "autogen_ServerException.h" #include "BoxTime.h" @@ -126,8 +129,8 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer) mpBIO = ::BIO_new(::BIO_s_socket()); if(mpBIO == 0) { - CryptoUtils::LogError("creating socket bio"); - THROW_EXCEPTION(ServerException, TLSAllocationFailed) + THROW_EXCEPTION_MESSAGE(ServerException, TLSAllocationFailed, + "Failed to create SSL BIO: " << CryptoUtils::LogError("creating socket bio")); } tOSSocketHandle socket = GetSocketHandle(); @@ -137,8 +140,8 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer) mpSSL = ::SSL_new(rContext.GetRawContext()); if(mpSSL == 0) { - CryptoUtils::LogError("creating SSL object"); - THROW_EXCEPTION(ServerException, TLSAllocationFailed) + THROW_EXCEPTION_MESSAGE(ServerException, TLSAllocationFailed, + "Failed to create SSL object: " << CryptoUtils::LogError("creating SSL object")); } // Make the socket non-blocking so timeouts on Read work @@ -203,15 +206,43 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer) default: // (and SSL_ERROR_ZERO_RETURN) // Error occured +#if HAVE_DECL_SSL_R_EE_KEY_TOO_SMALL + int err_reason = ERR_GET_REASON(ERR_peek_error()); + const char *file, *data; + int line, flags; + ERR_peek_error_line_data(&file, &line, &data, &flags); + long verify_result = SSL_get_verify_result(mpSSL); + + if(se == SSL_ERROR_SSL && verify_result == X509_V_ERR_CA_KEY_TOO_SMALL) + { + // Would be nice to use GetPeerCommonName() in these error messages, + // but since the certificate isn't trusted, that might be misleading, + // and it's not available to us anyway :( + + THROW_EXCEPTION_MESSAGE(ConnectionException, TLSPeerWeakCertificate, + (IsServer ? "Failed to accept connection from" : + "Failed to connect to") << " " << mPeerSocketDesc << + ": key too short for current security level"); + } + else if(se == SSL_ERROR_SSL && verify_result == X509_V_ERR_CA_MD_TOO_WEAK) + { + THROW_EXCEPTION_MESSAGE(ConnectionException, TLSPeerWeakCertificate, + (IsServer ? "Failed to accept connection from" : + "Failed to connect to") << " " << mPeerSocketDesc << + ": hash too weak for current security level"); + } + else +#endif // HAVE_DECL_SSL_R_EE_KEY_TOO_SMALL if(IsServer) { - CryptoUtils::LogError("accepting connection"); - THROW_EXCEPTION(ConnectionException, TLSHandshakeFailed) + THROW_EXCEPTION_MESSAGE(ConnectionException, TLSHandshakeFailed, + "Failed to accept connection: " << + CryptoUtils::LogError("accepting connection")); } else { - CryptoUtils::LogError("connecting"); - THROW_EXCEPTION(ConnectionException, TLSHandshakeFailed) + THROW_EXCEPTION_MESSAGE(ConnectionException, TLSHandshakeFailed, + "Failed to connect: " << CryptoUtils::LogError("connecting")); } } } diff --git a/lib/server/SocketStreamTLS.h b/lib/server/SocketStreamTLS.h index 3fda98c1..8a793966 100644 --- a/lib/server/SocketStreamTLS.h +++ b/lib/server/SocketStreamTLS.h @@ -34,10 +34,11 @@ public: SocketStreamTLS(); SocketStreamTLS(int socket); ~SocketStreamTLS(); + private: SocketStreamTLS(const SocketStreamTLS &rToCopy); -public: +public: void Open(const TLSContext &rContext, Socket::Type Type, const std::string& rName, int Port = 0); void Handshake(const TLSContext &rContext, bool IsServer = false); diff --git a/lib/server/TLSContext.cpp b/lib/server/TLSContext.cpp index 1a6d4a53..9c01452b 100644 --- a/lib/server/TLSContext.cpp +++ b/lib/server/TLSContext.cpp @@ -10,10 +10,12 @@ #include "Box.h" #define TLS_CLASS_IMPLEMENTATION_CPP +#include #include #include "autogen_ConnectionException.h" #include "autogen_ServerException.h" +#include "BoxPortsAndFiles.h" #include "CryptoUtils.h" #include "SSLLib.h" #include "TLSContext.h" @@ -21,7 +23,7 @@ #include "MemLeakFindOn.h" #define MAX_VERIFICATION_DEPTH 2 -#define CIPHER_LIST "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH" +#define CIPHER_LIST "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH" // Macros to allow compatibility with OpenSSL 1.0 and 1.1 APIs. See // https://github.com/charybdis-ircd/charybdis/blob/release/3.5/libratbox/src/openssl_ratbox.h @@ -71,7 +73,8 @@ TLSContext::~TLSContext() // Created: 2003/08/06 // // -------------------------------------------------------------------------- -void TLSContext::Initialise(bool AsServer, const char *CertificatesFile, const char *PrivateKeyFile, const char *TrustedCAsFile) +void TLSContext::Initialise(bool AsServer, const char *CertificatesFile, const char *PrivateKeyFile, + const char *TrustedCAsFile, int SSLSecurityLevel) { if(mpContext != 0) { @@ -84,29 +87,65 @@ void TLSContext::Initialise(bool AsServer, const char *CertificatesFile, const c THROW_EXCEPTION(ServerException, TLSAllocationFailed) } +#ifdef HAVE_SSL_CTX_SET_SECURITY_LEVEL + if(SSLSecurityLevel == -1) + { + BOX_WARNING("SSLSecurityLevel not set. Your connection may not be secure. " + "Please see https://github.com/boxbackup/boxbackup/wiki/WeakSSLCertificates " + "for details"); + SSLSecurityLevel = 1; // Default for now. Unsafe, but we warned the user. + // TODO: upgrade to level 2 soon. + } + + SSL_CTX_set_security_level(mpContext, SSLSecurityLevel); +#else + if(SSLSecurityLevel != BOX_DEFAULT_SSL_SECURITY_LEVEL) + { + BOX_WARNING("SSLSecurityLevel is set, but this Box Backup is not compiled with " + "OpenSSL 1.1 or higher, so will be ignored (compiled with " + OPENSSL_VERSION_TEXT ")"); + } +#endif + // Setup our identity if(::SSL_CTX_use_certificate_chain_file(mpContext, CertificatesFile) != 1) { - std::string msg = "loading certificates from "; - msg += CertificatesFile; - CryptoUtils::LogError(msg); - THROW_EXCEPTION(ServerException, TLSLoadCertificatesFailed) +#if HAVE_DECL_SSL_R_EE_KEY_TOO_SMALL + int err_reason = ERR_GET_REASON(ERR_peek_error()); + if(err_reason == SSL_R_EE_KEY_TOO_SMALL) + { + THROW_EXCEPTION_MESSAGE(ServerException, TLSServerWeakCertificate, + "Failed to load certificates from " << CertificatesFile << ": " + "key too short for current security level"); + } + else if(err_reason == SSL_R_CA_MD_TOO_WEAK) + { + THROW_EXCEPTION_MESSAGE(ServerException, TLSServerWeakCertificate, + "Failed to load certificates from " << CertificatesFile << ": " + "hash too weak for current security level"); + } + else +#endif // HAVE_DECL_SSL_R_EE_KEY_TOO_SMALL + { + THROW_EXCEPTION_MESSAGE(ServerException, TLSLoadCertificatesFailed, + "Failed to load certificates from " << CertificatesFile << ": " << + CryptoUtils::LogError("loading certificates")); + } } + if(::SSL_CTX_use_PrivateKey_file(mpContext, PrivateKeyFile, SSL_FILETYPE_PEM) != 1) { - std::string msg = "loading private key from "; - msg += PrivateKeyFile; - CryptoUtils::LogError(msg); - THROW_EXCEPTION(ServerException, TLSLoadPrivateKeyFailed) + THROW_EXCEPTION_MESSAGE(ServerException, TLSLoadPrivateKeyFailed, + "Failed to load private key from " << PrivateKeyFile << ": " << + CryptoUtils::LogError("loading private key")); } // Setup the identify of CAs we trust if(::SSL_CTX_load_verify_locations(mpContext, TrustedCAsFile, NULL) != 1) { - std::string msg = "loading CA cert from "; - msg += TrustedCAsFile; - CryptoUtils::LogError(msg); - THROW_EXCEPTION(ServerException, TLSLoadTrustedCAsFailed) + THROW_EXCEPTION_MESSAGE(ServerException, TLSLoadTrustedCAsFailed, + "Failed to load CA certificate from " << TrustedCAsFile << ": " << + CryptoUtils::LogError("loading CA cert")); } // Setup options to require these certificates @@ -117,8 +156,9 @@ void TLSContext::Initialise(bool AsServer, const char *CertificatesFile, const c // Setup allowed ciphers if(::SSL_CTX_set_cipher_list(mpContext, CIPHER_LIST) != 1) { - CryptoUtils::LogError("setting cipher list to " CIPHER_LIST); - THROW_EXCEPTION(ServerException, TLSSetCiphersFailed) + THROW_EXCEPTION_MESSAGE(ServerException, TLSSetCiphersFailed, + "Failed to set cipher list to " << CIPHER_LIST << ": " << + CryptoUtils::LogError("setting cipher list")); } } diff --git a/lib/server/TLSContext.h b/lib/server/TLSContext.h index f52f5457..6239d136 100644 --- a/lib/server/TLSContext.h +++ b/lib/server/TLSContext.h @@ -30,7 +30,8 @@ public: private: TLSContext(const TLSContext &); public: - void Initialise(bool AsServer, const char *CertificatesFile, const char *PrivateKeyFile, const char *TrustedCAsFile); + void Initialise(bool AsServer, const char *CertificatesFile, const char *PrivateKeyFile, + const char *TrustedCAsFile, int SSLSecurityLevel = -1); SSL_CTX *GetRawContext() const; private: -- cgit v1.2.3