address parsing

- straightened out setting of public addr and public port in config vs listen ("bind") addr
- fixed small bug in router contact writing and saving

daemon/lokinet.cpp

@@ -593,7 +593,7 @@ namespace
         exit_code.set_value(1);
         return;
       }
-      catch (std::exception& ex)
+      catch (const std::exception& ex)
       {
         llarp::LogError(fmt::format("failed to start up lokinet: {}", ex.what()));
         exit_code.set_value(1);
@@ -604,9 +604,9 @@ namespace
       auto result = ctx->Run(opts);
       exit_code.set_value(result);
     }
-    catch (std::exception& e)
+    catch (const std::exception& e)
     {
-      llarp::LogError("Fatal: caught exception while running: ", e.what());
+      llarp::LogError("Fatal: caught exception while running: {}", e.what());
       exit_code.set_exception(std::current_exception());
     }
     catch (...)

llarp/config/config.cpp

@@ -19,8 +19,6 @@
 
 namespace llarp
 {
-  constexpr int DEFAULT_PUBLIC_PORT = 1090;
-
   using namespace config;
   namespace
   {
@@ -134,19 +132,7 @@ namespace llarp
             "this setting specifies the public IP at which this router is reachable. When",
             "provided the public-port option must also be specified.",
         },
-        [this, net = params.Net_ptr()](std::string arg) {
+        [this](std::string arg) { public_ip = std::move(arg); });
-          if (arg.empty())
-            return;
-          nuint32_t addr{};
-          if (not addr.FromString(arg))
-            throw std::invalid_argument{fmt::format("{} is not a valid IPv4 address", arg)};
-
-          if (net->IsBogonIP(addr))
-            throw std::invalid_argument{
-                fmt::format("{} is not a publicly routable ip address", addr)};
-
-          public_ip = addr;
-        });
 
     conf.define_option<std::string>("router", "public-address", Hidden, [](std::string) {
       throw std::invalid_argument{
@@ -154,19 +140,18 @@ namespace llarp
           "[router]:public-port instead"};
     });
 
