recursor: Correctly validate wildcard RRSIGs

diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc
index a7bebe03fe6ffad0c8dfa06829231f89d55bf2ac..859dc824db00912d008eae828fc67d14856b2ee7 100644 (file)
--- a/pdns/dnssecinfra.cc
+++ b/pdns/dnssecinfra.cc
@@ -308,7 +308,20 @@ bool sharedDNSSECCompare(const shared_ptr<DNSRecordContent>& a, const shared_ptr
   return a->serialize(DNSName("."), true, true) < b->serialize(DNSName("."), true, true);
 }
 
-string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, vector<shared_ptr<DNSRecordContent> >& signRecords) 
+/**
+ * Returns the string that should be hashed to create/verify the RRSIG content
+ *
+ * @param qname               DNSName of the RRSIG's owner name.
+ * @param rrc                 The RRSIGRecordContent we take the Type Covered and
+ *                            original TTL fields from.
+ * @param signRecords         A vector of DNSRecordContent shared_ptr's that are covered
+ *                            by the RRSIG, where we get the RDATA from.
+ * @param processRRSIGLabels  A boolean to trigger processing the RRSIG's "Labels"
+ *                            field. This is usually only needed for validation
+ *                            purposes, as the authoritative server correctly
+ *                            sets qname to the wildcard.
+ */
+string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, vector<shared_ptr<DNSRecordContent> >& signRecords, bool processRRSIGLabels)
 {
   sort(signRecords.begin(), signRecords.end(), sharedDNSSECCompare);
 
@@ -316,8 +329,26 @@ string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, v
   toHash.append(const_cast<RRSIGRecordContent&>(rrc).serialize(DNSName("."), true, true));
   toHash.resize(toHash.size() - rrc.d_signature.length()); // chop off the end, don't sign the signature!
 
+  string nameToHash(qname.toDNSStringLC());
+
+  if (processRRSIGLabels) {
+    unsigned int rrsig_labels = rrc.d_labels;
+    unsigned int fqdn_labels = qname.countLabels();
+
+    if (rrsig_labels < fqdn_labels) {
+      DNSName choppedQname(qname);
+      while (choppedQname.countLabels() > rrsig_labels)
+        choppedQname.chopOff();
+      nameToHash = "\x01*" + choppedQname.toDNSStringLC();
+    } else if (rrsig_labels > fqdn_labels) {
+      // The RRSIG Labels field is a lie (or the qname is wrong) and the RRSIG
+      // can never be valid
+      return "";
+    }
+  }
+
   for(shared_ptr<DNSRecordContent>& add :  signRecords) {
-    toHash.append(qname.toDNSStringLC()); 
+    toHash.append(nameToHash);
     uint16_t tmp=htons(rrc.d_type);
     toHash.append((char*)&tmp, 2);
     tmp=htons(1); // class

diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh
index bd95b439b07c2a3f3cf9b25408e57a4ed73c2c0d..9d5cdd82b9d146ccf68311d528fe911a86f2a2d9 100644 (file)
--- a/pdns/dnssecinfra.hh
+++ b/pdns/dnssecinfra.hh
@@ -110,7 +110,7 @@ struct CanonicalCompare: public std::binary_function<string, string, bool>
 };
 
 bool sharedDNSSECCompare(const std::shared_ptr<DNSRecordContent>& a, const shared_ptr<DNSRecordContent>& b);
-string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, std::vector<std::shared_ptr<DNSRecordContent> >& signRecords);
+string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, std::vector<std::shared_ptr<DNSRecordContent> >& signRecords, bool processRRSIGLabels = false);
 
 DSRecordContent makeDSFromDNSKey(const DNSName& qname, const DNSKEYRecordContent& drc, int digest=1);
 

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 7e223a694e244b4e06f565110d88d131d8d05d73..e479e7c3a108dc7cee3498c9fe9855e5664e336e 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -97,7 +97,7 @@ void validateWithKeySet(const cspmap_t& rrsets, cspmap_t& validated, const keyse
        continue;
       }
       
-      string msg=getMessageForRRSET(i->first.first, *signature, toSign);
+      string msg=getMessageForRRSET(i->first.first, *signature, toSign, true);
       auto r = getByTag(keys,signature->d_tag); // FIXME: also take algorithm into account? right now we wrongly validate unknownalgorithm.bad-dnssec.wb.sidnlabs.nl
       for(const auto& l : r) {
        bool isValid = false;