fix #1655

* make it so that we don't set up unbound resolver when we have no resolvers provided by config
* clean up dns codepath and make it use llarp::SockAddr instead of llarp::IpAddress

llarp/config/config.cpp

@@ -729,12 +729,9 @@ namespace llarp
           }
           if (!arg.empty())
           {
-            auto& addr = m_upstreamDNS.emplace_back(std::move(arg));
-            if (auto p = addr.getPort(); p && *p != 53)
-              // unbound doesn't support non-default ports so bail if the user gave one
-              throw std::invalid_argument(
-                  "Invalid [dns] upstream setting: non-default DNS ports are not supported");
-            addr.setPort(std::nullopt);
+            auto& entry = m_upstreamDNS.emplace_back(std::move(arg));
+            if (!entry.getPort())
+              entry.setPort(53);
           }
         });
 
@@ -746,7 +743,7 @@ namespace llarp
             "Address to bind to for handling DNS requests.",
         },
         [=](std::string arg) {
-          m_bind = IpAddress{std::move(arg)};
+          m_bind = SockAddr{std::move(arg)};
           if (!m_bind.getPort())
             m_bind.setPort(53);
         });

llarp/config/config.hpp

@@ -135,8 +135,8 @@ namespace llarp
 
   struct DnsConfig
   {
-    IpAddress m_bind;
-    std::vector<IpAddress> m_upstreamDNS;
+    SockAddr m_bind;
+    std::vector<SockAddr> m_upstreamDNS;
 
     void
     defineConfigOptions(ConfigDefinition& conf, const ConfigGenParameters& params);

llarp/dns/server.cpp

@@ -26,7 +26,7 @@ namespace llarp::dns
   }
 
   bool
-  Proxy::Start(SockAddr addr, std::vector<IpAddress> resolvers)
+  Proxy::Start(SockAddr addr, std::vector<SockAddr> resolvers)
   {
     if (not PacketHandler::Start(addr, std::move(resolvers)))
       return false;
@@ -44,14 +44,18 @@ namespace llarp::dns
   }
 
   bool
-  PacketHandler::Start(SockAddr, std::vector<IpAddress> resolvers)
+  PacketHandler::Start(SockAddr, std::vector<SockAddr> resolvers)
   {
     return SetupUnboundResolver(std::move(resolvers));
   }
 
   bool
