Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify the checking logic of cname cache, #14918

Closed
wants to merge 3 commits into from
Closed

Modify the checking logic of cname cache, #14918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df45e66
update find cname cache
Dec 3, 2024
365c77c
Format and clang-tidy tweaks, still need to validate the logic
omoerbeek Dec 5, 2024
e1128f9
Better names for a few vars
omoerbeek Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions pdns/recursordist/syncres.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,7 @@ static string resToString(int res)
return res >= 0 ? RCode::to_s(res) : std::to_string(res);
}

int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context) // NOLINT(readability-function-cognitive-complexity)
int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context, bool* doneResolveFromCache) // NOLINT(readability-function-cognitive-complexity)
{
auto prefix = getPrefix(depth);
auto luaconfsLocal = g_luaconfs.getLocal();
Expand Down Expand Up @@ -1733,6 +1733,9 @@ int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord
if (d_appliedPolicy.d_type != DNSFilterEngine::PolicyType::None && (d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::NXDOMAIN || d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::NODATA)) {
ret.clear();
}
if (doneResolveFromCache != nullptr) {
*doneResolveFromCache = true;
}
return res;
}
// Erase unwanted cache lookup preliminary results in the returned records
Expand Down Expand Up @@ -1957,10 +1960,11 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName& qname, const QType qtyp
}
}

bool cnameTargetFromCache = false;
/* When we are looking for a DS, we want to the non-CNAME cache check first
because we can actually have a DS (from the parent zone) AND a CNAME (from
the child zone), and what we really want is the DS */
if (qtype != QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, prefix, res, context, wasAuthZone, wasForwardRecurse, loop == 1)) { // will reroute us if needed
if (qtype != QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, prefix, res, context, wasAuthZone, wasForwardRecurse, loop == 1, &cnameTargetFromCache)) { // will reroute us if needed
d_wasOutOfBand = wasAuthZone;
// Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
// are in QM Step0) we might have a CNAME but not the corresponding target.
Expand All @@ -1986,6 +1990,10 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName& qname, const QType qtyp
}
}
}
if (fromCache != nullptr && cnameTargetFromCache) {
*fromCache = true;
}

return res;
}

Expand All @@ -2009,7 +2017,7 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName& qname, const QType qtyp
}

/* if we have not found a cached DS (or denial of), now is the time to look for a CNAME */
if (qtype == QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, prefix, res, context, wasAuthZone, wasForwardRecurse, loop == 1)) { // will reroute us if needed
if (qtype == QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, prefix, res, context, wasAuthZone, wasForwardRecurse, loop == 1, nullptr)) { // will reroute us if needed
d_wasOutOfBand = wasAuthZone;
// Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
// are in QM Step0) we might have a CNAME but not the corresponding target.
Expand Down Expand Up @@ -2519,7 +2527,7 @@ static pair<bool, unsigned int> scanForCNAMELoop(const DNSName& name, const vect
return {false, numCNames};
}

bool SyncRes::doCNAMECacheCheck(const DNSName& qname, const QType qtype, vector<DNSRecord>& ret, unsigned int depth, const string& prefix, int& res, Context& context, bool wasAuthZone, bool wasForwardRecurse, bool checkForDups) // NOLINT(readability-function-cognitive-complexity)
bool SyncRes::doCNAMECacheCheck(const DNSName& qname, const QType qtype, vector<DNSRecord>& ret, unsigned int depth, const string& prefix, int& res, Context& context, bool wasAuthZone, bool wasForwardRecurse, bool checkForDups, bool* cnameTargetInCache) // NOLINT(readability-function-cognitive-complexity)
{
vector<DNSRecord> cset;
vector<std::shared_ptr<const RRSIGRecordContent>> signatures;
Expand Down Expand Up @@ -2724,10 +2732,17 @@ bool SyncRes::doCNAMECacheCheck(const DNSName& qname, const QType qtype, vector<
Context cnameContext;
// Be aware that going out on the network might be disabled (cache-only), for example because we are in QM Step0,
// so you can't trust that a real lookup will have been made.
res = doResolve(newTarget, qtype, ret, depth + 1, beenthere, cnameContext);
bool fromCache = false;
res = doResolve(newTarget, qtype, ret, depth + 1, beenthere, cnameContext, &fromCache);
LOG(prefix << qname << ": Updating validation state for response to " << qname << " from " << context.state << " with the state from the DNAME/CNAME quest: " << cnameContext.state << endl);
updateValidationState(qname, context.state, cnameContext.state, prefix);

if (fromCache && foundQT == QType::CNAME) {
if (cnameTargetInCache != nullptr) {
*cnameTargetInCache = true;
}
}

return true;
}
}
Expand Down
4 changes: 2 additions & 2 deletions pdns/recursordist/syncres.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,14 +637,14 @@ private:
bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, QType qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool sendRDQuery, bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional<EDNSExtendedError>& extendedError, bool dontThrottle = false);
bool processAnswer(unsigned int depth, const string& prefix, LWResult& lwr, const DNSName& qname, QType qtype, DNSName& auth, bool wasForwarded, const boost::optional<Netmask>& ednsmask, bool sendRDQuery, NsSet& nameservers, std::vector<DNSRecord>& ret, const DNSFilterEngine& dfe, bool* gotNewServers, int* rcode, vState& state, const ComboAddress& remoteIP);

int doResolve(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context);
int doResolve(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context, bool* doneResolveFromCache = nullptr);
int doResolveNoQNameMinimization(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context, bool* fromCache = nullptr, StopAtDelegation* stopAtDelegation = nullptr);
bool doOOBResolve(const AuthDomain& domain, const DNSName& qname, QType qtype, vector<DNSRecord>& ret, int& res);
bool doOOBResolve(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, const string& prefix, int& res);
static bool isRecursiveForwardOrAuth(const DNSName& qname);
static bool isForwardOrAuth(const DNSName& qname);
static domainmap_t::const_iterator getBestAuthZone(DNSName* qname);
bool doCNAMECacheCheck(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, const string& prefix, int& res, Context& context, bool wasAuthZone, bool wasForwardRecurse, bool checkForDups);
bool doCNAMECacheCheck(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, const string& prefix, int& res, Context& context, bool wasAuthZone, bool wasForwardRecurse, bool checkForDups, bool* cnameTargetInCache);
bool doCacheCheck(const DNSName& qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, bool wasForwardRecurse, QType qtype, vector<DNSRecord>& ret, unsigned int depth, const string& prefix, int& res, Context& context);
void getBestNSFromCache(const DNSName& qname, QType qtype, vector<DNSRecord>& bestns, bool* flawedNSSet, unsigned int depth, const string& prefix, set<GetBestNSAnswer>& beenthere, const boost::optional<DNSName>& cutOffDomain = boost::none);
DNSName getBestNSNamesFromCache(const DNSName& qname, QType qtype, NsSet& nsset, bool* flawedNSSet, unsigned int depth, const string& prefix, set<GetBestNSAnswer>& beenthere);
Expand Down
Loading