Windows DNS fixes

- windivert was being set up *before* DNS is set up, so the DNS port was
  nullopt and thus we couldn't properly identify upstream DNS traffic.
- close() doesn't close a socket on Windows, so the socket-bind-close
  approach to get a free UDP port wasn't actually closing, and thus
  unbound upstream constrained to the given port were completely
  failing.
- The unbound thread was accessing the same shared_ptr instance as the
  outer code, which isn't thread-safe; changed it to copy a weak_ptr
  into the lambda instead.
- Exclude upstream DNS traffic in the filter rather than capturing and
  reinjecting it.

llarp/dns/server.cpp

@@ -286,27 +286,40 @@ namespace llarp::dns
             // on our system and use it so we KNOW what it is before giving it to unbound to
             // explicitly bind to JUST that port.
 
-            int fd = socket(AF_INET, SOCK_DGRAM, 0);
+            auto fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+#ifdef _WIN32
+            if (fd == INVALID_SOCKET)
+#else
             if (fd == -1)
+#endif
+            {
               throw std::invalid_argument{
                   fmt::format("Failed to create UDP socket for unbound: {}", strerror(errno))};
+            }
+
+#ifdef _WIN32
+#define CLOSE closesocket
+#else
+#define CLOSE close
+#endif
             if (0 != bind(fd, static_cast<const sockaddr*>(addr), addr.sockaddr_len()))
             {
-              close(fd);
+              CLOSE(fd);
               throw std::invalid_argument{
                   fmt::format("Failed to bind UDP socket for unbound: {}", strerror(errno))};
             }
             struct sockaddr_storage sas;
             auto* sa = reinterpret_cast<struct sockaddr*>(&sas);
             socklen_t sa_len = sizeof(sas);
-            if (0 != getsockname(fd, sa, &sa_len))
+            int rc = getsockname(fd, sa, &sa_len);
+            CLOSE(fd);
+#undef CLOSE
+            if (rc != 0)
             {
-              close(fd);
               throw std::invalid_argument{
                   fmt::format("Failed to query UDP port for unbound: {}", strerror(errno))};
             }
             addr = SockAddr{*sa};
-            close(fd);
           }
           m_LocalAddr = addr;
 
@@ -323,15 +336,15 @@ namespace llarp::dns
         // setup mainloop
 #ifdef _WIN32
         running = true;
