From d510ee0e47793a664af4fb0130f2cdd5e1e1ed9b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 22 Feb 2016 18:00:05 -0800 Subject: [PATCH] complete ice-server signature verification --- domain-server/src/DomainGatekeeper.cpp | 1 - domain-server/src/DomainServer.cpp | 2 +- ice-server/CMakeLists.txt | 14 ++++++ ice-server/src/IceServer.cpp | 44 ++++++++++++++++--- libraries/networking/src/AccountManager.cpp | 2 +- .../networking/src/DataServerAccountInfo.cpp | 13 +++--- 6 files changed, 61 insertions(+), 15 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 3e4ee7b758..f385f5c489 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -331,7 +331,6 @@ bool DomainGatekeeper::verifyUserSignature(const QString& username, QCryptographicHash::Sha256); if (rsaPublicKey) { - QByteArray decryptedArray(RSA_size(rsaPublicKey), 0); int decryptResult = RSA_verify(NID_sha256, reinterpret_cast(usernameWithToken.constData()), usernameWithToken.size(), diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5e5a843fae..46aa9ab2ee 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1099,7 +1099,7 @@ void DomainServer::sendHeartbeatToIceServer() { auto signature = accountManager.getAccountInfo().signPlaintext(plaintext); // pack the signature with the data - heartbeatDataStream << plaintext; + heartbeatDataStream << signature; } // send the heartbeat packet to the ice server now diff --git a/ice-server/CMakeLists.txt b/ice-server/CMakeLists.txt index cfec3c966c..e5bdffe2e2 100644 --- a/ice-server/CMakeLists.txt +++ b/ice-server/CMakeLists.txt @@ -6,3 +6,17 @@ setup_hifi_project(Network) # link the shared hifi libraries link_hifi_libraries(embedded-webserver networking shared) package_libraries_for_deployment() + +# find OpenSSL +find_package(OpenSSL REQUIRED) + +if (APPLE AND ${OPENSSL_INCLUDE_DIR} STREQUAL "/usr/include") + # this is a user on OS X using system OpenSSL, which is going to throw warnings since they're deprecating for their common crypto + message(WARNING "The found version of OpenSSL is the OS X system version. This will produce deprecation warnings." + "\nWe recommend you install a newer version (at least 1.0.1h) in a different directory and set OPENSSL_ROOT_DIR in your env so Cmake can find it.") +endif () + +include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}") + +# append OpenSSL to our list of libraries to link +target_link_libraries(${TARGET_NAME} ${OPENSSL_LIBRARIES}) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index 31f576fa03..d0dad812aa 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -11,6 +11,9 @@ #include "IceServer.h" +#include +#include + #include #include #include @@ -159,14 +162,43 @@ bool IceServer::isVerifiedHeartbeat(const QUuid& domainID, const QByteArray& pla auto it = _domainPublicKeys.find(domainID); if (it != _domainPublicKeys.end()) { - return true; - } else { - // we don't have a public key for this domain so we can't verify this heartbeat - // ask the metaverse API for the right public key and return false to indicate that this is not verified - requestDomainPublicKey(domainID); + // attempt to verify the signature for this heartbeat + const unsigned char* publicKeyData = reinterpret_cast(it->second.constData()); - return false; + // first load up the public key into an RSA struct + RSA* rsaPublicKey = d2i_RSA_PUBKEY(NULL, &publicKeyData, it->second.size()); + + if (rsaPublicKey) { + auto hashedPlaintext = QCryptographicHash::hash(plaintext, QCryptographicHash::Sha256); + int verificationResult = RSA_verify(NID_sha256, + reinterpret_cast(hashedPlaintext.constData()), + hashedPlaintext.size(), + reinterpret_cast(signature.constData()), + signature.size(), + rsaPublicKey); + + // free up the public key and remove connection token before we return + RSA_free(rsaPublicKey); + + if (verificationResult == 1) { + // this is the only success case - we return true here to indicate that the heartbeat is verified + return true; + } else { + qDebug() << "Failed to verify heartbeat for" << domainID << "- re-requesting public key from API."; + } + + } else { + // we can't let this user in since we couldn't convert their public key to an RSA key we could use + qWarning() << "Could not convert in-memory public key for" << domainID << "to usable RSA public key."; + qWarning() << "Re-requesting public key from API"; + } } + + // we could not verify this heartbeat (missing public key, could not load public key, bad actor) + // ask the metaverse API for the right public key and return false to indicate that this is not verified + requestDomainPublicKey(domainID); + + return false; } void IceServer::requestDomainPublicKey(const QUuid& domainID) { diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index a24b0b471c..fc1d8e9458 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -33,7 +33,7 @@ #include "AccountManager.h" #include "NetworkLogging.h" -const bool VERBOSE_HTTP_REQUEST_DEBUGGING = true; +const bool VERBOSE_HTTP_REQUEST_DEBUGGING = false; AccountManager& AccountManager::getInstance(bool forceReset) { static std::unique_ptr sharedInstance(new AccountManager()); diff --git a/libraries/networking/src/DataServerAccountInfo.cpp b/libraries/networking/src/DataServerAccountInfo.cpp index 706da1ce72..9455fb1b88 100644 --- a/libraries/networking/src/DataServerAccountInfo.cpp +++ b/libraries/networking/src/DataServerAccountInfo.cpp @@ -117,11 +117,10 @@ void DataServerAccountInfo::setProfileInfoFromJSON(const QJsonObject& jsonObject } QByteArray DataServerAccountInfo::getUsernameSignature(const QUuid& connectionToken) { - QByteArray lowercaseUsername = _username.toLower().toUtf8(); - QByteArray usernameWithToken = QCryptographicHash::hash(lowercaseUsername.append(connectionToken.toRfc4122()), - QCryptographicHash::Sha256); + auto lowercaseUsername = _username.toLower().toUtf8(); + auto plaintext = lowercaseUsername.append(connectionToken.toRfc4122()); - auto signature = signPlaintext(usernameWithToken); + auto signature = signPlaintext(plaintext); if (!signature.isEmpty()) { qDebug(networking) << "Returning username" << _username << "signed with connection UUID" << uuidStringWithoutCurlyBraces(connectionToken); @@ -143,9 +142,11 @@ QByteArray DataServerAccountInfo::signPlaintext(const QByteArray& plaintext) { QByteArray signature(RSA_size(rsaPrivateKey), 0); unsigned int signatureBytes = 0; + QByteArray hashedPlaintext = QCryptographicHash::hash(plaintext, QCryptographicHash::Sha256); + int encryptReturn = RSA_sign(NID_sha256, - reinterpret_cast(plaintext.constData()), - plaintext.size(), + reinterpret_cast(hashedPlaintext.constData()), + hashedPlaintext.size(), reinterpret_cast(signature.data()), &signatureBytes, rsaPrivateKey);