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 5586b230e7b75340a939bcc14cb424d19283cebd..dd0ff0b119ec7c7b2e6a779324fbb56790f67220 100644 (file)
--- 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<uint8_t> qtc = {static_cast<uint8_t>(qtype>>8), static_cast<uint8_t>(qtype&0xff), static_cast<uint8_t>(qclass>>8), static_cast<uint8_t>(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 2c02beb5dd5f5ef3922a8e5ab6ee30e7ead40cfd..5703db46488d7baa4985c3bca926569d5b8ef614 100644 (file)
--- 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<ComboAddress>& addresses, const ResponseConfig& responseConfig);
 bool generateAnswerFromRDataEntries(DNSQuestion& dnsQuestion, const std::vector<std::string>& entries, std::optional<uint16_t> 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 86e475ad2caec0e6a021969f08a469dcf8e2ef2a..421f03d232b4e2cfd37280e1c9c2b801f4d0c1ae 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-settings-definitions.yml
+++ b/pdns/dnsdistdist/dnsdist-settings-definitions.yml
@@ -146,7 +146,7 @@ global:
       type: "Vec<ResponseRuleConfiguration>"
       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 c4e57b2fff7969d29a598324b8eda9fb87986b5d..9bf8d7d31f56b067d31c7f16f4419ec051054199 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -456,6 +456,9 @@ bool applyRulesToResponse(const std::vector<dnsdist::rules::ResponseRuleAction>&
         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<dnsdist::rules::ResponseRuleAction>&
         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<dnsdist::rules::ResponseRuleAction>& rules, InternalQueryState& ids, const std::shared_ptr<DownstreamState>& d_ds, const std::shared_ptr<TCPQuerySender>& 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 f78358e745900b1c398cdbd2e13e2366ebfbcd25..6aa4a9b981ad21b94517e75ef5c01a3b59e40259 100644 (file)
--- 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