add regression test for key backup bug

include/llarp.hpp

@@ -107,6 +107,7 @@ namespace llarp
     std::string configfile;
     std::unique_ptr<std::promise<void>> closeWaiter;
   };
+
 }  // namespace llarp
 
 #endif

llarp/config/config.cpp

@@ -171,13 +171,13 @@ namespace llarp
     conf.defineOption<int>("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<int>("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<std::string>("network", "exit-node", false, "", [this](std::string arg) {
@@ -407,8 +407,15 @@ namespace llarp
 
     conf.defineOption<std::string>(
         "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
     {

llarp/config/config.hpp

@@ -72,11 +72,11 @@ namespace llarp
     std::string m_ifname;
     std::string m_ifaddr;
 
-    std::string m_keyfile;
+    std::optional<fs::path> m_keyfile;
     std::string m_endpointType;
     bool m_reachable = false;
-    int m_hops = -1;
-    int m_paths = -1;
+    std::optional<int> m_Hops;
+    std::optional<int> m_Paths;
     bool m_AllowExit = false;
     std::set<RouterID> m_snodeBlacklist;
     std::optional<service::Address> m_exitNode;
@@ -152,7 +152,9 @@ namespace llarp
 
   struct BootstrapConfig
   {
-    std::vector<std::string> routers;
+    std::vector<fs::path> 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

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 = "<anonymous string>";
     return Parse();
   }
 

llarp/config/ini.hpp

@@ -7,6 +7,7 @@
 #include <memory>
 #include <unordered_map>
 #include <vector>
+#include <util/fs.hpp>
 
 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<char> m_Data;
     Config_impl_t m_Config;
-    std::string m_FileName;
+    fs::path m_FileName;
   };
 
 }  // namespace llarp

llarp/context.cpp

@@ -234,15 +234,9 @@ struct llarp_main
   std::shared_ptr<llarp::Context> 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
 {

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.");

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())
       {

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;

llarp/util/logging/logger.cpp

@@ -90,6 +90,12 @@ namespace llarp
     }
   }
 
+  LogLevel
+  GetLogLevel()
+  {
+    return LogContext::Instance().curLevel;
+  }
+
   void
   LogContext::ImmediateFlush()
   {

llarp/util/logging/logger.hpp

@@ -63,6 +63,9 @@ namespace llarp
   void
   SetLogLevel(LogLevel lvl);
 
+  LogLevel
+  GetLogLevel();
+
   /** internal */
   template <typename... TArgs>
   inline static void

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

test/regress/2020-06-08-key-backup-bug.cpp

@@ -0,0 +1,51 @@
+#include <llarp.h>
+#include <llarp.hpp>
+#include <config/config.hpp>
+#include <router/abstractrouter.hpp>
+#include <service/context.hpp>
+#include <catch2/catch.hpp>
+
+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);
+}