From 95823c07c2e77b6c26b81a9c979ba41979d9d460 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 21 Sep 2017 16:49:18 +0200 Subject: [PATCH] rec: NS-consistency check is only when we expect an insecure delegation --- pdns/dnssecinfra.cc | 30 +++++ pdns/dnssecinfra.hh | 3 + pdns/packethandler.cc | 31 ----- pdns/recursordist/test-syncres_cc.cc | 192 +++++++++++++++++++++++++-- pdns/syncres.cc | 10 +- pdns/syncres.hh | 2 +- pdns/validate.cc | 38 +++++- pdns/validate.hh | 2 +- 8 files changed, 254 insertions(+), 54 deletions(-) diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc index 95cbc3bdae..8964e1e72a 100644 --- a/pdns/dnssecinfra.cc +++ b/pdns/dnssecinfra.cc @@ -498,6 +498,36 @@ string hashQNameWithSalt(const std::string& salt, unsigned int iterations, const return toHash; } +void incrementHash(std::string& raw) // I wonder if this is correct, cmouse? ;-) +{ + if(raw.empty()) + return; + + for(string::size_type pos=raw.size(); pos; ) { + --pos; + unsigned char c = (unsigned char)raw[pos]; + ++c; + raw[pos] = (char) c; + if(c) + break; + } +} + +void decrementHash(std::string& raw) // I wonder if this is correct, cmouse? ;-) +{ + if(raw.empty()) + return; + + for(string::size_type pos=raw.size(); pos; ) { + --pos; + unsigned char c = (unsigned char)raw[pos]; + --c; + raw[pos] = (char) c; + if(c != 0xff) + break; + } +} + DNSKEYRecordContent DNSSECPrivateKey::getDNSKEY() const { return makeDNSKEYFromDNSCryptoKeyEngine(getKey(), d_algorithm, d_flags); diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh index d094fdb552..f5b1c0e247 100644 --- a/pdns/dnssecinfra.hh +++ b/pdns/dnssecinfra.hh @@ -155,6 +155,9 @@ uint32_t getStartOfWeek(); string hashQNameWithSalt(const NSEC3PARAMRecordContent& ns3prc, const DNSName& qname); string hashQNameWithSalt(const std::string& salt, unsigned int iterations, const DNSName& qname); +void incrementHash(std::string& raw); +void decrementHash(std::string& raw); + void addRRSigs(DNSSECKeeper& dk, UeberBackend& db, const std::set& authMap, vector& rrs); void addTSIG(DNSPacketWriter& pw, TSIGRecordContent& trc, const DNSName& tsigkeyname, const string& tsigsecret, const string& tsigprevious, bool timersonly); diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc index 853128404f..dd33e75a9c 100644 --- a/pdns/packethandler.cc +++ b/pdns/packethandler.cc @@ -544,37 +544,6 @@ void PacketHandler::addNSECX(DNSPacket *p, DNSPacket *r, const DNSName& target, } } -static void incrementHash(std::string& raw) // I wonder if this is correct, cmouse? ;-) -{ - if(raw.empty()) - return; - - for(string::size_type pos=raw.size(); pos; ) { - --pos; - unsigned char c = (unsigned char)raw[pos]; - ++c; - raw[pos] = (char) c; - if(c) - break; - } -} - -static void decrementHash(std::string& raw) // I wonder if this is correct, cmouse? ;-) -{ - if(raw.empty()) - return; - - for(string::size_type pos=raw.size(); pos; ) { - --pos; - unsigned char c = (unsigned char)raw[pos]; - --c; - raw[pos] = (char) c; - if(c != 0xff) - break; - } -} - - bool getNSEC3Hashes(bool narrow, DNSBackend* db, int id, const std::string& hashed, bool decrement, DNSName& unhashed, std::string& before, std::string& after, int mode) { bool ret; diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 554986240a..2301a097e9 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -3,6 +3,7 @@ #include #include "arguments.hh" +#include "base32.hh" #include "dnssecinfra.hh" #include "dnsseckeeper.hh" #include "lua-recursor4.hh" @@ -339,6 +340,48 @@ static void addNSECRecordToLW(const DNSName& domain, const DNSName& next, const records.push_back(rec); } +static void addNSEC3RecordToLW(const DNSName& hashedName, const std::string& hashedNext, const std::string& salt, unsigned int iterations, const std::set& types, uint32_t ttl, std::vector& records) +{ + NSEC3RecordContent nrc; + nrc.d_algorithm = 1; + nrc.d_flags = 0; + nrc.d_iterations = iterations; + nrc.d_salt = salt; + nrc.d_nexthash = hashedNext; + nrc.d_set = types; + + DNSRecord rec; + rec.d_name = hashedName; + rec.d_ttl = ttl; + rec.d_type = QType::NSEC3; + rec.d_content = std::make_shared(nrc); + rec.d_place = DNSResourceRecord::AUTHORITY; + + records.push_back(rec); +} + +static void addNSEC3UnhashedRecordToLW(const DNSName& domain, const std::string& next, const std::set& types, uint32_t ttl, std::vector& records) +{ + static const std::string salt = "deadbeef"; + static const unsigned int iterations = 10; + std::string hashed = hashQNameWithSalt(salt, iterations, domain); + + addNSEC3RecordToLW(DNSName(toBase32Hex(hashed)), next, salt, iterations, types, ttl, records); +} + +void addNSEC3NarrowRecordToLW(const DNSName& domain, const std::set& types, uint32_t ttl, std::vector& records) +{ + static const std::string salt = "deadbeef"; + static const unsigned int iterations = 10; + std::string hashed = hashQNameWithSalt(salt, iterations, domain); + + std::string hashedNext(hashed); + incrementHash(hashedNext); + decrementHash(hashed); + + addNSEC3RecordToLW(DNSName(toBase32Hex(hashed)), hashedNext, salt, iterations, types, ttl, records); +} + static void generateKeyMaterial(const DNSName& name, unsigned int algo, uint8_t digest, testkeysset_t& keys) { auto dcke = std::shared_ptr(DNSCryptoKeyEngine::make(algo)); @@ -6801,10 +6844,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_nowrap) { cspmap_t denialMap; denialMap[std::make_pair(DNSName("a.example.org."), QType::NSEC)] = pair; - dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A); + dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false); BOOST_CHECK_EQUAL(denialState, NXDOMAIN); - denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A); + denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false); /* let's check that d.example.org. is not denied by this proof */ BOOST_CHECK_EQUAL(denialState, NODATA); } @@ -6836,10 +6879,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_wrap_case_1) { cspmap_t denialMap; denialMap[std::make_pair(DNSName("z.example.org."), QType::NSEC)] = pair; - dState denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A); + dState denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A, false); BOOST_CHECK_EQUAL(denialState, NXDOMAIN); - denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A); + denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false); /* let's check that d.example.org. is not denied by this proof */ BOOST_CHECK_EQUAL(denialState, NODATA); } @@ -6871,10 +6914,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_wrap_case_2) { cspmap_t denialMap; denialMap[std::make_pair(DNSName("y.example.org."), QType::NSEC)] = pair; - dState denialState = getDenial(denialMap, DNSName("z.example.org."), QType::A); + dState denialState = getDenial(denialMap, DNSName("z.example.org."), QType::A, false); BOOST_CHECK_EQUAL(denialState, NXDOMAIN); - denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A); + denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false); /* let's check that d.example.org. is not denied by this proof */ BOOST_CHECK_EQUAL(denialState, NODATA); } @@ -6906,10 +6949,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_only_one_nsec) { cspmap_t denialMap; denialMap[std::make_pair(DNSName("a.example.org."), QType::NSEC)] = pair; - dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A); + dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false); BOOST_CHECK_EQUAL(denialState, NXDOMAIN); - denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A); + denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A, false); /* let's check that d.example.org. is not denied by this proof */ BOOST_CHECK_EQUAL(denialState, NODATA); } @@ -6941,7 +6984,7 @@ BOOST_AUTO_TEST_CASE(test_nsec_root_nxd_denial) { cspmap_t denialMap; denialMap[std::make_pair(DNSName("a."), QType::NSEC)] = pair; - dState denialState = getDenial(denialMap, DNSName("b."), QType::A); + dState denialState = getDenial(denialMap, DNSName("b."), QType::A, false); BOOST_CHECK_EQUAL(denialState, NXDOMAIN); } @@ -6981,15 +7024,142 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial) { owner name regardless of type. */ - dState denialState = getDenial(denialMap, DNSName("a."), QType::A); + dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false); /* no data means the qname/qtype is not denied, because an ancestor delegation NSEC can only deny the DS */ BOOST_CHECK_EQUAL(denialState, NODATA); - denialState = getDenial(denialMap, DNSName("a."), QType::DS); + denialState = getDenial(denialMap, DNSName("a."), QType::DS, true); BOOST_CHECK_EQUAL(denialState, NXQTYPE); } +BOOST_AUTO_TEST_CASE(test_nsec_insecure_delegation_denial) { + init(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + + vector records; + + vector> recordContents; + vector> signatureContents; + + /* + * RFC 5155 section 8.9: + * If there is an NSEC3 RR present in the response that matches the + * delegation name, then the validator MUST ensure that the NS bit is + * set and that the DS bit is not set in the Type Bit Maps field of the + * NSEC3 RR. + */ + /* + The RRSIG from "." denies the existence of any type at a. + NS should be set if it was proving an insecure delegation, let's check that + we correctly detect that it's not. + */ + addNSECRecordToLW(DNSName("a."), DNSName("b."), { }, 600, records); + recordContents.push_back(records.at(0).d_content); + addRRSIG(keys, records, DNSName("."), 300); + signatureContents.push_back(getRR(records.at(1))); + records.clear(); + + ContentSigPair pair; + pair.records = recordContents; + pair.signatures = signatureContents; + cspmap_t denialMap; + denialMap[std::make_pair(DNSName("a."), QType::NSEC)] = pair; + + /* Insecure because the NS is not set, so while it does + denies the DS, it can't prove an insecure delegation */ + dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true); + BOOST_CHECK_EQUAL(denialState, INSECURE); +} + +BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) { + init(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + + vector records; + + vector> recordContents; + vector> signatureContents; + + /* + The RRSIG from "." denies the existence of any type except NS at a. + However since it's an ancestor delegation NSEC (NS bit set, SOA bit clear, + signer field that is shorter than the owner name of the NSEC RR) it can't + be used to deny anything except the whole name or a DS. + */ + addNSEC3UnhashedRecordToLW(DNSName("a."), "whatever", { QType::NS }, 600, records); + recordContents.push_back(records.at(0).d_content); + addRRSIG(keys, records, DNSName("."), 300); + signatureContents.push_back(getRR(records.at(1))); + + ContentSigPair pair; + pair.records = recordContents; + pair.signatures = signatureContents; + cspmap_t denialMap; + denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair; + records.clear(); + + /* RFC 6840 section 4.1 "Clarifications on Nonexistence Proofs": + Ancestor delegation NSEC or NSEC3 RRs MUST NOT be used to assume + nonexistence of any RRs below that zone cut, which include all RRs at + that (original) owner name other than DS RRs, and all RRs below that + owner name regardless of type. + */ + + dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false); + /* no data means the qname/qtype is not denied, because an ancestor + delegation NSEC3 can only deny the DS */ + BOOST_CHECK_EQUAL(denialState, NODATA); + + denialState = getDenial(denialMap, DNSName("a."), QType::DS, true); + BOOST_CHECK_EQUAL(denialState, NXQTYPE); +} + +BOOST_AUTO_TEST_CASE(test_nsec3_insecure_delegation_denial) { + init(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + + vector records; + + vector> recordContents; + vector> signatureContents; + + /* + * RFC 5155 section 8.9: + * If there is an NSEC3 RR present in the response that matches the + * delegation name, then the validator MUST ensure that the NS bit is + * set and that the DS bit is not set in the Type Bit Maps field of the + * NSEC3 RR. + */ + /* + The RRSIG from "." denies the existence of any type at a. + NS should be set if it was proving an insecure delegation, let's check that + we correctly detect that it's not. + */ + addNSEC3UnhashedRecordToLW(DNSName("a."), "whatever", { }, 600, records); + recordContents.push_back(records.at(0).d_content); + addRRSIG(keys, records, DNSName("."), 300); + signatureContents.push_back(getRR(records.at(1))); + + ContentSigPair pair; + pair.records = recordContents; + pair.signatures = signatureContents; + cspmap_t denialMap; + denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair; + records.clear(); + + /* Insecure because the NS is not set, so while it does + denies the DS, it can't prove an insecure delegation */ + dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true); + BOOST_CHECK_EQUAL(denialState, INSECURE); +} + /* // cerr<<"asyncresolve called to ask "<& dnskeys, const std::vector >& signatures, unsigned int depth); vState getDSRecords(const DNSName& zone, dsmap_t& ds, bool onlyTA, unsigned int depth, bool bogusOnNXD=true, bool* foundCut=nullptr); vState getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int depth); - void getDenialValidationState(NegCache::NegCacheEntry& ne, vState& state, const dState expectedState, bool allowOptOut); + void getDenialValidationState(NegCache::NegCacheEntry& ne, vState& state, const dState expectedState, bool allowOptOut, bool referralToUnsigned); vState getTA(const DNSName& zone, dsmap_t& ds); bool haveExactValidationStatus(const DNSName& domain); vState getValidationStatus(const DNSName& subdomain, bool allowIndeterminate=true); diff --git a/pdns/validate.cc b/pdns/validate.cc index 148a320eda..7b159e367b 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -98,7 +98,7 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector& // FIXME: needs a zone argument, to avoid things like 6840 4.1 // FIXME: Add ENT support // FIXME: Make usable for non-DS records and hook up to validateRecords (or another place) -dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype) +dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned) { for(const auto& v : validrrsets) { LOG("Do have: "<d_set.count(QType::NS)) { + LOG("However, no NS record exists at this level!"<d_set.count(QType::NS) && !nsec3->d_set.count(QType::SOA) && + getSigner(v.second.signatures).countLabels() < v.first.first.countLabels()) { + LOG("type is "<d_set.count(QType::NS))<<", SOA is "<d_set.count(QType::SOA))<<", signer is "<d_set.count(qtype)) { - LOG("Does _not_ deny existence of type "<d_set.count(QType::NS)) { + if (referralToUnsigned && qtype == QType::DS && !nsec3->d_set.count(QType::NS)) { LOG("However, no NS record exists at this level!"<& anchors, const DNSName& zone, dsmap_t &res); bool haveNegativeTrustAnchor(const map& negAnchors, const DNSName& zone, std::string& reason); void validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t& dsmap, const skeyset_t& tkeys, vector >& toSign, const vector >& sigs, skeyset_t& validkeys); -dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype); +dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned); bool isSupportedDS(const DSRecordContent& ds); DNSName getSigner(const std::vector >& signatures); bool denialProvesNoDelegation(const DNSName& zone, const std::vector& dsrecords); -- 2.47.2