From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 17 Jul 2017 08:29:45 +0000 (+0200)
Subject: rec: Fix validation at the exact RRSIG inception or expiration time
X-Git-Tag: rec-4.1.0-alpha1~11^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=179b340d522e36a65e799b048dcdae85c0237fdc;p=pdns

rec: Fix validation at the exact RRSIG inception or expiration time

Reported by Petr Špaček of cz.nic (thanks!).
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 9d1d8873f..cca6af233 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -216,7 +216,7 @@ static bool isRootServer(const ComboAddress& ip)
   return false;
 }
 
-static void computeRRSIG(const DNSSECPrivateKey& dpk, const DNSName& signer, const DNSName& signQName, uint16_t signQType, uint32_t signTTL, uint32_t sigValidity, RRSIGRecordContent& rrc, vector<shared_ptr<DNSRecordContent> >& toSign, boost::optional<uint8_t> algo=boost::none)
+static void computeRRSIG(const DNSSECPrivateKey& dpk, const DNSName& signer, const DNSName& signQName, uint16_t signQType, uint32_t signTTL, uint32_t sigValidity, RRSIGRecordContent& rrc, vector<shared_ptr<DNSRecordContent> >& toSign, boost::optional<uint8_t> algo=boost::none, boost::optional<uint32_t> inception=boost::none)
 {
   time_t now = time(nullptr);
   DNSKEYRecordContent drc = dpk.getDNSKEY();
@@ -225,7 +225,7 @@ static void computeRRSIG(const DNSSECPrivateKey& dpk, const DNSName& signer, con
   rrc.d_type = signQType;
   rrc.d_labels = signQName.countLabels() - signQName.isWildcard();
   rrc.d_originalttl = signTTL;
-  rrc.d_siginception = now - 10;
+  rrc.d_siginception = inception ? *inception : (now - 10);
   rrc.d_sigexpire = now + sigValidity;
   rrc.d_signer = signer;
   rrc.d_tag = 0;
@@ -3190,8 +3190,10 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig) {
 
   DNSName qname("powerdns.com.");
 
+  time_t now = time(nullptr);
   RRSIGRecordContent rrc;
-  computeRRSIG(dpk, qname, qname, QType::A, 600, 300, rrc, recordcontents);
+  /* this RRSIG is valid for the current second only */
+  computeRRSIG(dpk, qname, qname, QType::A, 600, 0, rrc, recordcontents, boost::none, now);
 
   skeyset_t keyset;
   keyset.insert(std::make_shared<DNSKEYRecordContent>(dpk.getDNSKEY()));
@@ -3199,7 +3201,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig) {
   std::vector<std::shared_ptr<RRSIGRecordContent> > sigs;
   sigs.push_back(std::make_shared<RRSIGRecordContent>(rrc));
 
-  BOOST_CHECK(validateWithKeySet(time(nullptr), qname, recordcontents, sigs, keyset));
+  BOOST_CHECK(validateWithKeySet(now, qname, recordcontents, sigs, keyset));
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_root_validation_csk) {
diff --git a/pdns/validate.cc b/pdns/validate.cc
index 5778c08ab..f9bc5153d 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -244,13 +244,17 @@ static bool checkSignatureWithKey(time_t now, const shared_ptr<RRSIGRecordConten
 {
   bool result = false;
   try {
-    if(sig->d_siginception < now && sig->d_sigexpire > now) {
+    /* rfc4035:
+       - The validator's notion of the current time MUST be less than or equal to the time listed in the RRSIG RR's Expiration field.
+       - The validator's notion of the current time MUST be greater than or equal to the time listed in the RRSIG RR's Inception field.
+    */
+    if(sig->d_siginception <= now && sig->d_sigexpire >= now) {
       std::shared_ptr<DNSCryptoKeyEngine> dke = shared_ptr<DNSCryptoKeyEngine>(DNSCryptoKeyEngine::makeFromPublicKeyString(key->d_algorithm, key->d_key));
       result = dke->verify(msg, sig->d_signature);
       LOG("signature by key with tag "<<sig->d_tag<<" and algorithm "<<DNSSECKeeper::algorithm2name(sig->d_algorithm)<<" was " << (result ? "" : "NOT ")<<"valid"<<endl);
     }
     else {
-      LOG("Signature is "<<((sig->d_siginception >= now) ? "not yet valid" : "expired")<<" (inception: "<<sig->d_siginception<<", expiration: "<<sig->d_sigexpire<<", now: "<<now<<")"<<endl);
+      LOG("Signature is "<<((sig->d_siginception > now) ? "not yet valid" : "expired")<<" (inception: "<<sig->d_siginception<<", expiration: "<<sig->d_sigexpire<<", now: "<<now<<")"<<endl);
     }
   }
   catch(const std::exception& e) {