From 6f2088b4fef682cd640b908e4c4f652b854257a1 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 8 Jul 2020 12:24:43 +0200 Subject: [PATCH] rec: Check that DNSKEYs have the 'zone' flag set, 'revoked' one cleared --- pdns/recursordist/test-syncres_cc4.cc | 153 ++++++++++++++++++++++++++ pdns/validate.cc | 48 ++++++-- 2 files changed, 190 insertions(+), 11 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc index a7d6a057b2..5971b9163b 100644 --- a/pdns/recursordist/test-syncres_cc4.cc +++ b/pdns/recursordist/test-syncres_cc4.cc @@ -684,6 +684,159 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_dnskey) BOOST_CHECK_EQUAL(queriesCount, 2U); } +BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_without_zone_flag) +{ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("."); + testkeysset_t keys; + + /* Generate key material for "." */ + auto dcke = std::shared_ptr(DNSCryptoKeyEngine::make(DNSSECKeeper::ECDSA256)); + dcke->create(dcke->getBits()); + DNSSECPrivateKey csk; + csk.d_flags = 0; + csk.setKey(dcke); + DSRecordContent ds = makeDSFromDNSKey(target, csk.getDNSKEY(), DNSSECKeeper::DIGEST_SHA256); + + keys[target] = std::pair(csk, ds); + + /* Set the root DS */ + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + luaconfsCopy.dsAnchors[g_rootdnsname].insert(ds); + 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, LWResult* res, bool* chained) { + queriesCount++; + + if (domain == target && type == QType::NS) { + + setLWResult(res, 0, true, false, true); + char addr[] = "a.root-servers.net."; + for (char idx = 'a'; idx <= 'm'; idx++) { + addr[0] = idx; + addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 3600); + } + + addRRSIG(keys, res->d_records, domain, 300); + addRecordToLW(res, "a.root-servers.net.", QType::A, "198.41.0.4", DNSResourceRecord::ADDITIONAL, 3600); + addRecordToLW(res, "a.root-servers.net.", QType::AAAA, "2001:503:ba3e::2:30", DNSResourceRecord::ADDITIONAL, 3600); + + return 1; + } + else if (domain == target && type == QType::DNSKEY) { + + setLWResult(res, 0, true, false, true); + + addDNSKEY(keys, domain, 300, res->d_records); + addRRSIG(keys, res->d_records, domain, 300); + + return 1; + } + + return 0; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Bogus); + /* 13 NS + 1 RRSIG */ + BOOST_REQUIRE_EQUAL(ret.size(), 14U); + BOOST_CHECK_EQUAL(queriesCount, 2U); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Bogus); + BOOST_REQUIRE_EQUAL(ret.size(), 14U); + BOOST_CHECK_EQUAL(queriesCount, 2U); +} + +BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_revoked) +{ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("."); + testkeysset_t keys; + + /* Generate key material for "." */ + auto dcke = std::shared_ptr(DNSCryptoKeyEngine::make(DNSSECKeeper::ECDSA256)); + dcke->create(dcke->getBits()); + DNSSECPrivateKey csk; + csk.d_flags = 257 | 128; + csk.setKey(dcke); + DSRecordContent ds = makeDSFromDNSKey(target, csk.getDNSKEY(), DNSSECKeeper::DIGEST_SHA256); + + keys[target] = std::pair(csk, ds); + + /* Set the root DS */ + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + luaconfsCopy.dsAnchors[g_rootdnsname].insert(ds); + 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, LWResult* res, bool* chained) { + queriesCount++; + + if (domain == target && type == QType::NS) { + + setLWResult(res, 0, true, false, true); + char addr[] = "a.root-servers.net."; + for (char idx = 'a'; idx <= 'm'; idx++) { + addr[0] = idx; + addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 3600); + } + + addRRSIG(keys, res->d_records, domain, 300); + addRecordToLW(res, "a.root-servers.net.", QType::A, "198.41.0.4", DNSResourceRecord::ADDITIONAL, 3600); + addRecordToLW(res, "a.root-servers.net.", QType::AAAA, "2001:503:ba3e::2:30", DNSResourceRecord::ADDITIONAL, 3600); + + return 1; + } + else if (domain == target && type == QType::DNSKEY) { + + setLWResult(res, 0, true, false, true); + + addDNSKEY(keys, domain, 300, res->d_records); + addRRSIG(keys, res->d_records, domain, 300); + + return 1; + } + + return 0; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Bogus); + /* 13 NS + 1 RRSIG */ + BOOST_REQUIRE_EQUAL(ret.size(), 14U); + BOOST_CHECK_EQUAL(queriesCount, 2U); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Bogus); + BOOST_REQUIRE_EQUAL(ret.size(), 14U); + BOOST_CHECK_EQUAL(queriesCount, 2U); +} BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_doesnt_match_ds) { std::unique_ptr sr; diff --git a/pdns/validate.cc b/pdns/validate.cc index 996870fc99..8a0c92fd93 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -11,12 +11,47 @@ uint16_t g_maxNSEC3Iterations{0}; #define LOG(x) if(g_dnssecLOG) { g_log < > getByTag(const skeyset_t& keys, uint16_t tag, uint8_t algorithm) { vector> ret; - for(const auto& key : keys) - if(key->d_protocol == 3 && key->getTag() == tag && key->d_algorithm == algorithm) + + for (const auto& key : keys) { + if (!isAZoneKey(*key)) { + LOG("Key for tag "<d_protocol == 3 && key->getTag() == tag && key->d_algorithm == algorithm) { ret.push_back(key); + } + } + return ret; } @@ -757,15 +792,6 @@ bool validateWithKeySet(time_t now, const DNSName& name, const sortedRecords_t& string msg = getMessageForRRSET(name, *signature, toSign, true); for(const auto& key : keysMatchingTag) { - /* rfc4034 Section 5.2: - "The DNSKEY RR Flags MUST have Flags bit 7 set." - Let's check that this is a ZONE key, even though there is no other - types of DNSKEYs at the moment. - */ - if (!(key->d_flags & 256)) { - continue; - } - bool signIsValid = checkSignatureWithKey(now, signature, key, msg); if (signIsValid) { isValid = true; -- 2.47.2