From: Remi Gacogne Date: Fri, 11 Oct 2019 12:51:11 +0000 (+0200) Subject: dnsdist: Always allocate at least 4096 bytes for the cached response X-Git-Tag: dnsdist-1.4.0-rc4~22^2~3 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8179b6d695b4645d06924687b13d48f699558d4b;p=pdns dnsdist: Always allocate at least 4096 bytes for the cached response --- diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index 3e1c9c144..bea6c584b 100644 --- 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& 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(512)) < s_maxPacketCacheEntrySize ? s_maxPacketCacheEntrySize : (state->d_querySize + static_cast(512))); state->d_currentPos = 0; } } diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 177cd4493..4b826767c 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -515,7 +515,7 @@ void responderThread(std::shared_ptr 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 39dd6d443..3efda3575 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -1240,8 +1240,8 @@ extern DNSDistSNMPAgent* g_snmpAgent; extern bool g_addEDNSToSelfGeneratedResponses; extern std::set 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& selectedBackend); diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index f93abdd82..76647dac2 100644 --- 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(du->query.c_str()); struct dnsheader* dh = reinterpret_cast(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;