From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 24 Dec 2018 16:28:37 +0000 (+0100)
Subject: rec: Refuse records in the answer section of AA=0 responses
X-Git-Tag: rec-4.2.0-alpha1~4^2~9
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=70ea0c651a0a223877ec84368762666dfccb0a78;p=pdns

rec: Refuse records in the answer section of AA=0 responses
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 130274d47..a6810e223 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -2608,6 +2608,35 @@ BOOST_AUTO_TEST_CASE(test_qclass_none) {
   BOOST_CHECK_EQUAL(queriesCount, 0);
 }
 
+BOOST_AUTO_TEST_CASE(test_answer_no_aa) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  primeHints();
+
+  const DNSName target("powerdns.com.");
+
+  sr->setAsyncCallback([target](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) {
+
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, domain, QType::A, "192.0.2.1");
+      return 1;
+    });
+
+  const time_t now = sr->getNow().tv_sec;
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::ServFail);
+  BOOST_CHECK_EQUAL(ret.size(), 0);
+
+  /* check that the record in the answer section has not been cached */
+  const ComboAddress who;
+  vector<DNSRecord> cached;
+  vector<std::shared_ptr<RRSIGRecordContent>> signatures;
+  BOOST_REQUIRE_EQUAL(t_RC->get(now, target, QType(QType::A), false, &cached, who, &signatures), -1);
+}
+
 BOOST_AUTO_TEST_CASE(test_special_types) {
   std::unique_ptr<SyncRes> sr;
   initSR(sr);
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index c463a5320..ae7b72252 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2043,6 +2043,11 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
       continue;
     }
 
+    if (!(lwr.d_aabit || wasForwardRecurse) && rec.d_place == DNSResourceRecord::ANSWER) {
+      LOG("NO! - we don't accept records in the answers section without the AA bit set"<<endl);
+      continue;
+    }
+
     if(rec.d_name.isPartOf(auth)) {
       if(rec.d_type == QType::RRSIG) {
         LOG("RRSIG - separate"<<endl);
@@ -2125,7 +2130,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
     /* if we forwarded the query to a recursor, we can expect the answer to be signed,
        even if the answer is not AA. Of course that's not only true inside a Secure
        zone, but we check that below. */
-    bool expectSignature = isAA || wasForwardRecurse;
+    bool expectSignature = i->first.place == DNSResourceRecord::ANSWER || ((lwr.d_aabit || wasForwardRecurse) && i->first.place != DNSResourceRecord::ADDITIONAL);
     if (isCNAMEAnswer && (i->first.place != DNSResourceRecord::ANSWER || i->first.type != QType::CNAME || i->first.name != qname)) {
       /*
         rfc2181 states:
@@ -2252,6 +2257,10 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
     if (rec.d_type!=QType::OPT && rec.d_class!=QClass::IN)
       continue;
 
+    if (rec.d_place==DNSResourceRecord::ANSWER && !(lwr.d_aabit || sendRDQuery)) {
+      continue;
+    }
+
     if(rec.d_place==DNSResourceRecord::AUTHORITY && rec.d_type==QType::SOA &&
        lwr.d_rcode==RCode::NXDomain && qname.isPartOf(rec.d_name) && rec.d_name.isPartOf(auth)) {
       LOG(prefix<<qname<<": got negative caching indication for name '"<<qname<<"' (accept="<<rec.d_name.isPartOf(auth)<<"), newtarget='"<<newtarget<<"'"<<endl);