diff --git a/.github/workflows/build-docker-images.yml b/.github/workflows/build-docker-images.yml index 92888cc68236..98fdfd2dfbe0 100644 --- a/.github/workflows/build-docker-images.yml +++ b/.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: diff --git a/docs/lua-records/index.rst b/docs/lua-records/index.rst index 86b30d5a4404..a141ede79b7d 100644 --- a/docs/lua-records/index.rst +++ b/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, diff --git a/docs/settings.rst b/docs/settings.rst index 21987db1e310..5380cfff6608 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -796,6 +796,19 @@ Globally enable the :doc:`LUA records ` 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 ` as part of AXFR/IXFR, +DNS Update or API operations. + .. _setting-entropy-source: ``entropy-source`` diff --git a/docs/upgrading.rst b/docs/upgrading.rst index 17da15368628..e4416c02cc3e 100644 --- a/docs/upgrading.rst +++ b/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 `, 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 -------------- diff --git a/pdns/auth-main.cc b/pdns/auth-main.cc index 4084740ecc65..cb684a63ff1b 100644 --- a/pdns/auth-main.cc +++ b/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"; diff --git a/pdns/auth-secondarycommunicator.cc b/pdns/auth-secondarycommunicator.cc index c7517ed97899..726134a4eb1f 100644 --- a/pdns/auth-secondarycommunicator.cc +++ b/pdns/auth-secondarycommunicator.cc @@ -521,6 +521,36 @@ void CommunicatorClass::ixfrSuck(const TSIGTriplet& tsig, const ComboAddress& la ctx.numDeltas = deltas.size(); // cout<<"Got "<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")); diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index 9437192ce945..db684e736f6c 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/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 \ diff --git a/pdns/dnsdistdist/bpf-filter.cc b/pdns/dnsdistdist/bpf-filter.cc index b5072cb58e20..e93d0ce30fa4 100644 --- a/pdns/dnsdistdist/bpf-filter.cc +++ b/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> 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> 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> BPFFilter::getAddrStats() std::vector> BPFFilter::getRangeRule() { - CIDR4 cidr4[2]; - CIDR6 cidr6[2]; + CIDR4 cidr4{}; + CIDR6 cidr6{}; std::vector> 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> 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,18 +867,18 @@ std::vector> BPFFilter::getQNameStats() std::vector> 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(nextKey.qname), sizeof(nextKey.qname), 0, false), value.qtype, value.counter); } @@ -879,21 +886,20 @@ std::vector> BPFFilter::getQNameStats() } } 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(nextKey.qname), sizeof(nextKey.qname), 0, false), key.qtype, value.counter); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + result.emplace_back(DNSName(reinterpret_cast(nextKey.qname), sizeof(nextKey.qname), 0, false), nextKey.qtype, value.counter); } res = bpf_get_next_key(map.d_fd.getHandle(), &nextKey, &nextKey); diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.cc b/pdns/dnsdistdist/dnsdist-actions-factory.cc index ded8a537dc83..4ae2be6304c5 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.cc +++ b/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(res) <= sizeof(struct dnsheader)) { + if (res < 0 || static_cast(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) { diff --git a/pdns/dnsdistdist/dnsdist-ecs.cc b/pdns/dnsdistdist/dnsdist-ecs.cc index 9b585127402c..40c1296d38f3 100644 --- a/pdns/dnsdistdist/dnsdist-ecs.cc +++ b/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; } diff --git a/pdns/dnsdistdist/dnsdist-logging.cc b/pdns/dnsdistdist/dnsdist-logging.cc index 5fbe6a47a9c5..1a59ddff9750 100644 --- a/pdns/dnsdistdist/dnsdist-logging.cc +++ b/pdns/dnsdistdist/dnsdist-logging.cc @@ -40,10 +40,8 @@ static const char* convertTime(const timeval& tval, std::array& buffer { auto format = dnsdist::configuration::getImmutableConfiguration().d_structuredLoggingTimeFormat; if (format == dnsdist::configuration::TimeFormat::ISO8601) { - time_t now{}; - time(&now); struct tm localNow{}; - localtime_r(&now, &localNow); + localtime_r(&tval.tv_sec, &localNow); { // strftime is not thread safe, it can access locale information diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings.cc b/pdns/dnsdistdist/dnsdist-lua-bindings.cc index 175304a63ed1..fcb25c3e43e9 100644 --- a/pdns/dnsdistdist/dnsdist-lua-bindings.cc +++ b/pdns/dnsdistdist/dnsdist-lua-bindings.cc @@ -383,7 +383,7 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck) return; } if (dnsdist::configuration::isImmutableConfigurationDone()) { - throw std::runtime_error("setHealthCheckResponseValidator cannot be used at configuration time!"); + throw std::runtime_error("setHealthCheckResponseValidator cannot be used at runtime!"); return; } state->d_config.d_healthCheckResponseValidationCallback = std::move(validator); diff --git a/pdns/dnsdistdist/dnsdist-lua-configuration-items.cc b/pdns/dnsdistdist/dnsdist-lua-configuration-items.cc index f6abc6cab21d..6b0d277f6972 100644 --- a/pdns/dnsdistdist/dnsdist-lua-configuration-items.cc +++ b/pdns/dnsdistdist/dnsdist-lua-configuration-items.cc @@ -162,11 +162,10 @@ static const std::map s {"setMaxTCPReadIOsPerQuery", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPReadIOsPerQuery = newValue; }, std::numeric_limits::max()}}, {"setBanDurationForExceedingMaxReadIOsPerQuery", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpBanDurationForExceedingMaxReadIOsPerQuery = newValue; }, std::numeric_limits::max()}}, {"setBanDurationForExceedingTCPTLSRate", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpBanDurationForExceedingTCPTLSRate = newValue; }, std::numeric_limits::max()}}, - {"setTCPConnectionsOverloadThreshold", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpConnectionsOverloadThreshold = newValue; }, std::numeric_limits::max()}}, {"setTCPConnectionsMaskV4", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpConnectionsMaskV4 = newValue; }, std::numeric_limits::max()}}, {"setTCPConnectionsMaskV6", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpConnectionsMaskV6 = newValue; }, std::numeric_limits::max()}}, {"setTCPConnectionsMaskV4Port", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpConnectionsMaskV4Port = newValue; }, std::numeric_limits::max()}}, - {"setTCPConnectionsOverloadThreshold", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpConnectionsOverloadThreshold = newValue; }, 100}}, + {"setTCPConnectionsOverloadThreshold", {[](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpConnectionsOverloadThreshold = newValue; }, 100U}}, }; static const std::map s_doubleImmutableConfigItems{ diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection.cc b/pdns/dnsdistdist/dnsdist-lua-inspection.cc index 9a793ef30d1a..a01c6f8a6e78 100644 --- a/pdns/dnsdistdist/dnsdist-lua-inspection.cc +++ b/pdns/dnsdistdist/dnsdist-lua-inspection.cc @@ -691,7 +691,7 @@ void setupLuaInspection(LuaContext& luaCtx) iter->second++; } else { - histo.rbegin()++; + histo.rbegin()->second++; } totlat += entry.usec; } diff --git a/pdns/dnsdistdist/dnsdist-lua-network.cc b/pdns/dnsdistdist/dnsdist-lua-network.cc index 95af07beb87f..2802e4e3e7ef 100644 --- a/pdns/dnsdistdist/dnsdist-lua-network.cc +++ b/pdns/dnsdistdist/dnsdist-lua-network.cc @@ -161,12 +161,9 @@ void NetworkListener::runOnce(timeval& now, uint32_t timeout) runOnce(*d_data, now, timeout); } -void NetworkListener::mainThread(std::shared_ptr& dataArg) +// NOLINTNEXTLINE(performance-unnecessary-value-param): take our own copy of the shared_ptr so it's still alive if the NetworkListener object gets destroyed while we are still running +void NetworkListener::mainThread(std::shared_ptr data) { - /* take our own copy of the shared_ptr so it's still alive if the NetworkListener object - gets destroyed while we are still running */ - // NOLINTNEXTLINE(performance-unnecessary-copy-initialization): we really need a copy here, or we end up with use-after-free as explained above - auto data = dataArg; setThreadName("dnsdist/lua-net"); timeval now{}; @@ -177,8 +174,8 @@ void NetworkListener::mainThread(std::shared_ptr& dataArg) void NetworkListener::start() { - std::thread main = std::thread([this] { - mainThread(d_data); + std::thread main = std::thread([data = d_data]() mutable { + mainThread(std::move(data)); }); main.detach(); } diff --git a/pdns/dnsdistdist/dnsdist-lua-network.hh b/pdns/dnsdistdist/dnsdist-lua-network.hh index 3cd2f0844271..85db8277ee3e 100644 --- a/pdns/dnsdistdist/dnsdist-lua-network.hh +++ b/pdns/dnsdistdist/dnsdist-lua-network.hh @@ -58,7 +58,7 @@ private: }; static void readCB(int desc, FDMultiplexer::funcparam_t& param); - static void mainThread(std::shared_ptr& data); + static void mainThread(std::shared_ptr data); static void runOnce(ListenerData& data, timeval& now, uint32_t timeout); struct CBData diff --git a/pdns/dnsdistdist/dnsdist-mac-address.cc b/pdns/dnsdistdist/dnsdist-mac-address.cc index eed10b3f735b..c568bd8558b1 100644 --- a/pdns/dnsdistdist/dnsdist-mac-address.cc +++ b/pdns/dnsdistdist/dnsdist-mac-address.cc @@ -39,7 +39,7 @@ int MacAddressesCache::get(const ComboAddress& ca, unsigned char* dest, size_t d { auto cache = s_cache.read_lock(); for (const auto& entry : *cache) { - if (entry.ttd >= now && compare(entry.ca, ca) == true) { + if (entry.ttd >= now && compare(entry.ca, ca)) { if (!entry.found) { // negative entry return ENOENT; @@ -50,8 +50,12 @@ int MacAddressesCache::get(const ComboAddress& ca, unsigned char* dest, size_t d } } + // in theory we might encounter a MAC address smaller than 6 bytes, + // don't enter garbage data into our cache + memset(dest, 0, destLen); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) auto res = getMACAddress(ca, reinterpret_cast(dest), destLen); - Entry entry; + Entry entry{}; entry.ca = ca; if (res == 0) { memcpy(entry.mac.data(), dest, entry.mac.size()); diff --git a/pdns/dnsdistdist/dnsdist-opentelemetry.cc b/pdns/dnsdistdist/dnsdist-opentelemetry.cc index ee0c84f8c993..820dc540bc3e 100644 --- a/pdns/dnsdistdist/dnsdist-opentelemetry.cc +++ b/pdns/dnsdistdist/dnsdist-opentelemetry.cc @@ -175,11 +175,14 @@ void Tracer::closeSpan([[maybe_unused]] const SpanID& spanID) // Only closers are allowed, so this can never happen assert(!data->d_spanIDStack.empty()); - // Preferebly, we'd use d_spanIDStack.pop() after verifing that that back() is the correct spanID. + // Preferably, we'd use d_spanIDStack.pop() after verifing that that back() is the correct spanID. // It turns out that due to dnsdist's multi-threaded nature some backend receivers can create new // spans when receiving backend responses before the closer in the frontend thread is destructed. // So we find the SpanID in the stack and remove it. - data->d_spanIDStack.erase(std::find(data->d_spanIDStack.begin(), data->d_spanIDStack.end(), spanID)); + auto stackIt = std::find(data->d_spanIDStack.begin(), data->d_spanIDStack.end(), spanID); + if (stackIt != data->d_spanIDStack.end()) { + data->d_spanIDStack.erase(stackIt); + } } #endif } diff --git a/pdns/dnsdistdist/dnsdist-session-cache.cc b/pdns/dnsdistdist/dnsdist-session-cache.cc index de4e6c3639a9..0e2bd3989937 100644 --- a/pdns/dnsdistdist/dnsdist-session-cache.cc +++ b/pdns/dnsdistdist/dnsdist-session-cache.cc @@ -28,10 +28,10 @@ TLSSessionCache g_sessionCache; void TLSSessionCache::cleanup(time_t now, LockGuardedHolder& data) { const auto& runtimeConfig = dnsdist::configuration::getCurrentRuntimeConfiguration(); - time_t cutOff = now + runtimeConfig.d_tlsSessionCacheSessionValidity; + time_t cutOff = now - runtimeConfig.d_tlsSessionCacheSessionValidity; for (auto it = data->d_sessions.begin(); it != data->d_sessions.end();) { - if (it->second.d_lastUsed > cutOff || it->second.d_sessions.size() == 0) { + if (it->second.d_lastUsed < cutOff || it->second.d_sessions.empty()) { it = data->d_sessions.erase(it); } else { @@ -50,6 +50,10 @@ void TLSSessionCache::putSessions(const boost::uuids::uuid& backendID, time_t no } const auto& runtimeConfig = dnsdist::configuration::getCurrentRuntimeConfiguration(); + if (runtimeConfig.d_tlsSessionCacheMaxSessionsPerBackend == 0) { + return; + } + for (auto& session : sessions) { auto& entry = data->d_sessions[backendID]; if (entry.d_sessions.size() >= runtimeConfig.d_tlsSessionCacheMaxSessionsPerBackend) { diff --git a/pdns/dnsdistdist/dnsdist-session-cache.hh b/pdns/dnsdistdist/dnsdist-session-cache.hh index f4e4cdbaad26..4fa1b860893e 100644 --- a/pdns/dnsdistdist/dnsdist-session-cache.hh +++ b/pdns/dnsdistdist/dnsdist-session-cache.hh @@ -31,10 +31,6 @@ class TLSSessionCache { public: - TLSSessionCache() - { - } - void putSessions(const boost::uuids::uuid& backendID, time_t now, std::vector>&& sessions); std::unique_ptr getSession(const boost::uuids::uuid& backendID, time_t now); diff --git a/pdns/dnsdistdist/doq.cc b/pdns/dnsdistdist/doq.cc index 7bf6c153bd34..15750860eb98 100644 --- a/pdns/dnsdistdist/doq.cc +++ b/pdns/dnsdistdist/doq.cc @@ -665,9 +665,9 @@ static void handleReadableStream(DOQFrontend& frontend, ClientState& clientState return; } - if (received > std::numeric_limits::max() || (std::numeric_limits::max() - streamBuffer.size()) < static_cast(received)) { - VERBOSESLOG(infolog("DoQ data frame of size %d is too large for a DNS query (we already have %d)", received, streamBuffer.size()), - frontend.d_logger->info(Logr::Info, "DoQ data frame is too large for a DNS query", "stream_id", Logging::Loggable(streamID), "frame_size", Logging::Loggable(received), "existing_payload_size", Logging::Loggable(streamBuffer.size()))); + if (received > std::numeric_limits::max() || (std::numeric_limits::max() - existingLength) < static_cast(received)) { + VERBOSESLOG(infolog("DoQ data frame of size %d is too large for a DNS query (we already have %d)", received, existingLength), + frontend.d_logger->info(Logr::Info, "DoQ data frame is too large for a DNS query", "stream_id", Logging::Loggable(streamID), "frame_size", Logging::Loggable(received), "existing_payload_size", Logging::Loggable(existingLength))); conn.d_streamBuffers.erase(streamID); ++dnsdist::metrics::g_stats.nonCompliantQueries; ++clientState.nonCompliantQueries; diff --git a/pdns/dnsdistdist/meson.build b/pdns/dnsdistdist/meson.build index dcce5a547023..1a80faf486dc 100644 --- a/pdns/dnsdistdist/meson.build +++ b/pdns/dnsdistdist/meson.build @@ -549,6 +549,7 @@ test_sources += files( src_dir / 'test-dnsdistrules_cc.cc', src_dir / 'test-dnsdistsvc_cc.cc', src_dir / 'test-dnsdistserverpool.cc', + src_dir / 'test-dnsdist-session-cache.cc', src_dir / 'test-dnsdisttcp_cc.cc', src_dir / 'test-dnsdist-xsk.cc', src_dir / 'test-dnsparser_cc.cc', diff --git a/pdns/dnsdistdist/test-dnsdist-session-cache.cc b/pdns/dnsdistdist/test-dnsdist-session-cache.cc new file mode 100644 index 000000000000..8bdd33ca94cb --- /dev/null +++ b/pdns/dnsdistdist/test-dnsdist-session-cache.cc @@ -0,0 +1,86 @@ +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ +#ifndef BOOST_TEST_DYN_LINK +#define BOOST_TEST_DYN_LINK +#endif + +#define BOOST_TEST_NO_MAIN + +#include + +#include "dnsdist-session-cache.hh" +#include "dnsdist-configuration.hh" + +class MockTLSSession : public TLSSession +{ +public: + ~MockTLSSession() override = default; +}; + +BOOST_AUTO_TEST_SUITE(test_dnsdist_session_cache) + +BOOST_AUTO_TEST_CASE(test_No_Rate_Limiting) +{ + TLSSessionCache cache{}; + const auto backendID1 = getUniqueID(); + const auto backendID2 = getUniqueID(); + auto now = time(nullptr); + + BOOST_REQUIRE_EQUAL(cache.getSize(), 0U); + + /* store one session */ + std::vector> sessions; + sessions.push_back(std::make_unique()); + cache.putSessions(backendID1, now, std::move(sessions)); + BOOST_REQUIRE_EQUAL(cache.getSize(), 1U); + + /* we can retrieve it */ + auto session = cache.getSession(backendID1, now); + BOOST_REQUIRE(session != nullptr); + BOOST_REQUIRE_EQUAL(cache.getSize(), 0U); + + /* but only once */ + session = cache.getSession(backendID1, now); + BOOST_REQUIRE(session == nullptr); + BOOST_REQUIRE_EQUAL(cache.getSize(), 0U); + + /* add a new session */ + sessions.clear(); + sessions.push_back(std::make_unique()); + cache.putSessions(backendID1, now, std::move(sessions)); + BOOST_REQUIRE_EQUAL(cache.getSize(), 1U); + + /* wait until the session expires */ + const auto& runtimeConfig = dnsdist::configuration::getCurrentRuntimeConfiguration(); + now += std::max(runtimeConfig.d_tlsSessionCacheSessionValidity, runtimeConfig.d_tlsSessionCacheCleanupDelay) + 1; + + /* trigger cache cleaning by inserting a session for a different backend */ + sessions.clear(); + sessions.push_back(std::make_unique()); + cache.putSessions(backendID2, now, std::move(sessions)); + BOOST_REQUIRE_EQUAL(cache.getSize(), 1U); + + session = cache.getSession(backendID1, now); + BOOST_REQUIRE(session == nullptr); +} + +BOOST_AUTO_TEST_SUITE_END(); diff --git a/pdns/dnsdistdist/xsk.cc b/pdns/dnsdistdist/xsk.cc index 475b5c6a25df..a88389576971 100644 --- a/pdns/dnsdistdist/xsk.cc +++ b/pdns/dnsdistdist/xsk.cc @@ -391,7 +391,7 @@ void XskSocket::recv(std::vector& packets, uint32_t recvSizeMax, uint try { const auto* desc = xsk_ring_cons__rx_desc(&rx, idx++); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast,performance-no-int-to-ptr) - XskPacket packet = XskPacket(reinterpret_cast(desc->addr + baseAddr), desc->len, frameSize); + auto packet = XskPacket(reinterpret_cast(desc->addr + baseAddr), desc->len, frameSize); #ifdef DEBUG_UMEM checkUmemIntegrity(__PRETTY_FUNCTION__, __LINE__, sharedEmptyFrameOffset, frameOffset(packet), {UmemEntryStatus::Status::FillQueue}, UmemEntryStatus::Status::Received); #endif /* DEBUG_UMEM */ @@ -407,12 +407,12 @@ void XskSocket::recv(std::vector& packets, uint32_t recvSizeMax, uint catch (const std::exception& exp) { ++failed; ++processed; - break; + continue; } catch (...) { ++failed; ++processed; - break; + continue; } } @@ -819,7 +819,7 @@ bool XskPacket::isIPV6() const noexcept return v6; } -XskPacket::XskPacket(uint8_t* frame_, size_t dataSize, size_t frameSize_) : +XskPacket::XskPacket(uint8_t* frame_, size_t dataSize, size_t frameSize_) noexcept : frame(frame_), frameLength(dataSize), frameSize(frameSize_ - XDP_PACKET_HEADROOM) { } diff --git a/pdns/dnsdistdist/xsk.hh b/pdns/dnsdistdist/xsk.hh index bc1bb0265430..fc594118060f 100644 --- a/pdns/dnsdistdist/xsk.hh +++ b/pdns/dnsdistdist/xsk.hh @@ -260,7 +260,7 @@ public: /* Rewrite the headers, usually called after setAddr() then setPayload() */ void rewrite() noexcept; void setHeader(PacketBuffer& buf); - XskPacket(uint8_t* frame, size_t dataSize, size_t frameSize); + XskPacket(uint8_t* frame, size_t dataSize, size_t frameSize) noexcept; void addDelay(int relativeMilliseconds) noexcept; /* If the payload has been updated, and the headers have not been rewritten via rewrite() yet, exchange the source and destination addresses (ethernet and IP) and rewrite the headers. diff --git a/pdns/iputils.cc b/pdns/iputils.cc index 74b61c9e4a76..9181c8b66685 100644 --- a/pdns/iputils.cc +++ b/pdns/iputils.cc @@ -710,6 +710,10 @@ std::vector getListOfRangesOfNetworkInterface(const std::string& itf) if (ifa->ifa_addr == nullptr || (ifa->ifa_addr->sa_family != AF_INET && ifa->ifa_addr->sa_family != AF_INET6)) { continue; } + if (ifa->ifa_netmask == nullptr) { + continue; + } + ComboAddress addr; try { addr.setSockaddr(ifa->ifa_addr, ifa->ifa_addr->sa_family == AF_INET ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6)); diff --git a/pdns/libssl.cc b/pdns/libssl.cc index f8c6e2e6363d..4d1c3242230a 100644 --- a/pdns/libssl.cc +++ b/pdns/libssl.cc @@ -452,29 +452,29 @@ static std::map libssl_load_ocsp_responses(const std::vector buffer{}; + file.read(buffer.data(), buffer.size()); if (file.bad()) { file.close(); warnings.push_back("Unable to load OCSP response from " + filename); continue; } - content.append(buffer, file.gcount()); + content.append(buffer.data(), file.gcount()); } file.close(); try { libssl_validate_ocsp_response(content); - ocspResponses.insert({keyTypes.at(count), std::move(content)}); + ocspResponses.insert({keyTypes.at(idx), std::move(content)}); } catch (const std::exception& e) { warnings.push_back("Error checking the validity of OCSP response from '" + filename + "': " + e.what()); continue; } - ++count; } return ocspResponses; @@ -1400,7 +1400,7 @@ std::string libssl_get_error_string() size_t len = BIO_get_mem_data(mem, &p); std::string msg(p, len); // replace newlines by / - if (msg.back() == '\n') { + if (!msg.empty() && msg.back() == '\n') { msg.pop_back(); } std::replace(msg.begin(), msg.end(), '\n', '/'); diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index 801f660e2137..f18293f2c092 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -700,6 +700,8 @@ static void loadMainConfig(const std::string& configdir) ::arg().set("max-include-depth", "Maximum nested $INCLUDE depth when loading a zone from a file")="20"; ::arg().setSwitch("upgrade-unknown-types","Transparently upgrade known TYPExxx records. Recommended to keep off, except for PowerDNS upgrades until data sources are cleaned up")="no"; ::arg().setSwitch("views", "Enable views (variants) of zones, for backends which support them") = "no"; + // Needed by Lua backend + ::arg().set("lua-global-include-dir", "Include *.lua files from this directory into Lua contexts") = ""; ::arg().laxFile(configname); // FIXME520: remove when branching 5.2 diff --git a/pdns/rfc2136handler.cc b/pdns/rfc2136handler.cc index 0921136e7d57..fb231cced999 100644 --- a/pdns/rfc2136handler.cc +++ b/pdns/rfc2136handler.cc @@ -905,6 +905,18 @@ static uint8_t updatePrereqCheck323(const MOADNSParser::answers_t& answers, cons static uint8_t updateRecords(const MOADNSParser::answers_t& answers, DNSSECKeeper& dsk, uint& changedRecords, const std::unique_ptr& update_policy_lua, DNSPacket& packet, updateContext& ctx) { + // Reject the complete update if it contains Lua records, unless explicitly + // allowed, regardless of any other policy. + if (!::arg().mustDo("enable-lua-record-updates")) { + for (const auto& rec : answers) { + if (QType(rec.d_type) == QType::LUA) { + SLOG(g_log << Logger::Warning << ctx.msgPrefix << "Refusing update due to Lua record " << rec.d_name << ": Not permitted by global settings" << endl, + ctx.slog->info(Logr::Warning, "Update: refusing update due to Lua record, not permitted by global settings", "name", Logging::Loggable(rec.d_name))); + return RCode::Refused; + } + } + } + vector cnamesToAdd; vector nonCnamesToAdd; vector nsRRtoDelete; @@ -914,6 +926,7 @@ static uint8_t updateRecords(const MOADNSParser::answers_t& answers, DNSSECKeepe for (const auto& rec : answers) { if (rec.d_place == DNSResourceRecord::AUTHORITY) { anyRecordProcessed = true; + /* see if it's permitted by policy */ if (update_policy_lua != nullptr) { if (!update_policy_lua->updatePolicy(rec.d_name, QType(rec.d_type), ctx.di.zone.operator const DNSName&(), packet)) { diff --git a/pdns/snmp-agent.cc b/pdns/snmp-agent.cc index 0e7ca82d65e8..c58ebca47579 100644 --- a/pdns/snmp-agent.cc +++ b/pdns/snmp-agent.cc @@ -92,6 +92,7 @@ void SNMPAgent::handleSNMPQueryEvent(int fd) NETSNMP_LARGE_FD_ZERO(&fdset); NETSNMP_LARGE_FD_SET(fd, &fdset); snmp_read2(&fdset); + netsnmp_large_fd_set_cleanup(&fdset); } void SNMPAgent::handleTrapsCB(int /* fd */, FDMultiplexer::funcparam_t& var) @@ -173,6 +174,7 @@ void SNMPAgent::worker() } } } + netsnmp_large_fd_set_cleanup(&fdset); } #endif /* HAVE_NET_SNMP */ } diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index ddc8aeb439e7..2e5fc7d58890 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -1753,6 +1753,15 @@ static bool checkNewRecords(HttpResponse* resp, vector& recor { std::vector> errors; + // Do not perform Lua records updates if not allowed to. + if (!::arg().mustDo("enable-lua-record-updates")) { + for (const auto& rec : records) { + if (rec.qtype == QType::LUA) { + errors.emplace_back(std::make_pair(rec, std::string("update of Lua records is not allowed"))); + } + } + } + Check::checkRRSet({}, records, zone, flags, errors); if (errors.empty()) { return true; @@ -2569,6 +2578,12 @@ enum applyResult // Apply a DELETE changetype. static applyResult applyDelete(const DomainInfo& domainInfo, DNSName& qname, QType& qtype, bool returnRRset, std::vector& rrset) { + // Do not perform Lua records deletions if not allowed to. + if (!::arg().mustDo("enable-lua-record-updates")) { + if (qtype == QType::LUA) { + throw ApiException("Update of Lua records is not allowed"); + } + } // Delete all matching qname/qtype RRs (and implicitly, comments). if (!domainInfo.backend->replaceRRSet(domainInfo.id, qname, qtype, {})) { throw ApiException("Hosting backend does not support editing records."); diff --git a/regression-tests.api/runtests.py b/regression-tests.api/runtests.py index 21fa6e5314cd..5bb27408193a 100755 --- a/regression-tests.api/runtests.py +++ b/regression-tests.api/runtests.py @@ -291,7 +291,11 @@ def run_check_call(cmd, *args, **kwargs): run_check_call(PDNSUTIL_CMD + ["load-zone", zone, ZONE_DIR + zone]) run_check_call(PDNSUTIL_CMD + ["secure-zone", "powerdnssec.org"]) - servercmd = [pdns_server] + common_args + ["--no-shuffle", "--dnsupdate=yes", "--cache-ttl=0", "--api=yes"] + servercmd = ( + [pdns_server] + + common_args + + ["--no-shuffle", "--dnsupdate=yes", "--cache-ttl=0", "--api=yes", "--enable-lua-record-updates"] + ) else: conf_dir = "rec-conf.d" diff --git a/regression-tests.dnsdist/test_EBPF.py b/regression-tests.dnsdist/test_EBPF.py index c4c6403239ed..6c3613c8b5ce 100644 --- a/regression-tests.dnsdist/test_EBPF.py +++ b/regression-tests.dnsdist/test_EBPF.py @@ -145,6 +145,8 @@ def testQNameBlockedOnlyForAny(self): def testClientIPBlocked(self): # block 127.0.0.1 self.sendConsoleCommand('bpf:block(newCA("127.0.0.1"))') + stats = self.sendConsoleCommand("bpf:getStats()") + self.assertIn("127.0.0.1: 0", stats) name = "ip-blocked.ebpf.tests.powerdns.com." query = dns.message.make_query(name, "A", "IN", use_edns=False) @@ -175,6 +177,13 @@ def testClientIPBlocked(self): # unblock 127.0.0.1 self.sendConsoleCommand('bpf:unblock(newCA("127.0.0.1"))') + stats = self.sendConsoleCommand("bpf:getStats()") + self.assertNotIn("127.0.0.1: 0", stats) + + # block 0.0.0.0 + self.sendConsoleCommand('bpf:block(newCA("0.0.0.0"))') + stats = self.sendConsoleCommand("bpf:getStats()") + self.assertIn("0.0.0.0: 0", stats) @unittest.skipUnless("ENABLE_SUDO_TESTS" in os.environ, "sudo is not available") diff --git a/regression-tests.dnsdist/test_HealthChecks.py b/regression-tests.dnsdist/test_HealthChecks.py index 430313a0adca..7d5528925d33 100644 --- a/regression-tests.dnsdist/test_HealthChecks.py +++ b/regression-tests.dnsdist/test_HealthChecks.py @@ -741,6 +741,13 @@ def testLatency(self): # introduce 500 ms of latency self.setDelay(0.5) + # consume any value received in the meantime (it does happen on GH actions runners) + try: + while self.wait1(False): + pass + except queue.Empty: + pass + self.wait1(True) # should have no failures, still up