From: Oliver Chen Date: Thu, 10 Apr 2025 14:00:55 +0000 (+0000) Subject: Document usage of timeout response rule and add defensive checks X-Git-Tag: dnsdist-2.0.0-alpha2~75^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d4a12e8e3ff5c576e986c2bb9257ac205cedda49;p=thirdparty%2Fpdns.git Document usage of timeout response rule and add defensive checks Removed unnecessary packet buffer generation that is no value --- diff --git a/pdns/dnsdistdist/dnsdist-self-answers.cc b/pdns/dnsdistdist/dnsdist-self-answers.cc index 5586b230e7..dd0ff0b119 100644 --- a/pdns/dnsdistdist/dnsdist-self-answers.cc +++ b/pdns/dnsdistdist/dnsdist-self-answers.cc @@ -240,28 +240,4 @@ bool generateAnswerFromRawPacket(DNSQuestion& dnsQuestion, const PacketBuffer& p return true; } -bool generateAnswerForTimeoutQuery(const PacketBuffer& query, PacketBuffer& answer, const DNSName& dnsQName, uint16_t qtype, uint16_t qclass) -{ - auto& qname = dnsQName.getStorage(); - if (query.size() < sizeof(dnsheader) || qname.length() <= 1) { - return false; - } - std::vector qtc = {static_cast(qtype>>8), static_cast(qtype&0xff), static_cast(qclass>>8), static_cast(qclass&0xff)}; - - answer.resize(sizeof(dnsheader) + qname.length() + 4); - memcpy(&answer.at(0), &query.at(0), sizeof(dnsheader)); - memcpy(&answer.at(sizeof(dnsheader)), qname.c_str(), qname.length()); - memcpy(&answer.at(sizeof(dnsheader)+qname.length()), &qtc.at(0), 4); - - dnsdist::PacketMangling::editDNSHeaderFromPacket(answer, [](dnsheader& header) { - header.qr = true; - header.qdcount = htons(1); - header.ancount = 0; - header.nscount = 0; - header.arcount = 0; - return true; - }); - return true; -} - } diff --git a/pdns/dnsdistdist/dnsdist-self-answers.hh b/pdns/dnsdistdist/dnsdist-self-answers.hh index 2c02beb5dd..5703db4648 100644 --- a/pdns/dnsdistdist/dnsdist-self-answers.hh +++ b/pdns/dnsdistdist/dnsdist-self-answers.hh @@ -29,5 +29,4 @@ bool generateAnswerFromCNAME(DNSQuestion& dnsQuestion, const DNSName& cname, con bool generateAnswerFromIPAddresses(DNSQuestion& dnsQuestion, const std::vector& addresses, const ResponseConfig& responseConfig); bool generateAnswerFromRDataEntries(DNSQuestion& dnsQuestion, const std::vector& entries, std::optional typeForAny, const ResponseConfig& responseConfig); bool generateAnswerFromRawPacket(DNSQuestion& dnsQuestion, const PacketBuffer& packet); -bool generateAnswerForTimeoutQuery(const PacketBuffer& query, PacketBuffer& answer, const DNSName& dnsQName, uint16_t qtype, uint16_t qclass); } diff --git a/pdns/dnsdistdist/dnsdist-settings-definitions.yml b/pdns/dnsdistdist/dnsdist-settings-definitions.yml index 86e475ad2c..421f03d232 100644 --- a/pdns/dnsdistdist/dnsdist-settings-definitions.yml +++ b/pdns/dnsdistdist/dnsdist-settings-definitions.yml @@ -146,7 +146,7 @@ global: type: "Vec" default: true skip-serde: true - description: "List of rules executed when a timeout event triggered from timer expiration or I/O error" + description: "List of rules executed when a timeout event triggered from timer expiration or network I/O error. Note that this rule is intent only for an action to restart a timed-out or network I/O failed query." metrics: description: "Metrics-related settings" diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc index c4e57b2fff..9bf8d7d31f 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -456,6 +456,9 @@ bool applyRulesToResponse(const std::vector& return true; break; case DNSResponseAction::Action::ServFail: + if (dnsResponse.getData().size() < sizeof(dnsheader)) { + return false; + } dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsResponse.getMutableData(), [](dnsheader& header) { header.rcode = RCode::ServFail; return true; @@ -463,6 +466,9 @@ bool applyRulesToResponse(const std::vector& return true; break; case DNSResponseAction::Action::Truncate: + if (dnsResponse.getData().size() < sizeof(dnsheader)) { + return false; + } if (!dnsResponse.overTCP()) { dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsResponse.getMutableData(), [](dnsheader& header) { header.tc = true; @@ -1567,16 +1573,8 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ bool handleTimeoutResponseRules(const std::vector& rules, InternalQueryState& ids, const std::shared_ptr& d_ds, const std::shared_ptr& sender) { - if (!ids.d_packet || ids.d_packet->size() < sizeof(struct dnsheader)) { - return false; - } - - PacketBuffer answer; - if (!dnsdist::self_answers::generateAnswerForTimeoutQuery(*ids.d_packet, answer, ids.qname, ids.qtype, ids.qclass)) { - return false; - } - - DNSResponse dnsResponse(ids, answer, d_ds); + PacketBuffer empty; + DNSResponse dnsResponse(ids, empty, d_ds); auto protocol = dnsResponse.getProtocol(); vinfolog("Handling timeout response rules for incoming protocol = %s", protocol.toString()); diff --git a/pdns/dnsdistdist/docs/reference/rules-management.rst b/pdns/dnsdistdist/docs/reference/rules-management.rst index f78358e745..6aa4a9b981 100644 --- a/pdns/dnsdistdist/docs/reference/rules-management.rst +++ b/pdns/dnsdistdist/docs/reference/rules-management.rst @@ -520,7 +520,7 @@ For Rules related to timed out queries: .. versionadded:: 2.0.0 - Add a Rule and Action for timeout triggered from timer expiration or I/O error. + Add a Rule and Action for timeout triggered from timer expiration or network I/O error. Note that this rule is intent only for an action to restart a timed-out or network I/O failed query. :param DNSrule rule: A :class:`DNSRule`, e.g. an :func:`AllRule`, or a compounded bunch of rules using e.g. :func:`AndRule`. :param action: The action to take