Skip to content

Commit

Permalink
rec: Fix DNSSEC validation of wildcards expanded onto themselves
Browse files Browse the repository at this point in the history
  • Loading branch information
@rgacogne
rgacogne committed Apr 25, 2019
1 parent 3f675ce commit 78cdf52
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 14 deletions.
30 changes: 21 additions & 9 deletions pdns/syncres.cc
View file
Expand Up @@ -2162,7 +2162,7 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN
}
}

RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, vState& state, bool& needWildcardProof, unsigned int& wildcardLabelsCount, bool rdQuery)
RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool rdQuery)
{
bool wasForwardRecurse = wasForwarded && rdQuery;
tcache_t tcache;
Expand Down Expand Up @@ -2190,7 +2190,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
/* if we have a positive answer synthetized from a wildcard,
we need to store the corresponding NSEC/NSEC3 records proving
that the exact name did not exist in the negative cache */
if(needWildcardProof) {
if(gatherWildcardProof) {
if (nsecTypes.count(rec.d_type)) {
authorityRecs.push_back(std::make_shared<DNSRecord>(rec));
}
Expand All @@ -2208,9 +2208,20 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
count can be lower than the name's label count if it was
synthetized from the wildcard. Note that the difference might
be > 1. */
if (rec.d_name == qname && rrsig->d_labels < labelCount) {
LOG(prefix<<qname<<": RRSIG indicates the name was synthetized from a wildcard, we need a wildcard proof"<<endl);
needWildcardProof = true;
if (rec.d_name == qname && isWildcardExpanded(labelCount, rrsig)) {
gatherWildcardProof = true;
if (!isWildcardExpandedOntoItself(rec.d_name, labelCount, rrsig)) {
/* if we have a wildcard expanded onto itself, we don't need to prove
that the exact name doesn't exist because it actually does.
We still want to gather the corresponding NSEC/NSEC3 records
to pass them to our client in case it wants to validate by itself.
*/
LOG(prefix<<qname<<": RRSIG indicates the name was synthetized from a wildcard, we need a wildcard proof"<<endl);
needWildcardProof = true;
}
else {
LOG(prefix<<qname<<": RRSIG indicates the name was synthetized from a wildcard expanded onto itself, we need to gather wildcard proof"<<endl);
}
wildcardLabelsCount = rrsig->d_labels;
}

Expand Down Expand Up @@ -2483,7 +2494,7 @@ dState SyncRes::getDenialValidationState(const NegCache::NegCacheEntry& ne, cons
return getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned, expectedState == NXQTYPE);
}

bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const unsigned int wildcardLabelsCount)
bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherWildcardProof, const unsigned int wildcardLabelsCount)
{
bool done = false;

Expand Down Expand Up @@ -2554,7 +2565,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
/* if we have a positive answer synthetized from a wildcard, we need to
return the corresponding NSEC/NSEC3 records from the AUTHORITY section
proving that the exact name did not exist */
else if(needWildcardProof && (rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::AUTHORITY) {
else if(gatherWildcardProof && (rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::AUTHORITY) {
ret.push_back(rec); // enjoy your DNSSEC
}
// for ANY answers we *must* have an authoritative answer, unless we are forwarding recursively
Expand Down Expand Up @@ -2857,8 +2868,9 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
}

bool needWildcardProof = false;
bool gatherWildcardProof = false;
unsigned int wildcardLabelsCount;
*rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, wildcardLabelsCount, sendRDQuery);
*rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount, sendRDQuery);
if (*rcode != RCode::NoError) {
return true;
}
Expand All @@ -2870,7 +2882,7 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
DNSName newauth;
DNSName newtarget;

bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, wildcardLabelsCount);
bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount);

if(done){
LOG(prefix<<qname<<": status=got results, this level of recursion done"<<endl);
Expand Down
4 changes: 2 additions & 2 deletions pdns/syncres.hh
View file
Expand Up @@ -789,8 +789,8 @@ private:
vector<ComboAddress> retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<DNSName >::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<DNSName >& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet, bool cacheOnly);

