From: Otto Moerbeek Date: Tue, 12 Apr 2022 08:12:37 +0000 (+0200) Subject: Process review comments: use correct auth and nsname for task X-Git-Tag: rec-4.7.0-beta1^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=378437c9f14c1751a3b13aa1bfb5bf8258619238;p=thirdparty%2Fpdns.git Process review comments: use correct auth and nsname for task --- diff --git a/pdns/rec_control.cc b/pdns/rec_control.cc index 0bcf476e10..2cbd70bc5a 100644 --- a/pdns/rec_control.cc +++ b/pdns/rec_control.cc @@ -97,7 +97,8 @@ int main(int argc, char** argv) "dump-throttlemap", "dump-non-resolving", "dump-saved-parent-ns-sets", - "dump-dot-probe-map"}; + "dump-dot-probe-map", + }; try { initArguments(argc, argv); string sockname = "pdns_recursor"; diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index 8c72a3e531..7773ac9b84 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -1082,8 +1082,7 @@ DoT probes are used to check if an authoritative server's IP address supports Do If the probe determines an IP address supports DoT, the Recursor will use DoT to contact it for subsequent queries. The results of probes are remembered and can be viewed by the ``rec_control dump-dot-probe-map`` command. If the maximum number of pending probes is reached, no probes will be scheduled, even if no DoT status is known for an address. -If the result of a probe is not yet available, the Recursor will contact the authoritative server in the regular way, -unless an authoritative server is configured to be contacted over DoT always using :ref:`setting-dot-to-auth-names`. +If the result of a probe is not yet available, the Recursor will contact the authoritative server in the regular way, unless an authoritative server is configured to be contacted over DoT always using :ref:`setting-dot-to-auth-names`. In that case no probe will be scheduled. Note:: diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc index b56e404ffb..a4ba8f88ae 100644 --- a/pdns/recursordist/rec-taskqueue.cc +++ b/pdns/recursordist/rec-taskqueue.cc @@ -173,7 +173,7 @@ static void tryDoT(const struct timeval& now, bool logErrors, const pdns::Resolv bool ex = true; try { log->info(Logr::Warning, "trying DoT"); - bool ok = sr.tryDoT(task.d_qname, QType(task.d_qtype), DNSName("auth"), DNSName("ns"), task.d_ip, now.tv_sec); + bool ok = sr.tryDoT(task.d_qname, QType(task.d_qtype), task.d_nsname, task.d_ip, now.tv_sec); ex = false; log->info(Logr::Warning, "done", "ok", Logging::Loggable(ok)); } @@ -245,7 +245,7 @@ void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline log->error(Logr::Error, "Cannot push task", "qtype unsupported"); return; } - pdns::ResolveTask task{qname, qtype, deadline, true, resolve, {}}; + pdns::ResolveTask task{qname, qtype, deadline, true, resolve, {}, {}}; if (s_taskQueue.lock()->queue.push(std::move(task))) { ++s_almost_expired_tasks.pushed; } @@ -258,7 +258,7 @@ void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t de log->error(Logr::Error, "Cannot push task", "qtype unsupported"); return; } - pdns::ResolveTask task{qname, qtype, deadline, false, resolve, {}}; + pdns::ResolveTask task{qname, qtype, deadline, false, resolve, {}, {}}; auto lock = s_taskQueue.lock(); bool inserted = lock->rateLimitSet.insert(now, task); if (inserted) { @@ -268,7 +268,7 @@ void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t de } } -bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline) +bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline, const DNSName& nsname) { if (SyncRes::isUnsupported(qtype)) { auto log = g_slog->withName("taskq")->withValues("name", Logging::Loggable(qname), "qtype", Logging::Loggable(QType(qtype).toString())); @@ -276,7 +276,7 @@ bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip return false; } - pdns::ResolveTask task{qname, qtype, deadline, false, tryDoT, ip}; + pdns::ResolveTask task{qname, qtype, deadline, false, tryDoT, ip, nsname}; bool pushed = s_taskQueue.lock()->queue.push(std::move(task)); if (pushed) { ++s_almost_expired_tasks.pushed; diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh index 06633b2f08..cb9e71bf85 100644 --- a/pdns/recursordist/rec-taskqueue.hh +++ b/pdns/recursordist/rec-taskqueue.hh @@ -35,7 +35,7 @@ void runTasks(size_t max, bool logErrors); void runTaskOnce(bool logErrors); void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline); void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline); -bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline); +bool pushTryDoTTask(const DNSName& qname, uint16_t qtype, const ComboAddress& ip, time_t deadline, const DNSName& nsname); void taskQueueClear(); pdns::ResolveTask taskQueuePop(); diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh index 8740130f7e..92e3f176d1 100644 --- a/pdns/recursordist/taskqueue.hh +++ b/pdns/recursordist/taskqueue.hh @@ -59,6 +59,8 @@ struct ResolveTask TaskFunction d_func; // IP used by DoT probe tasks ComboAddress d_ip; + // NS name used by DoT probe task + DNSName d_nsname; bool operator<(const ResolveTask& a) const { diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 9f868d0f08..1cfe5aadf3 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -4735,7 +4735,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co } -static void submitTryDotTask(ComboAddress address, const DNSName& auth, time_t now) +static void submitTryDotTask(ComboAddress address, const DNSName& auth, const DNSName nsname, time_t now) { if (address.getPort() == 853) { return; @@ -4762,7 +4762,7 @@ static void submitTryDotTask(ComboAddress address, const DNSName& auth, time_t n } } lock->d_map.modify(it, [=] (DoTStatus& st){ st.d_ttd = now + dotFailWait; }); - bool pushed = pushTryDoTTask(auth, QType::SOA, address, std::numeric_limits::max()); + bool pushed = pushTryDoTTask(auth, QType::SOA, address, std::numeric_limits::max(), nsname); if (pushed) { it->d_status = DoTStatus::Busy; ++lock->d_numBusy; @@ -4784,7 +4784,7 @@ static bool shouldDoDoT(ComboAddress address, time_t now) return false; } -static void updateDoTStatus(ComboAddress address, const DNSName auth, DoTStatus::Status status, time_t time, bool updateBusy = false) +static void updateDoTStatus(ComboAddress address, DoTStatus::Status status, time_t time, bool updateBusy = false) { address.setPort(853); auto lock = s_dotMap.lock(); @@ -4798,17 +4798,18 @@ static void updateDoTStatus(ComboAddress address, const DNSName auth, DoTStatus: } } -bool SyncRes::tryDoT(const DNSName& qname, const QType qtype, const DNSName& auth, const DNSName& nsName, ComboAddress address, time_t now) +bool SyncRes::tryDoT(const DNSName& qname, const QType qtype, const DNSName& nsName, ComboAddress address, time_t now) { LWResult lwr; bool truncated; bool spoofed; boost::optional nm; address.setPort(853); - bool ok = doResolveAtThisIP("", qname, qtype, lwr, nm, auth, false, false, nsName, address, true, true, truncated, spoofed); + // We use the fact that qname equals auth + bool ok = doResolveAtThisIP("", qname, qtype, lwr, nm, qname, false, false, nsName, address, true, true, truncated, spoofed); ok = ok && lwr.d_rcode == RCode::NoError && lwr.d_records.size() > 0; - updateDoTStatus(address, auth, ok ? DoTStatus::Good : DoTStatus::Bad, now + (ok ? dotSuccessWait : dotFailWait), true); + updateDoTStatus(address, ok ? DoTStatus::Good : DoTStatus::Bad, now + (ok ? dotSuccessWait : dotFailWait), true); return ok; } @@ -5354,7 +5355,7 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con bool forceTCP = doDoT; if (!doDoT && s_max_busy_dot_probes > 0) { - submitTryDotTask(*remoteIP, auth, d_now.tv_sec); + submitTryDotTask(*remoteIP, auth, tns->first, d_now.tv_sec); } if (!forceTCP) { gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded, @@ -5369,7 +5370,7 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con if (!gotAnswer) { if (doDoT && s_max_busy_dot_probes > 0) { // This is quite pessimistic... - updateDoTStatus(*remoteIP, auth, DoTStatus::Bad, d_now.tv_sec + dotFailWait); + updateDoTStatus(*remoteIP, DoTStatus::Bad, d_now.tv_sec + dotFailWait); } continue; } @@ -5377,7 +5378,7 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con LOG(prefix<first<<" ("<< remoteIP->toString() <<"), rcode="< 0) { - updateDoTStatus(*remoteIP, auth, DoTStatus::Good, d_now.tv_sec + dotSuccessWait); + updateDoTStatus(*remoteIP, DoTStatus::Good, d_now.tv_sec + dotSuccessWait); } /* // for you IPv6 fanatics :-) if(remoteIP->sin4.sin_family==AF_INET6) diff --git a/pdns/syncres.hh b/pdns/syncres.hh index 2f0de9b969..ef62a591f1 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -398,7 +398,7 @@ public: explicit SyncRes(const struct timeval& now); int beginResolve(const DNSName &qname, QType qtype, QClass qclass, vector&ret, unsigned int depth = 0); - bool tryDoT(const DNSName& qname, QType qtype, const DNSName& auth, const DNSName& nsName, ComboAddress address, time_t); + bool tryDoT(const DNSName& qname, QType qtype, const DNSName& nsName, ComboAddress address, time_t); void setId(int id) {