From afea2f63ecf37b171a395d0a00aad8b93fa44be4 Mon Sep 17 00:00:00 2001 From: John Knox Date: Mon, 3 Sep 2018 11:10:53 -0700 Subject: [PATCH] Extract file system interaction from SonarWebSocketImpl Summary: SonarWebSocketImpl has got pretty bloated. So I'm extracting all the file interaction out of it into ConnectionContextStore. The purpose of this class is to provide all the context needed to establish a connection. This makes SonarWebSocketImpl more functional and therefore testable. Reviewed By: priteshrnandgaonkar Differential Revision: D9540089 fbshipit-source-id: 0cd1d69f2b11eaf9f569245a2da14f85cc140427 --- xplat/Sonar/ConnectionContextStore.cpp | 134 +++++++++++++++++ xplat/Sonar/ConnectionContextStore.h | 33 +++++ xplat/Sonar/SonarClient.cpp | 4 +- xplat/Sonar/SonarWebSocketImpl.cpp | 136 +++--------------- xplat/Sonar/SonarWebSocketImpl.h | 8 +- .../SonarTestLib/ConnectionContextStoreMock.h | 28 ++++ .../SonarWebSocketImplTerminationTests.cpp | 21 ++- 7 files changed, 235 insertions(+), 129 deletions(-) create mode 100644 xplat/Sonar/ConnectionContextStore.cpp create mode 100644 xplat/Sonar/ConnectionContextStore.h create mode 100644 xplat/SonarTestLib/ConnectionContextStoreMock.h diff --git a/xplat/Sonar/ConnectionContextStore.cpp b/xplat/Sonar/ConnectionContextStore.cpp new file mode 100644 index 000000000..163527e1f --- /dev/null +++ b/xplat/Sonar/ConnectionContextStore.cpp @@ -0,0 +1,134 @@ +#include "ConnectionContextStore.h" +#include "CertificateUtils.h" +#include +#include +#include +#include + +#define CSR_FILE_NAME "app.csr" +#define SONAR_CA_FILE_NAME "sonarCA.crt" +#define CLIENT_CERT_FILE_NAME "device.crt" +#define PRIVATE_KEY_FILE "privateKey.pem" +#define CONNECTION_CONFIG_FILE "connection_config.json" + +#ifdef __ANDROID__ +#include +#define SONAR_LOG(message) \ + __android_log_print(ANDROID_LOG_INFO, "sonar", "sonar: %s", message) +#else +#define SONAR_LOG(message) printf("sonar: %s\n", message) +#endif + +using namespace facebook::sonar; + +bool fileExists(std::string fileName); +std::string loadStringFromFile(std::string fileName); +void writeStringToFile(std::string content, std::string fileName); + +ConnectionContextStore::ConnectionContextStore(DeviceData deviceData): deviceData_(deviceData) {} + +bool ConnectionContextStore::hasRequiredFiles() { + std::string caCert = loadStringFromFile(absoluteFilePath(SONAR_CA_FILE_NAME)); + std::string clientCert = + loadStringFromFile(absoluteFilePath(CLIENT_CERT_FILE_NAME)); + std::string privateKey = + loadStringFromFile(absoluteFilePath(PRIVATE_KEY_FILE)); + + if (caCert == "" || clientCert == "" || privateKey == "") { + return false; + } + return true; +} + +std::string ConnectionContextStore::createCertificateSigningRequest() { + ensureSonarDirExists(); + generateCertSigningRequest( + deviceData_.appId.c_str(), + absoluteFilePath(CSR_FILE_NAME).c_str(), + absoluteFilePath(PRIVATE_KEY_FILE).c_str()); + std::string csr = loadStringFromFile(absoluteFilePath(CSR_FILE_NAME)); + + return csr; +} + +std::shared_ptr ConnectionContextStore::getSSLContext() { + std::shared_ptr sslContext = + std::make_shared(); + sslContext->loadTrustedCertificates( + absoluteFilePath(SONAR_CA_FILE_NAME).c_str()); + sslContext->setVerificationOption( + folly::SSLContext::SSLVerifyPeerEnum::VERIFY); + sslContext->loadCertKeyPairFromFiles( + absoluteFilePath(CLIENT_CERT_FILE_NAME).c_str(), + absoluteFilePath(PRIVATE_KEY_FILE).c_str()); + sslContext->authenticate(true, false); + return sslContext; +} + +std::string ConnectionContextStore::getDeviceId() { + /* On android we can't reliably get the serial of the current device + So rely on our locally written config, which is provided by the + desktop app. + For backwards compatibility, when this isn't present, fall back to the + unreliable source. */ + std::string config = loadStringFromFile(absoluteFilePath(CONNECTION_CONFIG_FILE)); + auto maybeDeviceId = folly::parseJson(config)["deviceId"]; + return maybeDeviceId.isString() ? maybeDeviceId.getString() : deviceData_.deviceId; +} + +void ConnectionContextStore::storeConnectionConfig(folly::dynamic& config) { + std::string json = folly::toJson(config); + writeStringToFile(json, absoluteFilePath(CONNECTION_CONFIG_FILE)); +} + +std::string ConnectionContextStore::absoluteFilePath(const char* filename) { + return std::string(deviceData_.privateAppDirectory + "/sonar/" + filename); +} + +std::string ConnectionContextStore::getCertificateDirectoryPath() { + return absoluteFilePath(""); +} + +bool ConnectionContextStore::ensureSonarDirExists() { + std::string dirPath = absoluteFilePath(""); + struct stat info; + if (stat(dirPath.c_str(), &info) != 0) { + int ret = mkdir(dirPath.c_str(), S_IRUSR | S_IWUSR | S_IXUSR); + return ret == 0; + } else if (info.st_mode & S_IFDIR) { + return true; + } else { + SONAR_LOG(std::string( + "ERROR: Sonar path exists but is not a directory: " + dirPath) + .c_str()); + return false; + } +} + +std::string loadStringFromFile(std::string fileName) { + if (!fileExists(fileName)) { + return ""; + } + std::stringstream buffer; + std::ifstream stream; + std::string line; + stream.open(fileName.c_str()); + if (!stream) { + SONAR_LOG( + std::string("ERROR: Unable to open ifstream: " + fileName).c_str()); + return ""; + } + buffer << stream.rdbuf(); + std::string s = buffer.str(); + return s; +} + +void writeStringToFile(std::string content, std::string fileName) { + std::ofstream out(fileName); + out << content; +} + +bool fileExists(std::string fileName) { + struct stat buffer; + return stat(fileName.c_str(), &buffer) == 0; +} diff --git a/xplat/Sonar/ConnectionContextStore.h b/xplat/Sonar/ConnectionContextStore.h new file mode 100644 index 000000000..9235153a1 --- /dev/null +++ b/xplat/Sonar/ConnectionContextStore.h @@ -0,0 +1,33 @@ +#pragma once + +#include +#include +#include +#include "SonarInitConfig.h" + +using namespace folly; + +namespace facebook { +namespace sonar { + +class ConnectionContextStore { + +public: + ConnectionContextStore(DeviceData deviceData); + bool hasRequiredFiles(); + std::string createCertificateSigningRequest(); + std::shared_ptr getSSLContext(); + std::string getCertificateDirectoryPath(); + std::string getDeviceId(); + void storeConnectionConfig(folly::dynamic& config); + +private: + DeviceData deviceData_; + + std::string absoluteFilePath(const char* filename); + bool ensureSonarDirExists(); + +}; + +} // namespace sonar +} //namespace facebook diff --git a/xplat/Sonar/SonarClient.cpp b/xplat/Sonar/SonarClient.cpp index 01913095e..544e97604 100644 --- a/xplat/Sonar/SonarClient.cpp +++ b/xplat/Sonar/SonarClient.cpp @@ -12,6 +12,7 @@ #include "SonarState.h" #include "SonarStep.h" #include "SonarWebSocketImpl.h" +#include "ConnectionContextStore.h" #include #ifdef __ANDROID__ @@ -33,8 +34,9 @@ using folly::dynamic; void SonarClient::init(SonarInitConfig config) { auto state = std::make_shared(); + auto context = std::make_shared(config.deviceData); kInstance = - new SonarClient(std::make_unique(std::move(config), state), state); + new SonarClient(std::make_unique(std::move(config), state, context), state); } SonarClient* SonarClient::instance() { diff --git a/xplat/Sonar/SonarWebSocketImpl.cpp b/xplat/Sonar/SonarWebSocketImpl.cpp index e4e0e9fba..199512789 100644 --- a/xplat/Sonar/SonarWebSocketImpl.cpp +++ b/xplat/Sonar/SonarWebSocketImpl.cpp @@ -8,6 +8,7 @@ #include "SonarWebSocketImpl.h" #include "SonarStep.h" +#include "ConnectionContextStore.h" #include #include #include @@ -15,14 +16,9 @@ #include #include #include -#include -#include -#include -#include #include #include #include -#include "CertificateUtils.h" #ifdef __ANDROID__ #include @@ -32,11 +28,6 @@ #define SONAR_LOG(message) printf("sonar: %s\n", message) #endif -#define CSR_FILE_NAME "app.csr" -#define SONAR_CA_FILE_NAME "sonarCA.crt" -#define CLIENT_CERT_FILE_NAME "device.crt" -#define PRIVATE_KEY_FILE "privateKey.pem" -#define CONNECTION_CONFIG_FILE "connection_config.json" #define WRONG_THREAD_EXIT_MSG \ "ERROR: Aborting sonar initialization because it's not running in the sonar thread." @@ -48,9 +39,6 @@ static constexpr int insecurePort = 8089; namespace facebook { namespace sonar { -bool fileExists(std::string fileName); -void writeStringToFile(std::string content, std::string fileName); - class ConnectionEvents : public rsocket::RSocketConnectionEvents { private: SonarWebSocketImpl* websocket_; @@ -96,8 +84,8 @@ class Responder : public rsocket::RSocketResponder { } }; -SonarWebSocketImpl::SonarWebSocketImpl(SonarInitConfig config, std::shared_ptr state) - : deviceData_(config.deviceData), sonarState_(state), sonarEventBase_(config.callbackWorker), connectionEventBase_(config.connectionWorker) { +SonarWebSocketImpl::SonarWebSocketImpl(SonarInitConfig config, std::shared_ptr state, std::shared_ptr contextStore) + : deviceData_(config.deviceData), sonarState_(state), sonarEventBase_(config.callbackWorker), connectionEventBase_(config.connectionWorker), contextStore_(contextStore) { CHECK_THROW(config.callbackWorker, std::invalid_argument); CHECK_THROW(config.connectionWorker, std::invalid_argument); } @@ -175,7 +163,6 @@ void SonarWebSocketImpl::doCertificateExchange() { .get(); connectingInsecurely->complete(); - ensureSonarDirExists(); requestSignedCertFromSonar(); } @@ -183,24 +170,17 @@ void SonarWebSocketImpl::connectSecurely() { rsocket::SetupParameters parameters; folly::SocketAddress address; - auto deviceId = getDeviceId(); - + auto loadingDeviceId = sonarState_->start("Load Device Id"); + auto deviceId = contextStore_->getDeviceId(); + if (deviceId.compare("unknown")) { + loadingDeviceId->complete(); + } parameters.payload = rsocket::Payload(folly::toJson(folly::dynamic::object( "os", deviceData_.os)("device", deviceData_.device)( "device_id", deviceId)("app", deviceData_.app))); address.setFromHostPort(deviceData_.host, securePort); - std::shared_ptr sslContext = - std::make_shared(); - sslContext->loadTrustedCertificates( - absoluteFilePath(SONAR_CA_FILE_NAME).c_str()); - sslContext->setVerificationOption( - folly::SSLContext::SSLVerifyPeerEnum::VERIFY); - sslContext->loadCertKeyPairFromFiles( - absoluteFilePath(CLIENT_CERT_FILE_NAME).c_str(), - absoluteFilePath(PRIVATE_KEY_FILE).c_str()); - sslContext->authenticate(true, false); - + std::shared_ptr sslContext = contextStore_->getSSLContext(); auto connectingSecurely = sonarState_->start("Connect securely"); connectionIsTrusted_ = true; client_ = @@ -251,27 +231,6 @@ void SonarWebSocketImpl::sendMessage(const folly::dynamic& message) { }); } -std::string SonarWebSocketImpl::getDeviceId() { - /* On android we can't reliably get the serial of the current device - So rely on our locally written config, which is provided by the - desktop app. - For backwards compatibility, when this isn't present, fall back to the - unreliable source. */ - auto gettingDeviceId = sonarState_->start("Get deviceId"); - std::string config = loadStringFromFile(absoluteFilePath(CONNECTION_CONFIG_FILE)); - auto maybeDeviceId = folly::parseJson(config)["deviceId"]; - std::string deviceId; - if (maybeDeviceId.isString()) { - deviceId = maybeDeviceId.getString(); - } else { - deviceId = deviceData_.deviceId; - } - if (deviceId.compare("unknown")) { - gettingDeviceId->complete(); - } - return deviceId; -} - bool SonarWebSocketImpl::isCertificateExchangeNeeded() { if (failedConnectionAttempts_ >= 2) { @@ -279,32 +238,20 @@ bool SonarWebSocketImpl::isCertificateExchangeNeeded() { } auto step = sonarState_->start("Check required certificates are present"); - std::string caCert = loadStringFromFile(absoluteFilePath(SONAR_CA_FILE_NAME)); - std::string clientCert = - loadStringFromFile(absoluteFilePath(CLIENT_CERT_FILE_NAME)); - std::string privateKey = - loadStringFromFile(absoluteFilePath(PRIVATE_KEY_FILE)); - - if (caCert == "" || clientCert == "" || privateKey == "") { - return true; + bool hasRequiredFiles = contextStore_->hasRequiredFiles(); + if (hasRequiredFiles) { + step->complete(); } - step->complete(); - return false; + return !hasRequiredFiles; } void SonarWebSocketImpl::requestSignedCertFromSonar() { auto generatingCSR = sonarState_->start("Generate CSR"); - generateCertSigningRequest( - deviceData_.appId.c_str(), - absoluteFilePath(CSR_FILE_NAME).c_str(), - absoluteFilePath(PRIVATE_KEY_FILE).c_str()); + std::string csr = contextStore_->createCertificateSigningRequest(); generatingCSR->complete(); - auto loadingCSR = sonarState_->start("Load CSR"); - std::string csr = loadStringFromFile(absoluteFilePath(CSR_FILE_NAME)); - loadingCSR->complete(); folly::dynamic message = folly::dynamic::object("method", "signCertificate")( - "csr", csr.c_str())("destination", absoluteFilePath("").c_str()); + "csr", csr.c_str())("destination", contextStore_->getCertificateDirectoryPath().c_str()); auto gettingCert = sonarState_->start("Getting cert from desktop"); sonarEventBase_->add([this, message, gettingCert]() { @@ -312,7 +259,10 @@ void SonarWebSocketImpl::requestSignedCertFromSonar() { ->requestResponse(rsocket::Payload(folly::toJson(message))) ->subscribe([this, gettingCert](rsocket::Payload p) { auto response = p.moveDataToString(); - writeStringToFile(response, absoluteFilePath(CONNECTION_CONFIG_FILE)); + if (!response.empty()) { + folly::dynamic config = folly::parseJson(response); + contextStore_->storeConnectionConfig(config); + } gettingCert->complete(); SONAR_LOG("Certificate exchange complete."); // Disconnect after message sending is complete. @@ -352,57 +302,9 @@ void SonarWebSocketImpl::sendLegacyCertificateRequest(folly::dynamic message) { }); } -std::string SonarWebSocketImpl::loadStringFromFile(std::string fileName) { - if (!fileExists(fileName)) { - return ""; - } - std::stringstream buffer; - std::ifstream stream; - std::string line; - stream.open(fileName.c_str()); - if (!stream) { - SONAR_LOG( - std::string("ERROR: Unable to open ifstream: " + fileName).c_str()); - return ""; - } - buffer << stream.rdbuf(); - std::string s = buffer.str(); - return s; -} - -void writeStringToFile(std::string content, std::string fileName) { - std::ofstream out(fileName); - out << content; -} - -std::string SonarWebSocketImpl::absoluteFilePath(const char* filename) { - return std::string(deviceData_.privateAppDirectory + "/sonar/" + filename); -} - -bool SonarWebSocketImpl::ensureSonarDirExists() { - std::string dirPath = absoluteFilePath(""); - struct stat info; - if (stat(dirPath.c_str(), &info) != 0) { - int ret = mkdir(dirPath.c_str(), S_IRUSR | S_IWUSR | S_IXUSR); - return ret == 0; - } else if (info.st_mode & S_IFDIR) { - return true; - } else { - SONAR_LOG(std::string( - "ERROR: Sonar path exists but is not a directory: " + dirPath) - .c_str()); - return false; - } -} - bool SonarWebSocketImpl::isRunningInOwnThread() { return sonarEventBase_->isInEventBaseThread(); } -bool fileExists(std::string fileName) { - struct stat buffer; - return stat(fileName.c_str(), &buffer) == 0; -} - } // namespace sonar } // namespace facebook diff --git a/xplat/Sonar/SonarWebSocketImpl.h b/xplat/Sonar/SonarWebSocketImpl.h index 5c6030c15..75991bbf9 100644 --- a/xplat/Sonar/SonarWebSocketImpl.h +++ b/xplat/Sonar/SonarWebSocketImpl.h @@ -20,6 +20,7 @@ namespace facebook { namespace sonar { class ConnectionEvents; +class ConnectionContextStore; class Responder; class SonarWebSocketImpl : public SonarWebSocket { @@ -27,7 +28,7 @@ class SonarWebSocketImpl : public SonarWebSocket { friend Responder; public: - SonarWebSocketImpl(SonarInitConfig config, std::shared_ptr state); + SonarWebSocketImpl(SonarInitConfig config, std::shared_ptr state, std::shared_ptr contextStore); ~SonarWebSocketImpl(); @@ -54,16 +55,13 @@ class SonarWebSocketImpl : public SonarWebSocket { std::unique_ptr client_; bool connectionIsTrusted_; int failedConnectionAttempts_ = 0; + std::shared_ptr contextStore_; void startSync(); void doCertificateExchange(); void connectSecurely(); - std::string loadCSRFromFile(); - std::string loadStringFromFile(std::string fileName); - std::string absoluteFilePath(const char* relativeFilePath); bool isCertificateExchangeNeeded(); void requestSignedCertFromSonar(); - bool ensureSonarDirExists(); bool isRunningInOwnThread(); void sendLegacyCertificateRequest(folly::dynamic message); std::string getDeviceId(); diff --git a/xplat/SonarTestLib/ConnectionContextStoreMock.h b/xplat/SonarTestLib/ConnectionContextStoreMock.h new file mode 100644 index 000000000..5befa0129 --- /dev/null +++ b/xplat/SonarTestLib/ConnectionContextStoreMock.h @@ -0,0 +1,28 @@ +#include + +namespace facebook { +namespace sonar { +namespace test { + + class ConnectionContextStoreMock : public ConnectionContextStore { + public: + ConnectionContextStoreMock() : ConnectionContextStore(DeviceData()) { + } + bool hasRequiredFiles() { + return true; + } + std::string createCertificateSigningRequest() { + return "thisIsACsr"; + } + std::shared_ptr getSSLContext() { + return nullptr; + } + dynamic getConnectionConfig() { + return nullptr; + } + std::string getCertificateDirectoryPath() { + return "/something/sonar/"; + } + }; + +}}} diff --git a/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp b/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp index 11f5cd435..846b6f710 100644 --- a/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp +++ b/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp @@ -7,6 +7,7 @@ */ #include +#include #include @@ -18,10 +19,14 @@ using folly::EventBase; class SonarWebSocketImplTerminationTest : public ::testing::Test { protected: + std::shared_ptr state; + std::shared_ptr contextStore; void SetUp() override { // Folly singletons must be registered before they are used. // Without this, test fails in phabricator. folly::SingletonVault::singleton()->registrationComplete(); + state = std::make_shared(); + contextStore = std::make_shared(); } }; @@ -31,7 +36,10 @@ TEST_F(SonarWebSocketImplTerminationTest, testNullEventBaseGetsRejected) { DeviceData {}, nullptr, new EventBase() - }, std::make_shared()); + }, + state, + contextStore + ); FAIL(); } catch (std::invalid_argument& e) { // Pass test @@ -41,7 +49,10 @@ TEST_F(SonarWebSocketImplTerminationTest, testNullEventBaseGetsRejected) { DeviceData {}, new EventBase(), nullptr - }, std::make_shared()); + }, + state, + contextStore + ); FAIL(); } catch (std::invalid_argument& e) { // Pass test @@ -54,8 +65,7 @@ TEST_F(SonarWebSocketImplTerminationTest, testNonStartedEventBaseDoesntHang) { new EventBase(), new EventBase() }; - auto state = std::make_shared(); - auto instance = std::make_shared(config, state); + auto instance = std::make_shared(config, state, contextStore); instance->start(); } @@ -73,8 +83,7 @@ TEST_F(SonarWebSocketImplTerminationTest, testStartedEventBaseDoesntHang) { sonarEventBase, connectionEventBase }; - auto state = std::make_shared(); - auto instance = std::make_shared(config, state); + auto instance = std::make_shared(config, state, contextStore); instance->start();