From 29ad87969c8a24fcc812ff4cb445a9e130d256ae Mon Sep 17 00:00:00 2001
From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Tue, 6 Jul 2021 16:47:43 +0200
Subject: [PATCH] rec: do not use DNSKEYs found below an apex for validation,
 also from cache

---
 pdns/syncres.cc                               |  4 +-
 .../recursortests.py                          |  2 +
 .../test_Interop.py                           | 71 ++++++++++++++++++-
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index bf5b439e18..d5c097b73b 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1849,7 +1849,7 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
 
       if (recordState == vState::Secure) {
         LOG(prefix<<sqname<<": got vState::Indeterminate state from the cache, validating.."<<endl);
-        if (sqt == QType::DNSKEY) {
+        if (sqt == QType::DNSKEY && sqname == getSigner(signatures)) {
           cachedState = validateDNSKeys(sqname, cset, signatures, depth);
         }
         else {
@@ -1860,7 +1860,7 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
 
             for (const auto& type : types) {
               vState cachedRecordState;
-              if (type.first == QType::DNSKEY) {
+              if (type.first == QType::DNSKEY && sqname == getSigner(type.second.signatures)) {
                 cachedRecordState = validateDNSKeys(sqname, type.second.records, type.second.signatures, depth);
               }
               else {
diff --git a/regression-tests.recursor-dnssec/recursortests.py b/regression-tests.recursor-dnssec/recursortests.py
index 8ab62558bc..8cbdba1d26 100644
--- a/regression-tests.recursor-dnssec/recursortests.py
+++ b/regression-tests.recursor-dnssec/recursortests.py
@@ -183,6 +183,8 @@ dname-insecure.secure.example. 3600 IN DNAME insecure.example.
 dname-bogus.secure.example. 3600 IN DNAME bogus.example.
 
 non-apex-dnskey.secure.example. 3600 IN DNSKEY 257 3 13 CT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==
+non-apex-dnskey2.secure.example. 3600 IN DNSKEY 256 3 13 CT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==
+non-apex-dnskey3.secure.example. 3600 IN DNSKEY 256 3 13 DT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==
         """,
         'dname-secure.example': """
 dname-secure.example. 3600 IN SOA {soa}
diff --git a/regression-tests.recursor-dnssec/test_Interop.py b/regression-tests.recursor-dnssec/test_Interop.py
index 799633eac9..4b61759d71 100644
--- a/regression-tests.recursor-dnssec/test_Interop.py
+++ b/regression-tests.recursor-dnssec/test_Interop.py
@@ -121,7 +121,6 @@ forward-zones+=undelegated.insecure.example=%s.12
         self.assertRcodeEqual(res, dns.rcode.NOERROR)
         self.assertMessageHasFlags(res, ['QR', 'RD', 'RA', 'AD'], [])
 
-
     def testNonApexDNSKEY(self):
         """
         a DNSKEY not at the apex of a zone should not be treated as a DNSKEY in validation
@@ -152,6 +151,76 @@ forward-zones+=undelegated.insecure.example=%s.12
             cls._UDPResponder.setDaemon(True)
             cls._UDPResponder.start()
 
+class testInteropProcess(RecursorTest):
+    _confdir = 'InteropProcess'
+
+    _config_template = """dnssec=process
+packetcache-ttl=0 # explicitly disable packetcache
+forward-zones=undelegated.secure.example=%s.12
+forward-zones+=undelegated.insecure.example=%s.12
+    """ % (os.environ['PREFIX'], os.environ['PREFIX'])
+
+    def testNonApexDNSKEY2(self):
+        """
+        a DNSKEY not at the apex of a zone should not be treated as a DNSKEY in validation,
+        even when it was cached with Indeterminate validation state before
+        """
+
+        # send the query with +CD so the record ends up cached with Indeterminate validation state
+        query = dns.message.make_query('non-apex-dnskey2.secure.example.', 'DNSKEY')
+        query.flags |= dns.flags.CD
+
+        res = self.sendUDPQuery(query)
+        print(res)
+        expectedDNSKEY = dns.rrset.from_text('non-apex-dnskey2.secure.example.', 0, dns.rdataclass.IN, 'DNSKEY', '256 3 13 CT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==')
+
+        self.assertRRsetInAnswer(res, expectedDNSKEY)
+        self.assertRcodeEqual(res, dns.rcode.NOERROR)
+        self.assertMessageHasFlags(res, ['QR', 'RD', 'RA', 'CD'], [])
+
+        # now ask again with +AD so it has to be validated from cache
+        query = dns.message.make_query('non-apex-dnskey2.secure.example.', 'DNSKEY')
+        query.flags |= dns.flags.AD
+
+        res = self.sendUDPQuery(query)
+        print(res)
+        expectedDNSKEY = dns.rrset.from_text('non-apex-dnskey2.secure.example.', 0, dns.rdataclass.IN, 'DNSKEY', '256 3 13 CT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==')
+
+        self.assertRRsetInAnswer(res, expectedDNSKEY)
+        self.assertRcodeEqual(res, dns.rcode.NOERROR)
+        self.assertMessageHasFlags(res, ['QR', 'RD', 'RA', 'AD'], [])
+
+    def testNonApexDNSKEYANYQuery(self):
+        """
+        a DNSKEY not at the apex of a zone should not be treated as a DNSKEY in validation,
+        even when it was cached with Indeterminate validation state before.
+        This code tests the ANY path which is separate from the qtype=DNSKEY path tested in testNonApexDNSKEY2
+        """
+
+        # send the query with +CD so the record ends up cached with Indeterminate validation state
+        query = dns.message.make_query('non-apex-dnskey3.secure.example.', 'DNSKEY')
+        query.flags |= dns.flags.CD
+
+        res = self.sendUDPQuery(query)
+        print(res)
+        expectedDNSKEY = dns.rrset.from_text('non-apex-dnskey3.secure.example.', 0, dns.rdataclass.IN, 'DNSKEY', '256 3 13 DT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==')
+
+        self.assertRRsetInAnswer(res, expectedDNSKEY)
+        self.assertRcodeEqual(res, dns.rcode.NOERROR)
+        self.assertMessageHasFlags(res, ['QR', 'RD', 'RA', 'CD'], [])
+
+        # now ask again with +AD so it has to be validated from cache
+        query = dns.message.make_query('non-apex-dnskey3.secure.example.', 'ANY')
+        query.flags |= dns.flags.AD
+
+        res = self.sendUDPQuery(query)
+        print(res)
+        expectedDNSKEY = dns.rrset.from_text('non-apex-dnskey3.secure.example.', 0, dns.rdataclass.IN, 'DNSKEY', '256 3 13 DT6AJ4MEOtNDgj0+xLtTLGHf1WbLsKWZI8ONHOt/6q7hTjeWSnY/SGig1dIKZrHg+pJFUSPaxeShv48SYVRKEg==')
+
+        self.assertRRsetInAnswer(res, expectedDNSKEY)
+        self.assertRcodeEqual(res, dns.rcode.NOERROR)
+        self.assertMessageHasFlags(res, ['QR', 'RD', 'RA', 'AD'], [])
+
 class UDPResponder(DatagramProtocol):
     def datagramReceived(self, datagram, address):
         request = dns.message.from_wire(datagram)
-- 
2.47.2