From 5b07dc823354c22f1083d450b4c4d7b732be5881 Mon Sep 17 00:00:00 2001
From: Kees Monshouwer <mind04@monshouwer.org>
Date: Thu, 30 Nov 2017 13:40:04 +0100
Subject: [PATCH] auth: remove an other query for unsigned notifies and
 notifies signed with a wrong TSIG key

---
 pdns/packethandler.cc | 51 ++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index fdf5b8bcc..dba5379e5 100644
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -801,64 +801,65 @@ int PacketHandler::processNotify(DNSPacket *p)
      We determine the SOA at our (known) master
      if master is higher -> do stuff
   */
-  vector<string> meta;
 
   if(!::arg().mustDo("slave") && s_forwardNotify.empty()) {
-    g_log<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" but slave support is disabled in the configuration"<<endl;
-    return RCode::NotImp;
+    g_log<<Logger::Warning<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" but slave support is disabled in the configuration"<<endl;
+    return RCode::Refused;
   }
 
+  // Sender verification
+  //
   if(!s_allowNotifyFrom.match((ComboAddress *) &p->d_remote ) || p->d_havetsig) {
     if (p->d_havetsig && p->getTSIGKeyname().empty() == false) {
-        g_log<<Logger::Notice<<"Received secure NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<", allowed by TSIG key '"<<p->getTSIGKeyname()<<"'"<<endl;
+        g_log<<Logger::Notice<<"Received secure NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<", with TSIG key '"<<p->getTSIGKeyname()<<"'"<<endl;
     } else {
-      g_log<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" but remote is not permitted by TSIG or allow-notify-from"<<endl;
+      g_log<<Logger::Warning<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" but the remote is not providing a TSIG key or in allow-notify-from (Refused)"<<endl;
       return RCode::Refused;
     }
   }
 
-  DNSBackend *db=0;
-  DomainInfo di;
-  if(!B.getDomainInfo(p->qdomain, di, false) || !(db=di.backend)) {
-    g_log<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" for which we are not authoritative"<<endl;
-    return trySuperMaster(p, p->getTSIGKeyname());
-  }
-
-  meta.clear();
-  if (B.getDomainMetadata(p->qdomain,"AXFR-MASTER-TSIG",meta) && meta.size() > 0) {
+  if ((!::arg().mustDo("allow-unsigned-notify") && !p->d_havetsig) || p->d_havetsig) {
     if (!p->d_havetsig) {
-      if (::arg().mustDo("allow-unsigned-notify")) {
-        g_log<<Logger::Warning<<"Received unsigned NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<": permitted because allow-unsigned-notify";
-      } else {
-        g_log<<Logger::Warning<<"Received unsigned NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<": refused"<<endl;
+      g_log<<Logger::Warning<<"Received unsigned NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" while a TSIG key was required (Refused)"<<endl;
+      return RCode::Refused;
+    }
+    vector<string> meta;
+    if (B.getDomainMetadata(p->qdomain,"AXFR-MASTER-TSIG",meta) && meta.size() > 0) {
+      if (!pdns_iequals(meta[0], p->getTSIGKeyname().toStringNoDot())) {
+        g_log<<Logger::Warning<<"Received secure NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<": expected TSIG key '"<<meta[0]<<", got '"<<p->getTSIGKeyname()<<"' (Refused)"<<endl;
         return RCode::Refused;
       }
-    } else if (meta[0] != p->getTSIGKeyname().toStringNoDot()) {
-      g_log<<Logger::Error<<"Received secure NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<": expected TSIG key '"<<meta[0]<<", got '"<<p->getTSIGKeyname()<<"'"<<endl;
-      return RCode::Refused;
     }
   }
 
+  // Domain verification
+  //
+  DomainInfo di;
+  if(!B.getDomainInfo(p->qdomain, di, false) || !di.backend) {
+    g_log<<Logger::Warning<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" for which we are not authoritative, trying supermaster"<<endl;
+    return trySuperMaster(p, p->getTSIGKeyname()); // FIXME a global 'off' switch for supermaster support will save some resources in setups without supermasters
+  }
+
   if(::arg().contains("trusted-notification-proxy", p->getRemote().toString())) {
     g_log<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from trusted-notification-proxy "<< p->getRemote()<<endl;
     if(di.masters.empty()) {
-      g_log<<Logger::Error<<"However, "<<p->qdomain<<" does not have any masters defined"<<endl;
+      g_log<<Logger::Error<<"However, "<<p->qdomain<<" does not have any masters defined (Refused)"<<endl;
       return RCode::Refused;
     }
   }
   else if(::arg().mustDo("master") && di.kind == DomainInfo::Master) {
-    g_log<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" but we are master, rejecting"<<endl;
+    g_log<<Logger::Warning<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" but we are master (Refused)"<<endl;
     return RCode::Refused;
   }
   else if(!di.isMaster(p->getRemote())) {
-    g_log<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" which is not a master"<<endl;
+    g_log<<Logger::Warning<<"Received NOTIFY for "<<p->qdomain<<" from "<<p->getRemote()<<" which is not a master (Refused)"<<endl;
     return RCode::Refused;
   }
     
   if(!s_forwardNotify.empty()) {
     set<string> forwardNotify(s_forwardNotify);
     for(set<string>::const_iterator j=forwardNotify.begin();j!=forwardNotify.end();++j) {
-      g_log<<Logger::Warning<<"Relaying notification of domain "<<p->qdomain<<" from "<<p->getRemote()<<" to "<<*j<<endl;
+      g_log<<Logger::Notice<<"Relaying notification of domain "<<p->qdomain<<" from "<<p->getRemote()<<" to "<<*j<<endl;
       Communicator.notify(p->qdomain,*j);
     }
   }
-- 
2.40.0