-        runner = std::thread{[this]() {
+        runner = std::thread{[this, ctx = std::weak_ptr{m_ctx}]() {
           while (running)
           {
-            if (m_ctx.get())
-              ub_wait(m_ctx.get());
-            std::this_thread::sleep_for(25ms);
+            if (auto c = ctx.lock())
+              ub_wait(c.get());
+            std::this_thread::sleep_for(10ms);
           }
-          if (m_ctx.get())
-            ub_process(m_ctx.get());
+          if (auto c = ctx.lock())
+            ub_process(c.get());
         }};
 #else
         if (auto loop = m_Loop.lock())

llarp/handlers/tun.cpp

@@ -157,44 +157,6 @@ namespace llarp
       if (m_DnsConfig.m_raw_dns)
       {
         auto dns = std::make_shared<TunDNS>(this, m_DnsConfig);
-        if (auto vpn = Router()->GetVPNPlatform())
-        {
-          // get the first local address we know of
-          std::optional<SockAddr> localaddr;
-          for (auto res : dns->GetAllResolvers())
-          {
-            if (localaddr)
-              continue;
-            if (auto ptr = res.lock())
-              localaddr = ptr->GetLocalAddr();
-          }
-          if (platform::is_windows)
-          {
-            auto dns_io = vpn->create_packet_io(0);
-            LogInfo("doing dns queries from ", *localaddr);
-            Router()->loop()->add_ticker(
-                [r = Router(), dns_io, handler = m_PacketRouter, src = localaddr]() {
-                  net::IPPacket pkt = dns_io->ReadNextPacket();
-                  while (not pkt.empty())
-                  {
-                    // reinject if for upstream dns
-                    if (src and pkt.src() == *src)
-                    {
-                      LogInfo("reinject dns");
-                      std::function<void(net::IPPacket)> reply{std::move(pkt.reply)};
-                      reply(std::move(pkt));
-                    }
-                    else
-                    {
-                      LogInfo("got dns packet from ", pkt.src(), " of size ", pkt.size(), "B");
-                      handler->HandleIPPacket(std::move(pkt));
-                    }
-                    pkt = dns_io->ReadNextPacket();
-                  }
-                });
-            m_RawDNS = dns_io;
-          }
-        }
         m_DNS = dns;
 
         m_PacketRouter->AddUDPHandler(huint16_t{53}, [this, dns](net::IPPacket pkt) {
@@ -211,10 +173,42 @@ namespace llarp
       else
         m_DNS = std::make_shared<dns::Server>(Loop(), m_DnsConfig, info.index);
 
-      if (m_RawDNS)
-        m_RawDNS->Start();
       m_DNS->AddResolver(weak_from_this());
       m_DNS->Start();
+
+      if (m_DnsConfig.m_raw_dns)
+      {
+        if (auto vpn = Router()->GetVPNPlatform())
+        {
+          // get the first local address we know of
+          std::optional<SockAddr> localaddr;
+          for (auto res : m_DNS->GetAllResolvers())
+          {
+            if (auto ptr = res.lock())
+            {
+              localaddr = ptr->GetLocalAddr();
+              if (localaddr)
+                break;
+            }
+          }
+          if (platform::is_windows)
+          {
+            auto dns_io = vpn->create_packet_io(0, localaddr);
+            Router()->loop()->add_ticker([r = Router(), dns_io, handler = m_PacketRouter]() {
+              net::IPPacket pkt = dns_io->ReadNextPacket();
+              while (not pkt.empty())
+              {
+                handler->HandleIPPacket(std::move(pkt));
+                pkt = dns_io->ReadNextPacket();
+              }
+            });
+            m_RawDNS = dns_io;
+          }
+        }
+
+        if (m_RawDNS)
+          m_RawDNS->Start();
+      }
     }
 
     util::StatusObject

llarp/vpn/platform.hpp

@@ -154,7 +154,9 @@ namespace llarp::vpn
     /// @param index the interface index of the network interface to use or 0 for all
     /// interfaces on the system
     virtual std::shared_ptr<I_Packet_IO>
-    create_packet_io(unsigned int)
+    create_packet_io(
+        [[maybe_unused]] unsigned int ifindex,
+        [[maybe_unused]] const std::optional<SockAddr>& dns_upstream_src)
     {
       throw std::runtime_error{"raw packet io is unimplemented"};
     }

llarp/vpn/win32.cpp

@@ -1,4 +1,7 @@
 #include "vpn/win32.hpp"
+#include <llarp/win32/windivert.hpp>
+#include <llarp/win32/wintun.hpp>
+#include <fmt/core.h>
 
 namespace llarp::win32
 {
@@ -135,7 +138,8 @@ namespace llarp::win32
   }
 
   std::shared_ptr<I_Packet_IO>
-  VPNPlatform::create_packet_io(unsigned int ifindex)
+  VPNPlatform::create_packet_io(
+      unsigned int ifindex, const std::optional<SockAddr>& dns_upstream_src)
   {
     // we only want do this on all interfaes with windivert
     if (ifindex)
@@ -143,17 +147,13 @@ namespace llarp::win32
           "cannot create packet io on explicitly specified interface, not currently supported on "
           "windows (yet)"};
 
-    std::string filter{"outbound and ( udp.DstPort == 53 or tcp.DstPort == 53 )"};
+    uint16_t upstream_src_port = dns_upstream_src ? dns_upstream_src->getPort() : 0;
+    std::string udp_filter = upstream_src_port != 0
+        ? fmt::format("( udp.DstPort == 53 and udp.SrcPort != {} )", upstream_src_port)
+        : "udp.DstPort == 53";
+
+    auto filter = "outbound and ( " + udp_filter + " or tcp.DstPort == 53 )";
 
-    if (auto dscp = _ctx->router->GetConfig()->dns.m_queryDSCP.value_or(0))
-    {
-      // DSCP is the first 6 bits of the TOS field (the last 2 are ECN).
-      auto tos = dscp << 2;
-      fmt::format_to(std::back_inserter(filter), " and ip.TOS != {}", tos);
-    }
     return WinDivert::make_interceptor(filter, [router = _ctx->router] { router->TriggerPump(); });
   }
-
-};
-
 }  // namespace llarp::win32

llarp/vpn/win32.hpp

@@ -1,16 +1,11 @@
 #pragma once
 
+#include <winsock2.h>
 #include <windows.h>
 #include <iphlpapi.h>
-#include <io.h>
-#include <fcntl.h>
-#include <llarp/util/thread/queue.hpp>
 #include <llarp/router/abstractrouter.hpp>
 #include <llarp/win32/exec.hpp>
-#include <llarp/win32/windivert.hpp>
-#include <llarp/win32/wintun.hpp>
 #include <llarp.hpp>
-#include <fmt/std.h>
 
 #include "platform.hpp"
 
@@ -71,7 +66,7 @@ namespace llarp::win32
     ObtainInterface(InterfaceInfo info, AbstractRouter* router) override;
 
     std::shared_ptr<I_Packet_IO>
-    create_packet_io(unsigned int ifindex) override;
+    create_packet_io(unsigned int ifindex, const std::optional<SockAddr>& dns_upstream_src) override;
 
     IRouteManager&
     RouteManager() override

llarp/win32/windivert.cpp

@@ -69,7 +69,7 @@ namespace llarp::win32
       static constexpr size_t recv_queue_size = 64;
 
      public:
-      IO(std::string filter_spec, std::function<void(void)> wake)
+      IO(const std::string& filter_spec, std::function<void(void)> wake)
           : m_Wake{wake}, m_RecvQueue{recv_queue_size}
       {
         wd::Initialize();
@@ -193,7 +193,7 @@ namespace llarp::win32
     }
 
     std::shared_ptr<llarp::vpn::I_Packet_IO>
-    make_interceptor(std::string filter_spec, std::function<void(void)> wake)
+    make_interceptor(const std::string& filter_spec, std::function<void(void)> wake)
     {
       return std::make_shared<wd::IO>(filter_spec, wake);
     }

llarp/win32/windivert.hpp

@@ -16,6 +16,6 @@ namespace llarp::win32::WinDivert
   /// we hide all implementation details from other compilation units to prevent issues with
   /// linkage that may arrise.
   std::shared_ptr<llarp::vpn::I_Packet_IO>
-  make_interceptor(std::string filter_spec, std::function<void(void)> wakeup);
+  make_interceptor(const std::string& filter_spec, std::function<void(void)> wakeup);
 
 }  // namespace llarp::win32::WinDivert