-    conf.define_option<int>(
+    conf.define_option<uint16_t>(
         "router",
         "public-port",
         RelayOnly,
-        Default{DEFAULT_PUBLIC_PORT},
         Comment{
             "When specifying public-ip=, this specifies the public UDP port at which this lokinet",
             "router is reachable. Required when public-ip is used.",
         },
-        [this](int arg) {
+        [this](uint16_t arg) {
           if (arg <= 0 || arg > std::numeric_limits<uint16_t>::max())
             throw std::invalid_argument("public-port must be >= 0 and <= 65536");
-          public_port = ToNet(huint16_t{static_cast<uint16_t>(arg)});
+          public_port = arg;
         });
 
     conf.define_option<int>(
@@ -912,27 +897,38 @@ namespace llarp
     conf.define_option<std::string>(
         "bind",
         "public-ip",
+        Hidden,
         RelayOnly,
         Comment{
             "The IP address to advertise to the network instead of the incoming= or auto-detected",
             "IP.  This is typically required only when incoming= is used to listen on an internal",
             "private range IP address that received traffic forwarded from the public IP.",
         },
-        [this](std::string_view arg) {
+        [this](std::string arg) {
-          SockAddr pubaddr{arg};
+          public_addr = std::move(arg);
-          public_addr = pubaddr.getIP();
+          log::warning(
+              logcat,
+              "Using deprecated option; pass this value to [Router]:public-ip instead PLEASE");
         });
 
     conf.define_option<uint16_t>(
         "bind",
         "public-port",
+        Hidden,
         RelayOnly,
         Comment{
             "The port to advertise to the network instead of the incoming= (or default) port.",
             "This is typically required only when incoming= is used to listen on an internal",
             "private range IP address/port that received traffic forwarded from the public IP.",
         },
-        [this](uint16_t arg) { public_port = net::port_t::from_host(arg); });
+        [this](uint16_t arg) {
+          if (arg <= 0 || arg > std::numeric_limits<uint16_t>::max())
+            throw std::invalid_argument("public-port must be >= 0 and <= 65536");
+          public_port = arg;
+          log::warning(
+              logcat,
+              "Using deprecated option; pass this value to [Router]:public-port instead PLEASE");
+        });
 
     auto parse_addr_for_link = [net_ptr](const std::string& arg) {
       std::optional<oxen::quic::Address> maybe = std::nullopt;
@@ -1004,11 +1000,13 @@ namespace llarp
         },
         [this, parse_addr_for_link](const std::string& arg) {
           if (auto a = parse_addr_for_link(arg); a and a->is_addressable())
-            addr = *a;
+          {
+            listen_addr = *a;
+            using_user_value = true;
+            using_new_api = true;
+          }
           else
             throw std::invalid_argument{"Could not parse listen address!"};
-
-          using_new_api = true;
         });
 
     conf.define_option<std::string>(
@@ -1022,21 +1020,17 @@ namespace llarp
             throw std::runtime_error{"USE THE NEW API -- SPECIFY LOCAL ADDRESS UNDER [LISTEN]"};
 
           if (auto a = parse_addr_for_link(arg); a and a->is_addressable())
-            addr = *a;
+          {
+            log::warning(
+                logcat,
+                "Loaded address from deprecated [inbound] options; update your config to use "
+                "[bind]:listen instead PLEASE");
+            listen_addr = *a;
+            using_user_value = true;
+          }
         });
 
-    conf.define_option<std::string>(
+    conf.define_option<std::string>("bind", "outbound", MultiValue, Deprecated, Hidden);
-        "bind",
-        "outbound",
-        MultiValue,
-        Hidden,
-        [this, parse_addr_for_link](const std::string& arg) {
-          if (using_new_api)
-            throw std::runtime_error{"USE THE NEW API -- SPECIFY LOCAL ADDRESS UNDER [LISTEN]"};
-
-          if (auto a = parse_addr_for_link(arg); a and a->is_addressable())
-            addr = *a;
-        });
 
     conf.add_undeclared_handler(
         "bind", [this](std::string_view, std::string_view key, std::string_view val) {
@@ -1055,7 +1049,11 @@ namespace llarp
           // special case: wildcard for outbound
           if (key == "*")
           {
-            addr = oxen::quic::Address{port};
+            log::warning(
+                logcat,
+                "Wildcat address referencing port {} is referencing deprecated outbound config "
+                "options; use [bind]:listen instead",
+                port);
             return;
           }
 
@@ -1074,16 +1072,17 @@ namespace llarp
                 e.what())};
           }
 
-          if (temp.is_addressable())
+          if (not temp.is_addressable())
           {
-            addr = std::move(temp);
+            throw std::runtime_error{fmt::format(
-            return;
+                "Invalid address: {}; stop using this deprecated handler, update your config to "
+                "use "
+                "[bind]:listen instead PLEASE",
+                temp)};
           }
 
-          throw std::runtime_error{fmt::format(
+          listen_addr = std::move(temp);
-              "Invalid address: {}; stop using this deprecated handler, update your config to use "
+          using_user_value = true;
-              "[bind]:listen instead PLEASE",
-              temp)};
         });
   }
 

llarp/config/config.hpp

@@ -34,8 +34,9 @@ namespace llarp
   using SectionValues = llarp::ConfigParser::SectionValues;
   using ConfigMap = llarp::ConfigParser::ConfigMap;
 
-  inline static constexpr uint16_t DEFAULT_LISTEN_PORT{1090};
+  inline const std::string QUAD_ZERO{"0.0.0.0"};
-  constexpr int CLIENT_ROUTER_CONNECTIONS = 4;
+  inline constexpr uint16_t DEFAULT_LISTEN_PORT{1090};
+  inline constexpr int CLIENT_ROUTER_CONNECTIONS = 4;
 
   // TODO: don't use these maps. they're sloppy and difficult to follow
   /// Small struct to gather all parameters needed for config generation to reduce the number of
@@ -77,10 +78,9 @@ namespace llarp
     std::string transkey_file;
 
     bool is_relay = false;
-    /// deprecated
+
-    std::optional<net::ipaddr_t> public_ip;
+    std::optional<std::string> public_ip;
-    /// deprecated
+    std::optional<uint16_t> public_port;
-    std::optional<net::port_t> public_port;
 
     void
     define_config_options(ConfigDefinition& conf, const ConfigGenParameters& params);
