From 800668348a06c871f4789f25a171d239dce4bcfa Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 8 Jun 2020 08:42:10 -0400 Subject: [PATCH 1/5] add regression test for key backup bug --- include/llarp.hpp | 1 + llarp/config/config.cpp | 15 +++++-- llarp/config/config.hpp | 20 ++++++--- llarp/config/ini.cpp | 8 ++-- llarp/config/ini.hpp | 5 ++- llarp/context.cpp | 10 +---- llarp/router/router.cpp | 4 +- llarp/service/endpoint.cpp | 9 ++-- llarp/service/endpoint_state.cpp | 3 +- llarp/util/logging/logger.cpp | 6 +++ llarp/util/logging/logger.hpp | 3 ++ test/CMakeLists.txt | 1 + test/regress/2020-06-08-key-backup-bug.cpp | 51 ++++++++++++++++++++++ 13 files changed, 105 insertions(+), 31 deletions(-) create mode 100644 test/regress/2020-06-08-key-backup-bug.cpp diff --git a/include/llarp.hpp b/include/llarp.hpp index 48115c3e7..67606fb8b 100644 --- a/include/llarp.hpp +++ b/include/llarp.hpp @@ -107,6 +107,7 @@ namespace llarp std::string configfile; std::unique_ptr> closeWaiter; }; + } // namespace llarp #endif diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index b60fe3e2e..84027431e 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -171,13 +171,13 @@ namespace llarp conf.defineOption("network", "hops", false, HopsDefault, [this](int arg) { if (arg < 1 or arg > 8) throw std::invalid_argument("[endpoint]:hops must be >= 1 and <= 8"); - m_hops = arg; + m_Hops = arg; }); conf.defineOption("network", "paths", false, PathsDefault, [this](int arg) { if (arg < 1 or arg > 8) throw std::invalid_argument("[endpoint]:paths must be >= 1 and <= 8"); - m_paths = arg; + m_Paths = arg; }); conf.defineOption("network", "exit-node", false, "", [this](std::string arg) { @@ -407,8 +407,15 @@ namespace llarp conf.defineOption( "bootstrap", "add-node", false, true, "", [this](std::string arg) { - // TODO: validate as router fs path + if (arg.empty()) + { + throw std::invalid_argument("cannot use empty filename as bootstrap"); + } routers.emplace_back(std::move(arg)); + if (not fs::exists(routers.back())) + { + throw std::invalid_argument("file does not exist: " + arg); + } }); } @@ -444,7 +451,7 @@ namespace llarp } bool - Config::Load(const char* fname, bool isRelay, fs::path defaultDataDir) + Config::Load(const fs::path fname, bool isRelay, fs::path defaultDataDir) { try { diff --git a/llarp/config/config.hpp b/llarp/config/config.hpp index c27c3bde8..f73dba820 100644 --- a/llarp/config/config.hpp +++ b/llarp/config/config.hpp @@ -72,11 +72,11 @@ namespace llarp std::string m_ifname; std::string m_ifaddr; - std::string m_keyfile; + std::optional m_keyfile; std::string m_endpointType; bool m_reachable = false; - int m_hops = -1; - int m_paths = -1; + std::optional m_Hops; + std::optional m_Paths; bool m_AllowExit = false; std::set m_snodeBlacklist; std::optional m_exitNode; @@ -152,7 +152,9 @@ namespace llarp struct BootstrapConfig { - std::vector routers; + std::vector routers; + /// for unit tests + bool skipBootstrap = false; void defineConfigOptions(ConfigDefinition& conf, const ConfigGenParameters& params); }; @@ -192,7 +194,7 @@ namespace llarp // Load a config from the given file bool - Load(const char* fname, bool isRelay, fs::path defaultDataDir); + Load(const fs::path fname, bool isRelay, fs::path defaultDataDir); /// Load (initialize) a default config. /// @@ -224,4 +226,12 @@ namespace llarp } // namespace llarp +struct llarp_config +{ + llarp::Config impl; + llarp_config() = default; + + explicit llarp_config(const llarp_config* other); +}; + #endif diff --git a/llarp/config/ini.cpp b/llarp/config/ini.cpp index dc7df93e6..71f9e05e6 100644 --- a/llarp/config/ini.cpp +++ b/llarp/config/ini.cpp @@ -12,11 +12,10 @@ namespace llarp { bool - ConfigParser::LoadFile(std::string_view fname) + ConfigParser::LoadFile(const fs::path fname) { - std::string name{fname}; { - std::ifstream f(name, std::ios::in | std::ios::binary); + std::ifstream f(fname, std::ios::in | std::ios::binary); if (!f.is_open()) return false; f.seekg(0, std::ios::end); @@ -26,7 +25,7 @@ namespace llarp return false; f.read(m_Data.data(), m_Data.size()); } - m_FileName = name; + m_FileName = fname; return Parse(); } @@ -35,7 +34,6 @@ namespace llarp { m_Data.resize(str.size()); std::copy(str.begin(), str.end(), m_Data.begin()); - m_FileName = ""; return Parse(); } diff --git a/llarp/config/ini.hpp b/llarp/config/ini.hpp index c669d9485..2934567fe 100644 --- a/llarp/config/ini.hpp +++ b/llarp/config/ini.hpp @@ -7,6 +7,7 @@ #include #include #include +#include namespace llarp { @@ -22,7 +23,7 @@ namespace llarp /// return true on success /// return false on error bool - LoadFile(std::string_view fname); + LoadFile(const fs::path fname); /// load from string /// return true on success @@ -45,7 +46,7 @@ namespace llarp std::vector m_Data; Config_impl_t m_Config; - std::string m_FileName; + fs::path m_FileName; }; } // namespace llarp diff --git a/llarp/context.cpp b/llarp/context.cpp index ace399c3f..da9311730 100644 --- a/llarp/context.cpp +++ b/llarp/context.cpp @@ -234,15 +234,9 @@ struct llarp_main std::shared_ptr ctx; }; -struct llarp_config +llarp_config::llarp_config(const llarp_config* other) : impl(other->impl) { - llarp::Config impl; - llarp_config() = default; - - llarp_config(const llarp_config* other) : impl(other->impl) - { - } -}; +} namespace llarp { diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index f7c460bd7..fd6b3925b 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -442,8 +442,10 @@ namespace llarp // TODO: use constant fs::path defaultBootstrapFile = conf->router.m_dataDir / "bootstrap.signed"; if (fs::exists(defaultBootstrapFile)) + { configRouters.push_back(defaultBootstrapFile); - else + } + else if (not conf->bootstrap.skipBootstrap) { LogError("No bootstrap files specified in config file, and the default"); LogError("bootstrap file ", defaultBootstrapFile, " does not exist."); diff --git a/llarp/service/endpoint.cpp b/llarp/service/endpoint.cpp index 63d0c72f3..29f85c478 100644 --- a/llarp/service/endpoint.cpp +++ b/llarp/service/endpoint.cpp @@ -45,11 +45,11 @@ namespace llarp bool Endpoint::Configure(const NetworkConfig& conf, [[maybe_unused]] const DnsConfig& dnsConf) { - if (conf.m_paths > 0) - numPaths = conf.m_paths; + if (conf.m_Paths.has_value()) + numPaths = *conf.m_Paths; - if (conf.m_hops) - numHops = conf.m_hops; + if (conf.m_Hops.has_value()) + numHops = *conf.m_Hops; return m_state->Configure(conf); } @@ -387,7 +387,6 @@ namespace llarp bool Endpoint::LoadKeyFile() { - LogWarn("LoadKeyFile()"); const auto& keyfile = m_state->m_Keyfile; if (!keyfile.empty()) { diff --git a/llarp/service/endpoint_state.cpp b/llarp/service/endpoint_state.cpp index 7b8421e07..c2496a605 100644 --- a/llarp/service/endpoint_state.cpp +++ b/llarp/service/endpoint_state.cpp @@ -13,7 +13,8 @@ namespace llarp bool EndpointState::Configure(const NetworkConfig& conf) { - m_Keyfile = conf.m_keyfile; + if (conf.m_keyfile.has_value()) + m_Keyfile = *conf.m_keyfile; m_SnodeBlacklist = conf.m_snodeBlacklist; m_ExitEnabled = conf.m_AllowExit; m_ExitNode = conf.m_exitNode; diff --git a/llarp/util/logging/logger.cpp b/llarp/util/logging/logger.cpp index 42c8b8366..64867b824 100644 --- a/llarp/util/logging/logger.cpp +++ b/llarp/util/logging/logger.cpp @@ -90,6 +90,12 @@ namespace llarp } } + LogLevel + GetLogLevel() + { + return LogContext::Instance().curLevel; + } + void LogContext::ImmediateFlush() { diff --git a/llarp/util/logging/logger.hpp b/llarp/util/logging/logger.hpp index 394a25738..20fb4d8f6 100644 --- a/llarp/util/logging/logger.hpp +++ b/llarp/util/logging/logger.hpp @@ -63,6 +63,9 @@ namespace llarp void SetLogLevel(LogLevel lvl); + LogLevel + GetLogLevel(); + /** internal */ template inline static void diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index bae1d3b42..d070837ab 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -69,6 +69,7 @@ add_subdirectory(Catch2) add_executable(catchAll nodedb/test_nodedb.cpp path/test_path.cpp + regress/2020-06-08-key-backup-bug.cpp util/test_llarp_util_bits.cpp util/test_llarp_util_printer.cpp util/test_llarp_util_str.cpp diff --git a/test/regress/2020-06-08-key-backup-bug.cpp b/test/regress/2020-06-08-key-backup-bug.cpp new file mode 100644 index 000000000..a69281164 --- /dev/null +++ b/test/regress/2020-06-08-key-backup-bug.cpp @@ -0,0 +1,51 @@ +#include +#include +#include +#include +#include +#include + +static const fs::path keyfilePath = "2020-06-08-key-backup-regression-test.private"; + +static llarp_main* +make_context() +{ + auto config = llarp_default_config(); + config->impl.network.m_endpointType = "null"; + config->impl.network.m_keyfile = keyfilePath; + config->impl.bootstrap.skipBootstrap = true; + auto ptr = llarp_main_init_from_config(config, false); + llarp_config_free(config); + return ptr; +} + +TEST_CASE("key backup bug regression test", "[regress]") +{ + llarp::service::Address endpointAddress{}; + for (size_t index = 0; index < 10; index++) + { + auto context = make_context(); + REQUIRE(llarp_main_setup(context, false) == 0); + auto ctx = llarp::Context::Get(context); + ctx->CallSafe([ctx, index, &endpointAddress]() { + auto ep = ctx->router->hiddenServiceContext().GetDefault(); + REQUIRE(ep != nullptr); + if (index == 0) + { + // first iteration, we are getting our identity + endpointAddress = ep->GetIdentity().pub.Addr(); + REQUIRE(not endpointAddress.IsZero()); + } + else + { + REQUIRE(not endpointAddress.IsZero()); + // after the first iteration we expect the keys to stay the same + REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr()); + } + ctx->CloseAsync(); + }); + REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0); + llarp_main_free(context); + } + fs::remove(keyfilePath); +} From a73335579acbcc1a987ab258698af5a85965efc2 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 8 Jun 2020 09:07:49 -0400 Subject: [PATCH 2/5] silence logging in regression test --- llarp/ev/ev_libuv.cpp | 4 +- llarp/util/logging/logger.cpp | 10 ++++ llarp/util/logging/logger.hpp | 6 +- test/regress/2020-06-08-key-backup-bug.cpp | 70 +++++++++++++--------- 4 files changed, 59 insertions(+), 31 deletions(-) diff --git a/llarp/ev/ev_libuv.cpp b/llarp/ev/ev_libuv.cpp index 7242a71c8..0334b87cd 100644 --- a/llarp/ev/ev_libuv.cpp +++ b/llarp/ev/ev_libuv.cpp @@ -750,7 +750,9 @@ namespace libuv loop->process_timer_queue(); loop->process_cancel_queue(); loop->FlushLogic(); - llarp::LogContext::Instance().logStream->Tick(loop->time_now()); + auto& log = llarp::LogContext::Instance(); + if (log.logStream) + log.logStream->Tick(loop->time_now()); } Loop::Loop(size_t queue_size) diff --git a/llarp/util/logging/logger.cpp b/llarp/util/logging/logger.cpp index 64867b824..00daa34b3 100644 --- a/llarp/util/logging/logger.cpp +++ b/llarp/util/logging/logger.cpp @@ -174,6 +174,16 @@ namespace llarp } } + void + SilenceLog(std::function func) + { + auto& log = LogContext::Instance(); + ILogStream_ptr oldStream = std::move(log.logStream); + log.logStream = nullptr; + func(); + log.logStream = std::move(oldStream); + } + } // namespace llarp extern "C" diff --git a/llarp/util/logging/logger.hpp b/llarp/util/logging/logger.hpp index 20fb4d8f6..a01b54dbb 100644 --- a/llarp/util/logging/logger.hpp +++ b/llarp/util/logging/logger.hpp @@ -60,6 +60,10 @@ namespace llarp std::shared_ptr threadpool); }; + // call a function where the logging is slienced + void + SilenceLog(std::function func); + void SetLogLevel(LogLevel lvl); @@ -78,7 +82,7 @@ namespace llarp /* nop out logging for hive mode for now */ #ifndef LOKINET_HIVE auto& log = LogContext::Instance(); - if (log.curLevel > lvl) + if (log.curLevel > lvl || log.logStream == nullptr) return; std::stringstream ss; LogAppend(ss, std::forward(args)...); diff --git a/test/regress/2020-06-08-key-backup-bug.cpp b/test/regress/2020-06-08-key-backup-bug.cpp index a69281164..7763506be 100644 --- a/test/regress/2020-06-08-key-backup-bug.cpp +++ b/test/regress/2020-06-08-key-backup-bug.cpp @@ -5,14 +5,12 @@ #include #include -static const fs::path keyfilePath = "2020-06-08-key-backup-regression-test.private"; - static llarp_main* -make_context() +make_context(fs::path keyfile) { auto config = llarp_default_config(); config->impl.network.m_endpointType = "null"; - config->impl.network.m_keyfile = keyfilePath; + config->impl.network.m_keyfile = keyfile; config->impl.bootstrap.skipBootstrap = true; auto ptr = llarp_main_init_from_config(config, false); llarp_config_free(config); @@ -21,31 +19,45 @@ make_context() TEST_CASE("key backup bug regression test", "[regress]") { - llarp::service::Address endpointAddress{}; - for (size_t index = 0; index < 10; index++) - { - auto context = make_context(); - REQUIRE(llarp_main_setup(context, false) == 0); - auto ctx = llarp::Context::Get(context); - ctx->CallSafe([ctx, index, &endpointAddress]() { - auto ep = ctx->router->hiddenServiceContext().GetDefault(); - REQUIRE(ep != nullptr); - if (index == 0) + llarp::SilenceLog([]() { + for (const fs::path& path : {"regress-1.private", "regress-2.private", ""}) + { + llarp::service::Address endpointAddress{}; + for (size_t index = 0; index < 10; index++) { - // first iteration, we are getting our identity - endpointAddress = ep->GetIdentity().pub.Addr(); - REQUIRE(not endpointAddress.IsZero()); + auto context = make_context(path); + REQUIRE(llarp_main_setup(context, false) == 0); + auto ctx = llarp::Context::Get(context); + ctx->CallSafe([ctx, index, &endpointAddress, &path]() { + auto ep = ctx->router->hiddenServiceContext().GetDefault(); + REQUIRE(ep != nullptr); + if (index == 0) + { + REQUIRE(endpointAddress.IsZero()); + // first iteration, we are getting our identity + endpointAddress = ep->GetIdentity().pub.Addr(); + REQUIRE(not endpointAddress.IsZero()); + } + else + { + REQUIRE(not endpointAddress.IsZero()); + if (path.empty()) + { + // we want the keys to shift + REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr()); + } + else + { + // after the first iteration we expect the keys to stay the same + REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr()); + } + } + ctx->CloseAsync(); + }); + REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0); + llarp_main_free(context); } - else - { - REQUIRE(not endpointAddress.IsZero()); - // after the first iteration we expect the keys to stay the same - REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr()); - } - ctx->CloseAsync(); - }); - REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0); - llarp_main_free(context); - } - fs::remove(keyfilePath); + fs::remove(path); + } + }); } From 3d2990f90d040b2535c0523dbc30b2956f7d74d0 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 8 Jun 2020 09:26:53 -0400 Subject: [PATCH 3/5] use llarp::LogSilencer to shut up loging in unit tests --- llarp/util/logging/logger.cpp | 17 +++--- llarp/util/logging/logger.hpp | 14 ++++- test/main.cpp | 7 ++- test/regress/2020-06-08-key-backup-bug.cpp | 67 +++++++++++----------- 4 files changed, 59 insertions(+), 46 deletions(-) diff --git a/llarp/util/logging/logger.cpp b/llarp/util/logging/logger.cpp index 00daa34b3..ab00650ae 100644 --- a/llarp/util/logging/logger.cpp +++ b/llarp/util/logging/logger.cpp @@ -174,14 +174,17 @@ namespace llarp } } - void - SilenceLog(std::function func) + LogSilencer::LogSilencer() : LogSilencer(LogContext::Instance()) { - auto& log = LogContext::Instance(); - ILogStream_ptr oldStream = std::move(log.logStream); - log.logStream = nullptr; - func(); - log.logStream = std::move(oldStream); + } + + LogSilencer::LogSilencer(LogContext& ctx) : stream(std::move(ctx.logStream)) + { + } + + LogSilencer::~LogSilencer() + { + LogContext::Instance().logStream = std::move(stream); } } // namespace llarp diff --git a/llarp/util/logging/logger.hpp b/llarp/util/logging/logger.hpp index a01b54dbb..1b51e6b3c 100644 --- a/llarp/util/logging/logger.hpp +++ b/llarp/util/logging/logger.hpp @@ -60,9 +60,17 @@ namespace llarp std::shared_ptr threadpool); }; - // call a function where the logging is slienced - void - SilenceLog(std::function func); + /// RAII type to turn logging off + /// logging is suppressed as long as the silencer is in scope + struct LogSilencer + { + LogSilencer(); + ~LogSilencer(); + explicit LogSilencer(LogContext& ctx); + + private: + ILogStream_ptr stream; + }; void SetLogLevel(LogLevel lvl); diff --git a/test/main.cpp b/test/main.cpp index 3bef87c66..cb2449a6b 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -1,5 +1,7 @@ #include +#include + #ifdef _WIN32 #include int @@ -8,7 +10,7 @@ startWinsock() WSADATA wsockd; int err; err = ::WSAStartup(MAKEWORD(2, 2), &wsockd); - if(err) + if (err) { perror("Failed to start Windows Sockets"); return err; @@ -20,8 +22,9 @@ startWinsock() int main(int argc, char** argv) { + llarp::LogSilencer shutup; #ifdef _WIN32 - if(startWinsock()) + if (startWinsock()) return -1; #endif diff --git a/test/regress/2020-06-08-key-backup-bug.cpp b/test/regress/2020-06-08-key-backup-bug.cpp index 7763506be..396243801 100644 --- a/test/regress/2020-06-08-key-backup-bug.cpp +++ b/test/regress/2020-06-08-key-backup-bug.cpp @@ -19,45 +19,44 @@ make_context(fs::path keyfile) TEST_CASE("key backup bug regression test", "[regress]") { - llarp::SilenceLog([]() { - for (const fs::path& path : {"regress-1.private", "regress-2.private", ""}) + llarp::LogSilencer shutup; + for (const fs::path& path : {"regress-1.private", "regress-2.private", ""}) + { + llarp::service::Address endpointAddress{}; + for (size_t index = 0; index < 10; index++) { - llarp::service::Address endpointAddress{}; - for (size_t index = 0; index < 10; index++) - { - auto context = make_context(path); - REQUIRE(llarp_main_setup(context, false) == 0); - auto ctx = llarp::Context::Get(context); - ctx->CallSafe([ctx, index, &endpointAddress, &path]() { - auto ep = ctx->router->hiddenServiceContext().GetDefault(); - REQUIRE(ep != nullptr); - if (index == 0) + auto context = make_context(path); + REQUIRE(llarp_main_setup(context, false) == 0); + auto ctx = llarp::Context::Get(context); + ctx->CallSafe([ctx, index, &endpointAddress, &path]() { + auto ep = ctx->router->hiddenServiceContext().GetDefault(); + REQUIRE(ep != nullptr); + if (index == 0) + { + REQUIRE(endpointAddress.IsZero()); + // first iteration, we are getting our identity + endpointAddress = ep->GetIdentity().pub.Addr(); + REQUIRE(not endpointAddress.IsZero()); + } + else + { + REQUIRE(not endpointAddress.IsZero()); + if (path.empty()) { - REQUIRE(endpointAddress.IsZero()); - // first iteration, we are getting our identity - endpointAddress = ep->GetIdentity().pub.Addr(); - REQUIRE(not endpointAddress.IsZero()); + // we want the keys to shift + REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr()); } else { - REQUIRE(not endpointAddress.IsZero()); - if (path.empty()) - { - // we want the keys to shift - REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr()); - } - else - { - // after the first iteration we expect the keys to stay the same - REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr()); - } + // after the first iteration we expect the keys to stay the same + REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr()); } - ctx->CloseAsync(); - }); - REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0); - llarp_main_free(context); - } - fs::remove(path); + } + ctx->CloseAsync(); + }); + REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0); + llarp_main_free(context); } - }); + fs::remove(path); + } } From 3358a3371e3c0de74c90d25bfcabc2285de25e9a Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 8 Jun 2020 09:31:09 -0400 Subject: [PATCH 4/5] restore correct logstream --- llarp/util/logging/logger.cpp | 4 ++-- llarp/util/logging/logger.hpp | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/llarp/util/logging/logger.cpp b/llarp/util/logging/logger.cpp index ab00650ae..f604c07d5 100644 --- a/llarp/util/logging/logger.cpp +++ b/llarp/util/logging/logger.cpp @@ -178,13 +178,13 @@ namespace llarp { } - LogSilencer::LogSilencer(LogContext& ctx) : stream(std::move(ctx.logStream)) + LogSilencer::LogSilencer(LogContext& ctx) : parent(ctx), stream(std::move(ctx.logStream)) { } LogSilencer::~LogSilencer() { - LogContext::Instance().logStream = std::move(stream); + parent.logStream = std::move(stream); } } // namespace llarp diff --git a/llarp/util/logging/logger.hpp b/llarp/util/logging/logger.hpp index 1b51e6b3c..f717fbfa8 100644 --- a/llarp/util/logging/logger.hpp +++ b/llarp/util/logging/logger.hpp @@ -69,6 +69,7 @@ namespace llarp explicit LogSilencer(LogContext& ctx); private: + LogContext& parent; ILogStream_ptr stream; }; From b8e1ffa83eda88ffff0d3480442262e1aa008df7 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 8 Jun 2020 15:36:47 -0400 Subject: [PATCH 5/5] add comments and improve the regression test --- test/regress/2020-06-08-key-backup-bug.cpp | 32 +++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/test/regress/2020-06-08-key-backup-bug.cpp b/test/regress/2020-06-08-key-backup-bug.cpp index 396243801..76f811cc1 100644 --- a/test/regress/2020-06-08-key-backup-bug.cpp +++ b/test/regress/2020-06-08-key-backup-bug.cpp @@ -5,8 +5,9 @@ #include #include +/// make a llarp_main* with 1 endpoint that specifies a keyfile static llarp_main* -make_context(fs::path keyfile) +make_context(std::optional keyfile) { auto config = llarp_default_config(); config->impl.network.m_endpointType = "null"; @@ -17,12 +18,19 @@ make_context(fs::path keyfile) return ptr; } +/// test that we dont back up all keys when self.signed is missing or invalid as client TEST_CASE("key backup bug regression test", "[regress]") { + // kill logging, this code is noisy llarp::LogSilencer shutup; - for (const fs::path& path : {"regress-1.private", "regress-2.private", ""}) + // test 2 explicitly provided keyfiles, empty keyfile and no keyfile + for (std::optional path : {std::optional{"regress-1.private"}, + std::optional{"regress-2.private"}, + std::optional{""}, + {std::nullopt}}) { llarp::service::Address endpointAddress{}; + // try 10 start up and shut downs and see if our key changes or not for (size_t index = 0; index < 10; index++) { auto context = make_context(path); @@ -34,29 +42,33 @@ TEST_CASE("key backup bug regression test", "[regress]") if (index == 0) { REQUIRE(endpointAddress.IsZero()); - // first iteration, we are getting our identity + // first iteration, we are getting our identity that we start with endpointAddress = ep->GetIdentity().pub.Addr(); REQUIRE(not endpointAddress.IsZero()); } else { REQUIRE(not endpointAddress.IsZero()); - if (path.empty()) - { - // we want the keys to shift - REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr()); - } - else + if (path.has_value() and not path->empty()) { + // we have a keyfile provided // after the first iteration we expect the keys to stay the same REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr()); } + else + { + // we want the keys to shift because no keyfile was provided + REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr()); + } } + // close the router "later" so llarp_main_run exits ctx->CloseAsync(); }); REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0); llarp_main_free(context); } - fs::remove(path); + // remove keys if provied + if (path.has_value() and not path->empty()) + fs::remove(*path); } }