From c6383f042ce6937d7cbc7081075ed9b8c4f731b7 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Fri, 21 Dec 2018 18:08:07 -0800 Subject: [PATCH] Pin server cert to host during pairing --- app/backend/boxartmanager.cpp | 2 +- app/backend/computermanager.cpp | 14 +++++++------- app/backend/nvcomputer.cpp | 15 ++++++++++++++- app/backend/nvcomputer.h | 3 ++- app/backend/nvhttp.cpp | 18 ++++++++++++++---- app/backend/nvhttp.h | 3 ++- app/backend/nvpairingmanager.cpp | 13 +++++++------ app/backend/nvpairingmanager.h | 2 +- app/streaming/session.cpp | 4 ++-- 9 files changed, 50 insertions(+), 24 deletions(-) diff --git a/app/backend/boxartmanager.cpp b/app/backend/boxartmanager.cpp index 111bf7b1..12e95531 100644 --- a/app/backend/boxartmanager.cpp +++ b/app/backend/boxartmanager.cpp @@ -97,7 +97,7 @@ void BoxArtManager::handleBoxArtLoadComplete(NvComputer* computer, NvApp app, QU QUrl BoxArtManager::loadBoxArtFromNetwork(NvComputer* computer, int appId) { - NvHTTP http(computer->activeAddress); + NvHTTP http(computer->activeAddress, computer->serverCert); QString cachePath = getFilePathForBoxArt(computer, appId); QImage image; diff --git a/app/backend/computermanager.cpp b/app/backend/computermanager.cpp index 4b1970f0..d6357e47 100644 --- a/app/backend/computermanager.cpp +++ b/app/backend/computermanager.cpp @@ -27,7 +27,7 @@ public: private: bool tryPollComputer(QString address, bool& changed) { - NvHTTP http(address); + NvHTTP http(address, m_Computer->serverCert); QString serverInfo; try { @@ -36,7 +36,7 @@ private: return false; } - NvComputer newState(address, serverInfo); + NvComputer newState(address, serverInfo, QSslCertificate()); // Ensure the machine that responded is the one we intended to contact if (m_Computer->uuid != newState.uuid) { @@ -52,7 +52,7 @@ private: { Q_ASSERT(m_Computer->activeAddress != nullptr); - NvHTTP http(m_Computer->activeAddress); + NvHTTP http(m_Computer->activeAddress, m_Computer->serverCert); QVector appList; @@ -385,7 +385,7 @@ private: NvPairingManager pairingManager(m_Computer->activeAddress); try { - NvPairingManager::PairState result = pairingManager.pair(m_Computer->appVersion, m_Pin); + NvPairingManager::PairState result = pairingManager.pair(m_Computer->appVersion, m_Pin, m_Computer->serverCert); switch (result) { case NvPairingManager::PairState::PIN_WRONG: @@ -438,7 +438,7 @@ signals: private: void run() { - NvHTTP http(m_Computer->activeAddress); + NvHTTP http(m_Computer->activeAddress, m_Computer->serverCert); try { if (m_Computer->currentGameId != 0) { @@ -540,7 +540,7 @@ signals: private: void run() { - NvHTTP http(m_Address); + NvHTTP http(m_Address, QSslCertificate()); qInfo() << "Processing new PC at" << m_Address << "from" << (m_Mdns ? "mDNS" : "user"); @@ -571,7 +571,7 @@ private: return; } - NvComputer* newComputer = new NvComputer(m_Address, serverInfo); + NvComputer* newComputer = new NvComputer(m_Address, serverInfo, QSslCertificate()); // Update addresses depending on the context if (m_Mdns) { diff --git a/app/backend/nvcomputer.cpp b/app/backend/nvcomputer.cpp index 75f1cde5..1cb1bae9 100644 --- a/app/backend/nvcomputer.cpp +++ b/app/backend/nvcomputer.cpp @@ -10,6 +10,7 @@ #define SER_REMOTEADDR "remoteaddress" #define SER_MANUALADDR "manualaddress" #define SER_APPLIST "apps" +#define SER_SRVCERT "srvcert" #define SER_APPNAME "name" #define SER_APPID "id" @@ -23,6 +24,7 @@ NvComputer::NvComputer(QSettings& settings) this->localAddress = settings.value(SER_LOCALADDR).toString(); this->remoteAddress = settings.value(SER_REMOTEADDR).toString(); this->manualAddress = settings.value(SER_MANUALADDR).toString(); + this->serverCert = QSslCertificate(settings.value(SER_SRVCERT).toByteArray()); int appCount = settings.beginReadArray(SER_APPLIST); for (int i = 0; i < appCount; i++) { @@ -61,6 +63,7 @@ void NvComputer::serialize(QSettings& settings) settings.setValue(SER_LOCALADDR, localAddress); settings.setValue(SER_REMOTEADDR, remoteAddress); settings.setValue(SER_MANUALADDR, manualAddress); + settings.setValue(SER_SRVCERT, serverCert.toPem()); // Avoid deleting an existing applist if we couldn't get one if (!appList.isEmpty()) { @@ -84,8 +87,10 @@ void NvComputer::sortAppList() }); } -NvComputer::NvComputer(QString address, QString serverInfo) +NvComputer::NvComputer(QString address, QString serverInfo, QSslCertificate serverCert) { + this->serverCert = serverCert; + this->name = NvHTTP::getXmlString(serverInfo, "hostname"); if (this->name.isEmpty()) { this->name = "UNKNOWN"; @@ -255,6 +260,13 @@ bool NvComputer::update(NvComputer& that) changed = true; \ } +#define ASSIGN_IF_CHANGED_AND_NONNULL(field) \ + if (!that.field.isNull() && \ + this->field != that.field) { \ + this->field = that.field; \ + changed = true; \ + } + ASSIGN_IF_CHANGED(name); ASSIGN_IF_CHANGED_AND_NONEMPTY(macAddress); ASSIGN_IF_CHANGED_AND_NONEMPTY(localAddress); @@ -269,6 +281,7 @@ bool NvComputer::update(NvComputer& that) ASSIGN_IF_CHANGED(appVersion); ASSIGN_IF_CHANGED(maxLumaPixelsHEVC); ASSIGN_IF_CHANGED(gpuModel); + ASSIGN_IF_CHANGED_AND_NONNULL(serverCert); ASSIGN_IF_CHANGED_AND_NONEMPTY(appList); ASSIGN_IF_CHANGED_AND_NONEMPTY(displayModes); return changed; diff --git a/app/backend/nvcomputer.h b/app/backend/nvcomputer.h index 542a3779..c848dfd3 100644 --- a/app/backend/nvcomputer.h +++ b/app/backend/nvcomputer.h @@ -19,7 +19,7 @@ private: bool pendingQuit; public: - explicit NvComputer(QString address, QString serverInfo); + explicit NvComputer(QString address, QString serverInfo, QSslCertificate serverCert); explicit NvComputer(QSettings& settings); @@ -68,6 +68,7 @@ public: QByteArray macAddress; QString name; QString uuid; + QSslCertificate serverCert; QVector appList; // Synchronization diff --git a/app/backend/nvhttp.cpp b/app/backend/nvhttp.cpp index 3cb713ea..c9a08001 100644 --- a/app/backend/nvhttp.cpp +++ b/app/backend/nvhttp.cpp @@ -14,8 +14,9 @@ #define REQUEST_TIMEOUT_MS 5000 -NvHTTP::NvHTTP(QString address) : - m_Address(address) +NvHTTP::NvHTTP(QString address, QSslCertificate serverCert) : + m_Address(address), + m_ServerCert(serverCert) { Q_ASSERT(!address.isEmpty()); @@ -390,8 +391,17 @@ NvHTTP::openConnection(QUrl baseUrl, QNetworkReply* reply = m_Nam.get(request); - // Ignore self-signed certificate errors (since GFE uses them) - reply->ignoreSslErrors(); + if (m_ServerCert.isNull()) { + // No server cert yet + reply->ignoreSslErrors(); + } + else { + // Pin the server certificate received during pairing + QList expectedSslErrors; + expectedSslErrors.append(QSslError(QSslError::HostNameMismatch, m_ServerCert)); + expectedSslErrors.append(QSslError(QSslError::SelfSignedCertificate, m_ServerCert)); + reply->ignoreSslErrors(expectedSslErrors); + } // Run the request with a timeout if requested QEventLoop loop; diff --git a/app/backend/nvhttp.h b/app/backend/nvhttp.h index 1ebb9a7b..9a59f777 100644 --- a/app/backend/nvhttp.h +++ b/app/backend/nvhttp.h @@ -122,7 +122,7 @@ public: VERBOSE }; - explicit NvHTTP(QString address); + explicit NvHTTP(QString address, QSslCertificate serverCert); static int @@ -191,4 +191,5 @@ private: QString m_Address; QNetworkAccessManager m_Nam; + QSslCertificate m_ServerCert; }; diff --git a/app/backend/nvpairingmanager.cpp b/app/backend/nvpairingmanager.cpp index 049f977a..5d195315 100644 --- a/app/backend/nvpairingmanager.cpp +++ b/app/backend/nvpairingmanager.cpp @@ -9,7 +9,7 @@ #include NvPairingManager::NvPairingManager(QString address) : - m_Http(address) + m_Http(address, QSslCertificate()) { QByteArray cert = IdentityManager::get()->getCertificate(); BIO *bio = BIO_new_mem_buf(cert.data(), -1); @@ -161,7 +161,7 @@ NvPairingManager::saltPin(QByteArray salt, QString pin) } NvPairingManager::PairState -NvPairingManager::pair(QString appVersion, QString pin) +NvPairingManager::pair(QString appVersion, QString pin, QSslCertificate& serverCert) { int serverMajorVersion = NvHTTP::parseQuad(appVersion).at(0); qInfo() << "Pairing with server generation:" << serverMajorVersion; @@ -200,8 +200,8 @@ NvPairingManager::pair(QString appVersion, QString pin) return PairState::FAILED; } - QByteArray serverCert = NvHTTP::getXmlStringFromHex(getCert, "plaincert"); - if (serverCert == nullptr) + QByteArray serverCertStr = NvHTTP::getXmlStringFromHex(getCert, "plaincert"); + if (serverCertStr == nullptr) { qCritical() << "Server likely already pairing"; m_Http.openConnectionToString(m_Http.m_BaseUrlHttp, "unpair", nullptr, true); @@ -262,7 +262,7 @@ NvPairingManager::pair(QString appVersion, QString pin) if (!verifySignature(serverSecret, serverSignature, - serverCert)) + serverCertStr)) { qCritical() << "MITM detected"; m_Http.openConnectionToString(m_Http.m_BaseUrlHttp, "unpair", nullptr, true); @@ -271,7 +271,7 @@ NvPairingManager::pair(QString appVersion, QString pin) QByteArray expectedResponseData; expectedResponseData.append(randomChallenge); - expectedResponseData.append(getSignatureFromPemCert(serverCert)); + expectedResponseData.append(getSignatureFromPemCert(serverCertStr)); expectedResponseData.append(serverSecret); if (QCryptographicHash::hash(expectedResponseData, hashAlgo) != serverResponse) { @@ -309,5 +309,6 @@ NvPairingManager::pair(QString appVersion, QString pin) return PairState::FAILED; } + serverCert = QSslCertificate(serverCertStr); return PairState::PAIRED; } diff --git a/app/backend/nvpairingmanager.h b/app/backend/nvpairingmanager.h index b5778dfa..ecaed5c7 100644 --- a/app/backend/nvpairingmanager.h +++ b/app/backend/nvpairingmanager.h @@ -23,7 +23,7 @@ public: ~NvPairingManager(); PairState - pair(QString appVersion, QString pin); + pair(QString appVersion, QString pin, QSslCertificate& serverCert); private: QByteArray diff --git a/app/streaming/session.cpp b/app/streaming/session.cpp index 6a39a3f7..b356d562 100644 --- a/app/streaming/session.cpp +++ b/app/streaming/session.cpp @@ -593,7 +593,7 @@ private: // Perform a best-effort app quit if (shouldQuit) { - NvHTTP http(m_Session->m_Computer->activeAddress); + NvHTTP http(m_Session->m_Computer->activeAddress, m_Session->m_Computer->serverCert); // Logging is already done inside NvHTTP try { @@ -812,7 +812,7 @@ void Session::exec(int displayOriginX, int displayOriginY) m_Computer->currentGameId == m_App.id); try { - NvHTTP http(m_Computer->activeAddress); + NvHTTP http(m_Computer->activeAddress, m_Computer->serverCert); if (m_Computer->currentGameId != 0) { http.resumeApp(&m_StreamConfig); }