-  PacketHandler::SetupUnboundResolver(std::vector<IpAddress> resolvers)
+  PacketHandler::SetupUnboundResolver(std::vector<SockAddr> resolvers)
   {
+    // if we have no resolvers don't set up unbound
+    if (resolvers.empty())
+      return true;
+
     auto failFunc = [self = weak_from_this()](
                         const SockAddr& from, const SockAddr& to, Message msg) {
       if (auto this_ptr = self.lock())
@@ -73,9 +77,9 @@ namespace llarp::dns
     }
     for (const auto& resolver : resolvers)
     {
-      if (not m_UnboundResolver->AddUpstreamResolver(resolver.toHost()))
+      if (not m_UnboundResolver->AddUpstreamResolver(resolver))
       {
-        llarp::LogError("Failed to add upstream DNS server: ", resolver.toHost());
+        llarp::LogError("Failed to add upstream DNS server: ", resolver);
         m_UnboundResolver = nullptr;
         return false;
       }

llarp/dns/server.hpp

@@ -35,7 +35,7 @@ namespace llarp
       virtual ~PacketHandler() = default;
 
       virtual bool
-      Start(SockAddr localaddr, std::vector<IpAddress> upstreamResolvers);
+      Start(SockAddr localaddr, std::vector<SockAddr> upstreamResolvers);
 
       void
       Stop();
@@ -58,10 +58,10 @@ namespace llarp
       HandleUpstreamFailure(const SockAddr& from, const SockAddr& to, Message msg);
 
       bool
-      SetupUnboundResolver(std::vector<IpAddress> resolvers);
+      SetupUnboundResolver(std::vector<SockAddr> resolvers);
 
       IQueryHandler* const m_QueryHandler;
-      std::set<IpAddress> m_Resolvers;
+      std::set<SockAddr> m_Resolvers;
       std::shared_ptr<UnboundResolver> m_UnboundResolver;
       EventLoop_ptr m_Loop;
     };
@@ -73,7 +73,7 @@ namespace llarp
       explicit Proxy(EventLoop_ptr loop, IQueryHandler* handler);
 
       bool
-      Start(SockAddr localaddr, std::vector<IpAddress> resolvers) override;
+      Start(SockAddr localaddr, std::vector<SockAddr> resolvers) override;
 
      protected:
       void

llarp/dns/unbound_resolver.cpp

@@ -2,6 +2,7 @@
 
 #include "server.hpp"
 #include <llarp/util/buffer.hpp>
+#include <sstream>
 
 namespace llarp::dns
 {
@@ -105,9 +106,12 @@ namespace llarp::dns
   }
 
   bool
-  UnboundResolver::AddUpstreamResolver(const std::string& upstreamResolverIP)
+  UnboundResolver::AddUpstreamResolver(const SockAddr& upstreamResolver)
   {
-    if (ub_ctx_set_fwd(unboundContext, upstreamResolverIP.c_str()) != 0)
+    std::stringstream ss;
+    ss << upstreamResolver.hostString() << "@" << upstreamResolver.getPort();
+    const auto str = ss.str();
+    if (ub_ctx_set_fwd(unboundContext, str.c_str()) != 0)
     {
       Reset();
       return false;

llarp/dns/unbound_resolver.hpp

@@ -50,7 +50,7 @@ namespace llarp::dns
     Init();
 
     bool
-    AddUpstreamResolver(const std::string& upstreamResolverIP);
+    AddUpstreamResolver(const SockAddr& upstreamResolverIP);
 
     void
     Lookup(SockAddr to, SockAddr from, Message msg);

llarp/ev/ev_libuv.cpp

@@ -302,7 +302,7 @@ namespace llarp::uv
     handle->on<uvw::UDPDataEvent>([this](auto& event, auto& /*handle*/) {
       on_recv(
           *this,
-          SockAddr{event.sender.ip, static_cast<uint16_t>(event.sender.port)},
+          SockAddr{event.sender.ip, huint16_t{static_cast<uint16_t>(event.sender.port)}},
           OwnedBuffer{std::move(event.data), event.length});
     });
   }

llarp/handlers/exit.cpp

@@ -21,7 +21,7 @@ namespace llarp
         : m_Router(r)
         , m_Resolver(std::make_shared<dns::Proxy>(r->loop(), this))
         , m_Name(std::move(name))
-        , m_LocalResolverAddr("127.0.0.1", 53)
+        , m_LocalResolverAddr{"127.0.0.1:53"}
         , m_QUIC{std::make_shared<quic::TunnelManager>(*this)}
         , m_InetToNetwork(name + "_exit_rx", r->loop(), r->loop())
 
@@ -476,8 +476,8 @@ namespace llarp
 
         GetRouter()->loop()->add_ticker([this] { Flush(); });
 
-        llarp::LogInfo("Trying to start resolver ", m_LocalResolverAddr.toString());
-        return m_Resolver->Start(m_LocalResolverAddr.createSockAddr(), m_UpstreamResolvers);
+        llarp::LogInfo("Trying to start resolver ", m_LocalResolverAddr);
+        return m_Resolver->Start(m_LocalResolverAddr, m_UpstreamResolvers);
       }
       return true;
     }

llarp/handlers/exit.hpp

@@ -211,8 +211,8 @@ namespace llarp
 
       std::shared_ptr<vpn::NetworkInterface> m_NetIf;
 
-      IpAddress m_LocalResolverAddr;
-      std::vector<IpAddress> m_UpstreamResolvers;
+      SockAddr m_LocalResolverAddr;
+      std::vector<SockAddr> m_UpstreamResolvers;
 
       std::shared_ptr<quic::TunnelManager> m_QUIC;
 

llarp/handlers/tun.hpp

@@ -265,7 +265,7 @@ namespace llarp
       /// our ip range we are using
       llarp::IPRange m_OurRange;
       /// upstream dns resolver list
-      std::vector<IpAddress> m_UpstreamResolvers;
+      std::vector<SockAddr> m_UpstreamResolvers;
       /// local dns
       IpAddress m_LocalResolverAddr;
       /// list of strict connect addresses for hooks

llarp/net/sock_addr.cpp

@@ -75,10 +75,10 @@ namespace llarp
     init();
     fromString(addr);
   }
