From 4ed255ed5c601a18f91566965b813ef3a3d5e029 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 23 Jan 2024 12:01:02 +0100 Subject: [PATCH] dnsdist: Fix warnings from clang-tidy --- pdns/bpf-filter.cc | 76 +++++------ pdns/dnsdist-lua.cc | 177 +++++++++++++------------ pdns/dnsdist-tcp.cc | 2 +- pdns/dnsdist.cc | 79 +++++------ pdns/dnsdist.hh | 4 +- pdns/dnsdistdist/doh.cc | 2 +- pdns/dnsdistdist/doh3.cc | 2 +- pdns/dnsdistdist/doq.cc | 2 +- pdns/dnsdistdist/test-dnsdisttcp_cc.cc | 2 +- pdns/test-dnsdist_cc.cc | 4 +- 10 files changed, 173 insertions(+), 177 deletions(-) diff --git a/pdns/bpf-filter.cc b/pdns/bpf-filter.cc index 19343955c81fb..225432cc1c4a5 100644 --- a/pdns/bpf-filter.cc +++ b/pdns/bpf-filter.cc @@ -33,32 +33,18 @@ #include "misc.hh" -int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size, - int max_entries, int map_flags); -int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags); -int bpf_lookup_elem(int fd, void *key, void *value); -int bpf_delete_elem(int fd, void *key); -int bpf_get_next_key(int fd, void *key, void *next_key); - -int bpf_prog_load(enum bpf_prog_type prog_type, - const struct bpf_insn *insns, int insn_len, - const char *license, int kern_version); - -int bpf_obj_pin(int fd, const char *pathname); -int bpf_obj_get(const char *pathname); - -static __u64 ptr_to_u64(void *ptr) +static __u64 ptr_to_u64(const void *ptr) { return (__u64) (unsigned long) ptr; } /* these can be static as they are not declared in libbpf.h: */ -static int bpf_pin_map(int fd, const std::string& path) +static int bpf_pin_map(int descriptor, const std::string& path) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.bpf_fd = fd; - attr.pathname = ptr_to_u64(const_cast(path.c_str())); + attr.bpf_fd = descriptor; + attr.pathname = ptr_to_u64(path.c_str()); return syscall(SYS_bpf, BPF_OBJ_PIN, &attr, sizeof(attr)); } @@ -66,11 +52,11 @@ static int bpf_load_pinned_map(const std::string& path) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.pathname = ptr_to_u64(const_cast(path.c_str())); + attr.pathname = ptr_to_u64(path.c_str()); return syscall(SYS_bpf, BPF_OBJ_GET, &attr, sizeof(attr)); } -static void bpf_check_map_sizes(int fd, uint32_t expectedKeySize, uint32_t expectedValueSize) +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); @@ -78,7 +64,7 @@ static void bpf_check_map_sizes(int fd, uint32_t expectedKeySize, uint32_t expec union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.info.bpf_fd = fd; + attr.info.bpf_fd = descriptor; attr.info.info_len = info_len; attr.info.info = ptr_to_u64(&info); @@ -97,8 +83,8 @@ static void bpf_check_map_sizes(int fd, uint32_t expectedKeySize, uint32_t expec } } -int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size, - int max_entries, int map_flags) +static int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size, + int max_entries, int map_flags) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); @@ -110,49 +96,49 @@ int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size, return syscall(SYS_bpf, BPF_MAP_CREATE, &attr, sizeof(attr)); } -int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags) +static int bpf_update_elem(int descriptor, void *key, void *value, unsigned long long flags) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.map_fd = fd; + attr.map_fd = descriptor; attr.key = ptr_to_u64(key); attr.value = ptr_to_u64(value); attr.flags = flags; return syscall(SYS_bpf, BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr)); } -int bpf_lookup_elem(int fd, void *key, void *value) +static int bpf_lookup_elem(int descriptor, void *key, void *value) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.map_fd = fd; + attr.map_fd = descriptor; attr.key = ptr_to_u64(key); attr.value = ptr_to_u64(value); return syscall(SYS_bpf, BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr)); } -int bpf_delete_elem(int fd, void *key) +static int bpf_delete_elem(int descriptor, void *key) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.map_fd = fd; + attr.map_fd = descriptor; attr.key = ptr_to_u64(key); return syscall(SYS_bpf, BPF_MAP_DELETE_ELEM, &attr, sizeof(attr)); } -int bpf_get_next_key(int fd, void *key, void *next_key) +static int bpf_get_next_key(int descriptor, void *key, void *next_key) { union bpf_attr attr; memset(&attr, 0, sizeof(attr)); - attr.map_fd = fd; + attr.map_fd = descriptor; attr.key = ptr_to_u64(key); attr.next_key = ptr_to_u64(next_key); return syscall(SYS_bpf, BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr)); } -int bpf_prog_load(enum bpf_prog_type prog_type, - const struct bpf_insn *insns, int prog_len, - const char *license, int kern_version) +static int bpf_prog_load(enum bpf_prog_type prog_type, + const struct bpf_insn *insns, int prog_len, + const char *license, int kern_version) { char log_buf[65535]; union bpf_attr attr; @@ -351,15 +337,15 @@ BPFFilter::Map::Map(const BPFFilter::MapConfiguration& config, BPFFilter::MapFor static FDWrapper loadProgram(const struct bpf_insn* filter, size_t filterSize) { - auto fd = FDWrapper(bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, - filter, - filterSize, - "GPL", - 0)); - if (fd.getHandle() == -1) { + auto descriptor = FDWrapper(bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, + filter, + filterSize, + "GPL", + 0)); + if (descriptor.getHandle() == -1) { throw std::runtime_error("error loading BPF filter: " + stringerror()); } - return fd; + return descriptor; } @@ -442,8 +428,8 @@ BPFFilter::BPFFilter(std::unordered_map& configs, void BPFFilter::addSocket(int sock) { - int fd = d_mainfilter.getHandle(); - int res = setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &fd, sizeof(fd)); + int descriptor = d_mainfilter.getHandle(); + int res = setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &descriptor, sizeof(descriptor)); if (res != 0) { throw std::runtime_error("Error attaching BPF filter to this socket: " + stringerror()); @@ -452,8 +438,8 @@ void BPFFilter::addSocket(int sock) void BPFFilter::removeSocket(int sock) { - int fd = d_mainfilter.getHandle(); - int res = setsockopt(sock, SOL_SOCKET, SO_DETACH_BPF, &fd, sizeof(fd)); + int descriptor = d_mainfilter.getHandle(); + int res = setsockopt(sock, SOL_SOCKET, SO_DETACH_BPF, &descriptor, sizeof(descriptor)); if (res != 0) { throw std::runtime_error("Error detaching BPF filter from this socket: " + stringerror()); diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index e22a83cd68d61..50112a3730b0f 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -307,10 +307,101 @@ static bool checkConfigurationTime(const std::string& name) return false; } +using newserver_t = LuaAssociativeTable, LuaArray>, DownstreamState::checkfunc_t>>; + +static void handleNewServerHealthCheckParameters(boost::optional& vars, DownstreamState::Config& config) +{ + std::string valueStr; + + if (getOptionalValue(vars, "checkInterval", valueStr) > 0) { + config.checkInterval = static_cast(std::stoul(valueStr)); + } + + if (getOptionalValue(vars, "healthCheckMode", valueStr) > 0) { + const auto& mode = valueStr; + if (pdns_iequals(mode, "auto")) { + config.availability = DownstreamState::Availability::Auto; + } + else if (pdns_iequals(mode, "lazy")) { + config.availability = DownstreamState::Availability::Lazy; + } + else if (pdns_iequals(mode, "up")) { + config.availability = DownstreamState::Availability::Up; + } + else if (pdns_iequals(mode, "down")) { + config.availability = DownstreamState::Availability::Down; + } + else { + warnlog("Ignoring unknown value '%s' for 'healthCheckMode' on 'newServer'", mode); + } + } + + if (getOptionalValue(vars, "checkName", valueStr) > 0) { + config.checkName = DNSName(valueStr); + } + + getOptionalValue(vars, "checkType", config.checkType); + getOptionalIntegerValue("newServer", vars, "checkClass", config.checkClass); + getOptionalValue(vars, "checkFunction", config.checkFunction); + getOptionalIntegerValue("newServer", vars, "checkTimeout", config.checkTimeout); + getOptionalValue(vars, "checkTCP", config.d_tcpCheck); + getOptionalValue(vars, "setCD", config.setCD); + getOptionalValue(vars, "mustResolve", config.mustResolve); + + if (getOptionalValue(vars, "lazyHealthCheckSampleSize", valueStr) > 0) { + const auto& value = std::stoi(valueStr); + checkParameterBound("lazyHealthCheckSampleSize", value); + config.d_lazyHealthCheckSampleSize = value; + } + + if (getOptionalValue(vars, "lazyHealthCheckMinSampleCount", valueStr) > 0) { + const auto& value = std::stoi(valueStr); + checkParameterBound("lazyHealthCheckMinSampleCount", value); + config.d_lazyHealthCheckMinSampleCount = value; + } + + if (getOptionalValue(vars, "lazyHealthCheckThreshold", valueStr) > 0) { + const auto& value = std::stoi(valueStr); + checkParameterBound("lazyHealthCheckThreshold", value, std::numeric_limits::max()); + config.d_lazyHealthCheckThreshold = value; + } + + if (getOptionalValue(vars, "lazyHealthCheckFailedInterval", valueStr) > 0) { + const auto& value = std::stoi(valueStr); + checkParameterBound("lazyHealthCheckFailedInterval", value); + config.d_lazyHealthCheckFailedInterval = value; + } + + getOptionalValue(vars, "lazyHealthCheckUseExponentialBackOff", config.d_lazyHealthCheckUseExponentialBackOff); + + if (getOptionalValue(vars, "lazyHealthCheckMaxBackOff", valueStr) > 0) { + const auto& value = std::stoi(valueStr); + checkParameterBound("lazyHealthCheckMaxBackOff", value); + config.d_lazyHealthCheckMaxBackOff = value; + } + + if (getOptionalValue(vars, "lazyHealthCheckMode", valueStr) > 0) { + const auto& mode = valueStr; + if (pdns_iequals(mode, "TimeoutOnly")) { + config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOnly; + } + else if (pdns_iequals(mode, "TimeoutOrServFail")) { + config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOrServFail; + } + else { + warnlog("Ignoring unknown value '%s' for 'lazyHealthCheckMode' on 'newServer'", mode); + } + } + + getOptionalValue(vars, "lazyHealthCheckWhenUpgraded", config.d_upgradeToLazyHealthChecks); + + getOptionalIntegerValue("newServer", vars, "maxCheckFailures", config.maxCheckFailures); + getOptionalIntegerValue("newServer", vars, "rise", config.minRiseSuccesses); +} + // NOLINTNEXTLINE(readability-function-cognitive-complexity): this function declares Lua bindings, even with a good refactoring it will likely blow up the threshold static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) { - using newserver_t = LuaAssociativeTable, LuaArray>, DownstreamState::checkfunc_t>>; luaCtx.writeFunction("inClientStartup", [client]() { return client && !g_configurationDone; }); @@ -406,9 +497,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) getOptionalIntegerValue("newServer", vars, "tcpSendTimeout", config.tcpSendTimeout); getOptionalIntegerValue("newServer", vars, "tcpRecvTimeout", config.tcpRecvTimeout); - if (getOptionalValue(vars, "checkInterval", valueStr) > 0) { - config.checkInterval = static_cast(std::stoul(valueStr)); - } + handleNewServerHealthCheckParameters(vars, config); bool fastOpen{false}; if (getOptionalValue(vars, "tcpFastOpen", fastOpen) > 0) { @@ -430,84 +519,6 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.id = boost::uuids::string_generator()(valueStr); } - if (getOptionalValue(vars, "healthCheckMode", valueStr) > 0) { - const auto& mode = valueStr; - if (pdns_iequals(mode, "auto")) { - config.availability = DownstreamState::Availability::Auto; - } - else if (pdns_iequals(mode, "lazy")) { - config.availability = DownstreamState::Availability::Lazy; - } - else if (pdns_iequals(mode, "up")) { - config.availability = DownstreamState::Availability::Up; - } - else if (pdns_iequals(mode, "down")) { - config.availability = DownstreamState::Availability::Down; - } - else { - warnlog("Ignoring unknown value '%s' for 'healthCheckMode' on 'newServer'", mode); - } - } - - if (getOptionalValue(vars, "checkName", valueStr) > 0) { - config.checkName = DNSName(valueStr); - } - - getOptionalValue(vars, "checkType", config.checkType); - getOptionalIntegerValue("newServer", vars, "checkClass", config.checkClass); - getOptionalValue(vars, "checkFunction", config.checkFunction); - getOptionalIntegerValue("newServer", vars, "checkTimeout", config.checkTimeout); - getOptionalValue(vars, "checkTCP", config.d_tcpCheck); - getOptionalValue(vars, "setCD", config.setCD); - getOptionalValue(vars, "mustResolve", config.mustResolve); - - if (getOptionalValue(vars, "lazyHealthCheckSampleSize", valueStr) > 0) { - const auto& value = std::stoi(valueStr); - checkParameterBound("lazyHealthCheckSampleSize", value); - config.d_lazyHealthCheckSampleSize = value; - } - - if (getOptionalValue(vars, "lazyHealthCheckMinSampleCount", valueStr) > 0) { - const auto& value = std::stoi(valueStr); - checkParameterBound("lazyHealthCheckMinSampleCount", value); - config.d_lazyHealthCheckMinSampleCount = value; - } - - if (getOptionalValue(vars, "lazyHealthCheckThreshold", valueStr) > 0) { - const auto& value = std::stoi(valueStr); - checkParameterBound("lazyHealthCheckThreshold", value, std::numeric_limits::max()); - config.d_lazyHealthCheckThreshold = value; - } - - if (getOptionalValue(vars, "lazyHealthCheckFailedInterval", valueStr) > 0) { - const auto& value = std::stoi(valueStr); - checkParameterBound("lazyHealthCheckFailedInterval", value); - config.d_lazyHealthCheckFailedInterval = value; - } - - getOptionalValue(vars, "lazyHealthCheckUseExponentialBackOff", config.d_lazyHealthCheckUseExponentialBackOff); - - if (getOptionalValue(vars, "lazyHealthCheckMaxBackOff", valueStr) > 0) { - const auto& value = std::stoi(valueStr); - checkParameterBound("lazyHealthCheckMaxBackOff", value); - config.d_lazyHealthCheckMaxBackOff = value; - } - - if (getOptionalValue(vars, "lazyHealthCheckMode", valueStr) > 0) { - const auto& mode = valueStr; - if (pdns_iequals(mode, "TimeoutOnly")) { - config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOnly; - } - else if (pdns_iequals(mode, "TimeoutOrServFail")) { - config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOrServFail; - } - else { - warnlog("Ignoring unknown value '%s' for 'lazyHealthCheckMode' on 'newServer'", mode); - } - } - - getOptionalValue(vars, "lazyHealthCheckWhenUpgraded", config.d_upgradeToLazyHealthChecks); - getOptionalValue(vars, "useClientSubnet", config.useECS); getOptionalValue(vars, "useProxyProtocol", config.useProxyProtocol); getOptionalValue(vars, "proxyProtocolAdvertiseTLS", config.d_proxyProtocolAdvertiseTLS); @@ -515,8 +526,6 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) getOptionalValue(vars, "ipBindAddrNoPort", config.ipBindAddrNoPort); getOptionalIntegerValue("newServer", vars, "addXPF", config.xpfRRCode); - getOptionalIntegerValue("newServer", vars, "maxCheckFailures", config.maxCheckFailures); - getOptionalIntegerValue("newServer", vars, "rise", config.minRiseSuccesses); getOptionalValue(vars, "reconnectOnUp", config.reconnectOnUp); diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index 3baf478ea22b1..9d8edaf572ff9 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -674,7 +674,7 @@ IncomingTCPConnectionState::QueryProcessingResult IncomingTCPConnectionState::ha { /* this pointer will be invalidated the second the buffer is resized, don't hold onto it! */ const dnsheader_aligned dnsHeader(query.data()); - if (!checkQueryHeaders(dnsHeader.get(), *d_ci.cs)) { + if (!checkQueryHeaders(*dnsHeader.get(), *d_ci.cs)) { return QueryProcessingResult::InvalidHeaders; } diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 67382a04bbfa8..204db07f1413e 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -366,6 +366,7 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, uint16_t rqtype, rqclass; DNSName rqname; try { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) rqname = DNSName(reinterpret_cast(response.data()), response.size(), sizeof(dnsheader), false, &rqtype, &rqclass); } catch (const std::exception& e) { @@ -746,7 +747,7 @@ static void handleResponseForUDPClient(InternalQueryState& ids, PacketBuffer& re } bool muted = true; - if (ids.cs && !ids.cs->muted && !ids.isXSK()) { + if (ids.cs != nullptr && !ids.cs->muted && !ids.isXSK()) { sendUDPResponse(ids.cs->udpFD, response, dr.ids.delayMsec, ids.hopLocal, ids.hopRemote); muted = false; } @@ -775,15 +776,15 @@ static void handleResponseForUDPClient(InternalQueryState& ids, PacketBuffer& re bool processResponderPacket(std::shared_ptr& dss, PacketBuffer& response, const std::vector& localRespRuleActions, const std::vector& cacheInsertedRespRuleActions, InternalQueryState&& ids) { - const dnsheader_aligned dh(response.data()); - auto queryId = dh->id; + const dnsheader_aligned dnsHeader(response.data()); + auto queryId = dnsHeader->id; if (!responseContentMatches(response, ids.qname, ids.qtype, ids.qclass, dss)) { dss->restoreState(queryId, std::move(ids)); return false; } - auto du = std::move(ids.du); + auto dohUnit = std::move(ids.du); dnsdist::PacketMangling::editDNSHeaderFromPacket(response, [&ids](dnsheader& header) { header.id = ids.origID; return true; @@ -793,13 +794,13 @@ bool processResponderPacket(std::shared_ptr& dss, PacketBuffer& double udiff = ids.queryRealTime.udiff(); // do that _before_ the processing, otherwise it's not fair to the backend dss->latencyUsec = (127.0 * dss->latencyUsec / 128.0) + udiff / 128.0; - dss->reportResponse(dh->rcode); + dss->reportResponse(dnsHeader->rcode); /* don't call processResponse for DOH */ - if (du) { + if (dohUnit) { #ifdef HAVE_DNS_OVER_HTTPS - // DoH query, we cannot touch du after that - DOHUnitInterface::handleUDPResponse(std::move(du), std::move(response), std::move(ids), dss); + // DoH query, we cannot touch dohUnit after that + DOHUnitInterface::handleUDPResponse(std::move(dohUnit), std::move(response), std::move(ids), dss); #endif return false; } @@ -860,8 +861,8 @@ void responderThread(std::shared_ptr dss) } response.resize(static_cast(got)); - const dnsheader_aligned dh(response.data()); - queryId = dh->id; + const dnsheader_aligned dnsHeader(response.data()); + queryId = dnsHeader->id; auto ids = dss->getState(queryId); if (!ids) { @@ -1337,22 +1338,22 @@ bool checkDNSCryptQuery(const ClientState& cs, PacketBuffer& query, std::unique_ return false; } -bool checkQueryHeaders(const struct dnsheader* dh, ClientState& cs) +bool checkQueryHeaders(const struct dnsheader& dnsHeader, ClientState& clientState) { - if (dh->qr) { // don't respond to responses + if (dnsHeader.qr) { // don't respond to responses ++dnsdist::metrics::g_stats.nonCompliantQueries; - ++cs.nonCompliantQueries; + ++clientState.nonCompliantQueries; return false; } - if (dh->qdcount == 0) { + if (dnsHeader.qdcount == 0) { ++dnsdist::metrics::g_stats.emptyQueries; if (g_dropEmptyQueries) { return false; } } - if (dh->rd) { + if (dnsHeader.rd) { ++dnsdist::metrics::g_stats.rdQueries; } @@ -1684,38 +1685,38 @@ ProcessQueryResult processQuery(DNSQuestion& dq, LocalHolders& holders, std::sha return ProcessQueryResult::Drop; } -bool assignOutgoingUDPQueryToBackend(std::shared_ptr& ds, uint16_t queryID, DNSQuestion& dq, PacketBuffer& query, bool actuallySend) +bool assignOutgoingUDPQueryToBackend(std::shared_ptr& downstream, uint16_t queryID, DNSQuestion& dnsQuestion, PacketBuffer& query, bool actuallySend) { - bool doh = dq.ids.du != nullptr; + bool doh = dnsQuestion.ids.du != nullptr; bool failed = false; - if (ds->d_config.useProxyProtocol) { + if (downstream->d_config.useProxyProtocol) { try { - addProxyProtocol(dq, &dq.ids.d_proxyProtocolPayloadSize); + addProxyProtocol(dnsQuestion, &dnsQuestion.ids.d_proxyProtocolPayloadSize); } catch (const std::exception& e) { - vinfolog("Adding proxy protocol payload to %s query from %s failed: %s", (dq.ids.du ? "DoH" : ""), dq.ids.origDest.toStringWithPort(), e.what()); + vinfolog("Adding proxy protocol payload to %s query from %s failed: %s", (dnsQuestion.ids.du ? "DoH" : ""), dnsQuestion.ids.origDest.toStringWithPort(), e.what()); return false; } } - if (doh && !dq.ids.d_packet) { - dq.ids.d_packet = std::make_unique(query); + if (doh && !dnsQuestion.ids.d_packet) { + dnsQuestion.ids.d_packet = std::make_unique(query); } try { - int fd = ds->pickSocketForSending(); + int fd = downstream->pickSocketForSending(); if (actuallySend) { - dq.ids.backendFD = fd; + dnsQuestion.ids.backendFD = fd; } - dq.ids.origID = queryID; - dq.ids.forwardedOverUDP = true; + dnsQuestion.ids.origID = queryID; + dnsQuestion.ids.forwardedOverUDP = true; - vinfolog("Got query for %s|%s from %s%s, relayed to %s%s", dq.ids.qname.toLogString(), QType(dq.ids.qtype).toString(), dq.ids.origRemote.toStringWithPort(), (doh ? " (https)" : ""), ds->getNameWithAddr(), actuallySend ? "" : " (xsk)"); + vinfolog("Got query for %s|%s from %s%s, relayed to %s%s", dnsQuestion.ids.qname.toLogString(), QType(dnsQuestion.ids.qtype).toString(), dnsQuestion.ids.origRemote.toStringWithPort(), (doh ? " (https)" : ""), downstream->getNameWithAddr(), actuallySend ? "" : " (xsk)"); - /* make a copy since we cannot touch dq.ids after the move */ - auto proxyProtocolPayloadSize = dq.ids.d_proxyProtocolPayloadSize; - auto idOffset = ds->saveState(std::move(dq.ids)); + /* make a copy since we cannot touch dnsQuestion.ids after the move */ + auto proxyProtocolPayloadSize = dnsQuestion.ids.d_proxyProtocolPayloadSize; + auto idOffset = downstream->saveState(std::move(dnsQuestion.ids)); /* set the correct ID */ memcpy(&query.at(proxyProtocolPayloadSize), &idOffset, sizeof(idOffset)); @@ -1725,7 +1726,7 @@ bool assignOutgoingUDPQueryToBackend(std::shared_ptr& ds, uint1 /* you can't touch ids or du after this line, unless the call returned a non-negative value, because it might already have been freed */ - ssize_t ret = udpClientSendRequestToBackend(ds, fd, query); + ssize_t ret = udpClientSendRequestToBackend(downstream, fd, query); if (ret < 0) { failed = true; @@ -1734,12 +1735,12 @@ bool assignOutgoingUDPQueryToBackend(std::shared_ptr& ds, uint1 if (failed) { /* clear up the state. In the very unlikely event it was reused in the meantime, so be it. */ - auto cleared = ds->getState(idOffset); + auto cleared = downstream->getState(idOffset); if (cleared) { - dq.ids.du = std::move(cleared->du); + dnsQuestion.ids.du = std::move(cleared->du); } ++dnsdist::metrics::g_stats.downstreamSendErrors; - ++ds->sendErrors; + ++downstream->sendErrors; return false; } } @@ -1795,14 +1796,14 @@ static void processUDPQuery(ClientState& cs, LocalHolders& holders, const struct { /* this pointer will be invalidated the second the buffer is resized, don't hold onto it! */ - const dnsheader_aligned dh(query.data()); - queryId = ntohs(dh->id); + const dnsheader_aligned dnsHeader(query.data()); + queryId = ntohs(dnsHeader->id); - if (!checkQueryHeaders(dh.get(), cs)) { + if (!checkQueryHeaders(*dnsHeader.get(), cs)) { return; } - if (dh->qdcount == 0) { + if (dnsHeader->qdcount == 0) { dnsdist::PacketMangling::editDNSHeaderFromPacket(query, [](dnsheader& header) { header.rcode = RCode::NotImp; header.qr = true; @@ -1922,7 +1923,7 @@ bool XskProcessQuery(ClientState& cs, LocalHolders& holders, XskPacket& packet) dnsheader_aligned dnsHeader(query.data()); queryId = ntohs(dnsHeader->id); - if (!checkQueryHeaders(dnsHeader.get(), cs)) { + if (!checkQueryHeaders(*dnsHeader.get(), cs)) { return false; } diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 4dba2db925f80..19eb3c715a464 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -1171,7 +1171,7 @@ void resetLuaSideEffect(); // reset to indeterminate state bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr& remote); -bool checkQueryHeaders(const struct dnsheader* dh, ClientState& cs); +bool checkQueryHeaders(const struct dnsheader& dnsHeader, ClientState& clientState); extern std::vector> g_dnsCryptLocals; int handleDNSCryptQuery(PacketBuffer& packet, DNSCryptQuery& query, bool tcp, time_t now, PacketBuffer& response); @@ -1195,7 +1195,7 @@ bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::s bool processResponseAfterRules(PacketBuffer& response, const std::vector& cacheInsertedRespRuleActions, DNSResponse& dr, bool muted); bool processResponderPacket(std::shared_ptr& dss, PacketBuffer& response, const std::vector& localRespRuleActions, const std::vector& cacheInsertedRespRuleActions, InternalQueryState&& ids); -bool assignOutgoingUDPQueryToBackend(std::shared_ptr& ds, uint16_t queryID, DNSQuestion& dq, PacketBuffer& query, bool actuallySend = true); +bool assignOutgoingUDPQueryToBackend(std::shared_ptr& downstream, uint16_t queryID, DNSQuestion& dnsQuestion, PacketBuffer& query, bool actuallySend = true); ssize_t udpClientSendRequestToBackend(const std::shared_ptr& ss, const int sd, const PacketBuffer& request, bool healthCheck = false); bool sendUDPResponse(int origFD, const PacketBuffer& response, const int delayMsec, const ComboAddress& origDest, const ComboAddress& origRemote); diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 0ac9e33874fc5..f40923d385b18 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -719,7 +719,7 @@ static void processDOHQuery(DOHUnitUniquePtr&& unit, bool inMainThread = false) // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) const dnsheader_aligned dnsHeader(unit->query.data()); - if (!checkQueryHeaders(dnsHeader.get(), clientState)) { + if (!checkQueryHeaders(*dnsHeader.get(), clientState)) { unit->status_code = 400; handleImmediateResponse(std::move(unit), "DoH invalid headers"); return; diff --git a/pdns/dnsdistdist/doh3.cc b/pdns/dnsdistdist/doh3.cc index 2201e23b3d02d..66081eb08a6d1 100644 --- a/pdns/dnsdistdist/doh3.cc +++ b/pdns/dnsdistdist/doh3.cc @@ -513,7 +513,7 @@ static void processDOH3Query(DOH3UnitUniquePtr&& doh3Unit) /* don't keep that pointer around, it will be invalidated if the buffer is ever resized */ dnsheader_aligned dnsHeader(unit->query.data()); - if (!checkQueryHeaders(dnsHeader.get(), clientState)) { + if (!checkQueryHeaders(*dnsHeader.get(), clientState)) { dnsdist::PacketMangling::editDNSHeaderFromPacket(unit->query, [](dnsheader& header) { header.rcode = RCode::ServFail; header.qr = true; diff --git a/pdns/dnsdistdist/doq.cc b/pdns/dnsdistdist/doq.cc index af951f3957970..3f52d33dc962e 100644 --- a/pdns/dnsdistdist/doq.cc +++ b/pdns/dnsdistdist/doq.cc @@ -425,7 +425,7 @@ static void processDOQQuery(DOQUnitUniquePtr&& doqUnit) /* don't keep that pointer around, it will be invalidated if the buffer is ever resized */ dnsheader_aligned dnsHeader(unit->query.data()); - if (!checkQueryHeaders(dnsHeader.get(), clientState)) { + if (!checkQueryHeaders(*dnsHeader.get(), clientState)) { dnsdist::PacketMangling::editDNSHeaderFromPacket(unit->query, [](dnsheader& header) { header.rcode = RCode::ServFail; header.qr = true; diff --git a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc index fd37dda3196d0..22a27c692c1a0 100644 --- a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc +++ b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc @@ -51,7 +51,7 @@ bool checkDNSCryptQuery(const ClientState& cs, PacketBuffer& query, std::unique_ return false; } -bool checkQueryHeaders(const struct dnsheader* dh, ClientState&) +bool checkQueryHeaders(const struct dnsheader& dnsHeader, ClientState& clientState) { return true; } diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc index d3e0d29fc0b95..90a513fc88097 100644 --- a/pdns/test-dnsdist_cc.cc +++ b/pdns/test-dnsdist_cc.cc @@ -58,7 +58,7 @@ bool sendUDPResponse(int origFD, const PacketBuffer& response, const int delayMs return false; } -bool assignOutgoingUDPQueryToBackend(std::shared_ptr& ds, uint16_t queryID, DNSQuestion& dq, PacketBuffer& query, bool) +bool assignOutgoingUDPQueryToBackend(std::shared_ptr& downstream, uint16_t queryID, DNSQuestion& dnsQuestion, PacketBuffer& query, bool actuallySend) { return true; } @@ -76,7 +76,7 @@ bool DNSDistSNMPAgent::sendBackendStatusChangeTrap(DownstreamState const&) } namespace dnsdist::xsk { -bool XskProcessQuery(ClientState& cs, LocalHolders& holders, XskPacket& packet) +bool XskProcessQuery(ClientState& clientState, LocalHolders& holders, XskPacket& packet) { return false; }