void sanitizeRecords(const std::string& prefix, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, bool rdQuery);
RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask>, vState& state, bool& needWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery);
bool processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const unsigned int wildcardLabelsCount);
RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask>, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery);
bool processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherwildcardProof, const unsigned int wildcardLabelsCount);

bool doSpecialNamesResolve(const DNSName &qname, const QType &qtype, const uint16_t qclass, vector<DNSRecord> &ret);

Expand Down
36 changes: 33 additions & 3 deletions pdns/validate.cc
View file
Expand Up @@ -112,6 +112,15 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>&
Labels field of the covering RRSIG RR, then the RRset and its
covering RRSIG RR were created as a result of wildcard expansion."
*/
bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign)
{
if (sign && sign->d_labels < labelCount) {
return true;
}

return false;
}

static bool isWildcardExpanded(const DNSName& owner, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures)
{
if (signatures.empty()) {
Expand All @@ -120,13 +129,29 @@ static bool isWildcardExpanded(const DNSName& owner, const std::vector<std::shar

const auto& sign = signatures.at(0);
unsigned int labelsCount = owner.countLabels();
if (sign && sign->d_labels < labelsCount) {
return isWildcardExpanded(labelsCount, sign);
}

bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign)
{
if (owner.isWildcard() && (labelCount - 1) == sign->d_labels) {
/* this is a wildcard alright, but it has not been expanded */
return true;
}

return false;
}

static bool isWildcardExpandedOntoItself(const DNSName& owner, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures)
{
if (signatures.empty()) {
return false;
}

const auto& sign = signatures.at(0);
unsigned int labelsCount = owner.countLabels();
return isWildcardExpandedOntoItself(owner, labelsCount, sign);
}

/* if this is a wildcard NSEC, the owner name has been modified
to match the name. Make sure we use the original '*' form. */
static DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures)
Expand Down Expand Up @@ -381,7 +406,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
/* we know that the name exists (but this qtype doesn't) so except
if the answer was generated by a wildcard expansion, no wildcard
could have matched (rfc4035 section 5.4 bullet 1) */
if (!isWildcardExpanded(owner, v.second.signatures)) {
if (!isWildcardExpanded(owner, v.second.signatures) || isWildcardExpandedOntoItself(owner, v.second.signatures)) {
needWildcardProof = false;
}

Expand All @@ -395,6 +420,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16

/* check if the whole NAME is denied existing */
if(isCoveredByNSEC(qname, owner, nsec->d_next)) {
LOG(qname<<" is covered ");
/* if the name is an ENT and we received a NODATA answer,
we are fine with a NSEC proving that the name does not exist. */
if (wantsNoDataProof && nsecProvesENT(qname, owner, nsec->d_next)) {
Expand All @@ -403,15 +429,19 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
}

if (!needWildcardProof) {
LOG("and we did not need a wildcard proof"<<endl);
return NXDOMAIN;
}

LOG("but we do need a wildcard proof so ");
if (wantsNoDataProof) {
LOG("looking for NODATA proof"<<endl);
if (provesNoDataWildCard(qname, qtype, validrrsets)) {
return NXQTYPE;
}
}
else {
LOG("looking for NO wildcard proof"<<endl);
if (provesNoWildCard(qname, qtype, validrrsets)) {
return NXDOMAIN;
}
Expand Down
2 changes: 2 additions & 0 deletions pdns/validate.hh
View file
Expand Up @@ -77,3 +77,5 @@ bool isSupportedDS(const DSRecordContent& ds);
DNSName getSigner(const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures);
bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>& dsrecords);
bool isRRSIGNotExpired(const time_t now, const shared_ptr<RRSIGRecordContent> sig);
bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign);
bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign);

0 comments on commit 78cdf52

Please sign in to comment.