-  SockAddr::SockAddr(std::string_view addr, uint16_t port)
+  SockAddr::SockAddr(std::string_view addr, huint16_t port)
   {
     init();
-    setPort(huint16_t{port});
+    setPort(port);
     fromString(addr, false);
   }
 
@@ -289,7 +289,15 @@ namespace llarp
     // TODO: review
     if (isEmpty())
       return "";
+    std::string str = hostString();
+    str.append(1, ':');
+    str.append(std::to_string(getPort()));
+    return str;
+  }
 
+  std::string
+  SockAddr::hostString() const
+  {
     std::string str;
 
     if (isIPv4())
@@ -313,9 +321,6 @@ namespace llarp
       str.append(buf);
       str.append("]");
     }
-
-    str.append(1, ':');
-    str.append(std::to_string(getPort()));
     return str;
   }
 

llarp/net/sock_addr.hpp

@@ -40,7 +40,7 @@ namespace llarp
 
     // String ctors
     SockAddr(std::string_view addr);
-    SockAddr(std::string_view addr, uint16_t port);  // port is in native (host) order
+    SockAddr(std::string_view addr, huint16_t port);  // port is in native (host) order
 
     SockAddr(const AddressInfo&);
 
@@ -83,6 +83,9 @@ namespace llarp
     std::string
     toString() const;
 
+    std::string
+    hostString() const;
+
     /// Returns true if this is an empty SockAddr, defined by having no IP address set. An empty IP
     /// address with a valid port is still considered empty.
     ///

llarp/quic/tunnel.cpp

@@ -485,7 +485,7 @@ namespace llarp::quic
     }
 
     auto bound = tcp_tunnel->sock();
-    saddr = SockAddr{bound.ip, static_cast<uint16_t>(bound.port)};
+    saddr = SockAddr{bound.ip, huint16_t{static_cast<uint16_t>(bound.port)}};
 
     // Find the first unused psuedo-port value starting from next_pseudo_port_.
     if (auto p = find_unused_key(client_tunnels_, next_pseudo_port_))

llarp/router/route_poker.cpp

@@ -163,7 +163,7 @@ namespace llarp
 
     systemd_resolved_set_dns(
         m_Router->hiddenServiceContext().GetDefault()->GetIfName(),
-        m_Router->GetConfig()->dns.m_bind.createSockAddr(),
+        m_Router->GetConfig()->dns.m_bind,
         true /* route all DNS */);
   }
 
@@ -178,7 +178,7 @@ namespace llarp
 
     systemd_resolved_set_dns(
         m_Router->hiddenServiceContext().GetDefault()->GetIfName(),
-        m_Router->GetConfig()->dns.m_bind.createSockAddr(),
+        m_Router->GetConfig()->dns.m_bind,
         false /* route DNS only for .loki/.snode */);
   }
 

test/net/test_sock_addr.cpp

@@ -31,7 +31,7 @@ TEST_CASE("SockAddr fromString", "[SockAddr]")
   CHECK(llarp::SockAddr("255.255.255.255").toString() == "255.255.255.255:0");
   CHECK(llarp::SockAddr("255.255.255.255:255").toString() == "255.255.255.255:255");
   CHECK(llarp::SockAddr("255.255.255.255:65535").toString() == "255.255.255.255:65535");
-  CHECK(llarp::SockAddr("5.6.7.8", 5678).toString() == "5.6.7.8:5678");
+  CHECK(llarp::SockAddr("5.6.7.8", llarp::huint16_t{5678}).toString() == "5.6.7.8:5678");
 
   CHECK_THROWS_WITH(llarp::SockAddr("abcd"), "abcd is not a valid IPv4 address");
 
@@ -66,7 +66,7 @@ TEST_CASE("SockAddr fromString", "[SockAddr]")
 
   CHECK_THROWS_WITH(llarp::SockAddr("1.2.3.4:1a"), "1a is not a valid port");
 
-  CHECK_THROWS_WITH(llarp::SockAddr("5.6.7.8:1234", 5678), "invalid ip address (port not allowed here): 5.6.7.8:1234");
+  CHECK_THROWS_WITH(llarp::SockAddr("5.6.7.8:1234", llarp::huint16_t{5678}), "invalid ip address (port not allowed here): 5.6.7.8:1234");
 }
 
 TEST_CASE("SockAddr from sockaddr_in", "[SockAddr]")