From 4e18c711c800977989a288b47e585f5a9f959f02 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Tue, 16 Jun 2026 20:45:03 -0400 Subject: [PATCH 1/2] Add regression test pinning the resolve query/return-port asymmetry A resolve query advertises a return port in its payload but is transmitted from a different, unbound socket, so the datagram's source port does not match. A stateful firewall therefore sees the outlet's reply (sent to the advertised return port) as unsolicited inbound and drops it, so streams fail to resolve until the firewall is disabled (NETWORK_FAILURE_MODES.md 1a). This test points discovery at loopback, stands up a fake responder on :16571, captures the query, and asserts the datagram source port equals the return port embedded in the payload. It is deterministic and needs no firewall. It fails on the current code (source port is an ephemeral port, != return port); the following commit makes it pass. Sets the api_config singleton, so it lives in its own executable like lsl_test_runtime_config. --- testing/CMakeLists.txt | 13 +++- testing/int/resolve_query_port.cpp | 98 ++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 testing/int/resolve_query_port.cpp diff --git a/testing/CMakeLists.txt b/testing/CMakeLists.txt index 90928601..7a17bd26 100644 --- a/testing/CMakeLists.txt +++ b/testing/CMakeLists.txt @@ -107,6 +107,17 @@ add_executable(lsl_test_runtime_config int/runtime_config.cpp) target_link_libraries(lsl_test_runtime_config PRIVATE lslobj lslboost common catch_main) target_compile_definitions(lsl_test_runtime_config PRIVATE LIBLSL_EXPORTS) +# resolve_query_port test sets the api_config singleton, so it lives in its own executable. +add_executable(lsl_test_resolve_query_port int/resolve_query_port.cpp) +target_link_libraries(lsl_test_resolve_query_port PRIVATE lslobj lslboost common catch_main) +target_compile_definitions(lsl_test_resolve_query_port PRIVATE LIBLSL_EXPORTS) +# Needs pugixml headers (resolver_impl.h -> stream_info_impl.h includes pugixml) +if(LSL_PUGIXML_IS_FETCHED) + target_include_directories(lsl_test_resolve_query_port PRIVATE ${pugixml_SOURCE_DIR}/src) +else() + target_link_libraries(lsl_test_resolve_query_port PRIVATE pugixml::pugixml) +endif() + if(LSL_BENCHMARKS) # to get somewhat reproducible performance numbers: # /usr/bin/time -v testing/lsl_test_exported --benchmark-samples 100 bounce @@ -122,7 +133,7 @@ if(LSL_BENCHMARKS) ) endif() -set(LSL_TESTS lsl_test_exported lsl_test_internal lsl_test_runtime_config) +set(LSL_TESTS lsl_test_exported lsl_test_internal lsl_test_runtime_config lsl_test_resolve_query_port) foreach(lsltest ${LSL_TESTS}) add_test(NAME ${lsltest} COMMAND ${lsltest} --wait-for-keypress never) if(LSL_INSTALL) diff --git a/testing/int/resolve_query_port.cpp b/testing/int/resolve_query_port.cpp new file mode 100644 index 00000000..d1bb5531 --- /dev/null +++ b/testing/int/resolve_query_port.cpp @@ -0,0 +1,98 @@ +#include "api_config.h" +#include "resolver_impl.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using asio::ip::make_address; +using asio::ip::udp; + +// Regression test for the resolver "firewall asymmetry" (NETWORK_FAILURE_MODES.md 1a). +// +// A resolve query advertises a return port in its payload but, before the fix, is +// transmitted from a *different* socket (unicast_socket_) whose ephemeral source port +// does not match. A stateful firewall therefore sees the outlet's reply (sent to the +// advertised return port) as unsolicited inbound and drops it. The fix sends the query +// *from* recv_socket_ so the datagram source port equals the advertised return port, +// and the firewall sees a normal request/reply flow. +// +// We verify this deterministically, with NO firewall, by standing up a fake responder +// on loopback :16571, capturing the query, and comparing the datagram source port to +// the return port embedded in the payload. +// +// Sets the api_config singleton, so (like runtime_config) this lives in its own +// executable to start from a fresh, uninitialized config. + +TEST_CASE("resolve query is sent from the advertised return port", "[resolver][network]") { + // Point discovery at loopback so the query lands on our fake responder, and pin to + // IPv4 so exactly one query stream is produced. + lsl::api_config::set_api_config_content( + "[ports]\n" + "IPv6 = disable\n" + "[multicast]\n" + "AddressesOverride = {127.0.0.1}\n" + "[lab]\n" + "SessionID = resolve_query_port_test\n"); + + const uint16_t multicast_port = lsl::api_config::get_instance()->multicast_port(); + + // --- fake responder bound to loopback :16571 --- + asio::io_context resp_io(1); + udp::socket responder(resp_io); + responder.open(udp::v4()); + responder.set_option(udp::socket::reuse_address(true)); + // Bind specifically to 127.0.0.1 so a more-specific match wins over any wildcard-bound + // LSL outlet that might already be running on this machine. + responder.bind(udp::endpoint(make_address("127.0.0.1"), multicast_port)); + + udp::endpoint sender; + char buf[2048] = {0}; + bool received = false; + uint16_t datagram_source_port = 0; + uint16_t advertised_return_port = 0; + + // Deadline first, so the receive handler can cancel it (declared before the handler + // that references it). + asio::steady_timer deadline(resp_io, std::chrono::seconds(3)); + deadline.async_wait([&](const asio::error_code &) { responder.cancel(); }); + + responder.async_receive_from(asio::buffer(buf), sender, + [&](const asio::error_code &ec, std::size_t len) { + if (ec) return; + received = true; + datagram_source_port = sender.port(); + // Payload (resolve_attempt_udp): + // LSL:shortinfo\r\n + // \r\n + // \r\n + std::istringstream req(std::string(buf, buf + len)); + std::string method, query; + std::getline(req, method); + std::getline(req, query); + req >> advertised_return_port; + deadline.cancel(); // let resp_io.run() return promptly + }); + + // Drive a resolve in the background: it sends a query wave to 127.0.0.1:16571 and then + // times out (no real outlet to find). + std::thread resolver_thread([] { + lsl::resolver_impl resolver; + resolver.resolve_oneshot("session_id='resolve_query_port_test'", 1, 1.0); + }); + + resp_io.run(); // returns once a query is captured (or the deadline fires) + resolver_thread.join(); + + REQUIRE(received); // a query datagram reached our loopback responder + // Pre-fix (1a): source port != return port -> this CHECK fails (pins the bug). + // Post-fix: query sent from recv_socket_ -> they match. + CHECK(advertised_return_port == datagram_source_port); +} From 0cf0dc0fcbfa41beab904ddcf21448d6329f41eb Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Tue, 16 Jun 2026 20:45:12 -0400 Subject: [PATCH 2/2] Send resolve queries from the receive socket Route every resolve query (unicast / broadcast / multicast) through the socket that also receives replies, instead of three separate send sockets whose ephemeral source ports differ from the advertised return port. The receive socket is given the broadcast option, the multicast TTL, and the per-interface outbound-interface option so it can carry all query flavors. The query's source port now equals the return port embedded in it, so a stateful firewall sees a matching request/reply flow and no longer drops the reply. This is the long-standing "can't resolve until I disable the firewall" failure (NETWORK_FAILURE_MODES.md 1a), and it makes the regression test from the previous commit pass. --- src/resolve_attempt_udp.cpp | 48 ++++++++++++++----------------------- src/resolve_attempt_udp.h | 10 +++----- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/src/resolve_attempt_udp.cpp b/src/resolve_attempt_udp.cpp index 203e2a64..08e5a9ba 100644 --- a/src/resolve_attempt_udp.cpp +++ b/src/resolve_attempt_udp.cpp @@ -19,10 +19,11 @@ resolve_attempt_udp::resolve_attempt_udp(asio::io_context &io, const udp &protoc const std::vector &targets, const std::string &query, resolver_impl &resolver, double cancel_after) : io_(io), resolver_(resolver), cancel_after_(cancel_after), cancelled_(false), - targets_(targets), query_(query), unicast_socket_(io), broadcast_socket_(io), - multicast_socket_(io), multicast_interfaces(api_config::get_instance()->multicast_interfaces), - recv_socket_(io), cancel_timer_(io) { - // open the sockets that we might need + targets_(targets), query_(query), + multicast_interfaces(api_config::get_instance()->multicast_interfaces), recv_socket_(io), + cancel_timer_(io) { + // Open the single socket used for BOTH sending queries and receiving replies, so that the + // query's source port matches the return port we advertise in it (see header / 1a). recv_socket_.open(protocol); try { bind_port_in_range(recv_socket_, protocol); @@ -32,20 +33,14 @@ resolve_attempt_udp::resolve_attempt_udp(asio::io_context &io, const udp &protoc "%s", e.what()); } - unicast_socket_.open(protocol); - try { - broadcast_socket_.open(protocol); - broadcast_socket_.set_option(asio::socket_base::broadcast(true)); - } catch (std::exception &e) { - LOG_F(WARNING, "Cannot open UDP broadcast socket for resolves: %s", e.what()); - } - try { - multicast_socket_.open(protocol); - multicast_socket_.set_option( - asio::ip::multicast::hops(api_config::get_instance()->multicast_ttl())); - } catch (std::exception &e) { - LOG_F(WARNING, "Cannot open UDP multicast socket for resolves: %s", e.what()); - } + // The receive socket must also be able to send to broadcast addresses and to carry the + // multicast TTL, since it now sends every flavor of query (unicast / broadcast / multicast). + asio::error_code ec; + recv_socket_.set_option(asio::socket_base::broadcast(true), ec); + if (ec) LOG_F(WARNING, "Cannot enable broadcast on the resolve socket: %s", ec.message().c_str()); + recv_socket_.set_option( + asio::ip::multicast::hops(api_config::get_instance()->multicast_ttl()), ec); + if (ec) LOG_F(WARNING, "Cannot set the multicast TTL on the resolve socket: %s", ec.message().c_str()); // precalc the query id (hash of the query string, as string) query_id_ = std::to_string(std::hash()(query)); @@ -166,21 +161,17 @@ void resolve_attempt_udp::send_next_query( if (mcit->addr.is_v4() != (proto == asio::ip::udp::v4())) next = targets_.end(); else - multicast_socket_.set_option(mcit->addr.is_v4() ? outbound_interface(mcit->addr.to_v4()) - : outbound_interface(mcit->ifindex)); + recv_socket_.set_option(mcit->addr.is_v4() ? outbound_interface(mcit->addr.to_v4()) + : outbound_interface(mcit->ifindex)); } if (next != targets_.end()) { udp::endpoint ep(*next++); // endpoint matches our active protocol? if (ep.protocol() == recv_socket_.local_endpoint().protocol()) { - // select socket to use - udp_socket &sock = - (ep.address() == asio::ip::address_v4::broadcast()) - ? broadcast_socket_ - : (ep.address().is_multicast() ? multicast_socket_ : unicast_socket_); - // and send the query over it + // Send every query (unicast / broadcast / multicast) from the receive socket so the + // datagram source port equals the advertised return port (firewall-friendly; 1a). auto keepalive(shared_from_this()); - sock.async_send_to(asio::buffer(query_msg_), ep, + recv_socket_.async_send_to(asio::buffer(query_msg_), ep, [shared_this = shared_from_this(), next, mcit](err_t err, size_t /*unused*/) { if (!shared_this->cancelled_ && err != asio::error::operation_aborted && err != asio::error::not_connected && err != asio::error::not_socket) @@ -197,9 +188,6 @@ void resolve_attempt_udp::send_next_query( void resolve_attempt_udp::do_cancel() { try { cancelled_ = true; - if (unicast_socket_.is_open()) unicast_socket_.close(); - if (multicast_socket_.is_open()) multicast_socket_.close(); - if (broadcast_socket_.is_open()) broadcast_socket_.close(); if (recv_socket_.is_open()) recv_socket_.close(); cancel_timer_.cancel(); } catch (std::exception &e) { diff --git a/src/resolve_attempt_udp.h b/src/resolve_attempt_udp.h index c00181b1..1d75a174 100644 --- a/src/resolve_attempt_udp.h +++ b/src/resolve_attempt_udp.h @@ -120,15 +120,11 @@ class resolve_attempt_udp final : public cancellable_obj, char resultbuf_[65536]; // IO objects - /// socket to send data over (for unicasts) - udp_socket unicast_socket_; - /// socket to send data over (for broadcasts) - udp_socket broadcast_socket_; - /// socket to send data over (for multicasts) - udp_socket multicast_socket_; /// Interface addresses to send multicast packets from const mcast_interface_list &multicast_interfaces; - /// socket to receive replies (always unicast) + /// Socket used to BOTH send queries and receive replies. Sending from the receive socket + /// ensures the datagram's source port equals the return port advertised in the query, so a + /// stateful firewall sees a matching request/reply flow (see NETWORK_FAILURE_MODES.md 1a). udp_socket recv_socket_; /// timer to schedule the cancel action steady_timer cancel_timer_;