From fd7e68457a134102d1b30af5796c79f2aa623224 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Mon, 9 Sep 2024 19:13:54 -0500 Subject: [PATCH] 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 | 50 +++++++++++--------------------------------------- 1 file changed, 11 insertions(+), 39 deletions(-) diff --git a/src/nvhttp.cpp b/src/nvhttp.cpp index 8b610a4c21f..7eec1100008 100644 --- a/src/nvhttp.cpp +++ b/src/nvhttp.cpp @@ -142,7 +142,6 @@ namespace nvhttp { }; struct client_t { - std::vector certs; std::vector named_devices; }; @@ -150,6 +149,7 @@ namespace nvhttp { struct { std::string uniqueID; std::string cert; + std::string name; } client; std::unique_ptr cipher_key; @@ -277,7 +277,6 @@ namespace nvhttp { named_cert.cert = el.get_value(); 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(); named_cert.uuid = el.get_child("uuid").get_value(); 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++; }