]> granicus.if.org Git - pdns/commitdiff
rec: Refuse records in the answer section of AA=0 responses
authorRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 24 Dec 2018 16:28:37 +0000 (17:28 +0100)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 21 Jan 2019 15:38:47 +0000 (16:38 +0100)
pdns/recursordist/test-syncres_cc.cc
pdns/syncres.cc

index 130274d4786e6ccc88e65a38626b11aae9cfc2d7..a6810e22306b629cf1429262a530877462a6e2f6 100644 (file)
@@ -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);
index c463a5320d771018ff5d1b3cace1569d56798e98..ae7b72252628b0601fd3b012201d00c0b8ec38e4 100644 (file)
@@ -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);