Require cert pinning for HTTPS

app/backend/computermanager.cpp

@@ -538,12 +538,8 @@ signals:
     void computerStateChanged(NvComputer* computer);
 
 private:
-    void run()
+    QString fetchServerInfo(NvHTTP& http)
     {
-        NvHTTP http(m_Address, QSslCertificate());
-
-        qInfo() << "Processing new PC at" << m_Address << "from" << (m_Mdns ? "mDNS" : "user");
-
         QString serverInfo;
         try {
             // There's a race condition between GameStream servers reporting presence over
@@ -564,15 +560,52 @@ private:
                     throw e;
                 }
             }
+            return serverInfo;
         } catch (...) {
             if (!m_Mdns) {
                 emit computerAddCompleted(false);
             }
+            return QString();
+        }
+    }
+
+    void run()
+    {
+        NvHTTP http(m_Address, QSslCertificate());
+
+        qInfo() << "Processing new PC at" << m_Address << "from" << (m_Mdns ? "mDNS" : "user");
+
+        // Perform initial serverinfo fetch over HTTP since we don't know which cert to use
+        QString serverInfo = fetchServerInfo(http);
+        if (serverInfo.isEmpty()) {
             return;
         }
 
+        // Create initial newComputer using HTTP serverinfo with no pinned cert
         NvComputer* newComputer = new NvComputer(m_Address, serverInfo, QSslCertificate());
 
+        // Check if we have a record of this host UUID to pull the pinned cert
+        NvComputer* existingComputer;
+        {
+            QReadLocker lock(&m_ComputerManager->m_Lock);
+            existingComputer = m_ComputerManager->m_KnownHosts[newComputer->uuid];
+            if (existingComputer != nullptr) {
+                http.setServerCert(existingComputer->serverCert);
+            }
+        }
+
+        // Fetch serverinfo again over HTTPS with the pinned cert
+        if (existingComputer != nullptr) {
+            serverInfo = fetchServerInfo(http);
+            if (serverInfo.isEmpty()) {
+                return;
+            }
+
+            // Update the polled computer with the HTTPS information
+            NvComputer httpsComputer(m_Address, serverInfo, QSslCertificate());
+            newComputer->update(httpsComputer);
+        }
+
         // Update addresses depending on the context
         if (m_Mdns) {
             newComputer->localAddress = m_Address;
@@ -591,46 +624,48 @@ private:
             newComputer->manualAddress = m_Address;
         }
 
-        // Check if this PC already exists
+        {
-        QWriteLocker lock(&m_ComputerManager->m_Lock);
+            // Check if this PC already exists
-        NvComputer* existingComputer = m_ComputerManager->m_KnownHosts[newComputer->uuid];
+            QWriteLocker lock(&m_ComputerManager->m_Lock);
-        if (existingComputer != nullptr) {
+            NvComputer* existingComputer = m_ComputerManager->m_KnownHosts[newComputer->uuid];
-            // Fold it into the existing PC
+            if (existingComputer != nullptr) {
-            bool changed = existingComputer->update(*newComputer);
+                // Fold it into the existing PC
-            delete newComputer;
+                bool changed = existingComputer->update(*newComputer);
+                delete newComputer;
 
-            // Drop the lock before notifying
+                // Drop the lock before notifying
-            lock.unlock();
+                lock.unlock();
 
-            // For non-mDNS clients, let them know it succeeded
+                // For non-mDNS clients, let them know it succeeded
-            if (!m_Mdns) {
+                if (!m_Mdns) {
-                emit computerAddCompleted(true);
+                    emit computerAddCompleted(true);
+                }
+
+                // Tell our client if something changed
+                if (changed) {
+                    qInfo() << existingComputer->name << "is now at" << existingComputer->activeAddress;
+                    emit computerStateChanged(existingComputer);
+                }
             }
+            else {
+                // Store this in our active sets
+                m_ComputerManager->m_KnownHosts[newComputer->uuid] = newComputer;
 
-            // Tell our client if something changed
+                // Start polling if enabled (write lock required)
-            if (changed) {
+                m_ComputerManager->startPollingComputer(newComputer);
-                qInfo() << existingComputer->name << "is now at" << existingComputer->activeAddress;
+
-                emit computerStateChanged(existingComputer);
+                // Drop the lock before notifying
+                lock.unlock();
+
+                // For non-mDNS clients, let them know it succeeded
+                if (!m_Mdns) {
+                    emit computerAddCompleted(true);
+                }
+
+                // Tell our client about this new PC
+                emit computerStateChanged(newComputer);
             }
         }
-        else {
-            // Store this in our active sets
-            m_ComputerManager->m_KnownHosts[newComputer->uuid] = newComputer;
-
-            // Start polling if enabled (write lock required)
-            m_ComputerManager->startPollingComputer(newComputer);
-
-            // Drop the lock before notifying
-            lock.unlock();
-
-            // For non-mDNS clients, let them know it succeeded
-            if (!m_Mdns) {
-                emit computerAddCompleted(true);
-            }
-
-            // Tell our client about this new PC
-            emit computerStateChanged(newComputer);
-        }
     }
 
     ComputerManager* m_ComputerManager;

app/backend/nvhttp.cpp

@@ -32,6 +32,11 @@ NvHTTP::NvHTTP(QString address, QSslCertificate serverCert) :
     m_Nam.setProxy(noProxy);
 }
 
+void NvHTTP::setServerCert(QSslCertificate serverCert)
+{
+    m_ServerCert = serverCert;
+}
+
 QVector<int>
 NvHTTP::parseQuad(QString quad)
 {
@@ -74,36 +79,50 @@ NvHTTP::getServerInfo(NvLogLevel logLevel)
 {
     QString serverInfo;
 
-    try
+    // Check if we have a pinned cert for this host yet
+    if (!m_ServerCert.isNull())
     {
-        // Always try HTTPS first, since it properly reports
+        try
-        // pairing status (and a few other attributes).
-        serverInfo = openConnectionToString(m_BaseUrlHttps,
-                                            "serverinfo",
-                                            nullptr,
-                                            true,
-                                            logLevel);
-        // Throws if the request failed
-        verifyResponseStatus(serverInfo);
-    }
-    catch (const GfeHttpResponseException& e)
-    {
-        if (e.getStatusCode() == 401)
         {
-            // Certificate validation error, fallback to HTTP
+            // Always try HTTPS first, since it properly reports
-            serverInfo = openConnectionToString(m_BaseUrlHttp,
+            // pairing status (and a few other attributes).
+            serverInfo = openConnectionToString(m_BaseUrlHttps,
                                                 "serverinfo",
                                                 nullptr,
                                                 true,
                                                 logLevel);
+            // Throws if the request failed
             verifyResponseStatus(serverInfo);
         }
-        else
+        catch (const GfeHttpResponseException& e)
         {
-            // Rethrow real errors
+            if (e.getStatusCode() == 401)
-            throw e;
+            {
+                // Certificate validation error, fallback to HTTP
+                serverInfo = openConnectionToString(m_BaseUrlHttp,
+                                                    "serverinfo",
+                                                    nullptr,
+                                                    true,
+                                                    logLevel);
+                verifyResponseStatus(serverInfo);
+            }
+            else
+            {
+                // Rethrow real errors
+                throw e;
+            }
         }
     }
+    else
+    {
+        // Only use HTTP prior to pairing
+        serverInfo = openConnectionToString(m_BaseUrlHttp,
+                                            "serverinfo",
+                                            nullptr,
+                                            true,
+                                            logLevel);
+        verifyResponseStatus(serverInfo);
+    }
 
     return serverInfo;
 }
@@ -391,11 +410,7 @@ NvHTTP::openConnection(QUrl baseUrl,
 
     QNetworkReply* reply = m_Nam.get(request);
 
-    if (m_ServerCert.isNull()) {
+    if (!m_ServerCert.isNull()) {
-        // No server cert yet
-        reply->ignoreSslErrors();
-    }
-    else {
         // Pin the server certificate received during pairing
         QList<QSslError> expectedSslErrors;
         expectedSslErrors.append(QSslError(QSslError::HostNameMismatch, m_ServerCert));
@@ -435,7 +450,7 @@ NvHTTP::openConnection(QUrl baseUrl,
             qWarning() << command << " request failed with error " << reply->error();
         }
 
-        if (!m_ServerCert.isNull() && reply->error() == QNetworkReply::SslHandshakeFailedError) {
+        if (reply->error() == QNetworkReply::SslHandshakeFailedError) {
             // This will trigger falling back to HTTP for the serverinfo query
             // then pairing again to get the updated certificate.
             GfeHttpResponseException exception(401, "Server certificate mismatch");

app/backend/nvhttp.h

@@ -152,6 +152,8 @@ public:
                            bool enableTimeout,
                            NvLogLevel logLevel = NvLogLevel::VERBOSE);
 
+    void setServerCert(QSslCertificate serverCert);
+
     static
     QVector<int>
     parseQuad(QString quad);

app/backend/nvpairingmanager.cpp

@@ -208,6 +208,18 @@ NvPairingManager::pair(QString appVersion, QString pin, QSslCertificate& serverC
         return PairState::ALREADY_IN_PROGRESS;
     }
 
+    serverCert = QSslCertificate(serverCertStr);
+    if (serverCert.isNull()) {
+        Q_ASSERT(!serverCert.isNull());
+
+        qCritical() << "Failed to parse plaincert";
+        m_Http.openConnectionToString(m_Http.m_BaseUrlHttp, "unpair", nullptr, true);
+        return PairState::FAILED;
+    }
+
+    // Pin this cert for TLS
+    m_Http.setServerCert(serverCert);
+
     QByteArray randomChallenge = generateRandomBytes(16);
     QByteArray encryptedChallenge = encrypt(randomChallenge, &encKey);
     QString challengeXml = m_Http.openConnectionToString(m_Http.m_BaseUrlHttp,
@@ -309,6 +321,5 @@ NvPairingManager::pair(QString appVersion, QString pin, QSslCertificate& serverC
         return PairState::FAILED;
     }
 
-    serverCert = QSslCertificate(serverCertStr);
     return PairState::PAIRED;
 }