From: Pieter Lexis Date: Tue, 6 Jul 2021 10:57:32 +0000 (+0200) Subject: auth: send YXDOMAIN on too long DNAME synth X-Git-Tag: auth-4.6.0-alpha1~18^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F10231%2Fhead;p=thirdparty%2Fpdns.git auth: send YXDOMAIN on too long DNAME synth --- diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc index 87f398ae42..25fb63a31c 100644 --- a/pdns/packethandler.cc +++ b/pdns/packethandler.cc @@ -340,9 +340,9 @@ vector PacketHandler::getBestReferralNS(DNSPacket& p, const DNSNa return ret; } -vector PacketHandler::getBestDNAMESynth(DNSPacket& p, DNSName &target) +void PacketHandler::getBestDNAMESynth(DNSPacket& p, DNSName &target, vector &ret) { - vector ret; + ret.clear(); DNSZoneRecord rr; DNSName prefix; DNSName subdomain(target); @@ -356,18 +356,18 @@ vector PacketHandler::getBestDNAMESynth(DNSPacket& p, DNSName &ta rr.dr.d_name = prefix + rr.dr.d_name; rr.dr.d_content = std::make_shared(CNAMERecordContent(prefix + getRR(rr.dr)->getTarget())); rr.auth = false; // don't sign CNAME - target= getRR(rr.dr)->getTarget(); + target = getRR(rr.dr)->getTarget(); ret.push_back(rr); } if(!ret.empty()) - return ret; + return; if(subdomain.countLabels()) prefix.appendRawLabel(subdomain.getRawLabels()[0]); // XXX DNSName pain this feels wrong if(subdomain == d_sd.qname) // stop at SOA break; } while( subdomain.chopOff() ); // 'www.powerdns.org' -> 'powerdns.org' -> 'org' -> '' - return ret; + return; } @@ -1210,13 +1210,26 @@ bool PacketHandler::tryDNAME(DNSPacket& p, std::unique_ptr& r, DNSNam if(!d_doDNAME) return false; DLOG(g_log< rrset = getBestDNAMESynth(p, target); - if(!rrset.empty()) { - for(auto& rr: rrset) { - rr.dr.d_place = DNSResourceRecord::ANSWER; - r->addRecord(std::move(rr)); + vector rrset; + try { + getBestDNAMESynth(p, target, rrset); + if(!rrset.empty()) { + for(size_t i = 0; i < rrset.size(); i++) { + rrset.at(i).dr.d_place = DNSResourceRecord::ANSWER; + r->addRecord(std::move(rrset.at(i))); + } + return true; + } + } catch (const std::range_error &e) { + // Add the DNAME regardless, but throw to let the caller know we could not + // synthesize a CNAME + if(!rrset.empty()) { + for(size_t i = 0; i < rrset.size(); i++) { + rrset.at(i).dr.d_place = DNSResourceRecord::ANSWER; + r->addRecord(std::move(rrset.at(i))); + } } - return true; + throw e; } return false; } @@ -1647,16 +1660,20 @@ std::unique_ptr PacketHandler::doQuestion(DNSPacket& p) goto sendit; } - else if(tryDNAME(p, r, target)) { - retargetcount++; - goto retargeted; - } - else - { - if (!(((p.qtype.getCode() == QType::CNAME) || (p.qtype.getCode() == QType::ANY)) && retargetcount > 0)) - makeNXDomain(p, r, target, wildcard); + try { + if (tryDNAME(p, r, target)) { + retargetcount++; + goto retargeted; + } + } catch (const std::range_error &e) { + // We couldn't make a CNAME..... + r->setRcode(RCode::YXDomain); + goto sendit; } - + + if (!(((p.qtype.getCode() == QType::CNAME) || (p.qtype.getCode() == QType::ANY)) && retargetcount > 0)) + makeNXDomain(p, r, target, wildcard); + goto sendit; } diff --git a/pdns/packethandler.hh b/pdns/packethandler.hh index 01b4c403e9..e9e79f391f 100644 --- a/pdns/packethandler.hh +++ b/pdns/packethandler.hh @@ -97,7 +97,7 @@ private: void makeNXDomain(DNSPacket& p, std::unique_ptr& r, const DNSName& target, const DNSName& wildcard); void makeNOError(DNSPacket& p, std::unique_ptr& r, const DNSName& target, const DNSName& wildcard, int mode); vector getBestReferralNS(DNSPacket& p, const DNSName &target); - vector getBestDNAMESynth(DNSPacket& p, DNSName &target); + void getBestDNAMESynth(DNSPacket& p, DNSName &target, vector &ret); bool tryDNAME(DNSPacket& p, std::unique_ptr& r, DNSName &target); bool tryReferral(DNSPacket& p, std::unique_ptr& r, const DNSName &target, bool retargeted); diff --git a/regression-tests/tests/dname-too-long-synth/command b/regression-tests/tests/dname-too-long-synth/command new file mode 100755 index 0000000000..b6c9f647fd --- /dev/null +++ b/regression-tests/tests/dname-too-long-synth/command @@ -0,0 +1,2 @@ +#!/bin/sh +cleandig 1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.123456.www.d.test.com A dnssec diff --git a/regression-tests/tests/dname-too-long-synth/description b/regression-tests/tests/dname-too-long-synth/description new file mode 100644 index 0000000000..7b229f21db --- /dev/null +++ b/regression-tests/tests/dname-too-long-synth/description @@ -0,0 +1,2 @@ +Check that we send YXDOMAIN when we can't synthesise a too long CNAME +from a DNAME. diff --git a/regression-tests/tests/dname-too-long-synth/expected_result b/regression-tests/tests/dname-too-long-synth/expected_result new file mode 100644 index 0000000000..dd2d6579ff --- /dev/null +++ b/regression-tests/tests/dname-too-long-synth/expected_result @@ -0,0 +1,4 @@ +0 d.test.com. IN DNAME 3600 d2.test2.com. +2 . IN OPT 32768 +Rcode: 6 (Name Exists when it should not), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0 +Reply to question for qname='1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.123456.www.d.test.com.', qtype=A diff --git a/regression-tests/tests/dname-too-long-synth/expected_result.dnssec b/regression-tests/tests/dname-too-long-synth/expected_result.dnssec new file mode 100644 index 0000000000..c3054a93f9 --- /dev/null +++ b/regression-tests/tests/dname-too-long-synth/expected_result.dnssec @@ -0,0 +1,5 @@ +0 d.test.com. IN DNAME 3600 d2.test2.com. +0 d.test.com. IN RRSIG 3600 DNAME 13 3 3600 [expiry] [inception] [keytag] test.com. ... +2 . IN OPT 32768 +Rcode: 6 (Name Exists when it should not), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0 +Reply to question for qname='1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.123456.www.d.test.com.', qtype=A