From 71ea4f4fa20efb525ae20b5cf59cb493c152854c Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 19 Sep 2022 11:34:27 -0300 Subject: [PATCH 1/4] RPC: Relax token/range argument handling - Accept empty string or `null` for token to mean "no token." - Accept `null` for range to mean "default range." - Don't use a default range (::0/0) in lokinet-vpn because this will fail if IPv6 ranges aren't supported on the platform (e.g. on Windows), and isn't necessary: if we omit it then the rpc code already uses ::0/0 or 0.0.0.0/0 by default, as needed. --- daemon/lokinet-vpn.cpp | 28 +++++++++++----------------- llarp/rpc/rpc_server.cpp | 10 +++++----- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/daemon/lokinet-vpn.cpp b/daemon/lokinet-vpn.cpp index dff05d0ee..c5c6a4f50 100644 --- a/daemon/lokinet-vpn.cpp +++ b/daemon/lokinet-vpn.cpp @@ -74,8 +74,8 @@ main(int argc, char* argv[]) oxenmq::address rpcURL("tcp://127.0.0.1:1190"); std::string exitAddress; std::string endpoint = "default"; - std::optional token; - std::string range = "::/0"; + std::string token; + std::optional range; oxenmq::LogLevel logLevel = oxenmq::LogLevel::warn; bool goUp = false; bool goDown = false; @@ -216,20 +216,11 @@ main(int argc, char* argv[]) } if (goUp) { - std::optional maybe_result; - if (token.has_value()) - { - maybe_result = LMQ_Request( - lmq, - connID, - "llarp.exit", - nlohmann::json{{"exit", exitAddress}, {"range", range}, {"token", *token}}); - } - else - { - maybe_result = LMQ_Request( - lmq, connID, "llarp.exit", nlohmann::json{{"exit", exitAddress}, {"range", range}}); - } + nlohmann::json opts{{"exit", exitAddress}, {"token", token}}; + if (range) + opts["range"] = *range; + + auto maybe_result = LMQ_Request(lmq, connID, "llarp.exit", opts); if (not maybe_result.has_value()) { @@ -245,7 +236,10 @@ main(int argc, char* argv[]) } if (goDown) { - LMQ_Request(lmq, connID, "llarp.exit", nlohmann::json{{"range", range}, {"unmap", true}}); + nlohmann::json opts{{"unmap", true}}; + if (range) + opts["range"] = *range; + LMQ_Request(lmq, connID, "llarp.exit", std::move(opts)); } return 0; diff --git a/llarp/rpc/rpc_server.cpp b/llarp/rpc/rpc_server.cpp index ecff8c3f3..b07000d0c 100644 --- a/llarp/rpc/rpc_server.cpp +++ b/llarp/rpc/rpc_server.cpp @@ -469,7 +469,7 @@ namespace llarp::rpc map = false; } const auto range_itr = obj.find("range"); - if (range_itr == obj.end()) + if (range_itr == obj.end() or range_itr->is_null()) { // platforms without ipv6 support will shit themselves // here if we give them an exit mapping that is ipv6 @@ -492,9 +492,9 @@ namespace llarp::rpc reply(CreateJSONError("ipv6 ranges not supported on this platform")); return; } - std::optional token; + std::string token; const auto token_itr = obj.find("token"); - if (token_itr != obj.end()) + if (token_itr != obj.end() and not token_itr->is_null()) { token = token_itr->get(); } @@ -518,10 +518,10 @@ namespace llarp::rpc ep->MapExitRange(range, addr); bool shouldSendAuth = false; - if (token.has_value()) + if (not token.empty()) { shouldSendAuth = true; - ep->SetAuthInfoForEndpoint(*exit, service::AuthInfo{*token}); + ep->SetAuthInfoForEndpoint(*exit, service::AuthInfo{token}); } auto onGoodResult = [r, reply](std::string reason) { if (r->HasClientExit()) From f8f7f206669ef27ede52046266c3c5d50694f074 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 19 Sep 2022 12:12:41 -0300 Subject: [PATCH 2/4] Rename LMQ -> OMQ --- daemon/lokinet-vpn.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/daemon/lokinet-vpn.cpp b/daemon/lokinet-vpn.cpp index c5c6a4f50..4791455c6 100644 --- a/daemon/lokinet-vpn.cpp +++ b/daemon/lokinet-vpn.cpp @@ -16,11 +16,11 @@ #include #endif -/// do a oxenmq request on an lmq instance blocking style +/// do a oxenmq request on an omq instance blocking style /// returns a json object parsed from the result std::optional -LMQ_Request( - oxenmq::OxenMQ& lmq, +OMQ_Request( + oxenmq::OxenMQ& omq, const oxenmq::ConnectionID& id, std::string_view method, std::optional args = std::nullopt) @@ -37,11 +37,11 @@ LMQ_Request( }; if (args.has_value()) { - lmq.request(id, method, handleRequest, args->dump()); + omq.request(id, method, handleRequest, args->dump()); } else { - lmq.request(id, method, handleRequest); + omq.request(id, method, handleRequest); } auto ftr = result_promise.get_future(); const auto str = ftr.get(); @@ -147,17 +147,17 @@ main(int argc, char* argv[]) return 1; } - oxenmq::OxenMQ lmq{ + oxenmq::OxenMQ omq{ [](oxenmq::LogLevel lvl, const char* file, int line, std::string msg) { std::cout << lvl << " [" << file << ":" << line << "] " << msg << std::endl; }, logLevel}; - lmq.start(); + omq.start(); std::promise connectPromise; - const auto connID = lmq.connect_remote( + const auto connID = omq.connect_remote( rpcURL, [&connectPromise](auto) { connectPromise.set_value(true); }, [&connectPromise](auto, std::string_view msg) { @@ -173,7 +173,7 @@ main(int argc, char* argv[]) if (killDaemon) { - const auto maybe = LMQ_Request(lmq, connID, "llarp.halt"); + const auto maybe = OMQ_Request(lmq, connID, "llarp.halt"); if (not maybe.has_value()) { std::cout << "call to llarp.admin.die failed" << std::endl; @@ -184,7 +184,7 @@ main(int argc, char* argv[]) if (printStatus) { - const auto maybe_status = LMQ_Request(lmq, connID, "llarp.status"); + const auto maybe_status = OMQ_Request(lmq, connID, "llarp.status"); if (not maybe_status.has_value()) { std::cout << "call to llarp.status failed" << std::endl; @@ -220,7 +220,7 @@ main(int argc, char* argv[]) if (range) opts["range"] = *range; - auto maybe_result = LMQ_Request(lmq, connID, "llarp.exit", opts); + auto maybe_result = OMQ_Request(omq, connID, "llarp.exit", opts); if (not maybe_result.has_value()) { @@ -239,7 +239,7 @@ main(int argc, char* argv[]) nlohmann::json opts{{"unmap", true}}; if (range) opts["range"] = *range; - LMQ_Request(lmq, connID, "llarp.exit", std::move(opts)); + OMQ_Request(omq, connID, "llarp.exit", std::move(opts)); } return 0; From ab11a8128d8df520b26f2a1fffd8782353c95476 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 19 Sep 2022 12:17:01 -0300 Subject: [PATCH 3/4] lokinet-vpn: misc cleanups - Add a function to extract a value from parsed options, to DRY out the code a little bit. - Add a exit_error function to format a message to stdout and then return the code, to simplify the repeated print-and-return code used when errors occur. - Use fmt for output formatting - Add an error if multiple modes are specified at once (--up/--down/--status/--exit) - Add error printing around unmap --- daemon/lokinet-vpn.cpp | 137 ++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 62 deletions(-) diff --git a/daemon/lokinet-vpn.cpp b/daemon/lokinet-vpn.cpp index 4791455c6..cf8bfc9bd 100644 --- a/daemon/lokinet-vpn.cpp +++ b/daemon/lokinet-vpn.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -50,6 +51,50 @@ OMQ_Request( return std::nullopt; } +namespace +{ + template + constexpr bool is_optional = false; + template + constexpr bool is_optional> = true; + + // Extracts a value from a cxxopts result and assigns it into `value` if present. The value can + // either be a plain value or a std::optional. If not present, `value` is not touched. + template + void + extract_option(const cxxopts::ParseResult& r, const std::string& name, T& value) + { + if (r.count(name)) + { + if constexpr (is_optional) + value = r[name].as(); + else + value = r[name].as(); + } + } + + // Takes a code, prints a message, and returns the code. Intended use is: + // return exit_error(1, "blah: {}", 42); + // from within main(). + template + [[nodiscard]] int + exit_error(int code, const std::string& format, T&&... args) + { + fmt::print(format, std::forward(args)...); + fmt::print("\n"); + return code; + } + + // Same as above, but with code omitted (uses exit code 1) + template + [[nodiscard]] int + exit_error(const std::string& format, T&&... args) + { + return exit_error(1, format, std::forward(args)...); + } + +} // namespace + int main(int argc, char* argv[]) { @@ -95,57 +140,36 @@ main(int argc, char* argv[]) { logLevel = oxenmq::LogLevel::debug; } - if (result.count("rpc") > 0) - { - rpcURL = oxenmq::address(result["rpc"].as()); - } - if (result.count("exit") > 0) - { - exitAddress = result["exit"].as(); - } goUp = result.count("up") > 0; goDown = result.count("down") > 0; printStatus = result.count("status") > 0; killDaemon = result.count("kill") > 0; - if (result.count("endpoint") > 0) - { - endpoint = result["endpoint"].as(); - } - if (result.count("token") > 0) - { - token = result["token"].as(); - } - if (result.count("auth") > 0) - { - token = result["auth"].as(); - } - if (result.count("range") > 0) - { - range = result["range"].as(); - } + extract_option(result, "rpc", rpcURL); + extract_option(result, "exit", exitAddress); + extract_option(result, "endpoint", endpoint); + extract_option(result, "token", token); + extract_option(result, "auth", token); + extract_option(result, "range", range); } catch (const cxxopts::option_not_exists_exception& ex) { - std::cerr << ex.what(); - std::cout << opts.help() << std::endl; - return 1; + return exit_error(2, "{}\n{}", ex.what(), opts.help()); } catch (std::exception& ex) { - std::cout << ex.what() << std::endl; - return 1; - } - if ((not goUp) and (not goDown) and (not printStatus) and (not killDaemon)) - { - std::cout << opts.help() << std::endl; - return 1; + return exit_error(2, "{}", ex.what()); } + + int num_commands = goUp + goDown + printStatus + killDaemon; + + if (num_commands == 0) + return exit_error(3, "One of --up/--down/--status/--kill must be specified"); + if (num_commands != 1) + return exit_error(3, "Only one of --up/--down/--status/--kill may be specified"); + if (goUp and exitAddress.empty()) - { - std::cout << "no exit address provided" << std::endl; - return 1; - } + return exit_error("no exit address provided"); oxenmq::OxenMQ omq{ [](oxenmq::LogLevel lvl, const char* file, int line, std::string msg) { @@ -173,23 +197,16 @@ main(int argc, char* argv[]) if (killDaemon) { - const auto maybe = OMQ_Request(lmq, connID, "llarp.halt"); - if (not maybe.has_value()) - { - std::cout << "call to llarp.admin.die failed" << std::endl; - return 1; - } + if (not OMQ_Request(omq, connID, "llarp.halt")) + return exit_error("call to llarp.halt failed"); return 0; } if (printStatus) { - const auto maybe_status = OMQ_Request(lmq, connID, "llarp.status"); - if (not maybe_status.has_value()) - { - std::cout << "call to llarp.status failed" << std::endl; - return 1; - } + const auto maybe_status = OMQ_Request(omq, connID, "llarp.status"); + if (not maybe_status) + return exit_error("call to llarp.status failed"); try { @@ -209,8 +226,7 @@ main(int argc, char* argv[]) } catch (std::exception& ex) { - std::cout << "failed to parse result: " << ex.what() << std::endl; - return 1; + return exit_error("failed to parse result: {}", ex.what()); } return 0; } @@ -220,18 +236,14 @@ main(int argc, char* argv[]) if (range) opts["range"] = *range; - auto maybe_result = OMQ_Request(omq, connID, "llarp.exit", opts); + auto maybe_result = OMQ_Request(omq, connID, "llarp.exit", std::move(opts)); - if (not maybe_result.has_value()) - { - std::cout << "could not add exit" << std::endl; - return 1; - } + if (not maybe_result) + return exit_error("could not add exit"); - if (maybe_result->contains("error") and maybe_result->at("error").is_string()) + if (auto err_it = maybe_result->find("error"); err_it != maybe_result->end()) { - std::cout << maybe_result->at("error").get() << std::endl; - return 1; + return exit_error("{}", err_it->get()); } } if (goDown) @@ -239,7 +251,8 @@ main(int argc, char* argv[]) nlohmann::json opts{{"unmap", true}}; if (range) opts["range"] = *range; - OMQ_Request(omq, connID, "llarp.exit", std::move(opts)); + if (not OMQ_Request(omq, connID, "llarp.exit", std::move(opts))) + return exit_error("failed to unmap exit"); } return 0; From 66c79b232a6dc468b7f58c4f40f4ea763fab0f71 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Sep 2022 13:22:43 -0300 Subject: [PATCH 4/4] Fix log warning --- llarp/nodedb.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/llarp/nodedb.cpp b/llarp/nodedb.cpp index ceef0c95e..d2ff94c77 100644 --- a/llarp/nodedb.cpp +++ b/llarp/nodedb.cpp @@ -21,6 +21,8 @@ static const std::string RC_FILE_EXT = ".signed"; namespace llarp { + static auto logcat = log::Cat("nodedb"); + NodeDB::Entry::Entry(RouterContact value) : rc(std::move(value)), insertedAt(llarp::time_now_ms()) {} @@ -160,7 +162,7 @@ namespace llarp if (not purge.empty()) { - LogWarn("removing {} invalid RC from disk", purge.size()); + log::warning(logcat, "removing {} invalid RCs from disk", purge.size()); for (const auto& fpath : purge) fs::remove(fpath);