From: Remi Gacogne Date: Thu, 19 Jun 2025 09:18:27 +0000 (+0200) Subject: dnsdist: Enforce concurrent streams count for pending queries X-Git-Tag: dnsdist-2.0.1~10^2~6 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3ae96162ea71cd52366a5a313156e7516ec2605f;p=thirdparty%2Fpdns.git dnsdist: Enforce concurrent streams count for pending queries The gist of the `MadeYouRest` attack is that streams can be reset by the client, and thus no longer count towards the maximum number of a streams as far as the library is concerned, while the server is still processing the query and doing actual work. This pull request introduces a counter for "killed but still being processed streams" to prevent it. Signed-off-by: Remi Gacogne (cherry picked from commit 0214032c5f09fcfb440e5c5120f1491cb4f0fda4) --- diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc index 26f6d881b..b7983314b 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc @@ -93,6 +93,8 @@ private: }; #endif +static constexpr uint32_t MAX_CONCURRENT_STREAMS{100U}; + class IncomingDoHCrossProtocolContext : public DOHUnitInterface { public: @@ -292,7 +294,7 @@ bool IncomingHTTP2Connection::checkALPN() void IncomingHTTP2Connection::handleConnectionReady() { - constexpr std::array settings{{{NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 100U}}}; + constexpr std::array settings{{{NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, MAX_CONCURRENT_STREAMS}}}; constexpr std::array nearLimitsSettings{{{NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 1U}}}; auto ret = nghttp2_submit_settings(d_session.get(), NGHTTP2_FLAG_NONE, isNearTCPLimits() ? nearLimitsSettings.data() : settings.data(), isNearTCPLimits() ? nearLimitsSettings.size() : settings.size()); if (ret != 0) { @@ -552,13 +554,23 @@ void NGHTTP2Headers::addDynamicHeader(std::vector& headers, NGHTTP2H NGHTTP2Headers::addCustomDynamicHeader(headers, name, value); } +std::unordered_map::iterator IncomingHTTP2Connection::getStreamContext(StreamID streamID) +{ + auto streamIt = d_currentStreams.find(streamID); + if (streamIt == d_currentStreams.end()) { + /* it might have been closed by the remote end in the meantime */ + d_killedStreams.erase(streamID); + } + return streamIt; +} + IOState IncomingHTTP2Connection::sendResponse(const struct timeval& now, TCPResponse&& response) { (void)now; if (response.d_idstate.d_streamID == -1) { throw std::runtime_error("Invalid DoH stream ID while sending response"); } - auto streamIt = d_currentStreams.find(response.d_idstate.d_streamID); + auto streamIt = getStreamContext(response.d_idstate.d_streamID); if (streamIt == d_currentStreams.end()) { /* it might have been closed by the remote end in the meantime */ return hasPendingWrite() ? IOState::NeedWrite : IOState::Done; @@ -598,7 +610,7 @@ void IncomingHTTP2Connection::notifyIOError(const struct timeval& now, TCPRespon throw std::runtime_error("Invalid DoH stream ID while handling I/O error notification"); } - auto streamIt = d_currentStreams.find(response.d_idstate.d_streamID); + auto streamIt = getStreamContext(response.d_idstate.d_streamID); if (streamIt == d_currentStreams.end()) { /* it might have been closed by the remote end in the meantime */ return; @@ -926,7 +938,7 @@ int IncomingHTTP2Connection::on_frame_recv_callback(nghttp2_session* session, co /* is this the last frame for this stream? */ if ((frame->hd.type == NGHTTP2_HEADERS || frame->hd.type == NGHTTP2_DATA) && (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) != 0) { auto streamID = frame->hd.stream_id; - auto stream = conn->d_currentStreams.find(streamID); + auto stream = conn->getStreamContext(streamID); if (stream != conn->d_currentStreams.end()) { conn->handleIncomingQuery(std::move(stream->second), streamID); } @@ -948,7 +960,9 @@ int IncomingHTTP2Connection::on_stream_close_callback(nghttp2_session* session, (void)error_code; auto* conn = static_cast(user_data); - conn->d_currentStreams.erase(stream_id); + if (conn->d_currentStreams.erase(stream_id) > 0) { + conn->d_killedStreams.emplace(stream_id); + } return 0; } @@ -960,20 +974,29 @@ int IncomingHTTP2Connection::on_begin_headers_callback(nghttp2_session* session, } auto* conn = static_cast(user_data); - auto insertPair = conn->d_currentStreams.emplace(frame->hd.stream_id, PendingQuery()); - if (!insertPair.second) { - /* there is a stream ID collision, something is very wrong! */ - vinfolog("Stream ID collision (%d) on connection from %d", frame->hd.stream_id, conn->d_ci.remote.toStringWithPort()); - conn->d_connectionClosing = true; - conn->d_needFlush = true; - nghttp2_session_terminate_session(conn->d_session.get(), NGHTTP2_NO_ERROR); - auto ret = nghttp2_session_send(conn->d_session.get()); + auto close_connection = [](IncomingHTTP2Connection* connection, int32_t streamID, const ComboAddress& remote) -> int { + connection->d_connectionClosing = true; + connection->d_needFlush = true; + nghttp2_session_terminate_session(connection->d_session.get(), NGHTTP2_REFUSED_STREAM); + auto ret = nghttp2_session_send(connection->d_session.get()); if (ret != 0) { - vinfolog("Error flushing HTTP response for stream %d from %s: %s", frame->hd.stream_id, conn->d_ci.remote.toStringWithPort(), nghttp2_strerror(ret)); + vinfolog("Error flushing HTTP response for stream %d from %s: %s", streamID, remote.toStringWithPort(), nghttp2_strerror(ret)); return NGHTTP2_ERR_CALLBACK_FAILURE; } return 0; + }; + + if (conn->getConcurrentStreamsCount() >= MAX_CONCURRENT_STREAMS) { + vinfolog("Too many concurrent streams on connection from %d", conn->d_ci.remote.toStringWithPort()); + return close_connection(conn, frame->hd.stream_id, conn->d_ci.remote); + } + + auto insertPair = conn->d_currentStreams.emplace(frame->hd.stream_id, PendingQuery()); + if (!insertPair.second) { + /* there is a stream ID collision, something is very wrong! */ + vinfolog("Stream ID collision (%d) on connection from %d", frame->hd.stream_id, conn->d_ci.remote.toStringWithPort()); + return close_connection(conn, frame->hd.stream_id, conn->d_ci.remote); } return 0; @@ -1012,7 +1035,7 @@ int IncomingHTTP2Connection::on_header_callback(nghttp2_session* session, const return nameLen == expected.size() && memcmp(name, expected.data(), expected.size()) == 0; }; - auto stream = conn->d_currentStreams.find(frame->hd.stream_id); + auto stream = conn->getStreamContext(frame->hd.stream_id); if (stream == conn->d_currentStreams.end()) { vinfolog("Unable to match the stream ID %d to a known one!", frame->hd.stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; @@ -1077,7 +1100,7 @@ int IncomingHTTP2Connection::on_data_chunk_recv_callback(nghttp2_session* sessio (void)session; (void)flags; auto* conn = static_cast(user_data); - auto stream = conn->d_currentStreams.find(stream_id); + auto stream = conn->getStreamContext(stream_id); if (stream == conn->d_currentStreams.end()) { vinfolog("Unable to match the stream ID %d to a known one!", stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; @@ -1168,7 +1191,7 @@ void IncomingHTTP2Connection::stopIO() uint32_t IncomingHTTP2Connection::getConcurrentStreamsCount() const { - return d_currentStreams.size(); + return d_currentStreams.size() + d_killedStreams.size(); } boost::optional IncomingHTTP2Connection::getIdleClientReadTTD(struct timeval now) const @@ -1243,6 +1266,7 @@ void IncomingHTTP2Connection::handleIOError() d_outPos = 0; nghttp2_session_terminate_session(d_session.get(), NGHTTP2_PROTOCOL_ERROR); d_currentStreams.clear(); + d_killedStreams.clear(); stopIO(); } diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.hh b/pdns/dnsdistdist/dnsdist-nghttp2-in.hh index 47b9575c0..36e142151 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2-in.hh +++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.hh @@ -86,6 +86,7 @@ private: std::unique_ptr getDOHUnit(uint32_t streamID) override; void stopIO(); + std::unordered_map::iterator getStreamContext(StreamID streamID); uint32_t getConcurrentStreamsCount() const; void updateIO(IOState newState, const timeval& now) override; void updateIO(IOState newState, const FDMultiplexer::callbackfunc_t& callback); @@ -102,6 +103,7 @@ private: std::unique_ptr d_session{nullptr, nghttp2_session_del}; std::unordered_map d_currentStreams; + std::unordered_set d_killedStreams; PacketBuffer d_out; PacketBuffer d_in; size_t d_outPos{0};