From a14d29ed068a2a098d283922942fb03efb33ae0f Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 15 Apr 2025 10:34:47 +0200 Subject: [PATCH] dnsdist: Only pass source addresses on sockets bound to ANY FreeBSD refuses the use of `IP_SENDSRCADDR` on a socket that is bound to a specific address, returning `EINVAL` in that case. (cherry picked from commit 4ce6f5e8b532e103d9c16a54b92e9153ebfce2cb) --- pdns/dnsdistdist/doh3.cc | 8 ++++---- pdns/dnsdistdist/doq-common.cc | 34 ++++++++++++++++++++++------------ pdns/dnsdistdist/doq-common.hh | 6 +++--- pdns/dnsdistdist/doq.cc | 8 ++++---- pdns/iputils.hh | 7 +++++++ 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/pdns/dnsdistdist/doh3.cc b/pdns/dnsdistdist/doh3.cc index 236c19369b..be9718000b 100644 --- a/pdns/dnsdistdist/doh3.cc +++ b/pdns/dnsdistdist/doh3.cc @@ -912,14 +912,14 @@ static void handleSocketReadable(DOH3Frontend& frontend, ClientState& clientStat if (!quiche_version_is_supported(version)) { DEBUGLOG("Unsupported version"); ++frontend.d_doh3UnsupportedVersionErrors; - handleVersionNegociation(sock, clientConnID, serverConnID, client, localAddr, buffer); + handleVersionNegotiation(sock, clientConnID, serverConnID, client, localAddr, buffer, clientState.local.isUnspecified()); continue; } if (token_len == 0) { /* stateless retry */ DEBUGLOG("No token received"); - handleStatelessRetry(sock, clientConnID, serverConnID, client, localAddr, version, buffer); + handleStatelessRetry(sock, clientConnID, serverConnID, client, localAddr, version, buffer, clientState.local.isUnspecified()); continue; } @@ -966,7 +966,7 @@ static void handleSocketReadable(DOH3Frontend& frontend, ClientState& clientStat processH3Events(clientState, frontend, conn->get(), client, serverConnID, buffer); - flushEgress(sock, conn->get().d_conn, client, localAddr, buffer); + flushEgress(sock, conn->get().d_conn, client, localAddr, buffer, clientState.local.isUnspecified()); } else { DEBUGLOG("Connection not established"); @@ -1011,7 +1011,7 @@ void doh3Thread(ClientState* clientState) for (auto conn = frontend->d_server_config->d_connections.begin(); conn != frontend->d_server_config->d_connections.end();) { quiche_conn_on_timeout(conn->second.d_conn.get()); - flushEgress(sock, conn->second.d_conn, conn->second.d_peer, conn->second.d_localAddr, buffer); + flushEgress(sock, conn->second.d_conn, conn->second.d_peer, conn->second.d_localAddr, buffer, clientState->local.isUnspecified()); if (quiche_conn_is_closed(conn->second.d_conn.get())) { #ifdef DEBUGLOG_ENABLED diff --git a/pdns/dnsdistdist/doq-common.cc b/pdns/dnsdistdist/doq-common.cc index bb79ddc218..da25ec548f 100644 --- a/pdns/dnsdistdist/doq-common.cc +++ b/pdns/dnsdistdist/doq-common.cc @@ -126,10 +126,22 @@ std::optional validateToken(const PacketBuffer& token, const Combo } } -static void sendFromTo(Socket& sock, const ComboAddress& peer, const ComboAddress& local, PacketBuffer& buffer) +static void sendFromTo(Socket& sock, const ComboAddress& peer, const ComboAddress& local, PacketBuffer& buffer, [[maybe_unused]] bool socketBoundToAny) { - const int flags = 0; - if (local.sin4.sin_family == 0) { + /* we only want to specify the source address to use if we were able to + either harvest it from the incoming packet, or if our socket is already + bound to a specific address */ + bool setSourceAddress = local.sin4.sin_family != 0; +#if defined(__FreeBSD__) || defined(__DragonFly__) + /* FreeBSD and DragonFlyBSD refuse the use of IP_SENDSRCADDR on a socket that is bound to a + specific address, returning EINVAL in that case. */ + if (!socketBoundToAny) { + setSourceAddress = false; + } +#endif /* __FreeBSD__ || __DragonFly__ */ + + if (!setSourceAddress) { + const int flags = 0; // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) auto ret = sendto(sock.getHandle(), buffer.data(), buffer.size(), flags, reinterpret_cast(&peer), peer.getSocklen()); if (ret < 0) { @@ -147,7 +159,7 @@ static void sendFromTo(Socket& sock, const ComboAddress& peer, const ComboAddres } } -void handleStatelessRetry(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, uint32_t version, PacketBuffer& buffer) +void handleStatelessRetry(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, uint32_t version, PacketBuffer& buffer, bool socketBoundToAny) { auto newServerConnID = getCID(); if (!newServerConnID) { @@ -170,10 +182,10 @@ void handleStatelessRetry(Socket& sock, const PacketBuffer& clientConnID, const } buffer.resize(static_cast(written)); - sendFromTo(sock, peer, localAddr, buffer); + sendFromTo(sock, peer, localAddr, buffer, socketBoundToAny); } -void handleVersionNegociation(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer) +void handleVersionNegotiation(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer, bool socketBoundToAny) { buffer.resize(MAX_DATAGRAM_SIZE); @@ -187,10 +199,10 @@ void handleVersionNegociation(Socket& sock, const PacketBuffer& clientConnID, co } buffer.resize(static_cast(written)); - sendFromTo(sock, peer, localAddr, buffer); + sendFromTo(sock, peer, localAddr, buffer, socketBoundToAny); } -void flushEgress(Socket& sock, QuicheConnection& conn, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer) +void flushEgress(Socket& sock, QuicheConnection& conn, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer, bool socketBoundToAny) { buffer.resize(MAX_DATAGRAM_SIZE); quiche_send_info send_info; @@ -206,7 +218,7 @@ void flushEgress(Socket& sock, QuicheConnection& conn, const ComboAddress& peer, } // FIXME pacing (as send_info.at should tell us when to send the packet) ? buffer.resize(static_cast(written)); - sendFromTo(sock, peer, localAddr, buffer); + sendFromTo(sock, peer, localAddr, buffer, socketBoundToAny); } } @@ -312,9 +324,7 @@ bool recvAsync(Socket& socket, PacketBuffer& buffer, ComboAddress& clientAddr, C This is indicated by setting the family to 0 which is acted upon in sendUDPResponse() and DelayedPacket::(). */ - const ComboAddress bogusV4("0.0.0.0:0"); - const ComboAddress bogusV6("[::]:0"); - if ((localAddr.sin4.sin_family == AF_INET && localAddr == bogusV4) || (localAddr.sin4.sin_family == AF_INET6 && localAddr == bogusV6)) { + if (localAddr.isUnspecified()) { localAddr.sin4.sin_family = 0; } } diff --git a/pdns/dnsdistdist/doq-common.hh b/pdns/dnsdistdist/doq-common.hh index 9b04e4c835..d915705fb2 100644 --- a/pdns/dnsdistdist/doq-common.hh +++ b/pdns/dnsdistdist/doq-common.hh @@ -92,9 +92,9 @@ void fillRandom(PacketBuffer& buffer, size_t size); std::optional getCID(); PacketBuffer mintToken(const PacketBuffer& dcid, const ComboAddress& peer); std::optional validateToken(const PacketBuffer& token, const ComboAddress& peer); -void handleStatelessRetry(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, uint32_t version, PacketBuffer& buffer); -void handleVersionNegociation(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer); -void flushEgress(Socket& sock, QuicheConnection& conn, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer); +void handleStatelessRetry(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, uint32_t version, PacketBuffer& buffer, bool socketBoundToAny); +void handleVersionNegotiation(Socket& sock, const PacketBuffer& clientConnID, const PacketBuffer& serverConnID, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer, bool socketBoundToAny); +void flushEgress(Socket& sock, QuicheConnection& conn, const ComboAddress& peer, const ComboAddress& localAddr, PacketBuffer& buffer, bool socketBoundToAny); void configureQuiche(QuicheConfig& config, const QuicheParams& params, bool isHTTP); bool recvAsync(Socket& socket, PacketBuffer& buffer, ComboAddress& clientAddr, ComboAddress& localAddr); diff --git a/pdns/dnsdistdist/doq.cc b/pdns/dnsdistdist/doq.cc index 4c86fb4db6..02b5765cc0 100644 --- a/pdns/dnsdistdist/doq.cc +++ b/pdns/dnsdistdist/doq.cc @@ -718,14 +718,14 @@ static void handleSocketReadable(DOQFrontend& frontend, ClientState& clientState if (!quiche_version_is_supported(version)) { DEBUGLOG("Unsupported version"); ++frontend.d_doqUnsupportedVersionErrors; - handleVersionNegociation(sock, clientConnID, serverConnID, client, localAddr, buffer); + handleVersionNegotiation(sock, clientConnID, serverConnID, client, localAddr, buffer, clientState.local.isUnspecified()); continue; } if (token_len == 0) { /* stateless retry */ DEBUGLOG("No token received"); - handleStatelessRetry(sock, clientConnID, serverConnID, client, localAddr, version, buffer); + handleStatelessRetry(sock, clientConnID, serverConnID, client, localAddr, version, buffer, clientState.local.isUnspecified()); continue; } @@ -766,7 +766,7 @@ static void handleSocketReadable(DOQFrontend& frontend, ClientState& clientState handleReadableStream(frontend, clientState, *conn, streamID, client, serverConnID); } - flushEgress(sock, conn->get().d_conn, client, localAddr, buffer); + flushEgress(sock, conn->get().d_conn, client, localAddr, buffer, clientState.local.isUnspecified()); } else { DEBUGLOG("Connection not established"); @@ -811,7 +811,7 @@ void doqThread(ClientState* clientState) for (auto conn = frontend->d_server_config->d_connections.begin(); conn != frontend->d_server_config->d_connections.end();) { quiche_conn_on_timeout(conn->second.d_conn.get()); - flushEgress(sock, conn->second.d_conn, conn->second.d_peer, conn->second.d_localAddr, buffer); + flushEgress(sock, conn->second.d_conn, conn->second.d_peer, conn->second.d_localAddr, buffer, clientState->local.isUnspecified()); if (quiche_conn_is_closed(conn->second.d_conn.get())) { #ifdef DEBUGLOG_ENABLED diff --git a/pdns/iputils.hh b/pdns/iputils.hh index 2a318f7327..d7606b3ac6 100644 --- a/pdns/iputils.hh +++ b/pdns/iputils.hh @@ -266,6 +266,13 @@ union ComboAddress { return true; } + [[nodiscard]] bool isUnspecified() const + { + const ComboAddress unspecifiedV4("0.0.0.0:0"); + const ComboAddress unspecifiedV6("[::]:0"); + return *this == unspecifiedV4 || *this == unspecifiedV6; + } + ComboAddress mapToIPv4() const { if(!isMappedIPv4()) -- 2.47.2