[pull] master from PowerDNS:master

.github/workflows/build-docker-images.yml

@@ -178,7 +178,7 @@ jobs:
       IMAGE_NAME: ${{ secrets.DOCKERHUB_ORGANIZATION_NAME }}/${{ inputs.image-name }}
     steps:
       - name: Install cosign
-        uses: sigstore/cosign-installer@v4.1.1
+        uses: sigstore/cosign-installer@v4.1.2
       - name: Download digests
         uses: actions/download-artifact@v8
         with:

docs/lua-records/index.rst

@@ -216,7 +216,8 @@ they will not function, and will in fact leak the content of the LUA records.
 .. note::
   Under NO circumstances serve LUA records from zones from untrusted sources!
   LUA records will be able to bring down your system and possible take over
-  control of it. Use TSIG on AXFR even from trusted sources!
+  control of it. Use TSIG on AXFR even from trusted sources, and only
+  enable :ref:`setting-enable-lua-record-updates` if needed!
 
 LUA records can be DNSSEC signed, but because they are dynamic, it is not
 possible to combine pre-signed DNSSEC zone and LUA records. In other words,

docs/settings.rst

@@ -796,6 +796,19 @@ Globally enable the :doc:`LUA records <lua-records/index>` feature.
 
 To use shared LUA states, set this to ``shared``, see :ref:`lua-records-shared-state`.
 
+.. _setting-enable-lua-record-updates:
+
+``enable-lua-record-updates``
+-----------------------------
+
+.. versionadded:: 5.1.0
+
+-  Boolean
+-  Default: no
+
+Allow updating :doc:`LUA records <lua-records/index>` as part of AXFR/IXFR,
+DNS Update or API operations.
+
 .. _setting-entropy-source:
 
 ``entropy-source``

docs/upgrading.rst

@@ -49,6 +49,16 @@ If you are using an older version, the old query can be restored using::
 but it is advised to upgrade to a supported version of PostgreSQL whenever
 possible.
 
+LUA record updates no longer allowed by default
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Modifications of :doc:`LUA records <lua-records/index>`, either from AXFR/IXFR,
+DNS Update, or the API, are now only allowed if the new
+:ref:`setting-enable-lua-record-updates` configuration setting is set to
+``yes``.
+Its default value being ``no``, a configuration update will be
+necessary when upgrading to 5.1 in order to allow such updates.
+
 4.9.0 to 5.0.0
 --------------
 

pdns/auth-main.cc

@@ -323,6 +323,7 @@ static void declareArguments()
   ::arg().setSwitch("8bit-dns", "Allow 8bit dns queries") = "no";
 #ifdef HAVE_LUA_RECORDS
   ::arg().setSwitch("enable-lua-records", "Process Lua records for all zones (metadata overrides this)") = "no";
+  ::arg().setSwitch("enable-lua-record-updates", "Allow updates to Lua records") = "no";
   ::arg().setSwitch("lua-records-insert-whitespace", "Insert whitespace when combining Lua chunks") = "no";
   ::arg().set("lua-records-exec-limit", "Lua records scripts execution limit (instructions count). Values <= 0 mean no limit") = "1000";
   ::arg().set("lua-health-checks-expire-delay", "Stops doing health checks after the record hasn't been used for that delay (in seconds)") = "3600";

pdns/auth-secondarycommunicator.cc

