rec: Check that DNSKEYs have the 'zone' flag set, 'revoked' one cleared

diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc
index a7d6a057b2deaec17f20fd3068696ec99e1103b6..5971b9163b57cd7e00ee0063ef453763f5e7d146 100644 (file)
--- 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<SyncRes> 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>(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<DNSSECPrivateKey, DSRecordContent>(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<Netmask>& srcmask, boost::optional<const ResolveContext&> 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<DNSRecord> 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<SyncRes> 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>(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<DNSSECPrivateKey, DSRecordContent>(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<Netmask>& srcmask, boost::optional<const ResolveContext&> 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<DNSRecord> 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<SyncRes> sr;

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 996870fc99f02409694b7b879cc099152085cf71..8a0c92fd938522021095adc19dfdcfbf606a8c97 100644 (file)
--- 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 <<Logger::Warning << x; }
 
+static bool isAZoneKey(const DNSKEYRecordContent& key)
+{
+  /* rfc4034 Section 2.1.1:
+     "Bit 7 of the Flags field is the Zone Key flag.  If bit 7 has value 1,
+     then the DNSKEY record holds a DNS zone key, and the DNSKEY RR's
+     owner name MUST be the name of a zone.  If bit 7 has value 0, then
+     the DNSKEY record holds some other type of DNS public key and MUST
+     NOT be used to verify RRSIGs that cover RRsets."
+
+     Let's check that this is a ZONE key, even though there is no other
+     types of DNSKEYs at the moment.
+  */
+  return (key.d_flags & 256) != 0;
+}
+
+static bool isRevokedKey(const DNSKEYRecordContent& key)
+{
+  /* rfc5011 Section 3 */
+  return (key.d_flags & 128) != 0;
+}
+
 static vector<shared_ptr<DNSKEYRecordContent > > getByTag(const skeyset_t& keys, uint16_t tag, uint8_t algorithm)
 {
   vector<shared_ptr<DNSKEYRecordContent>> 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 "<<std::to_string(tag)<<" and algorithm "<<std::to_string(algorithm)<<" is not a zone key, skipping"<<endl;);
+      continue;
+    }
+
+    if (isRevokedKey(*key)) {
+      LOG("Key for tag "<<std::to_string(tag)<<" and algorithm "<<std::to_string(algorithm)<<" has been revoked, skipping"<<endl;);
+      continue;
+    }
+
+    if (key->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;