From 125058a0836c02844444d85f1fdfcb795a4274ab Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Thu, 28 Apr 2016 14:33:16 +0200 Subject: [PATCH] recursor: Correctly validate wildcard RRSIGs --- pdns/dnssecinfra.cc | 35 +++++++++++++++++++++++++++++++++-- pdns/dnssecinfra.hh | 2 +- pdns/validate.cc | 2 +- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc index a7bebe03fe..859dc824db 100644 --- a/pdns/dnssecinfra.cc +++ b/pdns/dnssecinfra.cc @@ -308,7 +308,20 @@ bool sharedDNSSECCompare(const shared_ptr& a, const shared_ptr return a->serialize(DNSName("."), true, true) < b->serialize(DNSName("."), true, true); } -string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, vector >& 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 >& 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(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& 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 bd95b439b0..9d5cdd82b9 100644 --- a/pdns/dnssecinfra.hh +++ b/pdns/dnssecinfra.hh @@ -110,7 +110,7 @@ struct CanonicalCompare: public std::binary_function }; bool sharedDNSSECCompare(const std::shared_ptr& a, const shared_ptr& b); -string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, std::vector >& signRecords); +string getMessageForRRSET(const DNSName& qname, const RRSIGRecordContent& rrc, std::vector >& 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 7e223a694e..e479e7c3a1 100644 --- 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; -- 2.47.2