dnsdist: Always allocate at least 4096 bytes for the cached response

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index 3e1c9c1441a09d8a4c49c8680706ea65c3cd54b1..bea6c584ba24fbdaebe32865cb631b6b739f51e9 100644 (file)
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -417,7 +417,7 @@ static void handleDownstreamIOCallback(int fd, FDMultiplexer::funcparam_t& param
 class IncomingTCPConnectionState
 {
 public:
-  IncomingTCPConnectionState(ConnectionInfo&& ci, TCPClientThreadData& threadData, const struct timeval& now): d_buffer(4096), d_responseBuffer(4096), d_threadData(threadData), d_ci(std::move(ci)), d_handler(d_ci.fd, g_tcpRecvTimeout, d_ci.cs->tlsFrontend ? d_ci.cs->tlsFrontend->getContext() : nullptr, now.tv_sec), d_connectionStartTime(now)
+  IncomingTCPConnectionState(ConnectionInfo&& ci, TCPClientThreadData& threadData, const struct timeval& now): d_buffer(s_maxPacketCacheEntrySize), d_responseBuffer(s_maxPacketCacheEntrySize), d_threadData(threadData), d_ci(std::move(ci)), d_handler(d_ci.fd, g_tcpRecvTimeout, d_ci.cs->tlsFrontend ? d_ci.cs->tlsFrontend->getContext() : nullptr, now.tv_sec), d_connectionStartTime(now)
   {
     d_ids.origDest.reset();
     d_ids.origDest.sin4.sin_family = d_ci.remote.sin4.sin_family;
@@ -1152,9 +1152,9 @@ static void handleIO(std::shared_ptr<IncomingTCPConnectionState>& state, struct
           return;
         }
 
-        /* allocate a bit more memory to be able to spoof the content,
+        /* allocate a bit more memory to be able to spoof the content, get an answer from the cache
            or to add ECS without allocating a new buffer */
-        state->d_buffer.resize(state->d_querySize + 512);
+        state->d_buffer.resize((state->d_querySize + static_cast<size_t>(512)) < s_maxPacketCacheEntrySize ? s_maxPacketCacheEntrySize : (state->d_querySize + static_cast<size_t>(512)));
         state->d_currentPos = 0;
       }
     }

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 177cd44931acad80ff99a8baffd67bf4d13c65fc..4b826767c3a2442fc0b2a2ccb07a322db926919e 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -515,7 +515,7 @@ void responderThread(std::shared_ptr<DownstreamState> dss)
 try {
   setThreadName("dnsdist/respond");
   auto localRespRulactions = g_resprulactions.getLocal();
-  char packet[4096 + DNSCRYPT_MAX_RESPONSE_PADDING_AND_MAC_SIZE];
+  char packet[s_maxPacketCacheEntrySize + DNSCRYPT_MAX_RESPONSE_PADDING_AND_MAC_SIZE];
   static_assert(sizeof(packet) <= UINT16_MAX, "Packet size should fit in a uint16_t");
   /* when the answer is encrypted in place, we need to get a copy
      of the original header before encryption to fill the ring buffer */
@@ -1681,7 +1681,7 @@ static void MultipleMessagesUDPClientThread(ClientState* cs, LocalHolders& holde
 {
   struct MMReceiver
   {
-    char packet[4096];
+    char packet[s_maxPacketCacheEntrySize];
     ComboAddress remote;
     ComboAddress dest;
     struct iovec iov;
@@ -1772,7 +1772,7 @@ try
   else
 #endif /* defined(HAVE_RECVMMSG) && defined(HAVE_SENDMMSG) && defined(MSG_WAITFORONE) */
   {
-    char packet[4096];
+    char packet[s_maxPacketCacheEntrySize];
     /* the actual buffer is larger because:
        - we may have to add EDNS and/or ECS
        - we use it for self-generated responses (from rule or cache)

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index 39dd6d443d9af6a3f2542862cbb6f13b22745cb1..3efda3575a2ad99f34d04f74ed978fd85f634419 100644 (file)
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -1240,8 +1240,8 @@ extern DNSDistSNMPAgent* g_snmpAgent;
 extern bool g_addEDNSToSelfGeneratedResponses;
 
 extern std::set<std::string> g_capabilitiesToRetain;
-
-static const size_t s_udpIncomingBufferSize{1500};
+static const uint16_t s_udpIncomingBufferSize{1500}; // don't accept UDP queries larger than this value
+static const size_t s_maxPacketCacheEntrySize{4096}; // don't cache responses larger than this value
 
 enum class ProcessQueryResult { Drop, SendAnswer, PassToBackend };
 ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders& holders, std::shared_ptr<DownstreamState>& selectedBackend);

diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc
index f93abdd82e46cfcca699b9851f4b1d192fe9fcd1..76647dac22ac5975de2f10a06080a1cf063ebf33 100644 (file)
--- a/pdns/dnsdistdist/doh.cc
+++ b/pdns/dnsdistdist/doh.cc
@@ -304,9 +304,9 @@ static int processDOHQuery(DOHUnit* du)
     struct timespec queryRealTime;
     gettime(&queryRealTime, true);
     uint16_t len = du->query.length();
-    /* allocate a bit more memory to be able to spoof the content,
-       or to add ECS without allocating a new buffer */
-    du->query.resize(du->query.size() + 512);
+    /* We reserve at least 512 additional bytes to be able to add EDNS, but we also want
+       at least s_maxPacketCacheEntrySize bytes to be able to spoof the content or fill the answer from the packet cache */
+    du->query.resize((du->query.size() + 512 < s_maxPacketCacheEntrySize) ? s_maxPacketCacheEntrySize : (du->query.size() + 512));
     size_t bufferSize = du->query.size();
     auto query = const_cast<char*>(du->query.c_str());
     struct dnsheader* dh = reinterpret_cast<struct dnsheader*>(query);
@@ -608,7 +608,9 @@ try
       ++dsc->df->d_http1Stats.d_nbQueries;
 
     std::string query;
-    query.reserve(req->entity.len + 512);
+    /* We reserve at least 512 additional bytes to be able to add EDNS, but we also want
+       at least s_maxPacketCacheEntrySize bytes to be able to fill the answer from the packet cache */
+    query.reserve(((req->entity.len + 512) < s_maxPacketCacheEntrySize) ? s_maxPacketCacheEntrySize : (req->entity.len + 512));
     query.assign(req->entity.base, req->entity.len);
     doh_dispatch_query(dsc, self, req, std::move(query), local, remote);
   }
@@ -633,7 +635,10 @@ try
 
       string decoded;
       /* rough estimate so we hopefully don't need a need allocation later */
-      decoded.reserve(((sdns.size() * 3) / 4) + 512);
+      /* We reserve at least 512 additional bytes to be able to add EDNS, but we also want
+         at least s_maxPacketCacheEntrySize bytes to be able to fill the answer from the packet cache */
+      size_t estimate = ((sdns.size() * 3) / 4);
+      decoded.reserve(((estimate + 512) < s_maxPacketCacheEntrySize) ? s_maxPacketCacheEntrySize : (estimate + 512));
       if(B64Decode(sdns, decoded) < 0) {
         h2o_send_error_400(req, "Bad Request", "Unable to decode BASE64-URL", 0);
         ++dsc->df->d_badrequests;