Fix DS and NS add in different order

PowerDNS · Jul 12, 2013 · 6a82ce4 · 6a82ce4
1 parent e70f262
commit 6a82ce4
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 14 deletions.
diff --git a/pdns/rfc2136handler.cc b/pdns/rfc2136handler.cc
@@ -261,19 +261,25 @@ uint16_t PacketHandler::performUpdate(const string &msgPrefix, const DNSRecord *
       // because we added a record, we need to fix DNSSEC data.
       string shorter(rrLabel);
       bool auth=newRec.auth;
+      bool fixDS = (rrType == QType::DS);
 
-      if ( ! pdns_iequals(di->zone, shorter)) {
+      if ( ! pdns_iequals(di->zone, shorter)) { // Everything at APEX is auth=1 && no ENT's
         do {
+
           if (pdns_iequals(di->zone, shorter))
             break;
+
           bool foundShorter = false;
           di->backend->lookup(QType(QType::ANY), shorter);
           while (di->backend->get(rec)) {
+            if (pdns_iequals(rec.qname, rrLabel) && rec.qtype == QType::DS)
+              fixDS = true;
             if ( ! pdns_iequals(shorter, rrLabel) )
               foundShorter = true;
             if (rec.qtype == QType::NS) // are we inserting below a delegate?
               auth=false;
           }
+
           if (!foundShorter && auth && !pdns_iequals(shorter, rrLabel)) // haven't found any record at current level, insert ENT.
             insnonterm.insert(shorter);
           if (foundShorter)
@@ -292,8 +298,9 @@ uint16_t PacketHandler::performUpdate(const string &msgPrefix, const DNSRecord *
         else
           di->backend->updateDNSSECOrderAndAuthAbsolute(di->id, rrLabel, hashed, auth);
 
-        if (rrType == QType::DS)
-          di->backend->setDNSSECAuthOnDsRecord(di->id, rrLabel);        
+        if (fixDS)
+          di->backend->setDNSSECAuthOnDsRecord(di->id, rrLabel);
+
         if(!auth)
         {
           if (ns3pr->d_flags) 
@@ -305,14 +312,12 @@ uint16_t PacketHandler::performUpdate(const string &msgPrefix, const DNSRecord *
       else // NSEC
       {
         di->backend->updateDNSSECOrderAndAuth(di->id, di->zone, rrLabel, auth);
-        if (rrType == QType::DS)
+        if (fixDS) {
           di->backend->setDNSSECAuthOnDsRecord(di->id, rrLabel);
-        else {
-          if(!auth)
-          {
-            di->backend->nullifyDNSSECOrderNameAndAuth(di->id, rrLabel, "A");
-            di->backend->nullifyDNSSECOrderNameAndAuth(di->id, rrLabel, "AAAA");
-          }
+        }
+        if(!auth) {
+          di->backend->nullifyDNSSECOrderNameAndAuth(di->id, rrLabel, "A");
+          di->backend->nullifyDNSSECOrderNameAndAuth(di->id, rrLabel, "AAAA");
         }
       }
 
@@ -321,11 +326,11 @@ uint16_t PacketHandler::performUpdate(const string &msgPrefix, const DNSRecord *
       // Auth can only be false when the rrLabel is not the zone 
       if (auth == false && rrType == QType::NS) {
         DLOG(L<<msgPrefix<<"Going to fix auth flags below "<<rrLabel<<endl);
-        insnonterm.clear(); // clean ENT's again, as it's a delegate
+        insnonterm.clear(); // No ENT's are needed below delegates (auth=0)
         vector<string> qnames;
         di->backend->listSubZone(rrLabel, di->id);
         while(di->backend->get(rec)) {
-          if (rec.qtype.getCode() && rec.qtype.getCode() != QType::DS) // Skip ENT and DS records.
+          if (rec.qtype.getCode() && rec.qtype.getCode() != QType::DS && !pdns_iequals(rrLabel, rec.qname)) // Skip ENT, DS and our already corrected record.
             qnames.push_back(rec.qname);
         }
         for(vector<string>::const_iterator qname=qnames.begin(); qname != qnames.end(); ++qname) {
@@ -334,7 +339,11 @@ uint16_t PacketHandler::performUpdate(const string &msgPrefix, const DNSRecord *
             if(! *narrow) 
               hashed=toLower(toBase32Hex(hashQNameWithSalt(ns3pr->d_iterations, ns3pr->d_salt, *qname)));
 
-            di->backend->updateDNSSECOrderAndAuthAbsolute(di->id, *qname, hashed, auth);
+            if (*narrow)
+              di->backend->nullifyDNSSECOrderNameAndUpdateAuth(di->id, rrLabel, auth);
+            else
+              di->backend->updateDNSSECOrderAndAuthAbsolute(di->id, *qname, hashed, auth);
+
             if (ns3pr->d_flags)
               di->backend->nullifyDNSSECOrderNameAndAuth(di->id, *qname, "NS");
           }

diff --git a/regression-tests/1dyndns-update-add-delete-ds/command b/regression-tests/1dyndns-update-add-delete-ds/command
@@ -28,4 +28,32 @@ answer
 !
 
 # check if the record was deleted
-mysqldiff 2 "Check if record is gone"
+mysqldiff 2 "Check if record is gone"
+
+# add a delegate + ds
+cleannsupdate <<!
+server $nameserver $port
+zone test.dyndns
+update add del.test.dyndns. 3600 DS 39274 8 2 8E8A8CFB40FD0C30BFA82E53752E1C257DAFB7B6206D12B9EDA43AF3 EAB2157D
+update add del.test.dyndns. 3600 NS ns1.del.test.dyndns
+update add ns1.del.test.dyndns. 3600 A 127.0.0.1
+send
+answer
+!
+
+# check if the record was added
+mysqldiff 3 "Check delegate and DS added correctly again (other way around)"
+
+# delete the just added record
+cleannsupdate <<!
+server $nameserver $port
+zone test.dyndns
+update delete del.test.dyndns. DS
+update delete del.test.dyndns. NS
+update delete ns1.del.test.dyndns. A
+send
+answer
+!
+
+# check if the record was deleted
+mysqldiff 4 "Check if record is gone again"
diff --git a/regression-tests/1dyndns-update-add-delete-ds/expected_result b/regression-tests/1dyndns-update-add-delete-ds/expected_result
@@ -22,3 +22,27 @@ Check if record is gone
 no difference
 --- End: diff start step.2 ---
 
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check delegate and DS added correctly again (other way around)
+--- Start: diff start step.3 ---
+> del.test.dyndns	DS	0	39274 8 2 8e8a8cfb40fd0c30bfa82e53752e1c257dafb7b6206d12b9eda43af3eab2157d	3600
+> del.test.dyndns	NS	0	ns1.del.test.dyndns	3600
+> ns1.del.test.dyndns	A	0	127.0.0.1	3600
+--- End: diff start step.3 ---
+
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check if record is gone again
+--- Start: diff start step.4 ---
+no difference
+--- End: diff start step.4 ---
+
diff --git a/regression-tests/1dyndns-update-add-delete-ds/expected_result.dnssec b/regression-tests/1dyndns-update-add-delete-ds/expected_result.dnssec
@@ -22,3 +22,27 @@ Check if record is gone
 no difference
 --- End: diff start step.2 ---
 
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check delegate and DS added correctly again (other way around)
+--- Start: diff start step.3 ---
+> del.test.dyndns	DS	0	39274 8 2 8e8a8cfb40fd0c30bfa82e53752e1c257dafb7b6206d12b9eda43af3eab2157d	3600	'del'	1
+> del.test.dyndns	NS	0	ns1.del.test.dyndns	3600	'del'	0
+> ns1.del.test.dyndns	A	0	127.0.0.1	3600	NULL	0
+--- End: diff start step.3 ---
+
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check if record is gone again
+--- Start: diff start step.4 ---
+no difference
+--- End: diff start step.4 ---
+
diff --git a/regression-tests/1dyndns-update-add-delete-ds/expected_result.narrow b/regression-tests/1dyndns-update-add-delete-ds/expected_result.narrow
@@ -22,3 +22,27 @@ Check if record is gone
 no difference
 --- End: diff start step.2 ---
 
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check delegate and DS added correctly again (other way around)
+--- Start: diff start step.3 ---
+> del.test.dyndns	DS	0	39274 8 2 8e8a8cfb40fd0c30bfa82e53752e1c257dafb7b6206d12b9eda43af3eab2157d	3600	NULL	1
+> del.test.dyndns	NS	0	ns1.del.test.dyndns	3600	NULL	0
+> ns1.del.test.dyndns	A	0	127.0.0.1	3600	NULL	0
+--- End: diff start step.3 ---
+
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check if record is gone again
+--- Start: diff start step.4 ---
+no difference
+--- End: diff start step.4 ---
+
diff --git a/regression-tests/1dyndns-update-add-delete-ds/expected_result.nsec3 b/regression-tests/1dyndns-update-add-delete-ds/expected_result.nsec3
@@ -22,3 +22,27 @@ Check if record is gone
 no difference
 --- End: diff start step.2 ---
 
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check delegate and DS added correctly again (other way around)
+--- Start: diff start step.3 ---
+> del.test.dyndns	DS	0	39274 8 2 8e8a8cfb40fd0c30bfa82e53752e1c257dafb7b6206d12b9eda43af3eab2157d	3600	'ott41kituq4b2adjpf8gs59se6liu8vh'	1
+> del.test.dyndns	NS	0	ns1.del.test.dyndns	3600	'ott41kituq4b2adjpf8gs59se6liu8vh'	0
+> ns1.del.test.dyndns	A	0	127.0.0.1	3600	NULL	0
+--- End: diff start step.3 ---
+
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check if record is gone again
+--- Start: diff start step.4 ---
+no difference
+--- End: diff start step.4 ---
+
diff --git a/regression-tests/1dyndns-update-add-delete-ds/expected_result.nsec3-optout b/regression-tests/1dyndns-update-add-delete-ds/expected_result.nsec3-optout
@@ -22,3 +22,27 @@ Check if record is gone
 no difference
 --- End: diff start step.2 ---
 
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check delegate and DS added correctly again (other way around)
+--- Start: diff start step.3 ---
+> del.test.dyndns	DS	0	39274 8 2 8e8a8cfb40fd0c30bfa82e53752e1c257dafb7b6206d12b9eda43af3eab2157d	3600	'ott41kituq4b2adjpf8gs59se6liu8vh'	1
+> del.test.dyndns	NS	0	ns1.del.test.dyndns	3600	NULL	0
+> ns1.del.test.dyndns	A	0	127.0.0.1	3600	NULL	0
+--- End: diff start step.3 ---
+
+Answer:
+;; ->>HEADER<<- opcode: UPDATE, status: NOERROR, id: [id]
+;; flags: qr aa; ZONE: 1, PREREQ: 0, UPDATE: 0, ADDITIONAL: 0
+;; ZONE SECTION:
+;test.dyndns.			IN	SOA
+
+Check if record is gone again
+--- Start: diff start step.4 ---
+no difference
+--- End: diff start step.4 ---
+