@@ -170,10 +170,14 @@ namespace llarp
 
   struct LinksConfig
   {
-    std::optional<net::ipaddr_t> public_addr;
+    // DEPRECATED -- use [Router]:public_addr
-    std::optional<net::port_t> public_port;
+    std::optional<std::string> public_addr;
+    // DEPRECATED -- use [Router]:public_port
+    std::optional<uint16_t> public_port;
 
-    oxen::quic::Address addr{""s, DEFAULT_LISTEN_PORT};
+    std::optional<oxen::quic::Address> listen_addr;
+
+    bool using_user_value = false;
     bool using_new_api = false;
 
     void

llarp/link/link_manager.hpp

@@ -417,8 +417,14 @@ namespace llarp
         connid_map.emplace(conn_interface->scid(), rc.router_id());
         auto [itr, b] = conns.emplace(rc.router_id(), nullptr);
 
-        auto control_stream =
+        auto control_stream = conn_interface->template get_new_stream<oxen::quic::BTRequestStream>(
-            conn_interface->template get_new_stream<oxen::quic::BTRequestStream>();
+            [](oxen::quic::Stream& s, uint64_t error_code) {
+              log::warning(
+                  logcat,
+                  "BTRequestStream closed unexpectedly (ec:{}); closing connection...",
+                  error_code);
+              s.conn.close_connection(error_code);
+            });
         itr->second = std::make_shared<link::Connection>(conn_interface, control_stream, rc);
 
         return true;

llarp/nodedb.hpp

@@ -165,7 +165,7 @@ namespace llarp
     std::atomic<int> fetch_failures{0}, bootstrap_failures{0};
 
     std::atomic<bool> _using_bootstrap_fallback{false}, _needs_rebootstrap{false},
-        _needs_initial_fetch{false};
+        _needs_initial_fetch{true};
 
     bool
     want_rc(const RouterID& rid) const;

llarp/router/router.cpp

@@ -563,38 +563,39 @@ namespace llarp
     transport_keyfile = _key_manager->transkey_path;
     identity_keyfile = _key_manager->idkey_path;
 
-    std::optional<SockAddr> _ourAddress;
+    std::optional<std::string> paddr = (conf.router.public_ip) ? conf.router.public_ip
+        : (conf.links.public_addr)                             ? conf.links.public_addr
+                                                               : std::nullopt;
+    std::optional<uint16_t> pport = (conf.router.public_port) ? conf.router.public_port
+        : (conf.links.public_port)                            ? conf.links.public_port
+                                                              : std::nullopt;
 
-    if (auto maybe_ip = conf.links.public_addr)
+    if (pport.has_value() and not paddr.has_value())
-      _ourAddress = var::visit([](auto&& ip) { return SockAddr{ip}; }, *maybe_ip);
+      throw std::runtime_error{"If public-port is specified, public-addr must be as well!"};
-    else if (auto maybe_ip = conf.router.public_ip)
-      _ourAddress = var::visit([](auto&& ip) { return SockAddr{ip}; }, *maybe_ip);
 
-    if (_ourAddress)
+    if (conf.links.listen_addr)
     {
-      if (auto maybe_port = conf.links.public_port)
+      _listen_address = *conf.links.listen_addr;
-        _ourAddress->setPort(*maybe_port);
-      else if (auto maybe_port = conf.router.public_port)
-        _ourAddress->setPort(*maybe_port);
-      else
-        throw std::runtime_error{"public ip provided without public port"};
-      log::debug(logcat, "Using {} for our public address", *_ourAddress);
-
-      _public_address = oxen::quic::Address{static_cast<const sockaddr*>(*_ourAddress)};
-      log::critical(logcat, "PUBLIC ADDR: {}", *_public_address);
     }
     else
     {
-      log::debug(logcat, "No explicit public address given; inferring now...");
+      if (paddr or pport)
+        throw std::runtime_error{"Must specify [bind]:listen in config with public ip/addr!"};
 
       if (auto maybe_addr = net().GetBestNetIF())
       {
-        _public_address = oxen::quic::Address{static_cast<const sockaddr*>(*maybe_addr)};
+        _listen_address = oxen::quic::Address{static_cast<const sockaddr*>(*maybe_addr)};
-        log::critical(logcat, "PUBLIC ADDR: {}", *_public_address);
+        _listen_address.set_port(DEFAULT_LISTEN_PORT);
       }
+      else
+        throw std::runtime_error{"Could not find net interface on current platform!"};
     }
 
-    _listen_addr = conf.links.addr;
+    _public_address = (not paddr and not pport)
+        ? _listen_address
+        : oxen::quic::Address{*paddr, pport ? *pport : DEFAULT_LISTEN_PORT};
+
+    log::critical(logcat, "listen_addr:{} \t public_addr:{}", _listen_address, _public_address);
 
     RouterContact::BLOCK_BOGONS = conf.router.block_bogons;
 
@@ -886,29 +887,27 @@ namespace llarp
         next_rc_gossip = now_timepoint + RouterContact::STALE_AGE - random_delta;
       }
     }
