From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 1 Feb 2024 08:32:16 +0000 (+0100)
Subject: dnsdist: Better handling of short, non-initial QUIC headers
X-Git-Tag: dnsdist-1.9.0~11^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F13755%2Fhead;p=thirdparty%2Fpdns.git

dnsdist: Better handling of short, non-initial QUIC headers

After a QUIC connection has been negotiated, we mostly get short QUIC
headers that do not contain the QUIC version. This is fine as long as
we still know about the connection, which might not be the case if it
has been idle for longer that our timer and thus we forgot about its
connection ID. What happens then is that we will mistakenly think that
the remote peer is trying to establish a new QUIC connection with a
version set to 0, which we don't support, and therefore start the
version negotiation process. What we should instead do is notice that
the packet is not a 'Initial' one and discard it, as we cannot open
a new connection from a short header anyway. Note that the peer knows
about our idle timer so in theory it should not try to reuse such a
connection, but it does happen from time to time.
---

diff --git a/pdns/dnsdistdist/doh3.cc b/pdns/dnsdistdist/doh3.cc
index 4c0823ab62..6b9ff9a0a2 100644
--- a/pdns/dnsdistdist/doh3.cc
+++ b/pdns/dnsdistdist/doh3.cc
@@ -880,6 +880,11 @@ static void handleSocketReadable(DOH3Frontend& frontend, ClientState& clientStat
 
     if (!conn) {
       DEBUGLOG("Connection not found");
+      if (type != static_cast<uint8_t>(DOQ_Packet_Types::QUIC_PACKET_TYPE_INITIAL)) {
+        DEBUGLOG("Packet is not initial");
+        continue;
+      }
+
       if (!quiche_version_is_supported(version)) {
         DEBUGLOG("Unsupported version");
         ++frontend.d_doh3UnsupportedVersionErrors;
diff --git a/pdns/dnsdistdist/doq-common.hh b/pdns/dnsdistdist/doq-common.hh
index 92af37f25f..2d1094c6bd 100644
--- a/pdns/dnsdistdist/doq-common.hh
+++ b/pdns/dnsdistdist/doq-common.hh
@@ -71,6 +71,17 @@ enum class DOQ_Error_Codes : uint64_t
   DOQ_UNSPECIFIED_ERROR = 5
 };
 
+/* Quiche type values do not match rfc9000 */
+enum class DOQ_Packet_Types : uint8_t
+{
+  QUIC_PACKET_TYPE_INITIAL = 1,
+  QUIC_PACKET_TYPE_RETRY = 2,
+  QUIC_PACKET_TYPE_HANDSHAKE = 3,
+  QUIC_PACKET_TYPE_ZERO_RTT = 4,
+  QUIC_PACKET_TYPE_SHORT = 5,
+  QUIC_PACKET_TYPE_VERSION_NEGOTIATION = 6
+};
+
 static constexpr size_t MAX_TOKEN_LEN = dnsdist::crypto::authenticated::getEncryptedSize(std::tuple_size<decltype(dnsdist::crypto::authenticated::Nonce::value)>{} /* nonce */ + sizeof(uint64_t) /* TTD */ + 16 /* IPv6 */ + QUICHE_MAX_CONN_ID_LEN);
 static constexpr size_t MAX_DATAGRAM_SIZE = 1200;
 static constexpr size_t LOCAL_CONN_ID_LEN = 16;
diff --git a/pdns/dnsdistdist/doq.cc b/pdns/dnsdistdist/doq.cc
index 277a8c5985..4b544dc850 100644
--- a/pdns/dnsdistdist/doq.cc
+++ b/pdns/dnsdistdist/doq.cc
@@ -674,6 +674,11 @@ static void handleSocketReadable(DOQFrontend& frontend, ClientState& clientState
 
     if (!conn) {
       DEBUGLOG("Connection not found");
+      if (type != static_cast<uint8_t>(DOQ_Packet_Types::QUIC_PACKET_TYPE_INITIAL)) {
+        DEBUGLOG("Packet is not initial");
+        continue;
+      }
+
       if (!quiche_version_is_supported(version)) {
         DEBUGLOG("Unsupported version");
         ++frontend.d_doqUnsupportedVersionErrors;