From: Pieter Lexis Date: Fri, 1 Mar 2019 17:33:50 +0000 (+0100) Subject: rec: Validate DNAME sigs X-Git-Tag: rec-4.2.0-beta1~6^2~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8838c88f478bc2bd20fd8d842ecda1b1f1b6cc95;p=thirdparty%2Fpdns.git rec: Validate DNAME sigs --- diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 1a336cf22c..e0e4ca17c4 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -10856,6 +10856,109 @@ BOOST_AUTO_TEST_CASE(test_dname_processing) { BOOST_CHECK(ret[2].d_type == QType::A); BOOST_CHECK_EQUAL(ret[2].d_name, cnameTarget); + + // TODO add cached tests once the caching of DNAME is implemented +} + +BOOST_AUTO_TEST_CASE(test_dname_dnssec_secure) { + std::unique_ptr sr; + initSR(sr, true); + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + + const DNSName dnameOwner("powerdns"); + const DNSName dnameTarget("example"); + + const DNSName target("dname.powerdns"); + const DNSName cnameTarget("dname.example"); + + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(dnameOwner, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + generateKeyMaterial(dnameTarget, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + g_luaconfs.setState(luaconfsCopy); + + size_t queries = 0; + + sr->setAsyncCallback([dnameOwner, dnameTarget, target, cnameTarget, keys, &queries](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, LWResult* res, bool* chained) { + queries++; + + if (type == QType::DS || type == QType::DNSKEY) { + bool proveCut=true; + auto auth{domain}; + if (domain.countLabels() > 1) { + auth.chopOff(); + proveCut = false; + } + return genericDSAndDNSKEYHandler(res, domain, auth, type, keys, proveCut); + } + + if (isRootServer(ip)) { + if (domain.isPartOf(dnameOwner)) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, dnameOwner, QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + addDS(dnameOwner, 300, res->d_records, keys); + addRRSIG(keys, res->d_records, DNSName("."), 300); + addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return 1; + } + if (domain.isPartOf(dnameTarget)) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, dnameTarget, QType::NS, "b.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + addDS(dnameTarget, 300, res->d_records, keys); + addRRSIG(keys, res->d_records, DNSName("."), 300); + addRecordToLW(res, "b.gtld-servers.net.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + return 1; + } + } else if (ip == ComboAddress("192.0.2.1:53")) { + if (domain == target) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, dnameOwner, QType::DNAME, dnameTarget.toString()); + addRRSIG(keys, res->d_records, DNSName("."), 300); + addRecordToLW(res, domain, QType::CNAME, cnameTarget.toString()); + return 1; + } + } else if (ip == ComboAddress("192.0.2.2:53")) { + if (domain == cnameTarget) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::A, "192.0.2.2"); + addRRSIG(keys, res->d_records, DNSName("."), 300); + } + return 1; + } + return 0; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 5); /* DNAME + RRSIG(DNAME) + CNAME + A + RRSIG(A) */ + + BOOST_CHECK_EQUAL(queries, 9); + + BOOST_CHECK(ret[0].d_type == QType::DNAME); + BOOST_CHECK(ret[0].d_name == dnameOwner); + BOOST_CHECK_EQUAL(getRR(ret[0])->getTarget(), dnameTarget); + + BOOST_CHECK(ret[1].d_type == QType::RRSIG); + BOOST_CHECK_EQUAL(ret[1].d_name, dnameOwner); + + BOOST_CHECK(ret[2].d_type == QType::CNAME); + BOOST_CHECK_EQUAL(ret[2].d_name, target); + + BOOST_CHECK(ret[3].d_type == QType::A); + BOOST_CHECK_EQUAL(ret[3].d_name, cnameTarget); + + BOOST_CHECK(ret[4].d_type == QType::RRSIG); + BOOST_CHECK_EQUAL(ret[4].d_name, cnameTarget); + + // TODO add cached tests once the caching of DNAME is implemented } /* diff --git a/pdns/syncres.cc b/pdns/syncres.cc index d247efd486..01d22c530d 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -2171,6 +2171,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr std::vector> authorityRecs; const unsigned int labelCount = qname.countLabels(); bool isCNAMEAnswer = false; + bool isDNAMEAnswer = false; for(const auto& rec : lwr.d_records) { if (rec.d_class != QClass::IN) { continue; @@ -2179,6 +2180,9 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr if(!isCNAMEAnswer && rec.d_place == DNSResourceRecord::ANSWER && rec.d_type == QType::CNAME && (!(qtype==QType(QType::CNAME))) && rec.d_name == qname) { isCNAMEAnswer = true; } + if(!isDNAMEAnswer && rec.d_place == DNSResourceRecord::ANSWER && rec.d_type == QType::DNAME && (!(qtype==QType(QType::DNAME))) && qname.isPartOf(rec.d_name)) { + isDNAMEAnswer = true; + } /* if we have a positive answer synthetized from a wildcard, we need to store the corresponding NSEC/NSEC3 records proving @@ -2338,6 +2342,10 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr expectSignature = false; } + if (isDNAMEAnswer && !isAA && i->first.place == DNSResourceRecord::ANSWER && i->first.type == QType::DNAME && qname.isPartOf(i->first.name)) { + isAA = true; + } + if (isCNAMEAnswer && i->first.place == DNSResourceRecord::AUTHORITY && i->first.type == QType::NS && auth == i->first.name) { /* These NS can't be authoritative since we have a CNAME answer for which (see above) only the record describing that alias is necessarily authoritative. @@ -2367,11 +2375,25 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr recordState = validateDNSKeys(i->first.name, i->second.records, i->second.signatures, depth); } else { - LOG(d_prefix<<"Validating non-additional record for "<first.name<first.name, i->second.records, i->second.signatures); - /* we might have missed a cut (zone cut within the same auth servers), causing the NS query for an Insecure zone to seem Bogus during zone cut determination */ - if (qtype == QType::NS && i->second.signatures.empty() && recordState == Bogus && haveExactValidationStatus(i->first.name) && getValidationStatus(i->first.name) == Indeterminate) { - recordState = Indeterminate; + /* + * RFC 6672 section 5.3.1 + * In any response, a signed DNAME RR indicates a non-terminal + * redirection of the query. There might or might not be a server- + * synthesized CNAME in the answer section; if there is, the CNAME will + * never be signed. For a DNSSEC validator, verification of the DNAME + * RR and then that the CNAME was properly synthesized is sufficient + * proof. + * + * We do the synthesis check in processRecords, here we make sure we + * don't validate the CNAME. + */ + if (!(isDNAMEAnswer && i->first.type == QType::CNAME)) { + LOG(d_prefix<<"Validating non-additional record for "<first.name<first.name, i->second.records, i->second.signatures); + /* we might have missed a cut (zone cut within the same auth servers), causing the NS query for an Insecure zone to seem Bogus during zone cut determination */ + if (qtype == QType::NS && i->second.signatures.empty() && recordState == Bogus && haveExactValidationStatus(i->first.name) && getValidationStatus(i->first.name) == Indeterminate) { + recordState = Indeterminate; + } } } }