+
+    if (needs_initial_fetch())
+    {
+      node_db()->fetch_initial();
+    }
+    else if (needs_rebootstrap() and next_bootstrap_attempt > now_timepoint)
+    {
+      node_db()->fallback_to_bootstrap();
+    }
     else
     {
-      if (needs_initial_fetch())
+      // (client-only) periodically fetch updated RCs
+      if (now_timepoint - last_rc_fetch > RC_UPDATE_INTERVAL)
       {
-        node_db()->fetch_initial();
+        node_db()->fetch_rcs();
       }
-      else if (needs_rebootstrap() and next_bootstrap_attempt > now_timepoint)
-      {
-        node_db()->fallback_to_bootstrap();
-      }
-      else
-      {
-        // (client-only) periodically fetch updated RCs
-        if (now_timepoint - last_rc_fetch > RC_UPDATE_INTERVAL)
-        {
-          node_db()->fetch_rcs();
-        }
 
-        // (client-only) periodically fetch updated RouterID list
+      // (client-only) periodically fetch updated RouterID list
-        if (now_timepoint - last_rid_fetch > ROUTERID_UPDATE_INTERVAL)
+      if (now_timepoint - last_rid_fetch > ROUTERID_UPDATE_INTERVAL)
-        {
+      {
-          node_db()->fetch_rids();
+        node_db()->fetch_rids();
-        }
       }
     }
 
@@ -1084,7 +1083,7 @@ namespace llarp
       return false;
 
     router_contact = LocalRC::make(
-        identity(), _is_service_node and _public_address ? *_public_address : _listen_addr);
+        identity(), _is_service_node and _public_address ? *_public_address : _listen_address);
 
     _link_manager = LinkManager::make(*this);
 
@@ -1374,7 +1373,7 @@ namespace llarp
   oxen::quic::Address
   Router::listen_addr() const
   {
-    return _listen_addr;
+    return _listen_address;
   }
 
   void

llarp/router/router.hpp

@@ -92,7 +92,7 @@ namespace llarp
     consensus::reachability_testing router_testing;
 
     std::optional<oxen::quic::Address> _public_address;  // public addr for relays
-    oxen::quic::Address _listen_addr;
+    oxen::quic::Address _listen_address;
 
     EventLoop_ptr _loop;
     std::shared_ptr<vpn::Platform> _vpn;

llarp/router_contact.hpp

@@ -312,7 +312,9 @@ namespace llarp
    public:
     RemoteRC() = default;
     RemoteRC(std::string_view data) : RemoteRC{oxenc::bt_dict_consumer{data}}
-    {}
+    {
+      _payload = {reinterpret_cast<const unsigned char*>(data.data()), data.size()};
+    }
     RemoteRC(ustring_view data) : RemoteRC{oxenc::bt_dict_consumer{data}}
     {
       _payload = data;

llarp/router_contact_local.cpp

@@ -79,7 +79,7 @@ namespace llarp
       return sig;
     });
 
-    _payload = btdp.view<unsigned char>();
+    _payload = ustring{btdp.view<unsigned char>()};
   }
 
   void

llarp/router_contact_remote.cpp

@@ -75,11 +75,11 @@ namespace llarp
   RemoteRC::read(const fs::path& fname)
   {
     ustring buf;
-    buf.reserve(MAX_RC_SIZE);
+    buf.resize(MAX_RC_SIZE);
 
     try
     {
-      util::file_to_buffer(fname, buf.data(), MAX_RC_SIZE);
+      util::file_to_buffer(fname, buf.data(), buf.size());
 
       oxenc::bt_dict_consumer btdc{buf};
       bt_load(btdc);