From a518e654c555838db16900de3434115fc42ea880 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 16:27:21 -0500 Subject: [PATCH 1/8] add much logging around dns and windivert --- llarp/dns/server.cpp | 41 +++++++++++++++++- llarp/win32/windivert.cpp | 91 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 130 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 4a0d22da4..a03f10cce 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -146,6 +146,7 @@ namespace llarp::dns static void Callback(void* data, int err, ub_result* _result) { + log::debug(logcat, "got dns response from libunbound"); // take ownership of ub_result std::unique_ptr result{_result}; // borrow query @@ -158,6 +159,8 @@ namespace llarp::dns return; } + log::trace(logcat, "queueing dns response from libunbound to userland"); + // rewrite response OwnedBuffer pkt{(const byte_t*)result->answer_packet, (size_t)result->answer_len}; llarp_buffer_t buf{pkt}; @@ -490,6 +493,11 @@ namespace llarp::dns // no questions, send fail if (query.questions.empty()) { + log::info( + logcat, + "dns from {} to {} has empty query questions, sending failure reply", + from, + to); tmp->Cancel(); return true; } @@ -499,6 +507,12 @@ namespace llarp::dns // dont process .loki or .snode if (q.HasTLD(".loki") or q.HasTLD(".snode")) { + log::warning( + logcat, + "dns from {} to {} is for .loki or .snode but got to the unbound resolver, sending " + "failure reply", + from, + to); tmp->Cancel(); return true; } @@ -506,6 +520,12 @@ namespace llarp::dns if (not m_ctx) { // we are down + log::debug( + logcat, + "dns from {} to {} got to the unbound resolver, but the resolver isn't set up, " + "sending failure reply", + from, + to); tmp->Cancel(); return true; } @@ -514,6 +534,12 @@ namespace llarp::dns if (not running) { // we are stopping the win32 thread + log::debug( + logcat, + "dns from {} to {} got to the unbound resolver, but the resolver isn't running, " + "sending failure reply", + from, + to); tmp->Cancel(); return true; } @@ -533,7 +559,10 @@ namespace llarp::dns tmp->Cancel(); } else + { + log::trace(logcat, "dns from {} to {} processing via libunbound", from, to); m_Pending.insert(std::move(tmp)); + } return true; } @@ -549,6 +578,12 @@ namespace llarp::dns { parent_ptr->call( [self = shared_from_this(), parent_ptr = std::move(parent_ptr), buf = replyBuf.copy()] { + log::trace( + logcat, + "forwarding dns response from libunbound to userland (resolverAddr: {}, " + "askerAddr: {})", + self->resolverAddr, + self->askerAddr); self->src->SendTo(self->askerAddr, self->resolverAddr, OwnedBuffer::copy_from(buf)); // remove query parent_ptr->RemovePending(self); @@ -742,10 +777,14 @@ namespace llarp::dns { if (auto res_ptr = resolver.lock()) { - log::debug( + log::trace( logcat, "check resolver {} for dns from {} to {}", res_ptr->ResolverName(), from, to); if (res_ptr->MaybeHookDNS(ptr, msg, to, from)) + { + log::trace( + logcat, "resolver {} handling dns from {} to {}", res_ptr->ResolverName(), from, to); return true; + } } } return false; diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 344718db1..4faeee3d0 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -55,6 +55,90 @@ namespace llarp::win32 WINDIVERT_ADDRESS addr; }; + void + log_windivert_addr(const WINDIVERT_ADDRESS& addr) + { + std::string layer_str{}; + std::string ifidx_str{}; + switch (addr.Layer) + { + case WINDIVERT_LAYER_NETWORK: + layer_str = "WINDIVERT_LAYER_NETWORK"; + ifidx_str = "Network: [IfIdx: {}, SubIfIdx: {}]"_format( + addr.Network.IfIdx, addr.Network.SubIfIdx); + break; + case WINDIVERT_LAYER_NETWORK_FORWARD: + layer_str = "WINDIVERT_LAYER_NETWORK_FORWARD"; + break; + case WINDIVERT_LAYER_FLOW: + layer_str = "WINDIVERT_LAYER_FLOW"; + break; + case WINDIVERT_LAYER_SOCKET: + layer_str = "WINDIVERT_LAYER_SOCKET"; + break; + case WINDIVERT_LAYER_REFLECT: + layer_str = "WINDIVERT_LAYER_REFLECT"; + break; + default: + layer_str = "unknown"; + } + + std::string event_str{}; + switch (addr.Event) + { + case WINDIVERT_EVENT_NETWORK_PACKET: + event_str = "WINDIVERT_EVENT_NETWORK_PACKET"; + break; + case WINDIVERT_EVENT_FLOW_ESTABLISHED: + event_str = "WINDIVERT_EVENT_FLOW_ESTABLISHED"; + break; + case WINDIVERT_EVENT_FLOW_DELETED: + event_str = "WINDIVERT_EVENT_FLOW_DELETED"; + break; + case WINDIVERT_EVENT_SOCKET_BIND: + event_str = "WINDIVERT_EVENT_SOCKET_BIND"; + break; + case WINDIVERT_EVENT_SOCKET_CONNECT: + event_str = "WINDIVERT_EVENT_SOCKET_CONNECT"; + break; + case WINDIVERT_EVENT_SOCKET_LISTEN: + event_str = "WINDIVERT_EVENT_SOCKET_LISTEN"; + break; + case WINDIVERT_EVENT_SOCKET_ACCEPT: + event_str = "WINDIVERT_EVENT_SOCKET_ACCEPT"; + break; + case WINDIVERT_EVENT_SOCKET_CLOSE: + event_str = "WINDIVERT_EVENT_SOCKET_CLOSE"; + break; + case WINDIVERT_EVENT_REFLECT_OPEN: + event_str = "WINDIVERT_EVENT_REFLECT_OPEN"; + break; + case WINDIVERT_EVENT_REFLECT_CLOSE: + event_str = "WINDIVERT_EVENT_REFLECT_CLOSE"; + break; + default: + event_str = "unknown"; + } + + log::trace( + logcat, + "Windivert WINDIVERT_ADDRESS -- Timestamp: {}, Layer: {}, Event: {}, Sniffed: {}, " + "Outbound: {}, Loopback: {}, Imposter: {}, IPv6: {}, IPChecksum: {}, TCPChecksum: {}, " + "UDPChecksum: {}, {}", + addr.Timestamp, + layer_str, + event_str, + addr.Sniffed ? "true" : "false", + addr.Outbound ? "true" : "false", + addr.Loopback ? "true" : "false", + addr.Impostor ? "true" : "false", + addr.IPv6 ? "true" : "false", + addr.IPChecksum ? "true" : "false", + addr.TCPChecksum ? "true" : "false", + addr.UDPChecksum ? "true" : "false", + ifidx_str); + } + class IO : public llarp::vpn::I_Packet_IO { std::function m_Wake; @@ -106,8 +190,11 @@ namespace llarp::win32 throw win32::error{ err, fmt::format("failed to receive packet from windivert (code={})", err)}; } - log::trace(logcat, "got packet of size {}B", sz); pkt.resize(sz); + + log::trace(logcat, "got packet of size {}B", sz); + log_windivert_addr(addr); + return Packet{std::move(pkt), std::move(addr)}; } @@ -117,6 +204,8 @@ namespace llarp::win32 const auto& pkt = w_pkt.pkt; const auto* addr = &w_pkt.addr; log::trace(logcat, "send dns packet of size {}B", pkt.size()); + log_windivert_addr(w_pkt.addr); + UINT sz{}; if (!wd::send(m_Handle, pkt.data(), pkt.size(), &sz, addr)) throw win32::error{"windivert send failed"}; From 133cee0fd9b3c95777701e397d4429d9e5287244 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 16:27:56 -0500 Subject: [PATCH 2/8] Remove obsolete/extraneous WouldLoop function The DNS resolver code should not and can not be responsible for preventing packet looping. --- llarp/dns/server.cpp | 17 ----------------- llarp/dns/server.hpp | 10 ---------- 2 files changed, 27 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index a03f10cce..ef1c01c03 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -455,20 +455,6 @@ namespace llarp::dns Up(m_conf); } - bool - WouldLoop(const SockAddr& to, const SockAddr& from) const override - { -#if defined(ANDROID) - (void)to; - (void)from; - return false; -#else - const auto& vec = m_conf.m_upstreamDNS; - return std::find(vec.begin(), vec.end(), to) != std::end(vec) - or std::find(vec.begin(), vec.end(), from) != std::end(vec); -#endif - } - template void call(Callable&& f) @@ -486,9 +472,6 @@ namespace llarp::dns const SockAddr& to, const SockAddr& from) override { - if (WouldLoop(to, from)) - return false; - auto tmp = std::make_shared(weak_from_this(), query, source, to, from); // no questions, send fail if (query.questions.empty()) diff --git a/llarp/dns/server.hpp b/llarp/dns/server.hpp index 7f22df48e..6b1ba54b3 100644 --- a/llarp/dns/server.hpp +++ b/llarp/dns/server.hpp @@ -197,16 +197,6 @@ namespace llarp::dns const Message& query, const SockAddr& to, const SockAddr& from) = 0; - - /// Returns true if a packet with to and from addresses is something that would cause a - /// resolution loop and thus should not be used on this resolver - virtual bool - WouldLoop(const SockAddr& to, const SockAddr& from) const - { - (void)to; - (void)from; - return false; - } }; // Base class for DNS proxy From 5238c3f1a0d5101d6f443a22a746d411e0fe9cc1 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 16:30:39 -0500 Subject: [PATCH 3/8] force windivert to recalc IP checksum --- llarp/win32/windivert.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 4faeee3d0..6f8a3aa58 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -23,6 +23,7 @@ namespace llarp::win32 decltype(::WinDivertOpen)* open = nullptr; decltype(::WinDivertClose)* close = nullptr; decltype(::WinDivertShutdown)* shutdown = nullptr; + decltype(::WinDivertHelperCalcChecksums)* calc_checksum = nullptr; decltype(::WinDivertSend)* send = nullptr; decltype(::WinDivertRecv)* recv = nullptr; decltype(::WinDivertHelperFormatIPv4Address)* format_ip4 = nullptr; @@ -41,6 +42,7 @@ namespace llarp::win32 "WinDivertOpen", open, "WinDivertClose", close, "WinDivertShutdown", shutdown, + "WinDivertHelperCalcChecksums", calc_checksum, "WinDivertSend", send, "WinDivertRecv", recv, "WinDivertHelperFormatIPv4Address", format_ip4, @@ -199,14 +201,18 @@ namespace llarp::win32 } void - send_packet(const Packet& w_pkt) const + send_packet(Packet&& w_pkt) const { - const auto& pkt = w_pkt.pkt; - const auto* addr = &w_pkt.addr; + auto& pkt = w_pkt.pkt; + auto* addr = &w_pkt.addr; + log::trace(logcat, "send dns packet of size {}B", pkt.size()); log_windivert_addr(w_pkt.addr); UINT sz{}; + // recalc IP packet checksum in case it needs it + wd::calc_checksum(pkt.data(), pkt.size(), addr, 0); + if (!wd::send(m_Handle, pkt.data(), pkt.size(), &sz, addr)) throw win32::error{"windivert send failed"}; } From 548ce5c3a2a097a2279825decc0ed61656a3c116 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 16:32:15 -0500 Subject: [PATCH 4/8] invert packet direction on WINDIVERT_ADDRESS We simply keep the WINDIVERT_ADDRESS struct given on recv, so when using it for send we need to invert the direction (the Output bit) --- llarp/win32/windivert.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 6f8a3aa58..d2dadc01a 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -206,6 +206,8 @@ namespace llarp::win32 auto& pkt = w_pkt.pkt; auto* addr = &w_pkt.addr; + addr->Outbound = !addr->Outbound; // re-used from recv, so invert direction + log::trace(logcat, "send dns packet of size {}B", pkt.size()); log_windivert_addr(w_pkt.addr); From d44ad497fd10f56720979ad626cd9857820ed271 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 17:48:03 -0500 Subject: [PATCH 5/8] rvalue ref -> value --- llarp/win32/windivert.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index d2dadc01a..67f7273dc 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -201,7 +201,7 @@ namespace llarp::win32 } void - send_packet(Packet&& w_pkt) const + send_packet(Packet w_pkt) const { auto& pkt = w_pkt.pkt; auto* addr = &w_pkt.addr; From 3d71bbd1e435f9eb4daa3a3dfc59e2b9a7fb62ba Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 17:59:54 -0500 Subject: [PATCH 6/8] log func should return a string instead --- llarp/win32/windivert.cpp | 175 +++++++++++++++++++------------------- 1 file changed, 88 insertions(+), 87 deletions(-) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 67f7273dc..157a4d881 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -12,6 +12,92 @@ extern "C" #include } +namespace +{ + std::string + windivert_addr_to_string(const WINDIVERT_ADDRESS& addr) + { + std::string layer_str{}; + std::string ifidx_str{}; + switch (addr.Layer) + { + case WINDIVERT_LAYER_NETWORK: + layer_str = "WINDIVERT_LAYER_NETWORK"; + ifidx_str = "Network: [IfIdx: {}, SubIfIdx: {}]"_format( + addr.Network.IfIdx, addr.Network.SubIfIdx); + break; + case WINDIVERT_LAYER_NETWORK_FORWARD: + layer_str = "WINDIVERT_LAYER_NETWORK_FORWARD"; + break; + case WINDIVERT_LAYER_FLOW: + layer_str = "WINDIVERT_LAYER_FLOW"; + break; + case WINDIVERT_LAYER_SOCKET: + layer_str = "WINDIVERT_LAYER_SOCKET"; + break; + case WINDIVERT_LAYER_REFLECT: + layer_str = "WINDIVERT_LAYER_REFLECT"; + break; + default: + layer_str = "unknown"; + } + + std::string event_str{}; + switch (addr.Event) + { + case WINDIVERT_EVENT_NETWORK_PACKET: + event_str = "WINDIVERT_EVENT_NETWORK_PACKET"; + break; + case WINDIVERT_EVENT_FLOW_ESTABLISHED: + event_str = "WINDIVERT_EVENT_FLOW_ESTABLISHED"; + break; + case WINDIVERT_EVENT_FLOW_DELETED: + event_str = "WINDIVERT_EVENT_FLOW_DELETED"; + break; + case WINDIVERT_EVENT_SOCKET_BIND: + event_str = "WINDIVERT_EVENT_SOCKET_BIND"; + break; + case WINDIVERT_EVENT_SOCKET_CONNECT: + event_str = "WINDIVERT_EVENT_SOCKET_CONNECT"; + break; + case WINDIVERT_EVENT_SOCKET_LISTEN: + event_str = "WINDIVERT_EVENT_SOCKET_LISTEN"; + break; + case WINDIVERT_EVENT_SOCKET_ACCEPT: + event_str = "WINDIVERT_EVENT_SOCKET_ACCEPT"; + break; + case WINDIVERT_EVENT_SOCKET_CLOSE: + event_str = "WINDIVERT_EVENT_SOCKET_CLOSE"; + break; + case WINDIVERT_EVENT_REFLECT_OPEN: + event_str = "WINDIVERT_EVENT_REFLECT_OPEN"; + break; + case WINDIVERT_EVENT_REFLECT_CLOSE: + event_str = "WINDIVERT_EVENT_REFLECT_CLOSE"; + break; + default: + event_str = "unknown"; + } + + return fmt::format( + "Windivert WINDIVERT_ADDRESS -- Timestamp: {}, Layer: {}, Event: {}, Sniffed: {}, " + "Outbound: {}, Loopback: {}, Imposter: {}, IPv6: {}, IPChecksum: {}, TCPChecksum: {}, " + "UDPChecksum: {}, {}", + addr.Timestamp, + layer_str, + event_str, + addr.Sniffed ? "true" : "false", + addr.Outbound ? "true" : "false", + addr.Loopback ? "true" : "false", + addr.Impostor ? "true" : "false", + addr.IPv6 ? "true" : "false", + addr.IPChecksum ? "true" : "false", + addr.TCPChecksum ? "true" : "false", + addr.UDPChecksum ? "true" : "false", + ifidx_str); + } +} + namespace llarp::win32 { static auto logcat = log::Cat("windivert"); @@ -57,90 +143,6 @@ namespace llarp::win32 WINDIVERT_ADDRESS addr; }; - void - log_windivert_addr(const WINDIVERT_ADDRESS& addr) - { - std::string layer_str{}; - std::string ifidx_str{}; - switch (addr.Layer) - { - case WINDIVERT_LAYER_NETWORK: - layer_str = "WINDIVERT_LAYER_NETWORK"; - ifidx_str = "Network: [IfIdx: {}, SubIfIdx: {}]"_format( - addr.Network.IfIdx, addr.Network.SubIfIdx); - break; - case WINDIVERT_LAYER_NETWORK_FORWARD: - layer_str = "WINDIVERT_LAYER_NETWORK_FORWARD"; - break; - case WINDIVERT_LAYER_FLOW: - layer_str = "WINDIVERT_LAYER_FLOW"; - break; - case WINDIVERT_LAYER_SOCKET: - layer_str = "WINDIVERT_LAYER_SOCKET"; - break; - case WINDIVERT_LAYER_REFLECT: - layer_str = "WINDIVERT_LAYER_REFLECT"; - break; - default: - layer_str = "unknown"; - } - - std::string event_str{}; - switch (addr.Event) - { - case WINDIVERT_EVENT_NETWORK_PACKET: - event_str = "WINDIVERT_EVENT_NETWORK_PACKET"; - break; - case WINDIVERT_EVENT_FLOW_ESTABLISHED: - event_str = "WINDIVERT_EVENT_FLOW_ESTABLISHED"; - break; - case WINDIVERT_EVENT_FLOW_DELETED: - event_str = "WINDIVERT_EVENT_FLOW_DELETED"; - break; - case WINDIVERT_EVENT_SOCKET_BIND: - event_str = "WINDIVERT_EVENT_SOCKET_BIND"; - break; - case WINDIVERT_EVENT_SOCKET_CONNECT: - event_str = "WINDIVERT_EVENT_SOCKET_CONNECT"; - break; - case WINDIVERT_EVENT_SOCKET_LISTEN: - event_str = "WINDIVERT_EVENT_SOCKET_LISTEN"; - break; - case WINDIVERT_EVENT_SOCKET_ACCEPT: - event_str = "WINDIVERT_EVENT_SOCKET_ACCEPT"; - break; - case WINDIVERT_EVENT_SOCKET_CLOSE: - event_str = "WINDIVERT_EVENT_SOCKET_CLOSE"; - break; - case WINDIVERT_EVENT_REFLECT_OPEN: - event_str = "WINDIVERT_EVENT_REFLECT_OPEN"; - break; - case WINDIVERT_EVENT_REFLECT_CLOSE: - event_str = "WINDIVERT_EVENT_REFLECT_CLOSE"; - break; - default: - event_str = "unknown"; - } - - log::trace( - logcat, - "Windivert WINDIVERT_ADDRESS -- Timestamp: {}, Layer: {}, Event: {}, Sniffed: {}, " - "Outbound: {}, Loopback: {}, Imposter: {}, IPv6: {}, IPChecksum: {}, TCPChecksum: {}, " - "UDPChecksum: {}, {}", - addr.Timestamp, - layer_str, - event_str, - addr.Sniffed ? "true" : "false", - addr.Outbound ? "true" : "false", - addr.Loopback ? "true" : "false", - addr.Impostor ? "true" : "false", - addr.IPv6 ? "true" : "false", - addr.IPChecksum ? "true" : "false", - addr.TCPChecksum ? "true" : "false", - addr.UDPChecksum ? "true" : "false", - ifidx_str); - } - class IO : public llarp::vpn::I_Packet_IO { std::function m_Wake; @@ -195,8 +197,7 @@ namespace llarp::win32 pkt.resize(sz); log::trace(logcat, "got packet of size {}B", sz); - log_windivert_addr(addr); - + log::trace(logcat, "{}", windivert_addr_to_string(addr)); return Packet{std::move(pkt), std::move(addr)}; } @@ -209,7 +210,7 @@ namespace llarp::win32 addr->Outbound = !addr->Outbound; // re-used from recv, so invert direction log::trace(logcat, "send dns packet of size {}B", pkt.size()); - log_windivert_addr(w_pkt.addr); + log::trace(logcat, "{}", windivert_addr_to_string(w_pkt.addr)); UINT sz{}; // recalc IP packet checksum in case it needs it From c4c81cc9f84988110a4dae7158d6e061b65bbe48 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 18:33:12 -0500 Subject: [PATCH 7/8] I hate clang-format sometimes --- llarp/win32/windivert.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 157a4d881..41d5d43a3 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -23,8 +23,8 @@ namespace { case WINDIVERT_LAYER_NETWORK: layer_str = "WINDIVERT_LAYER_NETWORK"; - ifidx_str = "Network: [IfIdx: {}, SubIfIdx: {}]"_format( - addr.Network.IfIdx, addr.Network.SubIfIdx); + ifidx_str = + "Network: [IfIdx: {}, SubIfIdx: {}]"_format(addr.Network.IfIdx, addr.Network.SubIfIdx); break; case WINDIVERT_LAYER_NETWORK_FORWARD: layer_str = "WINDIVERT_LAYER_NETWORK_FORWARD"; @@ -96,7 +96,7 @@ namespace addr.UDPChecksum ? "true" : "false", ifidx_str); } -} +} // namespace namespace llarp::win32 { From 1e294652375757cff9848f1e9b7d5cf14197bdbe Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Tue, 22 Nov 2022 18:39:22 -0500 Subject: [PATCH 8/8] fix missing namespace --- llarp/win32/windivert.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 41d5d43a3..48f5c27b5 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -14,6 +14,8 @@ extern "C" namespace { + using namespace oxen::log::literals; + std::string windivert_addr_to_string(const WINDIVERT_ADDRESS& addr) {