From 2e5c856cf3b33ba00184a162a7f1e8c4ccf4371d Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 14 Nov 2023 17:06:10 -0500 Subject: [PATCH 1/2] onion encrypt path build frames path build frames should be onioned at each hop to avoid a bad actor controlling two nodes in a path being able to know (with certainty, temporal correlation is hard to avoid) that they're hops on the same path. This is desirable as in the worst case someone could be your edge hop and terminal hop on a path, and now the terminal hop knows your IP making the path basically pointless. --- llarp/link/link_manager.cpp | 165 ++++++++++++++++++------------------ llarp/path/pathbuilder.cpp | 58 ++++++++++--- 2 files changed, 128 insertions(+), 95 deletions(-) diff --git a/llarp/link/link_manager.cpp b/llarp/link/link_manager.cpp index f935e306c..daf99ffd2 100644 --- a/llarp/link/link_manager.cpp +++ b/llarp/link/link_manager.cpp @@ -1117,114 +1117,97 @@ namespace llarp m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::NO_TRANSIT}}), true); return; } + try { - std::string payload{m.body()}, frame_payload; + std::string frame_payload; std::string frame, hash, hop_payload, commkey, rx_id, tx_id, upstream; ustring other_pubkey, outer_nonce, inner_nonce; uint64_t lifetime; - try + auto payload_list = oxenc::bt_deserialize>(m.body()); + if (payload_list.size() != path::MAX_LEN) { - oxenc::bt_list_consumer btlc{payload}; - frame_payload = btlc.consume_string(); - - oxenc::bt_dict_consumer frame_info{frame_payload}; - hash = frame_info.require("HASH"); - frame = frame_info.require("FRAME"); - - oxenc::bt_dict_consumer hop_dict{frame}; - hop_payload = frame_info.require("ENCRYPTED"); - outer_nonce = frame_info.require("NONCE"); - other_pubkey = frame_info.require("PUBKEY"); - - SharedSecret shared; - // derive shared secret using ephemeral pubkey and our secret key (and nonce) - if (!crypto::dh_server( - shared.data(), other_pubkey.data(), _router.pubkey(), inner_nonce.data())) - { - log::info(link_cat, "DH server initialization failed during path build"); - m.respond( - serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); - return; - } + log::info(link_cat, "Path build message with wrong number of frames"); + m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_FRAMES}}), true); + return; + } - // hash data and check against given hash - ShortHash digest; - if (!crypto::hmac( - digest.data(), - reinterpret_cast(frame.data()), - frame.size(), - shared)) - { - log::error(link_cat, "HMAC failed on path build request"); - m.respond( - serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); - return; - } - if (!std::equal( - digest.begin(), digest.end(), reinterpret_cast(hash.data()))) - { - log::info(link_cat, "HMAC mismatch on path build request"); - m.respond( - serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); - return; - } + oxenc::bt_dict_consumer frame_info{payload_list.front()}; + hash = frame_info.require("HASH"); + frame = frame_info.require("FRAME"); - // decrypt frame with our hop info - if (!crypto::xchacha20( - reinterpret_cast(hop_payload.data()), - hop_payload.size(), - shared.data(), - outer_nonce.data())) - { - log::info(link_cat, "Decrypt failed on path build request"); - m.respond( - serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); - return; - } + oxenc::bt_dict_consumer hop_dict{frame}; + hop_payload = frame_info.require("ENCRYPTED"); + outer_nonce = frame_info.require("NONCE"); + other_pubkey = frame_info.require("PUBKEY"); + + SharedSecret shared; + // derive shared secret using ephemeral pubkey and our secret key (and nonce) + if (!crypto::dh_server( + shared.data(), other_pubkey.data(), _router.pubkey(), inner_nonce.data())) + { + log::info(link_cat, "DH server initialization failed during path build"); + m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); + return; + } - oxenc::bt_dict_consumer hop_info{hop_payload}; - commkey = hop_info.require("COMMKEY"); - lifetime = hop_info.require("LIFETIME"); - inner_nonce = hop_info.require("NONCE"); - rx_id = hop_info.require("RX"); - tx_id = hop_info.require("TX"); - upstream = hop_info.require("UPSTREAM"); + // hash data and check against given hash + ShortHash digest; + if (!crypto::hmac( + digest.data(), reinterpret_cast(frame.data()), frame.size(), shared)) + { + log::error(link_cat, "HMAC failed on path build request"); + m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); + return; } - catch (...) + if (!std::equal( + digest.begin(), digest.end(), reinterpret_cast(hash.data()))) { - log::warning(link_cat, "Error: failed to deserialize path build message"); - throw; + log::info(link_cat, "HMAC mismatch on path build request"); + m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); + return; } - if (frame.empty()) + // decrypt frame with our hop info + if (!crypto::xchacha20( + reinterpret_cast(hop_payload.data()), + hop_payload.size(), + shared.data(), + outer_nonce.data())) { - log::info(link_cat, "Path build request received invalid frame"); - m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_FRAMES}}), true); + log::info(link_cat, "Decrypt failed on path build request"); + m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); return; } + oxenc::bt_dict_consumer hop_info{hop_payload}; + commkey = hop_info.require("COMMKEY"); + lifetime = hop_info.require("LIFETIME"); + inner_nonce = hop_info.require("NONCE"); + rx_id = hop_info.require("RX"); + tx_id = hop_info.require("TX"); + upstream = hop_info.require("UPSTREAM"); + // populate transit hop object with hop info // TODO: IP / path build limiting clients auto hop = std::make_shared(); hop->info.downstream = from; // extract pathIDs and check if zero or used - auto& hop_info = hop->info; - hop_info.txID.from_string(tx_id); - hop_info.rxID.from_string(rx_id); + hop->info.txID.from_string(tx_id); + hop->info.rxID.from_string(rx_id); - if (hop_info.txID.IsZero() || hop_info.rxID.IsZero()) + if (hop->info.txID.IsZero() || hop->info.rxID.IsZero()) { log::warning(link_cat, "Invalid PathID; PathIDs must be non-zero"); m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_PATHID}}), true); return; } - hop_info.upstream.from_string(upstream); + hop->info.upstream.from_string(upstream); - if (_router.path_context().HasTransitHop(hop_info)) + if (_router.path_context().HasTransitHop(hop->info)) { log::warning(link_cat, "Invalid PathID; PathIDs must be unique"); m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_PATHID}}), true); @@ -1255,9 +1238,9 @@ namespace llarp } hop->started = _router.now(); - _router.persist_connection_until(hop_info.downstream, hop->ExpireTime() + 10s); + _router.persist_connection_until(hop->info.downstream, hop->ExpireTime() + 10s); - if (hop_info.upstream == _router.pubkey()) + if (hop->info.upstream == _router.pubkey()) { hop->terminal_hop = true; // we are terminal hop and everything is okay @@ -1266,14 +1249,32 @@ namespace llarp return; } - // rotate our frame to the end of the list and forward upstream - auto payload_list = oxenc::bt_deserialize(payload); - payload_list.splice(payload_list.end(), payload_list, payload_list.begin()); + // pop our frame, to be randomized after onion step and appended + auto end_frame = std::move(payload_list.front()); + payload_list.pop_front(); + auto onion_nonce = SymmNonce{inner_nonce.data()} ^ hop->nonceXOR; + // (de-)onion each further frame using the established shared secret and + // onion_nonce = inner_nonce ^ nonceXOR + // Note: final value passed to crypto::onion is xor factor, but that's for *after* the + // onion round to compute the return value, so we don't care about it. + for (auto& element : payload_list) + { + crypto::onion( + reinterpret_cast(element.data()), + element.size(), + hop->pathKey, + onion_nonce, + onion_nonce); + } + // randomize final frame. could probably paste our frame on the end and onion it with the + // rest, but it gains nothing over random. + randombytes(reinterpret_cast(end_frame.data()), end_frame.size()); + payload_list.push_back(std::move(end_frame)); send_control_message( hop->info.upstream, "path_build", - bt_serialize(payload_list), + oxenc::bt_serialize(payload_list), [hop, this, prev_message = std::move(m)](oxen::quic::message m) { if (m) { diff --git a/llarp/path/pathbuilder.cpp b/llarp/path/pathbuilder.cpp index fc1697549..8ce56463a 100644 --- a/llarp/path/pathbuilder.cpp +++ b/llarp/path/pathbuilder.cpp @@ -429,40 +429,72 @@ namespace llarp path_cat, "{} building path -> {} : {}", Name(), path->ShortName(), path->HopsString()); oxenc::bt_list_producer frames; - + std::vector frame_str(path::MAX_LEN); auto& path_hops = path->hops; size_t n_hops = path_hops.size(); size_t last_len{0}; - for (size_t i = 0; i < n_hops; i++) + // each hop will be able to read the outer part of its frame and decrypt + // the inner part with that information. It will then do an onion step on the + // remaining frames so the next hop can read the outer part of its frame, + // and so on. As this de-onion happens from hop 1 to n, we create and onion + // the frames from hop n downto 1 (i.e. reverse order). The first frame is + // not onioned. + // + // Onion-ing the frames in this way will prevent relays controlled by + // the same entity from knowing they are part of the same path + // (unless they're adjacent in the path; nothing we can do about that obviously). + + // i from n_hops downto 0 + size_t i = n_hops; + while (i > 0) { + i--; bool lastHop = (i == (n_hops - 1)); const auto& nextHop = lastHop ? path_hops[i].rc.router_id() : path_hops[i + 1].rc.router_id(); PathBuildMessage::setup_hop_keys(path_hops[i], nextHop); - auto frame_str = PathBuildMessage::serialize(path_hops[i]); + frame_str[i] = PathBuildMessage::serialize(path_hops[i]); // all frames should be the same length...not sure what that is yet + // it may vary if path lifetime is non-default, as that is encoded as an + // integer in decimal, but it should be constant for a given path if (last_len != 0) - assert(frame_str.size() == last_len); + assert(frame_str[i].size() == last_len); + + last_len = frame_str[i].size(); - last_len = frame_str.size(); - frames.append(std::move(frame_str)); + // onion each previously-created frame using the established shared secret and + // onion_nonce = path_hops[i].nonce ^ path_hops[i].nonceXOR, which the transit hop + // will have recovered after decrypting its frame. + // Note: final value passed to crypto::onion is xor factor, but that's for *after* the + // onion round to compute the return value, so we don't care about it. + for (size_t j = n_hops - 1; j > i; j--) + { + auto onion_nonce = path_hops[i].nonce ^ path_hops[i].nonceXOR; + crypto::onion( + reinterpret_cast(frame_str[j].data()), + frame_str[j].size(), + path_hops[i].shared, + onion_nonce, + onion_nonce); + } } std::string dummy; dummy.reserve(last_len); - // append dummy frames; path build request must always have MAX_LEN frames - // TODO: with the data structured as it is now (bt-encoded dict as each frame) - // the dummy frames can't be completely random; they need to look like - // normal frames - for (size_t i = 0; i < path::MAX_LEN - n_hops; i++) + for (i = n_hops; i < path::MAX_LEN; i++) + { + frame_str[i].resize(last_len); + randombytes(reinterpret_cast(frame_str[i].data()), frame_str[i].size()); + } + + for (auto& str : frame_str) // NOLINT { - randombytes(reinterpret_cast(dummy.data()), dummy.size()); - frames.append(dummy); + frames.append(std::move(str)); } router->path_context().AddOwnPath(GetSelf(), path); From feaf0b91931e7748ed5c517828dc7e6f020eb598 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Thu, 16 Nov 2023 12:10:53 -0500 Subject: [PATCH 2/2] fix some copy/paste derping also deserialize to unsigned string where possible/useful so to not have unnecessary reinterpret_casts all over the place. --- llarp/link/link_manager.cpp | 51 +++++++++++++------------------------ 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/llarp/link/link_manager.cpp b/llarp/link/link_manager.cpp index daf99ffd2..494f607c7 100644 --- a/llarp/link/link_manager.cpp +++ b/llarp/link/link_manager.cpp @@ -1120,12 +1120,7 @@ namespace llarp try { - std::string frame_payload; - std::string frame, hash, hop_payload, commkey, rx_id, tx_id, upstream; - ustring other_pubkey, outer_nonce, inner_nonce; - uint64_t lifetime; - - auto payload_list = oxenc::bt_deserialize>(m.body()); + auto payload_list = oxenc::bt_deserialize>(m.body()); if (payload_list.size() != path::MAX_LEN) { log::info(link_cat, "Path build message with wrong number of frames"); @@ -1134,18 +1129,18 @@ namespace llarp } oxenc::bt_dict_consumer frame_info{payload_list.front()}; - hash = frame_info.require("HASH"); - frame = frame_info.require("FRAME"); + auto hash = frame_info.require("HASH"); + auto frame = frame_info.require("FRAME"); oxenc::bt_dict_consumer hop_dict{frame}; - hop_payload = frame_info.require("ENCRYPTED"); - outer_nonce = frame_info.require("NONCE"); - other_pubkey = frame_info.require("PUBKEY"); + auto hop_payload = hop_dict.require("ENCRYPTED"); + auto outer_nonce = hop_dict.require("NONCE"); + auto other_pubkey = hop_dict.require("PUBKEY"); SharedSecret shared; // derive shared secret using ephemeral pubkey and our secret key (and nonce) if (!crypto::dh_server( - shared.data(), other_pubkey.data(), _router.pubkey(), inner_nonce.data())) + shared.data(), other_pubkey.data(), _router.pubkey(), outer_nonce.data())) { log::info(link_cat, "DH server initialization failed during path build"); m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); @@ -1154,15 +1149,13 @@ namespace llarp // hash data and check against given hash ShortHash digest; - if (!crypto::hmac( - digest.data(), reinterpret_cast(frame.data()), frame.size(), shared)) + if (!crypto::hmac(digest.data(), frame.data(), frame.size(), shared)) { log::error(link_cat, "HMAC failed on path build request"); m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); return; } - if (!std::equal( - digest.begin(), digest.end(), reinterpret_cast(hash.data()))) + if (!std::equal(digest.begin(), digest.end(), hash.data())) { log::info(link_cat, "HMAC mismatch on path build request"); m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); @@ -1171,10 +1164,7 @@ namespace llarp // decrypt frame with our hop info if (!crypto::xchacha20( - reinterpret_cast(hop_payload.data()), - hop_payload.size(), - shared.data(), - outer_nonce.data())) + hop_payload.data(), hop_payload.size(), shared.data(), outer_nonce.data())) { log::info(link_cat, "Decrypt failed on path build request"); m.respond(serialize_response({{messages::STATUS_KEY, PathBuildMessage::BAD_CRYPTO}}), true); @@ -1182,12 +1172,12 @@ namespace llarp } oxenc::bt_dict_consumer hop_info{hop_payload}; - commkey = hop_info.require("COMMKEY"); - lifetime = hop_info.require("LIFETIME"); - inner_nonce = hop_info.require("NONCE"); - rx_id = hop_info.require("RX"); - tx_id = hop_info.require("TX"); - upstream = hop_info.require("UPSTREAM"); + auto commkey = hop_info.require("COMMKEY"); + auto lifetime = hop_info.require("LIFETIME"); + auto inner_nonce = hop_info.require("NONCE"); + auto rx_id = hop_info.require("RX"); + auto tx_id = hop_info.require("TX"); + auto upstream = hop_info.require("UPSTREAM"); // populate transit hop object with hop info // TODO: IP / path build limiting clients @@ -1259,16 +1249,11 @@ namespace llarp // onion round to compute the return value, so we don't care about it. for (auto& element : payload_list) { - crypto::onion( - reinterpret_cast(element.data()), - element.size(), - hop->pathKey, - onion_nonce, - onion_nonce); + crypto::onion(element.data(), element.size(), hop->pathKey, onion_nonce, onion_nonce); } // randomize final frame. could probably paste our frame on the end and onion it with the // rest, but it gains nothing over random. - randombytes(reinterpret_cast(end_frame.data()), end_frame.size()); + randombytes(end_frame.data(), end_frame.size()); payload_list.push_back(std::move(end_frame)); send_control_message(