diff --git a/llarp/apple/route_manager.cpp b/llarp/apple/route_manager.cpp index 3358d6c8b..5265faf58 100644 --- a/llarp/apple/route_manager.cpp +++ b/llarp/apple/route_manager.cpp @@ -31,10 +31,10 @@ namespace llarp::apple } if (enable) - saved_upstream_dns = - tun->ReconfigureDNS({SockAddr{127, 0, 0, 1, huint16_t{dns_trampoline_port}}}); + tun->ReconfigureDNS({SockAddr{127, 0, 0, 1, {dns_trampoline_port}}}); else - tun->ReconfigureDNS(std::move(saved_upstream_dns)); + tun->ReconfigureDNS(router->GetConfig()->dns.m_upstreamDNS); + trampoline_active = enable; } diff --git a/llarp/apple/route_manager.hpp b/llarp/apple/route_manager.hpp index d6e7c611a..69403c04d 100644 --- a/llarp/apple/route_manager.hpp +++ b/llarp/apple/route_manager.hpp @@ -48,7 +48,6 @@ namespace llarp::apple private: llarp::Context& context; bool trampoline_active = false; - std::vector saved_upstream_dns; void check_trampoline(bool enable); diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 8a85c6fdc..b102c0219 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -113,9 +113,9 @@ namespace llarp::dns }; /// Resolver_Base that uses libunbound - class Resolver : public Resolver_Base, public std::enable_shared_from_this + class Resolver final : public Resolver_Base, public std::enable_shared_from_this { - std::shared_ptr m_ctx; + ub_ctx* m_ctx = nullptr; std::weak_ptr m_Loop; #ifdef _WIN32 // windows is dumb so we do ub mainloop in a thread @@ -179,7 +179,7 @@ namespace llarp::dns if (const auto port = dns.getPort(); port != 53) fmt::format_to(std::back_inserter(str), "@{}", port); - if (auto err = ub_ctx_set_fwd(m_ctx.get(), str.c_str())) + if (auto err = ub_ctx_set_fwd(m_ctx, str.c_str())) { throw std::runtime_error{ fmt::format("cannot use {} as upstream dns: {}", str, ub_strerror(err))}; @@ -300,7 +300,7 @@ namespace llarp::dns void SetOpt(const std::string& key, const std::string& val) { - ub_ctx_set_option(m_ctx.get(), key.c_str(), val.c_str()); + ub_ctx_set_option(m_ctx, key.c_str(), val.c_str()); } // Wrapper around the above that takes 3+ arguments: the 2nd arg gets formatted with the @@ -312,24 +312,21 @@ namespace llarp::dns SetOpt(key, fmt::format(format, std::forward(args)...)); } + // Copy of the DNS config (a copy because on some platforms, like Apple, we change the applied + // upstream DNS settings when turning on/off exit mode). llarp::DnsConfig m_conf; public: explicit Resolver(const EventLoop_ptr& loop, llarp::DnsConfig conf) - : m_ctx{::ub_ctx_create(), ::ub_ctx_delete}, m_Loop{loop}, m_conf{std::move(conf)} + : m_Loop{loop}, m_conf{std::move(conf)} { Up(m_conf); } -#ifdef _WIN32 - virtual ~Resolver() + ~Resolver() override { - running = false; - runner.join(); + Down(); } -#else - virtual ~Resolver() = default; -#endif std::string_view ResolverName() const override @@ -346,6 +343,10 @@ namespace llarp::dns void Up(const llarp::DnsConfig& conf) { + if (m_ctx) + throw std::logic_error{"Internal error: attempt to Up() dns server multiple times"}; + + m_ctx = ::ub_ctx_create(); // set libunbound settings SetOpt("do-tcp:", "no"); @@ -357,7 +358,7 @@ namespace llarp::dns for (const auto& file : conf.m_hostfiles) { const auto str = file.u8string(); - if (auto ret = ub_ctx_hosts(m_ctx.get(), str.c_str())) + if (auto ret = ub_ctx_hosts(m_ctx, str.c_str())) { throw std::runtime_error{ fmt::format("Failed to add host file {}: {}", file, ub_strerror(ret))}; @@ -367,15 +368,14 @@ namespace llarp::dns ConfigureUpstream(conf); // set async - ub_ctx_async(m_ctx.get(), 1); + ub_ctx_async(m_ctx, 1); // setup mainloop #ifdef _WIN32 running = true; - runner = std::thread{[this, ctx = std::weak_ptr{m_ctx}]() { + runner = std::thread{[this]() { while (running) { - if (auto c = ctx.lock()) - ub_wait(c.get()); + ub_wait(ctx); std::this_thread::sleep_for(10ms); } if (auto c = ctx.lock()) @@ -386,10 +386,9 @@ namespace llarp::dns { if (auto loop_ptr = loop->MaybeGetUVWLoop()) { - m_Poller = loop_ptr->resource(ub_fd(m_ctx.get())); - m_Poller->on([ptr = std::weak_ptr{m_ctx}](auto&, auto&) { - if (auto ctx = ptr.lock()) - ub_process(ctx.get()); + m_Poller = loop_ptr->resource(ub_fd(m_ctx)); + m_Poller->on([this](auto&, auto&) { + ub_process(m_ctx); }); m_Poller->start(uvw::PollHandle::Event::READABLE); return; @@ -400,15 +399,19 @@ namespace llarp::dns } void - Down() + Down() override { #ifdef _WIN32 - running = false; - runner.join(); + if (running.exchange(false)) + runner.join(); #else - m_Poller->close(); + if (m_Poller) + m_Poller->close(); #endif - m_ctx.reset(); + if (m_ctx) { + ::ub_ctx_delete(m_ctx); + m_ctx = nullptr; + } } int @@ -418,20 +421,14 @@ namespace llarp::dns } void - ResetInternalState(std::optional> replace_upstream) override + ResetResolver(std::optional> replace_upstream) override { Down(); if (replace_upstream) - m_conf.m_upstreamDNS = *replace_upstream; + m_conf.m_upstreamDNS = std::move(*replace_upstream); Up(m_conf); } - void - CancelPendingQueries() override - { - Down(); - } - bool WouldLoop(const SockAddr& to, const SockAddr& from) const override { @@ -485,7 +482,7 @@ namespace llarp::dns } const auto& q = query.questions[0]; if (auto err = ub_resolve_async( - m_ctx.get(), + m_ctx, q.Name().c_str(), q.qtype, q.qclass, @@ -642,7 +639,7 @@ namespace llarp::dns for (const auto& resolver : m_Resolvers) { if (auto ptr = resolver.lock()) - ptr->CancelPendingQueries(); + ptr->Down(); } } @@ -652,7 +649,7 @@ namespace llarp::dns for (const auto& resolver : m_Resolvers) { if (auto ptr = resolver.lock()) - ptr->ResetInternalState(); + ptr->ResetResolver(); } } diff --git a/llarp/dns/server.hpp b/llarp/dns/server.hpp index eef7ee8cb..17ad38b3d 100644 --- a/llarp/dns/server.hpp +++ b/llarp/dns/server.hpp @@ -174,16 +174,17 @@ namespace llarp::dns virtual std::string_view ResolverName() const = 0; - /// reset state, replace upstream info with new info if desired + /// reset the resolver state, optionally replace upstream info with new info. The default base + /// implementation does nothing. virtual void - ResetInternalState(std::optional> replace_upstream = std::nullopt) + ResetResolver([[maybe_unused]] std::optional> replace_upstream = std::nullopt) { - (void)replace_upstream; - }; + } - /// cancel all pending requests and ceace further operation + /// cancel all pending requests and cease further operation. Default operation is a no-op. virtual void - CancelPendingQueries(){}; + Down(){} + /// attempt to handle a dns message /// returns true if we consumed this query and it should not be processed again virtual bool @@ -201,7 +202,7 @@ namespace llarp::dns (void)to; (void)from; return false; - }; + } }; // Base class for DNS proxy diff --git a/llarp/handlers/exit.hpp b/llarp/handlers/exit.hpp index 470cfea4c..79fc2be47 100644 --- a/llarp/handlers/exit.hpp +++ b/llarp/handlers/exit.hpp @@ -24,9 +24,6 @@ namespace llarp return "snode"; } - void - CancelPendingQueries() override{}; - bool MaybeHookDNS( std::shared_ptr source, diff --git a/llarp/handlers/tun.cpp b/llarp/handlers/tun.cpp index d4980c684..c2446ab1b 100644 --- a/llarp/handlers/tun.cpp +++ b/llarp/handlers/tun.cpp @@ -260,7 +260,7 @@ namespace llarp m_DNS->Reset(); } - std::vector + void TunEndpoint::ReconfigureDNS(std::vector servers) { if (m_DNS) @@ -268,10 +268,9 @@ namespace llarp for (auto weak : m_DNS->GetAllResolvers()) { if (auto ptr = weak.lock()) - ptr->ResetInternalState(servers); + ptr->ResetResolver(servers); } } - return servers; } bool @@ -903,7 +902,7 @@ namespace llarp return true; } - void TunEndpoint::ResetInternalState(std::optional>) + void TunEndpoint::ResetInternalState() { service::Endpoint::ResetInternalState(); } diff --git a/llarp/handlers/tun.hpp b/llarp/handlers/tun.hpp index 4335ce812..be7db4641 100644 --- a/llarp/handlers/tun.hpp +++ b/llarp/handlers/tun.hpp @@ -67,9 +67,8 @@ namespace llarp void Thaw() override; - // Reconfigures DNS servers and restarts libunbound with the new servers. Returns the old set - // of configured dns servers. - std::vector + // Reconfigures DNS servers and restarts libunbound with the new servers. + void ReconfigureDNS(std::vector servers); bool @@ -202,8 +201,7 @@ namespace llarp ObtainIPForAddr(std::variant addr) override; void - ResetInternalState( - std::optional> replace_upstream = std::nullopt) override; + ResetInternalState() override; protected: struct WritePacket