From 49c33a6cb388f732072a703082ab623647933bff Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 12 Nov 2018 17:20:17 +0100 Subject: [PATCH] dnsdist: Handle Client Subnet not being the first ECS option for 'zero-scope' --- pdns/dnsdist-ecs.cc | 13 +++++++++++-- pdns/dnsdist-ecs.hh | 3 +-- pdns/dnsdist.cc | 18 ++++++++++++------ pdns/dnsdist.hh | 3 ++- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/pdns/dnsdist-ecs.cc b/pdns/dnsdist-ecs.cc index d1cd71a86..d760fd3a5 100644 --- a/pdns/dnsdist-ecs.cc +++ b/pdns/dnsdist-ecs.cc @@ -488,7 +488,7 @@ int removeEDNSOptionFromOPT(char* optStart, size_t* optLen, const uint16_t optio return 0; } -bool isEDNSOptionInOpt(const std::string& packet, const size_t optStart, const size_t optLen, const uint16_t optionCodeToFind) +bool isEDNSOptionInOpt(const std::string& packet, const size_t optStart, const size_t optLen, const uint16_t optionCodeToFind, size_t* optContentStart, uint16_t* optContentLen) { /* we need at least: root label (1), type (2), class (2), ttl (4) + rdlen (2)*/ @@ -508,10 +508,20 @@ bool isEDNSOptionInOpt(const std::string& packet, const size_t optStart, const s p += sizeof(optionCode); const uint16_t optionLen = 0x100*packet.at(p) + packet.at(p+1); p += sizeof(optionLen); + if ((p + optionLen) > rdEnd) { return false; } + if (optionCode == optionCodeToFind) { + if (optContentStart != nullptr) { + *optContentStart = p; + } + + if (optContentLen != nullptr) { + *optContentLen = optionLen; + } + return true; } p += optionLen; @@ -731,4 +741,3 @@ bool queryHasEDNS(const DNSQuestion& dq) return false; } - diff --git a/pdns/dnsdist-ecs.hh b/pdns/dnsdist-ecs.hh index 40bf974bd..dd0a78293 100644 --- a/pdns/dnsdist-ecs.hh +++ b/pdns/dnsdist-ecs.hh @@ -31,7 +31,7 @@ void generateECSOption(const ComboAddress& source, string& res, uint16_t ECSPref int removeEDNSOptionFromOPT(char* optStart, size_t* optLen, const uint16_t optionCodeToRemove); int rewriteResponseWithoutEDNSOption(const std::string& initialPacket, const uint16_t optionCodeToSkip, vector& newContent); int getEDNSOptionsStart(const char* packet, const size_t offset, const size_t len, uint16_t* optRDPosition, size_t * remaining); -bool isEDNSOptionInOpt(const std::string& packet, const size_t optStart, const size_t optLen, const uint16_t optionCodeToFind); +bool isEDNSOptionInOpt(const std::string& packet, const size_t optStart, const size_t optLen, const uint16_t optionCodeToFind, size_t* optContentStart = nullptr, uint16_t* optContentLen = nullptr); bool addEDNS(dnsheader* dh, uint16_t& len, const size_t size, bool dnssecOK, uint16_t payloadSize); bool addEDNSToQueryTurnedResponse(DNSQuestion& dq); @@ -42,4 +42,3 @@ bool parseEDNSOptions(DNSQuestion& dq); int getEDNSZ(const DNSQuestion& dq); bool queryHasEDNS(const DNSQuestion& dq); - diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 177396a5b..a6c789907 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -301,10 +301,16 @@ bool fixUpResponse(char** response, uint16_t* responseLen, size_t* responseSize, int res = locateEDNSOptRR(responseStr, &optStart, &optLen, &last); if (res == 0) { - if(zeroScope) { // this finds if an EDNS scope was set, and if it is 0 - *zeroScope = optLen > 18 && !responseStr.at(optStart + 18) && !responseStr.at(optStart + 11) && responseStr.at(optStart + 12) ==8; + if (zeroScope) { // this finds if an EDNS Client Subnet scope was set, and if it is 0 + size_t optContentStart = 0; + uint16_t optContentLen = 0; + /* we need at least 4 bytes after the option length (family: 2, source prefix-length: 1, scope prefix-length: 1) */ + if (isEDNSOptionInOpt(responseStr, optStart, optLen, EDNSOptionCode::ECS, &optContentStart, &optContentLen) && optContentLen >= 4) { + /* see if the EDNS Client Subnet SCOPE PREFIX-LENGTH byte in position 3 is set to 0, which is the only thing + we care about. */ + *zeroScope = responseStr.at(optContentStart + 3) == 0; + } } - // this test only detects a /0 if ECS is the first option if (ednsAdded) { /* we added the entire OPT RR, @@ -518,8 +524,8 @@ try { addRoom = DNSCRYPT_MAX_RESPONSE_PADDING_AND_MAC_SIZE; } #endif - bool zeroScope=false; - if (!fixUpResponse(&response, &responseLen, &responseSize, ids->qname, ids->origFlags, ids->ednsAdded, ids->ecsAdded, rewrittenResponse, addRoom, &zeroScope)) { + bool zeroScope = false; + if (!fixUpResponse(&response, &responseLen, &responseSize, ids->qname, ids->origFlags, ids->ednsAdded, ids->ecsAdded, rewrittenResponse, addRoom, ids->useZeroScope ? &zeroScope : nullptr)) { continue; } @@ -1445,7 +1451,7 @@ static void processUDPQuery(ClientState& cs, LocalHolders& holders, const struct if (dq.useECS && ((ss && ss->useECS) || (!ss && serverPool->getECS()))) { // we special case our cache in case a downstream explicitly gave us a universally valid response with a 0 scope - if (packetCache && !dq.skipCache && packetCache->isECSParsingEnabled()) { + if (packetCache && !dq.skipCache && !ss->disableZeroScope && packetCache->isECSParsingEnabled()) { if (packetCache->get(dq, consumed, dh->id, query, &cachedResponseSize, &cacheKeyNoECS, subnet, dnssecOK, allowExpired)) { sendAndEncryptUDPResponse(holders, cs, dq, query, cachedResponseSize, dnsCryptQuery, delayMsec, dest, responsesVect, queuedResponses, respIOV, respCBuf, true); return; diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 242ba1e40..63a6c0192 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -718,6 +718,7 @@ struct DownstreamState bool upStatus{false}; bool useECS{false}; bool setCD{false}; + bool disableZeroScope{false}; std::atomic connected{false}; std::atomic_flag threadStarted; bool tcpFastOpen{false}; @@ -1022,7 +1023,7 @@ bool responseContentMatches(const char* response, const uint16_t responseLen, co bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int* delayMsec, const struct timespec& now); bool processResponse(LocalStateHolder >& localRespRulactions, DNSResponse& dr, int* delayMsec); bool fixUpQueryTurnedResponse(DNSQuestion& dq, const uint16_t origFlags); -bool fixUpResponse(char** response, uint16_t* responseLen, size_t* responseSize, const DNSName& qname, uint16_t origFlags, bool ednsAdded, bool ecsAdded, std::vector& rewrittenResponse, uint16_t addRoom, bool* zeroScope=0); +bool fixUpResponse(char** response, uint16_t* responseLen, size_t* responseSize, const DNSName& qname, uint16_t origFlags, bool ednsAdded, bool ecsAdded, std::vector& rewrittenResponse, uint16_t addRoom, bool* zeroScope); void restoreFlags(struct dnsheader* dh, uint16_t origFlags); bool checkQueryHeaders(const struct dnsheader* dh); -- 2.40.0