Merge commit from fork

PR #2042 introduced another location for storing authorized clients
but did not correctly consider how the load/store logic should differ
for those places. One location (named_devices) could contain clients
which had not completed pairing, while the other (certs) had only
fully paired clients.

Despite differences in trust level of clients in each list, the logic
for loading/saving config treated them identically. The result is that
clients that had not successfully completed pairing would be treated
as fully paired after a state reload.

Fix this state confusion by consolidating to a single location for
authorized client state and ensuring it only contains information on
fully paired clients.

src/nvhttp.cpp

@@ -142,14 +142,14 @@ namespace nvhttp {
   };
 
   struct client_t {
-    std::vector<std::string> certs;
     std::vector<named_cert_t> named_devices;
   };
 
   struct pair_session_t {
     struct {
       std::string uniqueID;
       std::string cert;
+      std::string name;
     } client;
 
     std::unique_ptr<crypto::aes_t> cipher_key;
@@ -277,7 +277,6 @@ namespace nvhttp {
             named_cert.cert = el.get_value<std::string>();
             named_cert.uuid = uuid_util::uuid_t::generate().string();
             client.named_devices.emplace_back(named_cert);
-            client.certs.emplace_back(named_cert.cert);
           }
         }
       }
@@ -290,15 +289,11 @@ namespace nvhttp {
         named_cert.cert = el.get_child("cert").get_value<std::string>();
         named_cert.uuid = el.get_child("uuid").get_value<std::string>();
         client.named_devices.emplace_back(named_cert);
-        client.certs.emplace_back(named_cert.cert);
       }
     }
 
     // Empty certificate chain and import certs from file
     cert_chain.clear();
-    for (auto &cert : client.certs) {
-      cert_chain.add(crypto::x509(cert));
-    }
     for (auto &named_cert : client.named_devices) {
       cert_chain.add(crypto::x509(named_cert.cert));
     }
@@ -307,17 +302,13 @@ namespace nvhttp {
   }
 
   void
-  update_id_client(const std::string &uniqueID, std::string &&cert, op_e op) {
-    switch (op) {
-      case op_e::ADD: {
-        client_t &client = client_root;
-        client.certs.emplace_back(std::move(cert));
-      } break;
-      case op_e::REMOVE:
-        client_t client;
-        client_root = client;
-        break;
-    }
+  add_authorized_client(const std::string &name, std::string &&cert) {
+    client_t &client = client_root;
+    named_cert_t named_cert;
+    named_cert.name = name;
+    named_cert.cert = std::move(cert);
+    named_cert.uuid = uuid_util::uuid_t::generate().string();
+    client.named_devices.emplace_back(named_cert);
 
     if (!config::sunshine.flags[config::flag::FRESH_STATE]) {
       save_state();
@@ -485,9 +476,9 @@ namespace nvhttp {
       tree.put("root.paired", 1);
       add_cert->raise(crypto::x509(client.cert));
 
+      // The client is now successfully paired and will be authorized to connect
       auto it = map_id_sess.find(client.uniqueID);
-
-      update_id_client(client.uniqueID, std::move(client.cert), op_e::ADD);
+      add_authorized_client(client.name, std::move(client.cert));
       map_id_sess.erase(it);
     }
     else {
@@ -654,14 +645,7 @@ namespace nvhttp {
 
     auto &sess = std::begin(map_id_sess)->second;
     getservercert(sess, tree, pin);
-
-    // set up named cert
-    client_t &client = client_root;
-    named_cert_t named_cert;
-    named_cert.name = name;
-    named_cert.cert = sess.client.cert;
-    named_cert.uuid = uuid_util::uuid_t::generate().string();
-    client.named_devices.emplace_back(named_cert);
+    sess.client.name = name;
 
     // response to the request for pin
     std::ostringstream data;
@@ -1191,18 +1175,6 @@ namespace nvhttp {
     client_t &client = client_root;
     for (auto it = client.named_devices.begin(); it != client.named_devices.end();) {
       if ((*it).uuid == uuid) {
-        // Find matching cert and remove it
-        for (auto cert = client.certs.begin(); cert != client.certs.end();) {
-          if ((*cert) == (*it).cert) {
-            cert = client.certs.erase(cert);
-            removed++;
-          }
-          else {
-            ++cert;
-          }
-        }
-
-        // And then remove the named cert
         it = client.named_devices.erase(it);
         removed++;
       }