@@ -521,6 +521,36 @@ void CommunicatorClass::ixfrSuck(const TSIGTriplet& tsig, const ComboAddress& la
     ctx.numDeltas = deltas.size();
     //    cout<<"Got "<<deltas.size()<<" deltas from serial "<<ctx.domain.serial<<", applying.."<<endl;
 
+    // Do not perform Lua records updates if not allowed to.
+    if (!::arg().mustDo("enable-lua-record-updates")) {
+      bool foundLua{false};
+      for (const auto& d : deltas) { // NOLINT(readability-identifier-length)
+        for (const auto& rec : d.first) {
+          if (rec.d_type == QType::LUA) {
+            foundLua = true;
+            break;
+          }
+        }
+        if (foundLua) {
+          break;
+        }
+        for (const auto& rec : d.second) {
+          if (rec.d_type == QType::LUA) {
+            foundLua = true;
+            break;
+          }
+        }
+        if (foundLua) {
+          break;
+        }
+      }
+      if (foundLua) {
+        SLOG(g_log << Logger::Warning << ctx.logPrefix << "refused as it contains Lua record updates" << endl,
+             ctx.slog->info(Logr::Warning, "IXFR: refused as it contains Lua record updates"));
+        return;
+      }
+    }
+
     for (const auto& d : deltas) { // NOLINT(readability-identifier-length)
       const auto& remove = d.first;
       const auto& add = d.second;
@@ -583,7 +613,6 @@ void CommunicatorClass::ixfrSuck(const TSIGTriplet& tsig, const ComboAddress& la
 
           replacement.emplace_back(std::move(rr));
         }
-
         ctx.domain.backend->replaceRRSet(ctx.domain.id, g.first.first.operator const DNSName&() + ctx.domain.zone.operator const DNSName&(), QType(g.first.second), replacement);
       }
       ctx.domain.backend->commitTransaction();
@@ -929,6 +958,16 @@ void CommunicatorClass::suck(const ZoneName& domain, const ComboAddress& remote,
       }
     }
 
+    // Do not perform Lua records updates if not allowed to.
+    if (!::arg().mustDo("enable-lua-record-updates")) {
+      for (DNSResourceRecord& drr : rrs) {
+        if (drr.qtype.getCode() == QType::LUA) {
+          SLOG(g_log << Logger::Warning << ctx.logPrefix << "refused as it contains Lua record updates" << endl,
+               ctx.slog->info(Logr::Warning, "AXFR: refused as it contains Lua record updates"));
+          return;
+        }
+      }
+    }
     transaction = ctx.domain.backend->startTransaction(domain, ctx.domain.id);
     SLOG(g_log << Logger::Info << ctx.logPrefix << "storage transaction started" << endl,
          ctx.slog->info(Logr::Info, "AXFR: storage transaction started"));

pdns/dnsdistdist/Makefile.am

@@ -420,6 +420,7 @@ testrunner_SOURCES = \
 	test-dnsdist-ipcrypt2_cc.cc \
 	test-dnsdist-lua-ffi.cc \
 	test-dnsdist-opentelemetry_cc.cc \
+	test-dnsdist-session-cache.cc \
 	test-dnsdist-xsk.cc \
 	test-dnsdist_cc.cc \
 	test-dnsdistasync.cc \

pdns/dnsdistdist/bpf-filter.cc

