From f5a747bbc131c0ad80ce31145af8b199c2488c75 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 20 Nov 2017 18:12:48 +0100 Subject: [PATCH] rec: Fix DNSSEC validation of DS denial from the negative cache There is two reasons you can get a proper DS denial: * Secure to insecure cut, and if we are getting a referral with a DS denial, we know that we have to check that the NS bit is set as described in section 8.9 of rfc5155 * No zone cut inside a secure zone, and then of course the NS is not set When we retrieve the DS denial from the negative cache with a validation status of Indeterminate, most likely because validation was not enabled during the query that landed it in the cache, we don't have enough data to know which case we are looking at, so let's just skip the NS check. --- pdns/recursordist/test-syncres_cc.cc | 61 ++++++++++++++++++++++++++++ pdns/syncres.cc | 2 +- pdns/validate.cc | 2 +- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 465bc364c..c91f95cae 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -8856,6 +8856,67 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure) { BOOST_CHECK_EQUAL(queriesCount, 4); } +BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure_ds) { + /* + Validation is optional, and the first query does not ask for it, + so the answer is negatively cached as Indeterminate. + The second query asks for validation, answer should be marked as + Secure. + The difference with test_dnssec_validation_from_negcache_secure is + that have one more level here, so we are going to look for the proof + that the DS does not exist for the last level. Since there is no cut, + we should accept the fact that the NSEC denies DS and NS both. + */ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::Process); + + primeHints(); + const DNSName target("www.com."); + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + g_luaconfs.setState(luaconfsCopy); + + size_t queriesCount = 0; + + sr->setAsyncCallback([target,&queriesCount,keys](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, std::shared_ptr outgoingLogger, LWResult* res) { + queriesCount++; + + if (type == QType::DS || type == QType::DNSKEY) { + if (domain == target) { + /* there is no cut */ + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys, false); + } + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + + return 0; + }); + + vector ret; + /* first query does not require validation */ + sr->setDNSSECValidationRequested(false); + int res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); + BOOST_REQUIRE_EQUAL(ret.size(), 4); + BOOST_CHECK_EQUAL(queriesCount, 1); + + ret.clear(); + /* second one _does_ require validation */ + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 4); + BOOST_CHECK_EQUAL(queriesCount, 4); +} + BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) { /* Validation is optional, and the first query does not ask for it, diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 5c026511a..40c69b777 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1030,7 +1030,7 @@ void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const if (state == Secure) { dState expectedState = res == RCode::NXDomain ? NXDOMAIN : NXQTYPE; - dState denialState = getDenialValidationState(ne, state, expectedState, qtype == QType::DS); + dState denialState = getDenialValidationState(ne, state, expectedState, false); updateDenialValidationState(ne, state, denialState, expectedState, qtype == QType::DS); } if (state != Indeterminate) { diff --git a/pdns/validate.cc b/pdns/validate.cc index 66b7d13ea..f7f37e6fa 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -298,7 +298,7 @@ static bool provesNSEC3NoWildCard(DNSName wildcard, uint16_t const qtype, const /* This function checks whether the existence of qname|qtype is denied by the NSEC and NSEC3 in validrrsets. - - If `referralToUnsigned` is true and qtype is QType::DS, this functions returns Insecure + - If `referralToUnsigned` is true and qtype is QType::DS, this functions returns NODATA if a NSEC or NSEC3 proves that the name exists but no NS type exists, as specified in RFC 5155 section 8.9. - If `wantsNoDataProof` is set but a NSEC proves that the whole name does not exist, the function will return NXQTYPE is the name is proven to be ENT and NXDOMAIN otherwise. -- 2.40.0