@@ -59,21 +59,20 @@ static int bpf_load_pinned_map(const std::string& path)
 
 static void bpf_check_map_sizes(int descriptor, uint32_t expectedKeySize, uint32_t expectedValueSize)
 {
-  struct bpf_map_info info;
-  uint32_t info_len = sizeof(info);
+  bpf_map_info info{};
   memset(&info, 0, sizeof(info));
 
-  union bpf_attr attr;
+  bpf_attr attr{};
   memset(&attr, 0, sizeof(attr));
   attr.info.bpf_fd = descriptor;
-  attr.info.info_len = info_len;
+  attr.info.info_len = sizeof(info);
   attr.info.info = ptr_to_u64(&info);
 
   int err = syscall(SYS_bpf, BPF_OBJ_GET_INFO_BY_FD, &attr, sizeof(attr));
   if (err != 0) {
     throw std::runtime_error("Error checking the size of eBPF map: " + stringerror());
   }
-  if (info_len != sizeof(info)) {
+  if (attr.info.info_len != sizeof(info)) {
     throw std::runtime_error("Error checking the size of eBPF map: invalid info size returned");
   }
   if (info.key_size != expectedKeySize) {
@@ -588,16 +587,20 @@ void BPFFilter::addRangeRule(const Netmask& addr, bool force, BPFFilter::MatchAc
       throw std::runtime_error("Table full when trying to add this rule: " + addr.toString());
     }
 
+    bool entryExisted = false;
     res = bpf_lookup_elem(map.d_fd.getHandle(), &key, &value);
     if (((res != -1 && value.action == action) || (res == -1 && action == BPFFilter::MatchAction::Pass)) && !force) {
       throw std::runtime_error("Trying to add a useless rule: " + addr.toString());
     }
+    if (res != -1) {
+      entryExisted = true;
+    }
 
     value.counter = 0;
     value.action = action;
 
     res = bpf_update_elem(map.d_fd.getHandle(), &key, &value, force ? BPF_ANY : BPF_NOEXIST);
-    if (res == 0) {
+    if (res == 0 && !entryExisted) {
       ++map.d_count;
     }
   }
@@ -613,16 +616,20 @@ void BPFFilter::addRangeRule(const Netmask& addr, bool force, BPFFilter::MatchAc
       throw std::runtime_error("Table full when trying to add this rule: " + addr.toString());
     }
 
+    bool entryExisted = false;
     res = bpf_lookup_elem(map.d_fd.getHandle(), &key, &value);
     if (((res != -1 && value.action == action) || (res == -1 && action == BPFFilter::MatchAction::Pass)) && !force) {
       throw std::runtime_error("Trying to add a useless rule: " + addr.toString());
     }
+    if (res != -1) {
+      entryExisted = true;
+    }
 
     value.counter = 0;
     value.action = action;
 
     res = bpf_update_elem(map.d_fd.getHandle(), &key, &value, BPF_NOEXIST);
-    if (res == 0) {
+    if (res == 0 && !entryExisted) {
       map.d_count++;
     }
   }
@@ -726,7 +733,7 @@ void BPFFilter::block(const DNSName& qname, BPFFilter::MatchAction action, uint1
 
 void BPFFilter::unblock(const DNSName& qname, uint16_t qtype)
 {
-  QNameAndQTypeKey key;
+  QNameAndQTypeKey key{};
   memset(&key, 0, sizeof(key));
   std::string keyStr = qname.toDNSStringLC();
 
@@ -778,7 +785,7 @@ std::vector<std::pair<ComboAddress, uint64_t>> BPFFilter::getAddrStats()
 
   {
     auto& map = maps->d_v4;
-    int res = bpf_get_next_key(map.d_fd.getHandle(), &v4Key, &nextV4Key);
+    int res = bpf_get_next_key(map.d_fd.getHandle(), nullptr, &nextV4Key);
 
     while (res == 0) {
       v4Key = nextV4Key;
@@ -793,7 +800,7 @@ std::vector<std::pair<ComboAddress, uint64_t>> BPFFilter::getAddrStats()
 
   {
     auto& map = maps->d_v6;
-    int res = bpf_get_next_key(map.d_fd.getHandle(), v6Key.data(), nextV6Key.data());
+    int res = bpf_get_next_key(map.d_fd.getHandle(), nullptr, nextV6Key.data());
 
     while (res == 0) {
       if (bpf_lookup_elem(map.d_fd.getHandle(), nextV6Key.data(), &value) == 0) {
@@ -811,16 +818,16 @@ std::vector<std::pair<ComboAddress, uint64_t>> BPFFilter::getAddrStats()
 
 std::vector<std::pair<Netmask, CounterAndActionValue>> BPFFilter::getRangeRule()
 {
-  CIDR4 cidr4[2];
-  CIDR6 cidr6[2];
+  CIDR4 cidr4{};
+  CIDR6 cidr6{};
   std::vector<std::pair<Netmask, CounterAndActionValue>> result;
 
-  sockaddr_in v4Addr;
-  sockaddr_in6 v6Addr;
+  sockaddr_in v4Addr{};
+  sockaddr_in6 v6Addr{};
   CounterAndActionValue value;
 
-  memset(cidr4, 0, sizeof(cidr4));
-  memset(cidr6, 0, sizeof(cidr6));
+  memset(&cidr4, 0, sizeof(cidr4));
+  memset(&cidr6, 0, sizeof(cidr6));
   memset(&v4Addr, 0, sizeof(v4Addr));
   memset(&v6Addr, 0, sizeof(v6Addr));
   v4Addr.sin_family = AF_INET;
@@ -829,27 +836,27 @@ std::vector<std::pair<Netmask, CounterAndActionValue>> BPFFilter::getRangeRule()
   result.reserve(maps->d_cidr4.d_count + maps->d_cidr6.d_count);
   {
     auto& map = maps->d_cidr4;
-    int res = bpf_get_next_key(map.d_fd.getHandle(), &cidr4[0], &cidr4[1]);
+    int res = bpf_get_next_key(map.d_fd.getHandle(), nullptr, &cidr4);
     while (res == 0) {
-      if (bpf_lookup_elem(map.d_fd.getHandle(), &cidr4[1], &value) == 0) {
-        v4Addr.sin_addr.s_addr = cidr4[1].addr.s_addr;
-        result.emplace_back(Netmask(&v4Addr, cidr4[1].cidr), value);
+      if (bpf_lookup_elem(map.d_fd.getHandle(), &cidr4, &value) == 0) {
+        v4Addr.sin_addr.s_addr = cidr4.addr.s_addr;
+        result.emplace_back(Netmask(&v4Addr, cidr4.cidr), value);
       }
 
-      res = bpf_get_next_key(map.d_fd.getHandle(), &cidr4[1], &cidr4[1]);
+      res = bpf_get_next_key(map.d_fd.getHandle(), &cidr4, &cidr4);
     }
   }
 
   {
     auto& map = maps->d_cidr6;
-    int res = bpf_get_next_key(map.d_fd.getHandle(), &cidr6[0], &cidr6[1]);
+    int res = bpf_get_next_key(map.d_fd.getHandle(), nullptr, &cidr6);
     while (res == 0) {
-      if (bpf_lookup_elem(map.d_fd.getHandle(), &cidr6[1], &value) == 0) {
-        v6Addr.sin6_addr = cidr6[1].addr;
-        result.emplace_back(Netmask(&v6Addr, cidr6[1].cidr), value);
+      if (bpf_lookup_elem(map.d_fd.getHandle(), &cidr6, &value) == 0) {
+        v6Addr.sin6_addr = cidr6.addr;
+        result.emplace_back(Netmask(&v6Addr, cidr6.cidr), value);
       }
 
-      res = bpf_get_next_key(map.d_fd.getHandle(), &cidr6[1], &cidr6[1]);
+      res = bpf_get_next_key(map.d_fd.getHandle(), &cidr6, &cidr6);
     }
   }
   return result;
@@ -860,40 +867,39 @@ std::vector<std::tuple<DNSName, uint16_t, uint64_t>> BPFFilter::getQNameStats()
   std::vector<std::tuple<DNSName, uint16_t, uint64_t>> result;
 
   if (d_mapFormat == MapFormat::Legacy) {
-    QNameKey key = {{0}};
     QNameKey nextKey = {{0}};
     QNameValue value;
 
     auto maps = d_maps.lock();
     auto& map = maps->d_qnames;
     result.reserve(map.d_count);
-    int res = bpf_get_next_key(map.d_fd.getHandle(), &key, &nextKey);
+    int res = bpf_get_next_key(map.d_fd.getHandle(), nullptr, &nextKey);
 
     while (res == 0) {
       if (bpf_lookup_elem(map.d_fd.getHandle(), &nextKey, &value) == 0) {
         nextKey.qname[sizeof(nextKey.qname) - 1] = '\0';
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
         result.emplace_back(DNSName(reinterpret_cast<const char*>(nextKey.qname), sizeof(nextKey.qname), 0, false), value.qtype, value.counter);
       }
 
       res = bpf_get_next_key(map.d_fd.getHandle(), &nextKey, &nextKey);
     }
   }
   else {
-    QNameAndQTypeKey key;
-    QNameAndQTypeKey nextKey;
-    memset(&key, 0, sizeof(key));
+    QNameAndQTypeKey nextKey{};
     memset(&nextKey, 0, sizeof(nextKey));
     CounterAndActionValue value;
 
     auto maps = d_maps.lock();
     auto& map = maps->d_qnames;
     result.reserve(map.d_count);
-    int res = bpf_get_next_key(map.d_fd.getHandle(), &key, &nextKey);
+    int res = bpf_get_next_key(map.d_fd.getHandle(), nullptr, &nextKey);
 
     while (res == 0) {
       if (bpf_lookup_elem(map.d_fd.getHandle(), &nextKey, &value) == 0) {
         nextKey.qname[sizeof(nextKey.qname) - 1] = '\0';
-        result.emplace_back(DNSName(reinterpret_cast<const char*>(nextKey.qname), sizeof(nextKey.qname), 0, false), key.qtype, value.counter);
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+        result.emplace_back(DNSName(reinterpret_cast<const char*>(nextKey.qname), sizeof(nextKey.qname), 0, false), nextKey.qtype, value.counter);
       }
 
       res = bpf_get_next_key(map.d_fd.getHandle(), &nextKey, &nextKey);

pdns/dnsdistdist/dnsdist-actions-factory.cc

@@ -296,12 +296,11 @@ void TeeAction::worker()
       continue;
     }
     res = recv(d_socket.getHandle(), packet.data(), packet.size(), 0);
-    if (static_cast<size_t>(res) <= sizeof(struct dnsheader)) {
+    if (res < 0 || static_cast<size_t>(res) <= sizeof(struct dnsheader)) {
       d_recverrors++;
+      continue;
     }
-    else {
-      d_responses++;
-    }
+    d_responses++;
 
     // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well
     if (dnsheader->rcode == RCode::NoError) {

pdns/dnsdistdist/dnsdist-ecs.cc

@@ -257,7 +257,12 @@ bool slowRewriteEDNSOptionInQueryWithRecords(const PacketBuffer& initialPacket,
   }
 
   if (ednsAdded) {
-    packetWriter.addOpt(dnsdist::configuration::s_EdnsUDPPayloadSize, 0, 0, {{optionToReplace, std::string(&newOptionContent.at(EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE), newOptionContent.size() - (EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE))}}, 0);
+    if (newOptionContent.size() == EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE) {
+      packetWriter.addOpt(dnsdist::configuration::s_EdnsUDPPayloadSize, 0, 0, {{optionToReplace, std::string()}}, 0);
+    }
+    else {
+      packetWriter.addOpt(dnsdist::configuration::s_EdnsUDPPayloadSize, 0, 0, {{optionToReplace, std::string(&newOptionContent.at(EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE), newOptionContent.size() - (EDNS_OPTION_CODE_SIZE + EDNS_OPTION_LENGTH_SIZE))}}, 0);
+    }
     optionAdded = true;
   }
 
@@ -1157,13 +1162,16 @@ bool setEDNSOption(PacketBuffer& buf, uint16_t ednsCode, const std::string& edns
     return true;
   }
 
-  if (generateOptRR(optRData, buf, maximumSize, dnsdist::configuration::s_EdnsUDPPayloadSize, 0, false)) {
-    dnsdist::PacketMangling::editDNSHeaderFromPacket(buf, [](dnsheader& header) {
-      header.arcount = htons(1);
-      return true;
-    });
+  if (!generateOptRR(optRData, buf, maximumSize, dnsdist::configuration::s_EdnsUDPPayloadSize, 0, false)) {
+    return false;
   }
 
+  dnsdist::PacketMangling::editDNSHeaderFromPacket(buf, [](dnsheader& header) {
+    header.arcount = htons(1);
+    return true;
+  });
+  ednsAdded = true;
+
   return true;
 }