From: Charles-Henri Bruyand Date: Tue, 16 Jan 2024 18:07:56 +0000 (+0100) Subject: dnsdist: clang-tidy fixes X-Git-Tag: dnsdist-1.9.0-rc1~31^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=94219a425ffd8f5fb2e6d45aa1f9aab74cd10fb1;p=thirdparty%2Fpdns.git dnsdist: clang-tidy fixes --- diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc index 6a6db66d17..22229b6dcb 100644 --- a/pdns/dnsdist-lua-actions.cc +++ b/pdns/dnsdist-lua-actions.cc @@ -48,11 +48,11 @@ class DropAction : public DNSAction { public: - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { return Action::Drop; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "drop"; } @@ -61,11 +61,11 @@ public: class AllowAction : public DNSAction { public: - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { return Action::Allow; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "allow"; } @@ -75,11 +75,11 @@ class NoneAction : public DNSAction { public: // this action does not stop the processing - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "no op"; } @@ -92,16 +92,14 @@ public: d_qps(QPSLimiter(limit, limit)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { if (d_qps.lock()->check()) { return Action::None; } - else { - return Action::Drop; - } + return Action::Drop; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "qps limit to " + std::to_string(d_qps.lock()->getRate()); } @@ -117,12 +115,12 @@ public: d_msec(msec) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { *ruleresult = std::to_string(d_msec); return Action::Delay; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "delay by " + std::to_string(d_msec) + " ms"; } @@ -136,9 +134,13 @@ class TeeAction : public DNSAction public: // this action does not stop the processing TeeAction(const ComboAddress& rca, const boost::optional& lca, bool addECS = false, bool addProxyProtocol = false); + TeeAction(TeeAction& other) = delete; + TeeAction(TeeAction&& other) = delete; + TeeAction& operator=(TeeAction& other) = delete; + TeeAction& operator=(TeeAction&& other) = delete; ~TeeAction() override; - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override; - std::string toString() const override; + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override; + [[nodiscard]] std::string toString() const override; std::map getStats() const override; private: @@ -184,9 +186,9 @@ TeeAction::~TeeAction() d_worker.join(); } -DNSAction::Action TeeAction::operator()(DNSQuestion* dq, std::string* ruleresult) const +DNSAction::Action TeeAction::operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const { - if (dq->overTCP()) { + if (dnsquestion->overTCP()) { d_tcpdrops++; return DNSAction::Action::None; } @@ -195,22 +197,22 @@ DNSAction::Action TeeAction::operator()(DNSQuestion* dq, std::string* ruleresult PacketBuffer query; if (d_addECS) { - query = dq->getData(); + query = dnsquestion->getData(); bool ednsAdded = false; bool ecsAdded = false; std::string newECSOption; - generateECSOption(dq->ecs ? dq->ecs->getNetwork() : dq->ids.origRemote, newECSOption, dq->ecs ? dq->ecs->getBits() : dq->ecsPrefixLength); + generateECSOption(dnsquestion->ecs ? dnsquestion->ecs->getNetwork() : dnsquestion->ids.origRemote, newECSOption, dnsquestion->ecs ? dnsquestion->ecs->getBits() : dnsquestion->ecsPrefixLength); - if (!handleEDNSClientSubnet(query, dq->getMaximumSize(), dq->ids.qname.wirelength(), ednsAdded, ecsAdded, dq->ecsOverride, newECSOption)) { + if (!handleEDNSClientSubnet(query, dnsquestion->getMaximumSize(), dnsquestion->ids.qname.wirelength(), ednsAdded, ecsAdded, dnsquestion->ecsOverride, newECSOption)) { return DNSAction::Action::None; } } if (d_addProxyProtocol) { - auto proxyPayload = getProxyProtocolPayload(*dq); + auto proxyPayload = getProxyProtocolPayload(*dnsquestion); if (query.empty()) { - query = dq->getData(); + query = dnsquestion->getData(); } if (!addProxyProtocol(query, proxyPayload)) { return DNSAction::Action::None; @@ -218,7 +220,7 @@ DNSAction::Action TeeAction::operator()(DNSQuestion* dq, std::string* ruleresult } { - const PacketBuffer& payload = query.empty() ? dq->getData() : query; + const PacketBuffer& payload = query.empty() ? dnsquestion->getData() : query; auto res = send(d_socket.getHandle(), payload.data(), payload.size(), 0); if (res <= 0) { @@ -253,7 +255,7 @@ void TeeAction::worker() setThreadName("dnsdist/TeeWork"); std::array packet{}; ssize_t res = 0; - const dnsheader_aligned dh(packet.data()); + const dnsheader_aligned dnsheader(packet.data()); for (;;) { res = waitForData(d_socket.getHandle(), 0, 250000); if (d_pleaseQuit) { @@ -276,27 +278,27 @@ void TeeAction::worker() } // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well - if (dh->rcode == RCode::NoError) { + if (dnsheader->rcode == RCode::NoError) { d_noerrors++; } // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well - else if (dh->rcode == RCode::ServFail) { + else if (dnsheader->rcode == RCode::ServFail) { d_servfails++; } // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well - else if (dh->rcode == RCode::NXDomain) { + else if (dnsheader->rcode == RCode::NXDomain) { d_nxdomains++; } // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well - else if (dh->rcode == RCode::Refused) { + else if (dnsheader->rcode == RCode::Refused) { d_refuseds++; } // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well - else if (dh->rcode == RCode::FormErr) { + else if (dnsheader->rcode == RCode::FormErr) { d_formerrs++; } // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions): rcode is unsigned, RCode::rcodes_ as well - else if (dh->rcode == RCode::NotImp) { + else if (dnsheader->rcode == RCode::NotImp) { d_notimps++; } } @@ -305,38 +307,36 @@ void TeeAction::worker() class PoolAction : public DNSAction { public: - PoolAction(const std::string& pool, bool stopProcessing) : - d_pool(pool), d_stopProcessing(stopProcessing) {} + PoolAction(std::string pool, bool stopProcessing) : + d_pool(std::move(pool)), d_stopProcessing(stopProcessing) {} - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { if (d_stopProcessing) { /* we need to do it that way to keep compatiblity with custom Lua actions returning DNSAction.Pool, 'poolname' */ *ruleresult = d_pool; return Action::Pool; } - else { - dq->ids.poolName = d_pool; - return Action::None; - } + dnsquestion->ids.poolName = d_pool; + return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "to pool " + d_pool; } private: - const std::string d_pool; - const bool d_stopProcessing; + std::string d_pool; + bool d_stopProcessing; }; class QPSPoolAction : public DNSAction { public: - QPSPoolAction(unsigned int limit, const std::string& pool, bool stopProcessing) : - d_qps(QPSLimiter(limit, limit)), d_pool(pool), d_stopProcessing(stopProcessing) {} - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + QPSPoolAction(unsigned int limit, std::string pool, bool stopProcessing) : + d_qps(QPSLimiter(limit, limit)), d_pool(std::move(pool)), d_stopProcessing(stopProcessing) {} + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { if (d_qps.lock()->check()) { if (d_stopProcessing) { @@ -344,24 +344,19 @@ public: *ruleresult = d_pool; return Action::Pool; } - else { - dq->ids.poolName = d_pool; - return Action::None; - } - } - else { - return Action::None; + dnsquestion->ids.poolName = d_pool; } + return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "max " + std::to_string(d_qps.lock()->getRate()) + " to pool " + d_pool; } private: mutable LockGuarded d_qps; - const std::string d_pool; - const bool d_stopProcessing; + std::string d_pool; + bool d_stopProcessing; }; class RCodeAction : public DNSAction @@ -369,9 +364,9 @@ class RCodeAction : public DNSAction public: RCodeAction(uint8_t rcode) : d_rcode(rcode) {} - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [this](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [this](dnsheader& header) { header.rcode = d_rcode; header.qr = true; // for good measure setResponseHeadersFromConfig(header, d_responseConfig); @@ -379,14 +374,17 @@ public: }); return Action::HeaderModify; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set rcode " + std::to_string(d_rcode); } - - ResponseConfig d_responseConfig; + [[nodiscard]] ResponseConfig& getResponseConfig() + { + return d_responseConfig; + } private: + ResponseConfig d_responseConfig; uint8_t d_rcode; }; @@ -395,25 +393,28 @@ class ERCodeAction : public DNSAction public: ERCodeAction(uint8_t rcode) : d_rcode(rcode) {} - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [this](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [this](dnsheader& header) { header.rcode = (d_rcode & 0xF); header.qr = true; // for good measure setResponseHeadersFromConfig(header, d_responseConfig); return true; }); - dq->ednsRCode = ((d_rcode & 0xFFF0) >> 4); + dnsquestion->ednsRCode = ((d_rcode & 0xFFF0) >> 4); return Action::HeaderModify; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set ercode " + ERCode::to_s(d_rcode); } - - ResponseConfig d_responseConfig; + [[nodiscard]] ResponseConfig& getResponseConfig() + { + return d_responseConfig; + } private: + ResponseConfig d_responseConfig; uint8_t d_rcode; }; @@ -443,80 +444,84 @@ public: } } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { /* it will likely be a bit bigger than that because of additionals */ - uint16_t numberOfRecords = d_payloads.size(); - const auto qnameWireLength = dq->ids.qname.wirelength(); - if (dq->getMaximumSize() < (sizeof(dnsheader) + qnameWireLength + 4 + numberOfRecords * 12 /* recordstart */ + d_totalPayloadsSize)) { + auto numberOfRecords = d_payloads.size(); + const auto qnameWireLength = dnsquestion->ids.qname.wirelength(); + if (dnsquestion->getMaximumSize() < (sizeof(dnsheader) + qnameWireLength + 4 + numberOfRecords * 12 /* recordstart */ + d_totalPayloadsSize)) { return Action::None; } PacketBuffer newPacket; newPacket.reserve(sizeof(dnsheader) + qnameWireLength + 4 + numberOfRecords * 12 /* recordstart */ + d_totalPayloadsSize); - GenericDNSPacketWriter pw(newPacket, dq->ids.qname, dq->ids.qtype); + GenericDNSPacketWriter packetWriter(newPacket, dnsquestion->ids.qname, dnsquestion->ids.qtype); for (const auto& payload : d_payloads) { - pw.startRecord(dq->ids.qname, dq->ids.qtype, d_responseConfig.ttl); - pw.xfrBlob(payload); - pw.commit(); + packetWriter.startRecord(dnsquestion->ids.qname, dnsquestion->ids.qtype, d_responseConfig.ttl); + packetWriter.xfrBlob(payload); + packetWriter.commit(); } - if (newPacket.size() < dq->getMaximumSize()) { + if (newPacket.size() < dnsquestion->getMaximumSize()) { for (const auto& additional : d_additionals4) { - pw.startRecord(additional.first.isRoot() ? dq->ids.qname : additional.first, QType::A, d_responseConfig.ttl, QClass::IN, DNSResourceRecord::ADDITIONAL); - pw.xfrCAWithoutPort(4, additional.second); - pw.commit(); + packetWriter.startRecord(additional.first.isRoot() ? dnsquestion->ids.qname : additional.first, QType::A, d_responseConfig.ttl, QClass::IN, DNSResourceRecord::ADDITIONAL); + packetWriter.xfrCAWithoutPort(4, additional.second); + packetWriter.commit(); } } - if (newPacket.size() < dq->getMaximumSize()) { + if (newPacket.size() < dnsquestion->getMaximumSize()) { for (const auto& additional : d_additionals6) { - pw.startRecord(additional.first.isRoot() ? dq->ids.qname : additional.first, QType::AAAA, d_responseConfig.ttl, QClass::IN, DNSResourceRecord::ADDITIONAL); - pw.xfrCAWithoutPort(6, additional.second); - pw.commit(); + packetWriter.startRecord(additional.first.isRoot() ? dnsquestion->ids.qname : additional.first, QType::AAAA, d_responseConfig.ttl, QClass::IN, DNSResourceRecord::ADDITIONAL); + packetWriter.xfrCAWithoutPort(6, additional.second); + packetWriter.commit(); } } - if (g_addEDNSToSelfGeneratedResponses && queryHasEDNS(*dq)) { - bool dnssecOK = getEDNSZ(*dq) & EDNS_HEADER_FLAG_DO; - pw.addOpt(g_PayloadSizeSelfGenAnswers, 0, dnssecOK ? EDNS_HEADER_FLAG_DO : 0); - pw.commit(); + if (g_addEDNSToSelfGeneratedResponses && queryHasEDNS(*dnsquestion)) { + bool dnssecOK = ((getEDNSZ(*dnsquestion) & EDNS_HEADER_FLAG_DO) != 0); + packetWriter.addOpt(g_PayloadSizeSelfGenAnswers, 0, dnssecOK ? EDNS_HEADER_FLAG_DO : 0); + packetWriter.commit(); } - if (newPacket.size() >= dq->getMaximumSize()) { + if (newPacket.size() >= dnsquestion->getMaximumSize()) { /* sorry! */ return Action::None; } - pw.getHeader()->id = dq->getHeader()->id; - pw.getHeader()->qr = true; // for good measure - setResponseHeadersFromConfig(*pw.getHeader(), d_responseConfig); - dq->getMutableData() = std::move(newPacket); + packetWriter.getHeader()->id = dnsquestion->getHeader()->id; + packetWriter.getHeader()->qr = true; // for good measure + setResponseHeadersFromConfig(*packetWriter.getHeader(), d_responseConfig); + dnsquestion->getMutableData() = std::move(newPacket); return Action::HeaderModify; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "spoof SVC record "; } - ResponseConfig d_responseConfig; + [[nodiscard]] ResponseConfig& getResponseConfig() + { + return d_responseConfig; + } private: - std::vector> d_payloads; - std::set> d_additionals4; - std::set> d_additionals6; + ResponseConfig d_responseConfig; + std::vector> d_payloads{}; + std::set> d_additionals4{}; + std::set> d_additionals6{}; size_t d_totalPayloadsSize{0}; }; class TCAction : public DNSAction { public: - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { return Action::Truncate; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "tc=1 answer"; } @@ -538,19 +543,19 @@ public: class LuaAction : public DNSAction { public: - typedef std::function>(DNSQuestion* dq)> func_t; - LuaAction(const LuaAction::func_t& func) : - d_func(func) + using func_t = std::function>(DNSQuestion* dnsquestion)>; + LuaAction(LuaAction::func_t func) : + d_func(std::move(func)) {} - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { try { - DNSAction::Action result; + DNSAction::Action result{}; { auto lock = g_lua.lock(); - auto ret = d_func(dq); - if (ruleresult) { + auto ret = d_func(dnsquestion); + if (ruleresult != nullptr) { if (boost::optional rule = std::get<1>(ret)) { *ruleresult = *rule; } @@ -573,7 +578,7 @@ public: return DNSAction::Action::ServFail; } - string toString() const override + [[nodiscard]] std::string toString() const override { return "Lua script"; } @@ -585,18 +590,18 @@ private: class LuaResponseAction : public DNSResponseAction { public: - typedef std::function>(DNSResponse* dr)> func_t; - LuaResponseAction(const LuaResponseAction::func_t& func) : - d_func(func) + using func_t = std::function>(DNSResponse* response)>; + LuaResponseAction(LuaResponseAction::func_t func) : + d_func(std::move(func)) {} - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { try { - DNSResponseAction::Action result; + DNSResponseAction::Action result{}; { auto lock = g_lua.lock(); - auto ret = d_func(dr); - if (ruleresult) { + auto ret = d_func(response); + if (ruleresult != nullptr) { if (boost::optional rule = std::get<1>(ret)) { *ruleresult = *rule; } @@ -619,7 +624,7 @@ public: return DNSResponseAction::Action::ServFail; } - string toString() const override + [[nodiscard]] std::string toString() const override { return "Lua response script"; } @@ -631,22 +636,22 @@ private: class LuaFFIAction : public DNSAction { public: - typedef std::function func_t; + using func_t = std::function; - LuaFFIAction(const LuaFFIAction::func_t& func) : - d_func(func) + LuaFFIAction(LuaFFIAction::func_t func) : + d_func(std::move(func)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dnsdist_ffi_dnsquestion_t dqffi(dq); + dnsdist_ffi_dnsquestion_t dqffi(dnsquestion); try { - DNSAction::Action result; + DNSAction::Action result{}; { auto lock = g_lua.lock(); auto ret = d_func(&dqffi); - if (ruleresult) { + if (ruleresult != nullptr) { if (dqffi.result) { *ruleresult = *dqffi.result; } @@ -669,7 +674,7 @@ public: return DNSAction::Action::ServFail; } - string toString() const override + [[nodiscard]] std::string toString() const override { return "Lua FFI script"; } @@ -681,14 +686,14 @@ private: class LuaFFIPerThreadAction : public DNSAction { public: - typedef std::function func_t; + using func_t = std::function; - LuaFFIPerThreadAction(const std::string& code) : - d_functionCode(code), d_functionID(s_functionsCounter++) + LuaFFIPerThreadAction(std::string code) : + d_functionCode(std::move(code)), d_functionID(s_functionsCounter++) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { try { auto& state = t_perThreadStates[d_functionID]; @@ -705,9 +710,9 @@ public: return DNSAction::Action::None; } - dnsdist_ffi_dnsquestion_t dqffi(dq); + dnsdist_ffi_dnsquestion_t dqffi(dnsquestion); auto ret = state.d_func(&dqffi); - if (ruleresult) { + if (ruleresult != nullptr) { if (dqffi.result) { *ruleresult = *dqffi.result; } @@ -728,7 +733,7 @@ public: return DNSAction::Action::ServFail; } - string toString() const override + [[nodiscard]] std::string toString() const override { return "Lua FFI per-thread script"; } @@ -742,8 +747,8 @@ private: }; static std::atomic s_functionsCounter; static thread_local std::map t_perThreadStates; - const std::string d_functionCode; - const uint64_t d_functionID; + std::string d_functionCode; + uint64_t d_functionID; }; std::atomic LuaFFIPerThreadAction::s_functionsCounter = 0; @@ -752,24 +757,24 @@ thread_local std::map LuaFFIPer class LuaFFIResponseAction : public DNSResponseAction { public: - typedef std::function func_t; + using func_t = std::function; - LuaFFIResponseAction(const LuaFFIResponseAction::func_t& func) : - d_func(func) + LuaFFIResponseAction(LuaFFIResponseAction::func_t func) : + d_func(std::move(func)) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - dnsdist_ffi_dnsresponse_t drffi(dr); + dnsdist_ffi_dnsresponse_t ffiResponse(response); try { - DNSResponseAction::Action result; + DNSResponseAction::Action result{}; { auto lock = g_lua.lock(); - auto ret = d_func(&drffi); - if (ruleresult) { - if (drffi.result) { - *ruleresult = *drffi.result; + auto ret = d_func(&ffiResponse); + if (ruleresult != nullptr) { + if (ffiResponse.result) { + *ruleresult = *ffiResponse.result; } else { // default to empty string @@ -790,7 +795,7 @@ public: return DNSResponseAction::Action::ServFail; } - string toString() const override + [[nodiscard]] std::string toString() const override { return "Lua FFI script"; } @@ -802,14 +807,14 @@ private: class LuaFFIPerThreadResponseAction : public DNSResponseAction { public: - typedef std::function func_t; + using func_t = std::function; - LuaFFIPerThreadResponseAction(const std::string& code) : - d_functionCode(code), d_functionID(s_functionsCounter++) + LuaFFIPerThreadResponseAction(std::string code) : + d_functionCode(std::move(code)), d_functionID(s_functionsCounter++) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { try { auto& state = t_perThreadStates[d_functionID]; @@ -826,11 +831,11 @@ public: return DNSResponseAction::Action::None; } - dnsdist_ffi_dnsresponse_t drffi(dr); - auto ret = state.d_func(&drffi); - if (ruleresult) { - if (drffi.result) { - *ruleresult = *drffi.result; + dnsdist_ffi_dnsresponse_t ffiResponse(response); + auto ret = state.d_func(&ffiResponse); + if (ruleresult != nullptr) { + if (ffiResponse.result) { + *ruleresult = *ffiResponse.result; } else { // default to empty string @@ -849,7 +854,7 @@ public: return DNSResponseAction::Action::ServFail; } - string toString() const override + [[nodiscard]] std::string toString() const override { return "Lua FFI per-thread script"; } @@ -864,8 +869,8 @@ private: static std::atomic s_functionsCounter; static thread_local std::map t_perThreadStates; - const std::string d_functionCode; - const uint64_t d_functionID; + std::string d_functionCode; + uint64_t d_functionID; }; std::atomic LuaFFIPerThreadResponseAction::s_functionsCounter = 0; @@ -873,9 +878,9 @@ thread_local std::map L thread_local std::default_random_engine SpoofAction::t_randomEngine; -DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresult) const +DNSAction::Action SpoofAction::operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const { - uint16_t qtype = dq->ids.qtype; + uint16_t qtype = dnsquestion->ids.qtype; // do we even have a response? if (d_cname.empty() && d_rawResponses.empty() && // make sure pre-forged response is greater than sizeof(dnsheader) @@ -884,18 +889,18 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresu } if (d_raw.size() >= sizeof(dnsheader)) { - auto id = dq->getHeader()->id; - dq->getMutableData() = d_raw; - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [id](dnsheader& header) { - header.id = id; + auto questionId = dnsquestion->getHeader()->id; + dnsquestion->getMutableData() = d_raw; + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [questionId](dnsheader& header) { + header.id = questionId; return true; }); return Action::HeaderModify; } - vector addrs; - vector rawResponses; + std::vector addrs = {}; + std::vector rawResponses = {}; unsigned int totrdatalen = 0; - uint16_t numberOfRecords = 0; + size_t numberOfRecords = 0; if (!d_cname.empty()) { qtype = QType::CNAME; totrdatalen += d_cname.getStorage().size(); @@ -928,24 +933,25 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresu } unsigned int qnameWireLength = 0; - DNSName ignore(reinterpret_cast(dq->getData().data()), dq->getData().size(), sizeof(dnsheader), false, 0, 0, &qnameWireLength); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + DNSName ignore(reinterpret_cast(dnsquestion->getData().data()), dnsquestion->getData().size(), sizeof(dnsheader), false, nullptr, nullptr, &qnameWireLength); - if (dq->getMaximumSize() < (sizeof(dnsheader) + qnameWireLength + 4 + numberOfRecords * 12 /* recordstart */ + totrdatalen)) { + if (dnsquestion->getMaximumSize() < (sizeof(dnsheader) + qnameWireLength + 4 + numberOfRecords * 12 /* recordstart */ + totrdatalen)) { return Action::None; } bool dnssecOK = false; bool hadEDNS = false; - if (g_addEDNSToSelfGeneratedResponses && queryHasEDNS(*dq)) { + if (g_addEDNSToSelfGeneratedResponses && queryHasEDNS(*dnsquestion)) { hadEDNS = true; - dnssecOK = getEDNSZ(*dq) & EDNS_HEADER_FLAG_DO; + dnssecOK = ((getEDNSZ(*dnsquestion) & EDNS_HEADER_FLAG_DO) != 0); } - auto& data = dq->getMutableData(); + auto& data = dnsquestion->getMutableData(); data.resize(sizeof(dnsheader) + qnameWireLength + 4 + numberOfRecords * 12 /* recordstart */ + totrdatalen); // there goes your EDNS uint8_t* dest = &(data.at(sizeof(dnsheader) + qnameWireLength + 4)); - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [this](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [this](dnsheader& header) { header.qr = true; // for good measure setResponseHeadersFromConfig(header, d_responseConfig); header.ancount = 0; @@ -954,13 +960,15 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresu }); uint32_t ttl = htonl(d_responseConfig.ttl); - uint16_t qclass = htons(dq->ids.qclass); - unsigned char recordstart[] = {0xc0, 0x0c, // compressed name - 0, 0, // QTYPE - 0, 0, // QCLASS - 0, 0, 0, 0, // TTL - 0, 0}; // rdata length - static_assert(sizeof(recordstart) == 12, "sizeof(recordstart) must be equal to 12, otherwise the above check is invalid"); + uint16_t qclass = htons(dnsquestion->ids.qclass); + std::array recordstart = { + 0xc0, 0x0c, // compressed name + 0, 0, // QTYPE + 0, 0, // QCLASS + 0, 0, 0, 0, // TTL + 0, 0 // rdata length + }; + static_assert(recordstart.size() == 12, "sizeof(recordstart) must be equal to 12, otherwise the above check is invalid"); memcpy(&recordstart[4], &qclass, sizeof(qclass)); memcpy(&recordstart[6], &ttl, sizeof(ttl)); bool raw = false; @@ -972,10 +980,11 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresu memcpy(&recordstart[2], &qtype, sizeof(qtype)); memcpy(&recordstart[10], &rdataLen, sizeof(rdataLen)); - memcpy(dest, recordstart, sizeof(recordstart)); - dest += sizeof(recordstart); + memcpy(dest, recordstart.data(), recordstart.size()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + dest += recordstart.size(); memcpy(dest, wireData.c_str(), wireData.length()); - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [](dnsheader& header) { header.ancount++; return true; }); @@ -990,13 +999,15 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresu memcpy(&recordstart[2], &qtype, sizeof(qtype)); memcpy(&recordstart[10], &rdataLen, sizeof(rdataLen)); - memcpy(dest, recordstart, sizeof(recordstart)); - dest += sizeof(recordstart); + memcpy(dest, recordstart.data(), sizeof(recordstart)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + dest += recordstart.size(); memcpy(dest, rawResponse.c_str(), rawResponse.size()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) dest += rawResponse.size(); - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [](dnsheader& header) { header.ancount++; return true; }); @@ -1010,28 +1021,31 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresu memcpy(&recordstart[2], &qtype, sizeof(qtype)); memcpy(&recordstart[10], &rdataLen, sizeof(rdataLen)); - memcpy(dest, recordstart, sizeof(recordstart)); + memcpy(dest, recordstart.data(), recordstart.size()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) dest += sizeof(recordstart); memcpy(dest, + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) addr.sin4.sin_family == AF_INET ? reinterpret_cast(&addr.sin4.sin_addr.s_addr) : reinterpret_cast(&addr.sin6.sin6_addr.s6_addr), addr.sin4.sin_family == AF_INET ? sizeof(addr.sin4.sin_addr.s_addr) : sizeof(addr.sin6.sin6_addr.s6_addr)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) dest += (addr.sin4.sin_family == AF_INET ? sizeof(addr.sin4.sin_addr.s_addr) : sizeof(addr.sin6.sin6_addr.s6_addr)); - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [](dnsheader& header) { header.ancount++; return true; }); } } - auto finalANCount = dq->getHeader()->ancount; - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [finalANCount](dnsheader& header) { + auto finalANCount = dnsquestion->getHeader()->ancount; + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [finalANCount](dnsheader& header) { header.ancount = htons(finalANCount); return true; }); - if (hadEDNS && raw == false) { - addEDNS(dq->getMutableData(), dq->getMaximumSize(), dnssecOK, g_PayloadSizeSelfGenAnswers, 0); + if (hadEDNS && !raw) { + addEDNS(dnsquestion->getMutableData(), dnsquestion->getMaximumSize(), dnssecOK, g_PayloadSizeSelfGenAnswers, 0); } return Action::HeaderModify; @@ -1046,52 +1060,53 @@ public: { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { dnsdist::MacAddress mac; - int res = dnsdist::MacAddressesCache::get(dq->ids.origRemote, mac.data(), mac.size()); + int res = dnsdist::MacAddressesCache::get(dnsquestion->ids.origRemote, mac.data(), mac.size()); if (res != 0) { return Action::None; } std::string optRData; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) generateEDNSOption(d_code, reinterpret_cast(mac.data()), optRData); - if (dq->getHeader()->arcount) { + if (dnsquestion->getHeader()->arcount > 0) { bool ednsAdded = false; bool optionAdded = false; PacketBuffer newContent; - newContent.reserve(dq->getData().size()); + newContent.reserve(dnsquestion->getData().size()); - if (!slowRewriteEDNSOptionInQueryWithRecords(dq->getData(), newContent, ednsAdded, d_code, optionAdded, true, optRData)) { + if (!slowRewriteEDNSOptionInQueryWithRecords(dnsquestion->getData(), newContent, ednsAdded, d_code, optionAdded, true, optRData)) { return Action::None; } - if (newContent.size() > dq->getMaximumSize()) { + if (newContent.size() > dnsquestion->getMaximumSize()) { return Action::None; } - dq->getMutableData() = std::move(newContent); - if (!dq->ids.ednsAdded && ednsAdded) { - dq->ids.ednsAdded = true; + dnsquestion->getMutableData() = std::move(newContent); + if (!dnsquestion->ids.ednsAdded && ednsAdded) { + dnsquestion->ids.ednsAdded = true; } return Action::None; } - auto& data = dq->getMutableData(); - if (generateOptRR(optRData, data, dq->getMaximumSize(), g_EdnsUDPPayloadSize, 0, false)) { - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [](dnsheader& header) { + auto& data = dnsquestion->getMutableData(); + if (generateOptRR(optRData, data, dnsquestion->getMaximumSize(), g_EdnsUDPPayloadSize, 0, false)) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [](dnsheader& header) { header.arcount = htons(1); return true; }); // make sure that any EDNS sent by the backend is removed before forwarding the response to the client - dq->ids.ednsAdded = true; + dnsquestion->ids.ednsAdded = true; } return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "add EDNS MAC (code=" + std::to_string(d_code) + ")"; } @@ -1104,18 +1119,18 @@ class SetEDNSOptionAction : public DNSAction { public: // this action does not stop the processing - SetEDNSOptionAction(uint16_t code, const std::string& data) : - d_code(code), d_data(data) + SetEDNSOptionAction(uint16_t code, std::string data) : + d_code(code), d_data(std::move(data)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - setEDNSOption(*dq, d_code, d_data); + setEDNSOption(*dnsquestion, d_code, d_data); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "add EDNS Option (code=" + std::to_string(d_code) + ")"; } @@ -1129,15 +1144,15 @@ class SetNoRecurseAction : public DNSAction { public: // this action does not stop the processing - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [](dnsheader& header) { header.rd = false; return true; }); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set rd=0"; } @@ -1147,9 +1162,7 @@ class LogAction : public DNSAction, public boost::noncopyable { public: // this action does not stop the processing - LogAction() - { - } + LogAction() = default; LogAction(const std::string& str, bool binary = true, bool append = false, bool buffered = true, bool verboseOnly = true, bool includeTimestamp = false) : d_fname(str), d_binary(binary), d_verboseOnly(verboseOnly), d_includeTimestamp(includeTimestamp), d_append(append), d_buffered(buffered) @@ -1163,54 +1176,54 @@ public: } } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - auto fp = std::atomic_load_explicit(&d_fp, std::memory_order_acquire); - if (!fp) { + auto filepointer = std::atomic_load_explicit(&d_fp, std::memory_order_acquire); + if (!filepointer) { if (!d_verboseOnly || g_verbose) { if (d_includeTimestamp) { - infolog("[%u.%u] Packet from %s for %s %s with id %d", static_cast(dq->getQueryRealTime().tv_sec), static_cast(dq->getQueryRealTime().tv_nsec), dq->ids.origRemote.toStringWithPort(), dq->ids.qname.toString(), QType(dq->ids.qtype).toString(), dq->getHeader()->id); + infolog("[%u.%u] Packet from %s for %s %s with id %d", static_cast(dnsquestion->getQueryRealTime().tv_sec), static_cast(dnsquestion->getQueryRealTime().tv_nsec), dnsquestion->ids.origRemote.toStringWithPort(), dnsquestion->ids.qname.toString(), QType(dnsquestion->ids.qtype).toString(), dnsquestion->getHeader()->id); } else { - infolog("Packet from %s for %s %s with id %d", dq->ids.origRemote.toStringWithPort(), dq->ids.qname.toString(), QType(dq->ids.qtype).toString(), dq->getHeader()->id); + infolog("Packet from %s for %s %s with id %d", dnsquestion->ids.origRemote.toStringWithPort(), dnsquestion->ids.qname.toString(), QType(dnsquestion->ids.qtype).toString(), dnsquestion->getHeader()->id); } } } else { if (d_binary) { - const auto& out = dq->ids.qname.getStorage(); + const auto& out = dnsquestion->ids.qname.getStorage(); if (d_includeTimestamp) { - uint64_t tv_sec = static_cast(dq->getQueryRealTime().tv_sec); - uint32_t tv_nsec = static_cast(dq->getQueryRealTime().tv_nsec); - fwrite(&tv_sec, sizeof(tv_sec), 1, fp.get()); - fwrite(&tv_nsec, sizeof(tv_nsec), 1, fp.get()); + auto tv_sec = static_cast(dnsquestion->getQueryRealTime().tv_sec); + auto tv_nsec = static_cast(dnsquestion->getQueryRealTime().tv_nsec); + fwrite(&tv_sec, sizeof(tv_sec), 1, filepointer.get()); + fwrite(&tv_nsec, sizeof(tv_nsec), 1, filepointer.get()); } - uint16_t id = dq->getHeader()->id; - fwrite(&id, sizeof(id), 1, fp.get()); - fwrite(out.c_str(), 1, out.size(), fp.get()); - fwrite(&dq->ids.qtype, sizeof(dq->ids.qtype), 1, fp.get()); - fwrite(&dq->ids.origRemote.sin4.sin_family, sizeof(dq->ids.origRemote.sin4.sin_family), 1, fp.get()); - if (dq->ids.origRemote.sin4.sin_family == AF_INET) { - fwrite(&dq->ids.origRemote.sin4.sin_addr.s_addr, sizeof(dq->ids.origRemote.sin4.sin_addr.s_addr), 1, fp.get()); + uint16_t queryId = dnsquestion->getHeader()->id; + fwrite(&queryId, sizeof(queryId), 1, filepointer.get()); + fwrite(out.c_str(), 1, out.size(), filepointer.get()); + fwrite(&dnsquestion->ids.qtype, sizeof(dnsquestion->ids.qtype), 1, filepointer.get()); + fwrite(&dnsquestion->ids.origRemote.sin4.sin_family, sizeof(dnsquestion->ids.origRemote.sin4.sin_family), 1, filepointer.get()); + if (dnsquestion->ids.origRemote.sin4.sin_family == AF_INET) { + fwrite(&dnsquestion->ids.origRemote.sin4.sin_addr.s_addr, sizeof(dnsquestion->ids.origRemote.sin4.sin_addr.s_addr), 1, filepointer.get()); } - else if (dq->ids.origRemote.sin4.sin_family == AF_INET6) { - fwrite(&dq->ids.origRemote.sin6.sin6_addr.s6_addr, sizeof(dq->ids.origRemote.sin6.sin6_addr.s6_addr), 1, fp.get()); + else if (dnsquestion->ids.origRemote.sin4.sin_family == AF_INET6) { + fwrite(&dnsquestion->ids.origRemote.sin6.sin6_addr.s6_addr, sizeof(dnsquestion->ids.origRemote.sin6.sin6_addr.s6_addr), 1, filepointer.get()); } - fwrite(&dq->ids.origRemote.sin4.sin_port, sizeof(dq->ids.origRemote.sin4.sin_port), 1, fp.get()); + fwrite(&dnsquestion->ids.origRemote.sin4.sin_port, sizeof(dnsquestion->ids.origRemote.sin4.sin_port), 1, filepointer.get()); } else { if (d_includeTimestamp) { - fprintf(fp.get(), "[%llu.%lu] Packet from %s for %s %s with id %u\n", static_cast(dq->getQueryRealTime().tv_sec), static_cast(dq->getQueryRealTime().tv_nsec), dq->ids.origRemote.toStringWithPort().c_str(), dq->ids.qname.toString().c_str(), QType(dq->ids.qtype).toString().c_str(), dq->getHeader()->id); + fprintf(filepointer.get(), "[%llu.%lu] Packet from %s for %s %s with id %u\n", static_cast(dnsquestion->getQueryRealTime().tv_sec), static_cast(dnsquestion->getQueryRealTime().tv_nsec), dnsquestion->ids.origRemote.toStringWithPort().c_str(), dnsquestion->ids.qname.toString().c_str(), QType(dnsquestion->ids.qtype).toString().c_str(), dnsquestion->getHeader()->id); } else { - fprintf(fp.get(), "Packet from %s for %s %s with id %u\n", dq->ids.origRemote.toStringWithPort().c_str(), dq->ids.qname.toString().c_str(), QType(dq->ids.qtype).toString().c_str(), dq->getHeader()->id); + fprintf(filepointer.get(), "Packet from %s for %s %s with id %u\n", dnsquestion->ids.origRemote.toStringWithPort().c_str(), dnsquestion->ids.qname.toString().c_str(), QType(dnsquestion->ids.qtype).toString().c_str(), dnsquestion->getHeader()->id); } } } return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { if (!d_fname.empty()) { return "log to " + d_fname; @@ -1231,20 +1244,21 @@ private: // we are using a naked pointer here because we don't want fclose to be called // with a nullptr, which would happen if we constructor a shared_ptr with fclose // as a custom deleter and nullptr as a FILE* - auto nfp = fopen(d_fname.c_str(), d_append ? "a+" : "w"); - if (!nfp) { + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + auto* nfp = fopen(d_fname.c_str(), d_append ? "a+" : "w"); + if (nfp == nullptr) { /* don't fall on our sword when reopening */ return false; } - auto fp = std::shared_ptr(nfp, fclose); + auto filepointer = std::shared_ptr(nfp, fclose); nfp = nullptr; if (!d_buffered) { - setbuf(fp.get(), 0); + setbuf(filepointer.get(), nullptr); } - std::atomic_store_explicit(&d_fp, std::move(fp), std::memory_order_release); + std::atomic_store_explicit(&d_fp, std::move(filepointer), std::memory_order_release); return true; } @@ -1260,9 +1274,7 @@ private: class LogResponseAction : public DNSResponseAction, public boost::noncopyable { public: - LogResponseAction() - { - } + LogResponseAction() = default; LogResponseAction(const std::string& str, bool append = false, bool buffered = true, bool verboseOnly = true, bool includeTimestamp = false) : d_fname(str), d_verboseOnly(verboseOnly), d_includeTimestamp(includeTimestamp), d_append(append), d_buffered(buffered) @@ -1276,31 +1288,31 @@ public: } } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - auto fp = std::atomic_load_explicit(&d_fp, std::memory_order_acquire); - if (!fp) { + auto filepointer = std::atomic_load_explicit(&d_fp, std::memory_order_acquire); + if (!filepointer) { if (!d_verboseOnly || g_verbose) { if (d_includeTimestamp) { - infolog("[%u.%u] Answer to %s for %s %s (%s) with id %u", static_cast(dr->getQueryRealTime().tv_sec), static_cast(dr->getQueryRealTime().tv_nsec), dr->ids.origRemote.toStringWithPort(), dr->ids.qname.toString(), QType(dr->ids.qtype).toString(), RCode::to_s(dr->getHeader()->rcode), dr->getHeader()->id); + infolog("[%u.%u] Answer to %s for %s %s (%s) with id %u", static_cast(response->getQueryRealTime().tv_sec), static_cast(response->getQueryRealTime().tv_nsec), response->ids.origRemote.toStringWithPort(), response->ids.qname.toString(), QType(response->ids.qtype).toString(), RCode::to_s(response->getHeader()->rcode), response->getHeader()->id); } else { - infolog("Answer to %s for %s %s (%s) with id %u", dr->ids.origRemote.toStringWithPort(), dr->ids.qname.toString(), QType(dr->ids.qtype).toString(), RCode::to_s(dr->getHeader()->rcode), dr->getHeader()->id); + infolog("Answer to %s for %s %s (%s) with id %u", response->ids.origRemote.toStringWithPort(), response->ids.qname.toString(), QType(response->ids.qtype).toString(), RCode::to_s(response->getHeader()->rcode), response->getHeader()->id); } } } else { if (d_includeTimestamp) { - fprintf(fp.get(), "[%llu.%lu] Answer to %s for %s %s (%s) with id %u\n", static_cast(dr->getQueryRealTime().tv_sec), static_cast(dr->getQueryRealTime().tv_nsec), dr->ids.origRemote.toStringWithPort().c_str(), dr->ids.qname.toString().c_str(), QType(dr->ids.qtype).toString().c_str(), RCode::to_s(dr->getHeader()->rcode).c_str(), dr->getHeader()->id); + fprintf(filepointer.get(), "[%llu.%lu] Answer to %s for %s %s (%s) with id %u\n", static_cast(response->getQueryRealTime().tv_sec), static_cast(response->getQueryRealTime().tv_nsec), response->ids.origRemote.toStringWithPort().c_str(), response->ids.qname.toString().c_str(), QType(response->ids.qtype).toString().c_str(), RCode::to_s(response->getHeader()->rcode).c_str(), response->getHeader()->id); } else { - fprintf(fp.get(), "Answer to %s for %s %s (%s) with id %u\n", dr->ids.origRemote.toStringWithPort().c_str(), dr->ids.qname.toString().c_str(), QType(dr->ids.qtype).toString().c_str(), RCode::to_s(dr->getHeader()->rcode).c_str(), dr->getHeader()->id); + fprintf(filepointer.get(), "Answer to %s for %s %s (%s) with id %u\n", response->ids.origRemote.toStringWithPort().c_str(), response->ids.qname.toString().c_str(), QType(response->ids.qtype).toString().c_str(), RCode::to_s(response->getHeader()->rcode).c_str(), response->getHeader()->id); } } return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { if (!d_fname.empty()) { return "log to " + d_fname; @@ -1321,20 +1333,21 @@ private: // we are using a naked pointer here because we don't want fclose to be called // with a nullptr, which would happen if we constructor a shared_ptr with fclose // as a custom deleter and nullptr as a FILE* - auto nfp = fopen(d_fname.c_str(), d_append ? "a+" : "w"); - if (!nfp) { + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + auto* nfp = fopen(d_fname.c_str(), d_append ? "a+" : "w"); + if (nfp == nullptr) { /* don't fall on our sword when reopening */ return false; } - auto fp = std::shared_ptr(nfp, fclose); + auto filepointer = std::shared_ptr(nfp, fclose); nfp = nullptr; if (!d_buffered) { - setbuf(fp.get(), 0); + setbuf(filepointer.get(), nullptr); } - std::atomic_store_explicit(&d_fp, std::move(fp), std::memory_order_release); + std::atomic_store_explicit(&d_fp, std::move(filepointer), std::memory_order_release); return true; } @@ -1350,15 +1363,15 @@ class SetDisableValidationAction : public DNSAction { public: // this action does not stop the processing - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [](dnsheader& header) { header.cd = true; return true; }); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set cd=1"; } @@ -1368,12 +1381,12 @@ class SetSkipCacheAction : public DNSAction { public: // this action does not stop the processing - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->ids.skipCache = true; + dnsquestion->ids.skipCache = true; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "skip cache"; } @@ -1382,12 +1395,12 @@ public: class SetSkipCacheResponseAction : public DNSResponseAction { public: - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - dr->ids.skipCache = true; + response->ids.skipCache = true; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "skip cache"; } @@ -1401,12 +1414,12 @@ public: d_ttl(ttl) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->ids.tempFailureTTL = d_ttl; + dnsquestion->ids.tempFailureTTL = d_ttl; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set tempfailure cache ttl to " + std::to_string(d_ttl); } @@ -1423,12 +1436,12 @@ public: d_v4PrefixLength(v4Length), d_v6PrefixLength(v6Length) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->ecsPrefixLength = dq->ids.origRemote.sin4.sin_family == AF_INET ? d_v4PrefixLength : d_v6PrefixLength; + dnsquestion->ecsPrefixLength = dnsquestion->ids.origRemote.sin4.sin_family == AF_INET ? d_v4PrefixLength : d_v6PrefixLength; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set ECS prefix length to " + std::to_string(d_v4PrefixLength) + "/" + std::to_string(d_v6PrefixLength); } @@ -1446,14 +1459,14 @@ public: d_ecsOverride(ecsOverride) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->ecsOverride = d_ecsOverride; + dnsquestion->ecsOverride = d_ecsOverride; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { - return "set ECS override to " + std::to_string(d_ecsOverride); + return "set ECS override to " + std::to_string(static_cast(d_ecsOverride)); } private: @@ -1464,12 +1477,12 @@ class SetDisableECSAction : public DNSAction { public: // this action does not stop the processing - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->useECS = false; + dnsquestion->useECS = false; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "disable ECS"; } @@ -1479,29 +1492,29 @@ class SetECSAction : public DNSAction { public: // this action does not stop the processing - SetECSAction(const Netmask& v4) : - d_v4(v4), d_hasV6(false) + SetECSAction(const Netmask& v4Netmask) : + d_v4(v4Netmask), d_hasV6(false) { } - SetECSAction(const Netmask& v4, const Netmask& v6) : - d_v4(v4), d_v6(v6), d_hasV6(true) + SetECSAction(const Netmask& v4Netmask, const Netmask& v6Netmask) : + d_v4(v4Netmask), d_v6(v6Netmask), d_hasV6(true) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { if (d_hasV6) { - dq->ecs = std::make_unique(dq->ids.origRemote.isIPv4() ? d_v4 : d_v6); + dnsquestion->ecs = std::make_unique(dnsquestion->ids.origRemote.isIPv4() ? d_v4 : d_v6); } else { - dq->ecs = std::make_unique(d_v4); + dnsquestion->ecs = std::make_unique(d_v4); } return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { std::string result = "set ECS to " + d_v4.toString(); if (d_hasV6) { @@ -1522,44 +1535,44 @@ static DnstapMessage::ProtocolType ProtocolToDNSTap(dnsdist::Protocol protocol) if (protocol == dnsdist::Protocol::DoUDP) { return DnstapMessage::ProtocolType::DoUDP; } - else if (protocol == dnsdist::Protocol::DoTCP) { + if (protocol == dnsdist::Protocol::DoTCP) { return DnstapMessage::ProtocolType::DoTCP; } - else if (protocol == dnsdist::Protocol::DoT) { + if (protocol == dnsdist::Protocol::DoT) { return DnstapMessage::ProtocolType::DoT; } - else if (protocol == dnsdist::Protocol::DoH || protocol == dnsdist::Protocol::DoH3) { + if (protocol == dnsdist::Protocol::DoH || protocol == dnsdist::Protocol::DoH3) { return DnstapMessage::ProtocolType::DoH; } - else if (protocol == dnsdist::Protocol::DNSCryptUDP) { + if (protocol == dnsdist::Protocol::DNSCryptUDP) { return DnstapMessage::ProtocolType::DNSCryptUDP; } - else if (protocol == dnsdist::Protocol::DNSCryptTCP) { + if (protocol == dnsdist::Protocol::DNSCryptTCP) { return DnstapMessage::ProtocolType::DNSCryptTCP; } - else if (protocol == dnsdist::Protocol::DoQ) { + if (protocol == dnsdist::Protocol::DoQ) { return DnstapMessage::ProtocolType::DoQ; } throw std::runtime_error("Unhandled protocol for dnstap: " + protocol.toPrettyString()); } -static void remoteLoggerQueueData(RemoteLoggerInterface& r, const std::string& data) +static void remoteLoggerQueueData(RemoteLoggerInterface& remoteLogger, const std::string& data) { - auto ret = r.queueData(data); + auto ret = remoteLogger.queueData(data); switch (ret) { case RemoteLoggerInterface::Result::Queued: break; case RemoteLoggerInterface::Result::PipeFull: { - vinfolog("%s: %s", r.name(), RemoteLoggerInterface::toErrorString(ret)); + vinfolog("%s: %s", remoteLogger.name(), RemoteLoggerInterface::toErrorString(ret)); break; } case RemoteLoggerInterface::Result::TooLarge: { - warnlog("%s: %s", r.name(), RemoteLoggerInterface::toErrorString(ret)); + warnlog("%s: %s", remoteLogger.name(), RemoteLoggerInterface::toErrorString(ret)); break; } case RemoteLoggerInterface::Result::OtherError: - warnlog("%s: %s", r.name(), RemoteLoggerInterface::toErrorString(ret)); + warnlog("%s: %s", remoteLogger.name(), RemoteLoggerInterface::toErrorString(ret)); } } @@ -1567,29 +1580,31 @@ class DnstapLogAction : public DNSAction, public boost::noncopyable { public: // this action does not stop the processing - DnstapLogAction(const std::string& identity, std::shared_ptr& logger, boost::optional> alterFunc) : - d_identity(identity), d_logger(logger), d_alterFunc(std::move(alterFunc)) + DnstapLogAction(std::string identity, std::shared_ptr& logger, boost::optional> alterFunc) : + d_identity(std::move(identity)), d_logger(logger), d_alterFunc(std::move(alterFunc)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { static thread_local std::string data; data.clear(); - DnstapMessage::ProtocolType protocol = ProtocolToDNSTap(dq->getProtocol()); - DnstapMessage message(data, !dq->getHeader()->qr ? DnstapMessage::MessageType::client_query : DnstapMessage::MessageType::client_response, d_identity, &dq->ids.origRemote, &dq->ids.origDest, protocol, reinterpret_cast(dq->getData().data()), dq->getData().size(), &dq->getQueryRealTime(), nullptr); + DnstapMessage::ProtocolType protocol = ProtocolToDNSTap(dnsquestion->getProtocol()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + DnstapMessage message(std::move(data), !dnsquestion->getHeader()->qr ? DnstapMessage::MessageType::client_query : DnstapMessage::MessageType::client_response, d_identity, &dnsquestion->ids.origRemote, &dnsquestion->ids.origDest, protocol, reinterpret_cast(dnsquestion->getData().data()), dnsquestion->getData().size(), &dnsquestion->getQueryRealTime(), nullptr); { if (d_alterFunc) { auto lock = g_lua.lock(); - (*d_alterFunc)(dq, &message); + (*d_alterFunc)(dnsquestion, &message); } } + data = message.getBuffer(); remoteLoggerQueueData(*d_logger, data); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "remote log as dnstap to " + (d_logger ? d_logger->toString() : ""); } @@ -1602,20 +1617,20 @@ private: namespace { -void addMetaDataToProtobuf(DNSDistProtoBufMessage& message, const DNSQuestion& dq, const std::vector>& metas) +void addMetaDataToProtobuf(DNSDistProtoBufMessage& message, const DNSQuestion& dnsquestion, const std::vector>& metas) { for (const auto& [name, meta] : metas) { - message.addMeta(name, meta.getValues(dq), {}); + message.addMeta(name, meta.getValues(dnsquestion), {}); } } -void addTagsToProtobuf(DNSDistProtoBufMessage& message, const DNSQuestion& dq, const std::unordered_set& allowed) +void addTagsToProtobuf(DNSDistProtoBufMessage& message, const DNSQuestion& dnsquestion, const std::unordered_set& allowed) { - if (!dq.ids.qTag) { + if (!dnsquestion.ids.qTag) { return; } - for (const auto& [key, value] : *dq.ids.qTag) { + for (const auto& [key, value] : *dnsquestion.ids.qTag) { if (!allowed.empty() && allowed.count(key) == 0) { continue; } @@ -1624,14 +1639,17 @@ void addTagsToProtobuf(DNSDistProtoBufMessage& message, const DNSQuestion& dq, c message.addTag(key); } else { - message.addTag(key + ":" + value); + auto tag = key; + tag.append(":"); + tag.append(value); + message.addTag(tag); } } } -void addExtendedDNSErrorToProtobuf(DNSDistProtoBufMessage& message, const DNSResponse& dr, const std::string& metaKey) +void addExtendedDNSErrorToProtobuf(DNSDistProtoBufMessage& message, const DNSResponse& response, const std::string& metaKey) { - auto [infoCode, extraText] = dnsdist::edns::getExtendedDNSError(dr.getData()); + auto [infoCode, extraText] = dnsdist::edns::getExtendedDNSError(response.getData()); if (!infoCode) { return; } @@ -1667,35 +1685,35 @@ public: { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - if (!dq->ids.d_protoBufData) { - dq->ids.d_protoBufData = std::make_unique(); + if (!dnsquestion->ids.d_protoBufData) { + dnsquestion->ids.d_protoBufData = std::make_unique(); } - if (!dq->ids.d_protoBufData->uniqueId) { - dq->ids.d_protoBufData->uniqueId = getUniqueID(); + if (!dnsquestion->ids.d_protoBufData->uniqueId) { + dnsquestion->ids.d_protoBufData->uniqueId = getUniqueID(); } - DNSDistProtoBufMessage message(*dq); + DNSDistProtoBufMessage message(*dnsquestion); if (!d_serverID.empty()) { message.setServerIdentity(d_serverID); } #if HAVE_IPCIPHER if (!d_ipEncryptKey.empty()) { - message.setRequestor(encryptCA(dq->ids.origRemote, d_ipEncryptKey)); + message.setRequestor(encryptCA(dnsquestion->ids.origRemote, d_ipEncryptKey)); } #endif /* HAVE_IPCIPHER */ if (d_tagsToExport) { - addTagsToProtobuf(message, *dq, *d_tagsToExport); + addTagsToProtobuf(message, *dnsquestion, *d_tagsToExport); } - addMetaDataToProtobuf(message, *dq, d_metas); + addMetaDataToProtobuf(message, *dnsquestion, d_metas); if (d_alterFunc) { auto lock = g_lua.lock(); - (*d_alterFunc)(dq, &message); + (*d_alterFunc)(dnsquestion, &message); } static thread_local std::string data; @@ -1705,7 +1723,7 @@ public: return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "remote log to " + (d_logger ? d_logger->toString() : ""); } @@ -1725,19 +1743,19 @@ class SNMPTrapAction : public DNSAction { public: // this action does not stop the processing - SNMPTrapAction(const std::string& reason) : - d_reason(reason) + SNMPTrapAction(std::string reason) : + d_reason(std::move(reason)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - if (g_snmpAgent && g_snmpTrapsEnabled) { - g_snmpAgent->sendDNSTrap(*dq, d_reason); + if (g_snmpAgent != nullptr && g_snmpTrapsEnabled) { + g_snmpAgent->sendDNSTrap(*dnsquestion, d_reason); } return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "send SNMP trap"; } @@ -1750,17 +1768,17 @@ class SetTagAction : public DNSAction { public: // this action does not stop the processing - SetTagAction(const std::string& tag, const std::string& value) : - d_tag(tag), d_value(value) + SetTagAction(std::string tag, std::string value) : + d_tag(std::move(tag)), d_value(std::move(value)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->setTag(d_tag, d_value); + dnsquestion->setTag(d_tag, d_value); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set tag '" + d_tag + "' to value '" + d_value + "'"; } @@ -1775,31 +1793,33 @@ class DnstapLogResponseAction : public DNSResponseAction, public boost::noncopya { public: // this action does not stop the processing - DnstapLogResponseAction(const std::string& identity, std::shared_ptr& logger, boost::optional> alterFunc) : - d_identity(identity), d_logger(logger), d_alterFunc(std::move(alterFunc)) + DnstapLogResponseAction(std::string identity, std::shared_ptr& logger, boost::optional> alterFunc) : + d_identity(std::move(identity)), d_logger(logger), d_alterFunc(std::move(alterFunc)) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { static thread_local std::string data; - struct timespec now; + struct timespec now = {}; gettime(&now, true); data.clear(); - DnstapMessage::ProtocolType protocol = ProtocolToDNSTap(dr->getProtocol()); - DnstapMessage message(data, DnstapMessage::MessageType::client_response, d_identity, &dr->ids.origRemote, &dr->ids.origDest, protocol, reinterpret_cast(dr->getData().data()), dr->getData().size(), &dr->getQueryRealTime(), &now); + DnstapMessage::ProtocolType protocol = ProtocolToDNSTap(response->getProtocol()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + DnstapMessage message(std::move(data), DnstapMessage::MessageType::client_response, d_identity, &response->ids.origRemote, &response->ids.origDest, protocol, reinterpret_cast(response->getData().data()), response->getData().size(), &response->getQueryRealTime(), &now); { if (d_alterFunc) { auto lock = g_lua.lock(); - (*d_alterFunc)(dr, &message); + (*d_alterFunc)(response, &message); } } + data = message.getBuffer(); remoteLoggerQueueData(*d_logger, data); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "log response as dnstap to " + (d_logger ? d_logger->toString() : ""); } @@ -1818,39 +1838,39 @@ public: d_tagsToExport(std::move(config.tagsToExport)), d_metas(std::move(config.metas)), d_logger(config.logger), d_alterFunc(std::move(config.alterResponseFunc)), d_serverID(config.serverID), d_ipEncryptKey(config.ipEncryptKey), d_exportExtendedErrorsToMeta(std::move(config.exportExtendedErrorsToMeta)), d_includeCNAME(config.includeCNAME) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - if (!dr->ids.d_protoBufData) { - dr->ids.d_protoBufData = std::make_unique(); + if (!response->ids.d_protoBufData) { + response->ids.d_protoBufData = std::make_unique(); } - if (!dr->ids.d_protoBufData->uniqueId) { - dr->ids.d_protoBufData->uniqueId = getUniqueID(); + if (!response->ids.d_protoBufData->uniqueId) { + response->ids.d_protoBufData->uniqueId = getUniqueID(); } - DNSDistProtoBufMessage message(*dr, d_includeCNAME); + DNSDistProtoBufMessage message(*response, d_includeCNAME); if (!d_serverID.empty()) { message.setServerIdentity(d_serverID); } #if HAVE_IPCIPHER if (!d_ipEncryptKey.empty()) { - message.setRequestor(encryptCA(dr->ids.origRemote, d_ipEncryptKey)); + message.setRequestor(encryptCA(response->ids.origRemote, d_ipEncryptKey)); } #endif /* HAVE_IPCIPHER */ if (d_tagsToExport) { - addTagsToProtobuf(message, *dr, *d_tagsToExport); + addTagsToProtobuf(message, *response, *d_tagsToExport); } - addMetaDataToProtobuf(message, *dr, d_metas); + addMetaDataToProtobuf(message, *response, d_metas); if (d_exportExtendedErrorsToMeta) { - addExtendedDNSErrorToProtobuf(message, *dr, *d_exportExtendedErrorsToMeta); + addExtendedDNSErrorToProtobuf(message, *response, *d_exportExtendedErrorsToMeta); } if (d_alterFunc) { auto lock = g_lua.lock(); - (*d_alterFunc)(dr, &message); + (*d_alterFunc)(response, &message); } static thread_local std::string data; @@ -1860,7 +1880,7 @@ public: return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "remote log response to " + (d_logger ? d_logger->toString() : ""); } @@ -1881,11 +1901,11 @@ private: class DropResponseAction : public DNSResponseAction { public: - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { return Action::Drop; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "drop"; } @@ -1894,11 +1914,11 @@ public: class AllowResponseAction : public DNSResponseAction { public: - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { return Action::Allow; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "allow"; } @@ -1911,12 +1931,12 @@ public: d_msec(msec) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { *ruleresult = std::to_string(d_msec); return Action::Delay; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "delay by " + std::to_string(d_msec) + " ms"; } @@ -1930,19 +1950,19 @@ class SNMPTrapResponseAction : public DNSResponseAction { public: // this action does not stop the processing - SNMPTrapResponseAction(const std::string& reason) : - d_reason(reason) + SNMPTrapResponseAction(std::string reason) : + d_reason(std::move(reason)) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { if (g_snmpAgent && g_snmpTrapsEnabled) { - g_snmpAgent->sendDNSTrap(*dr, d_reason); + g_snmpAgent->sendDNSTrap(*response, d_reason); } return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "send SNMP trap"; } @@ -1956,17 +1976,17 @@ class SetTagResponseAction : public DNSResponseAction { public: // this action does not stop the processing - SetTagResponseAction(const std::string& tag, const std::string& value) : - d_tag(tag), d_value(value) + SetTagResponseAction(std::string tag, std::string value) : + d_tag(std::move(tag)), d_value(std::move(value)) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - dr->setTag(d_tag, d_value); + response->setTag(d_tag, d_value); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set tag '" + d_tag + "' to value '" + d_value + "'"; } @@ -1979,20 +1999,20 @@ private: class ClearRecordTypesResponseAction : public DNSResponseAction, public boost::noncopyable { public: - ClearRecordTypesResponseAction(const std::unordered_set& qtypes) : - d_qtypes(qtypes) + ClearRecordTypesResponseAction(std::unordered_set qtypes) : + d_qtypes(std::move(qtypes)) { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - if (d_qtypes.size() > 0) { - clearDNSPacketRecordTypes(dr->getMutableData(), d_qtypes); + if (!d_qtypes.empty()) { + clearDNSPacketRecordTypes(response->getMutableData(), d_qtypes); } return DNSResponseAction::Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "clear record types"; } @@ -2010,28 +2030,26 @@ public: { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { if (d_action) { /* call the action */ - auto action = (*d_action)(dq, ruleresult); + auto action = (*d_action)(dnsquestion, ruleresult); bool drop = false; /* apply the changes if needed (pool selection, flags, etc */ - processRulesResult(action, *dq, *ruleresult, drop); + processRulesResult(action, *dnsquestion, *ruleresult, drop); } /* but ignore the resulting action no matter what */ return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { if (d_action) { return "continue after: " + (d_action ? d_action->toString() : ""); } - else { - return "no op"; - } + return "no op"; } private: @@ -2042,19 +2060,19 @@ private: class HTTPStatusAction : public DNSAction { public: - HTTPStatusAction(int code, const PacketBuffer& body, const std::string& contentType) : - d_body(body), d_contentType(contentType), d_code(code) + HTTPStatusAction(int code, PacketBuffer body, std::string contentType) : + d_body(std::move(body)), d_contentType(std::move(contentType)), d_code(code) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - if (!dq->ids.du) { + if (!dnsquestion->ids.du) { return Action::None; } - dq->ids.du->setHTTPResponse(d_code, PacketBuffer(d_body), d_contentType); - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [this](dnsheader& header) { + dnsquestion->ids.du->setHTTPResponse(d_code, PacketBuffer(d_body), d_contentType); + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [this](dnsheader& header) { header.qr = true; // for good measure setResponseHeadersFromConfig(header, d_responseConfig); return true; @@ -2062,14 +2080,18 @@ public: return Action::HeaderModify; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "return an HTTP status of " + std::to_string(d_code); } - ResponseConfig d_responseConfig; + [[nodiscard]] ResponseConfig& getResponseConfig() + { + return d_responseConfig; + } private: + ResponseConfig d_responseConfig; PacketBuffer d_body; std::string d_contentType; int d_code; @@ -2081,27 +2103,27 @@ class KeyValueStoreLookupAction : public DNSAction { public: // this action does not stop the processing - KeyValueStoreLookupAction(std::shared_ptr& kvs, std::shared_ptr& lookupKey, const std::string& destinationTag) : - d_kvs(kvs), d_key(lookupKey), d_tag(destinationTag) + KeyValueStoreLookupAction(std::shared_ptr& kvs, std::shared_ptr& lookupKey, std::string destinationTag) : + d_kvs(kvs), d_key(lookupKey), d_tag(std::move(destinationTag)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - std::vector keys = d_key->getKeys(*dq); + std::vector keys = d_key->getKeys(*dnsquestion); std::string result; for (const auto& key : keys) { - if (d_kvs->getValue(key, result) == true) { + if (d_kvs->getValue(key, result)) { break; } } - dq->setTag(d_tag, std::move(result)); + dnsquestion->setTag(d_tag, std::move(result)); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "lookup key-value store based on '" + d_key->toString() + "' and set the result in tag '" + d_tag + "'"; } @@ -2116,27 +2138,27 @@ class KeyValueStoreRangeLookupAction : public DNSAction { public: // this action does not stop the processing - KeyValueStoreRangeLookupAction(std::shared_ptr& kvs, std::shared_ptr& lookupKey, const std::string& destinationTag) : - d_kvs(kvs), d_key(lookupKey), d_tag(destinationTag) + KeyValueStoreRangeLookupAction(std::shared_ptr& kvs, std::shared_ptr& lookupKey, std::string destinationTag) : + d_kvs(kvs), d_key(lookupKey), d_tag(std::move(destinationTag)) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - std::vector keys = d_key->getKeys(*dq); + std::vector keys = d_key->getKeys(*dnsquestion); std::string result; for (const auto& key : keys) { - if (d_kvs->getRangeValue(key, result) == true) { + if (d_kvs->getRangeValue(key, result)) { break; } } - dq->setTag(d_tag, std::move(result)); + dnsquestion->setTag(d_tag, std::move(result)); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "do a range-based lookup in key-value store based on '" + d_key->toString() + "' and set the result in tag '" + d_tag + "'"; } @@ -2156,13 +2178,13 @@ public: { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - dq->ids.ttlCap = d_cap; + dnsquestion->ids.ttlCap = d_cap; return DNSAction::Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "cap the TTL of the returned response to " + std::to_string(d_cap); } @@ -2179,13 +2201,13 @@ public: { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { - dr->ids.ttlCap = d_cap; + response->ids.ttlCap = d_cap; return DNSResponseAction::Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "cap the TTL of the returned response to " + std::to_string(d_cap); } @@ -2197,18 +2219,27 @@ private: class NegativeAndSOAAction : public DNSAction { public: - NegativeAndSOAAction(bool nxd, const DNSName& zone, uint32_t ttl, const DNSName& mname, const DNSName& rname, uint32_t serial, uint32_t refresh, uint32_t retry, uint32_t expire, uint32_t minimum, bool soaInAuthoritySection) : - d_zone(zone), d_mname(mname), d_rname(rname), d_ttl(ttl), d_serial(serial), d_refresh(refresh), d_retry(retry), d_expire(expire), d_minimum(minimum), d_nxd(nxd), d_soaInAuthoritySection(soaInAuthoritySection) + struct SOAParams + { + uint32_t serial; + uint32_t refresh; + uint32_t retry; + uint32_t expire; + uint32_t minimum; + }; + + NegativeAndSOAAction(bool nxd, DNSName zone, uint32_t ttl, DNSName mname, DNSName rname, SOAParams params, bool soaInAuthoritySection) : + d_zone(std::move(zone)), d_mname(std::move(mname)), d_rname(std::move(rname)), d_ttl(ttl), d_params(params), d_nxd(nxd), d_soaInAuthoritySection(soaInAuthoritySection) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - if (!setNegativeAndAdditionalSOA(*dq, d_nxd, d_zone, d_ttl, d_mname, d_rname, d_serial, d_refresh, d_retry, d_expire, d_minimum, d_soaInAuthoritySection)) { + if (!setNegativeAndAdditionalSOA(*dnsquestion, d_nxd, d_zone, d_ttl, d_mname, d_rname, d_params.serial, d_params.refresh, d_params.retry, d_params.expire, d_params.minimum, d_soaInAuthoritySection)) { return Action::None; } - dnsdist::PacketMangling::editDNSHeaderFromPacket(dq->getMutableData(), [this](dnsheader& header) { + dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsquestion->getMutableData(), [this](dnsheader& header) { setResponseHeadersFromConfig(header, d_responseConfig); return true; }); @@ -2216,23 +2247,23 @@ public: return Action::Allow; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return std::string(d_nxd ? "NXD " : "NODATA") + " with SOA"; } + [[nodiscard]] ResponseConfig& getResponseConfig() + { + return d_responseConfig; + } +private: ResponseConfig d_responseConfig; -private: DNSName d_zone; DNSName d_mname; DNSName d_rname; uint32_t d_ttl; - uint32_t d_serial; - uint32_t d_refresh; - uint32_t d_retry; - uint32_t d_expire; - uint32_t d_minimum; + SOAParams d_params; bool d_nxd; bool d_soaInAuthoritySection; }; @@ -2249,18 +2280,18 @@ public: } } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - if (!dq->proxyProtocolValues) { - dq->proxyProtocolValues = make_unique>(); + if (!dnsquestion->proxyProtocolValues) { + dnsquestion->proxyProtocolValues = make_unique>(); } - *(dq->proxyProtocolValues) = d_values; + *(dnsquestion->proxyProtocolValues) = d_values; return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "set Proxy-Protocol values"; } @@ -2273,23 +2304,23 @@ class SetAdditionalProxyProtocolValueAction : public DNSAction { public: // this action does not stop the processing - SetAdditionalProxyProtocolValueAction(uint8_t type, const std::string& value) : - d_value(value), d_type(type) + SetAdditionalProxyProtocolValueAction(uint8_t type, std::string value) : + d_value(std::move(value)), d_type(type) { } - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + DNSAction::Action operator()(DNSQuestion* dnsquestion, std::string* ruleresult) const override { - if (!dq->proxyProtocolValues) { - dq->proxyProtocolValues = make_unique>(); + if (!dnsquestion->proxyProtocolValues) { + dnsquestion->proxyProtocolValues = make_unique>(); } - dq->proxyProtocolValues->push_back({d_value, d_type}); + dnsquestion->proxyProtocolValues->push_back({d_value, d_type}); return Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "add a Proxy-Protocol value of type " + std::to_string(d_type); } @@ -2308,16 +2339,18 @@ public: { } - DNSResponseAction::Action operator()(DNSResponse* dr, std::string* ruleresult) const override + DNSResponseAction::Action operator()(DNSResponse* response, std::string* ruleresult) const override { + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) auto visitor = [&](uint8_t section, uint16_t qclass, uint16_t qtype, uint32_t ttl) { return ttl * d_ratio; }; - editDNSPacketTTL(reinterpret_cast(dr->getMutableData().data()), dr->getData().size(), visitor); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + editDNSPacketTTL(reinterpret_cast(response->getMutableData().data()), response->getData().size(), visitor); return DNSResponseAction::Action::None; } - std::string toString() const override + [[nodiscard]] std::string toString() const override { return "reduce ttl to " + std::to_string(d_ratio * 100) + " percent of its value"; } @@ -2384,18 +2417,18 @@ static void addAction(GlobalStateHolder>* someRuleActions, const luadn setLuaSideEffect(); std::string name; - boost::uuids::uuid uuid; - uint64_t creationOrder; + boost::uuids::uuid uuid{}; + uint64_t creationOrder = 0; parseRuleParams(params, uuid, name, creationOrder); checkAllParametersConsumed("addAction", params); auto rule = makeRule(var, "addAction"); someRuleActions->modify([&rule, &action, &uuid, creationOrder, &name](vector& ruleactions) { - ruleactions.push_back({std::move(rule), std::move(action), std::move(name), std::move(uuid), creationOrder}); + ruleactions.push_back({std::move(rule), std::move(action), std::move(name), uuid, creationOrder}); }); } -typedef std::unordered_map> responseParams_t; +using responseParams_t = std::unordered_map>; static void parseResponseConfig(boost::optional& vars, ResponseConfig& config) { @@ -2405,41 +2438,41 @@ static void parseResponseConfig(boost::optional& vars, Respons getOptionalValue(vars, "ra", config.setRA); } -void setResponseHeadersFromConfig(dnsheader& dh, const ResponseConfig& config) +void setResponseHeadersFromConfig(dnsheader& dnsheader, const ResponseConfig& config) { if (config.setAA) { - dh.aa = *config.setAA; + dnsheader.aa = *config.setAA; } if (config.setAD) { - dh.ad = *config.setAD; + dnsheader.ad = *config.setAD; } else { - dh.ad = false; + dnsheader.ad = false; } if (config.setRA) { - dh.ra = *config.setRA; + dnsheader.ra = *config.setRA; } else { - dh.ra = dh.rd; // for good measure + dnsheader.ra = dnsheader.rd; // for good measure } } // NOLINTNEXTLINE(readability-function-cognitive-complexity): this function declares Lua bindings, even with a good refactoring it will likely blow up the threshold void setupLuaActions(LuaContext& luaCtx) { - luaCtx.writeFunction("newRuleAction", [](luadnsrule_t dnsrule, std::shared_ptr action, boost::optional params) { - boost::uuids::uuid uuid; - uint64_t creationOrder; + luaCtx.writeFunction("newRuleAction", [](const luadnsrule_t& dnsrule, std::shared_ptr action, boost::optional params) { + boost::uuids::uuid uuid{}; + uint64_t creationOrder = 0; std::string name; parseRuleParams(params, uuid, name, creationOrder); checkAllParametersConsumed("newRuleAction", params); auto rule = makeRule(dnsrule, "newRuleAction"); - DNSDistRuleAction ra({std::move(rule), std::move(action), std::move(name), uuid, creationOrder}); - return std::make_shared(ra); + DNSDistRuleAction ruleaction({std::move(rule), std::move(action), std::move(name), uuid, creationOrder}); + return std::make_shared(ruleaction); }); - luaCtx.writeFunction("addAction", [](luadnsrule_t var, boost::variant, std::shared_ptr> era, boost::optional params) { + luaCtx.writeFunction("addAction", [](const luadnsrule_t& var, boost::variant, std::shared_ptr> era, boost::optional params) { if (era.type() != typeid(std::shared_ptr)) { throw std::runtime_error("addAction() can only be called with query-related actions, not response-related ones. Are you looking for addResponseAction()?"); } @@ -2447,7 +2480,7 @@ void setupLuaActions(LuaContext& luaCtx) addAction(&g_ruleactions, var, boost::get>(era), params); }); - luaCtx.writeFunction("addResponseAction", [](luadnsrule_t var, boost::variant, std::shared_ptr> era, boost::optional params) { + luaCtx.writeFunction("addResponseAction", [](const luadnsrule_t& var, boost::variant, std::shared_ptr> era, boost::optional params) { if (era.type() != typeid(std::shared_ptr)) { throw std::runtime_error("addResponseAction() can only be called with response-related actions, not query-related ones. Are you looking for addAction()?"); } @@ -2455,7 +2488,7 @@ void setupLuaActions(LuaContext& luaCtx) addAction(&g_respruleactions, var, boost::get>(era), params); }); - luaCtx.writeFunction("addCacheHitResponseAction", [](luadnsrule_t var, boost::variant, std::shared_ptr> era, boost::optional params) { + luaCtx.writeFunction("addCacheHitResponseAction", [](const luadnsrule_t& var, boost::variant, std::shared_ptr> era, boost::optional params) { if (era.type() != typeid(std::shared_ptr)) { throw std::runtime_error("addCacheHitResponseAction() can only be called with response-related actions, not query-related ones. Are you looking for addAction()?"); } @@ -2463,7 +2496,7 @@ void setupLuaActions(LuaContext& luaCtx) addAction(&g_cachehitrespruleactions, var, boost::get>(era), params); }); - luaCtx.writeFunction("addCacheInsertedResponseAction", [](luadnsrule_t var, boost::variant, std::shared_ptr> era, boost::optional params) { + luaCtx.writeFunction("addCacheInsertedResponseAction", [](const luadnsrule_t& var, boost::variant, std::shared_ptr> era, boost::optional params) { if (era.type() != typeid(std::shared_ptr)) { throw std::runtime_error("addCacheInsertedResponseAction() can only be called with response-related actions, not query-related ones. Are you looking for addAction()?"); } @@ -2471,7 +2504,7 @@ void setupLuaActions(LuaContext& luaCtx) addAction(&g_cacheInsertedRespRuleActions, var, boost::get>(era), params); }); - luaCtx.writeFunction("addSelfAnsweredResponseAction", [](luadnsrule_t var, boost::variant, std::shared_ptr> era, boost::optional params) { + luaCtx.writeFunction("addSelfAnsweredResponseAction", [](const luadnsrule_t& var, boost::variant, std::shared_ptr> era, boost::optional params) { if (era.type() != typeid(std::shared_ptr)) { throw std::runtime_error("addSelfAnsweredResponseAction() can only be called with response-related actions, not query-related ones. Are you looking for addAction()?"); } @@ -2479,15 +2512,18 @@ void setupLuaActions(LuaContext& luaCtx) addAction(&g_selfansweredrespruleactions, var, boost::get>(era), params); }); - luaCtx.registerFunction("printStats", [](const DNSAction& ta) { + luaCtx.registerFunction("printStats", [](const DNSAction& action) { setLuaNoSideEffect(); - auto stats = ta.getStats(); - for (const auto& s : stats) { - g_outputBuffer += s.first + "\t"; - if ((uint64_t)s.second == s.second) - g_outputBuffer += std::to_string((uint64_t)s.second) + "\n"; - else - g_outputBuffer += std::to_string(s.second) + "\n"; + auto stats = action.getStats(); + for (const auto& stat : stats) { + g_outputBuffer += stat.first + "\t"; + double integral = 0; + if (std::modf(stat.second, &integral) == 0.0 && stat.second < static_cast(std::numeric_limits::max())) { + g_outputBuffer += std::to_string(static_cast(stat.second)) + "\n"; + } + else { + g_outputBuffer += std::to_string(stat.second) + "\n"; + } } }); @@ -2495,8 +2531,9 @@ void setupLuaActions(LuaContext& luaCtx) setLuaNoSideEffect(); boost::optional> ret; auto ruleactions = g_ruleactions.getCopy(); - if (num < ruleactions.size()) + if (num < ruleactions.size()) { ret = ruleactions[num].d_action; + } return ret; }); @@ -2506,12 +2543,12 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("LuaAction", [](LuaAction::func_t func) { setLuaSideEffect(); - return std::shared_ptr(new LuaAction(func)); + return std::shared_ptr(new LuaAction(std::move(func))); }); luaCtx.writeFunction("LuaFFIAction", [](LuaFFIAction::func_t func) { setLuaSideEffect(); - return std::shared_ptr(new LuaFFIAction(func)); + return std::shared_ptr(new LuaFFIAction(std::move(func))); }); luaCtx.writeFunction("LuaFFIPerThreadAction", [](const std::string& code) { @@ -2531,48 +2568,48 @@ void setupLuaActions(LuaContext& luaCtx) return std::shared_ptr(new SetEDNSOptionAction(code, data)); }); - luaCtx.writeFunction("PoolAction", [](const std::string& a, boost::optional stopProcessing) { - return std::shared_ptr(new PoolAction(a, stopProcessing ? *stopProcessing : true)); + luaCtx.writeFunction("PoolAction", [](const std::string& poolname, boost::optional stopProcessing) { + return std::shared_ptr(new PoolAction(poolname, stopProcessing ? *stopProcessing : true)); }); luaCtx.writeFunction("QPSAction", [](int limit) { return std::shared_ptr(new QPSAction(limit)); }); - luaCtx.writeFunction("QPSPoolAction", [](int limit, const std::string& a, boost::optional stopProcessing) { - return std::shared_ptr(new QPSPoolAction(limit, a, stopProcessing ? *stopProcessing : true)); + luaCtx.writeFunction("QPSPoolAction", [](int limit, const std::string& poolname, boost::optional stopProcessing) { + return std::shared_ptr(new QPSPoolAction(limit, poolname, stopProcessing ? *stopProcessing : true)); }); luaCtx.writeFunction("SpoofAction", [](LuaTypeOrArrayOf inp, boost::optional vars) { vector addrs; - if (auto s = boost::get(&inp)) { - addrs.push_back(ComboAddress(*s)); + if (auto* ipaddr = boost::get(&inp)) { + addrs.emplace_back(*ipaddr); } else { - const auto& v = boost::get>(inp); - for (const auto& a : v) { - addrs.push_back(ComboAddress(a.second)); + const auto& ipsArray = boost::get>(inp); + for (const auto& ipAddr : ipsArray) { + addrs.emplace_back(ipAddr.second); } } auto ret = std::shared_ptr(new SpoofAction(addrs)); - auto sa = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, sa->d_responseConfig); + auto spoofaction = std::dynamic_pointer_cast(ret); + parseResponseConfig(vars, spoofaction->getResponseConfig()); checkAllParametersConsumed("SpoofAction", vars); return ret; }); luaCtx.writeFunction("SpoofSVCAction", [](const LuaArray& parameters, boost::optional vars) { auto ret = std::shared_ptr(new SpoofSVCAction(parameters)); - auto sa = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, sa->d_responseConfig); + auto spoofaction = std::dynamic_pointer_cast(ret); + parseResponseConfig(vars, spoofaction->getResponseConfig()); return ret; }); - luaCtx.writeFunction("SpoofCNAMEAction", [](const std::string& a, boost::optional vars) { - auto ret = std::shared_ptr(new SpoofAction(DNSName(a))); - auto sa = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, sa->d_responseConfig); + luaCtx.writeFunction("SpoofCNAMEAction", [](const std::string& cname, boost::optional vars) { + auto ret = std::shared_ptr(new SpoofAction(DNSName(cname))); + auto spoofaction = std::dynamic_pointer_cast(ret); + parseResponseConfig(vars, spoofaction->getResponseConfig()); checkAllParametersConsumed("SpoofCNAMEAction", vars); return ret; }); @@ -2598,8 +2635,8 @@ void setupLuaActions(LuaContext& luaCtx) qtypeForAnyParam = static_cast(qtypeForAny); } auto ret = std::shared_ptr(new SpoofAction(raws, qtypeForAnyParam)); - auto sa = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, sa->d_responseConfig); + auto spoofaction = std::dynamic_pointer_cast(ret); + parseResponseConfig(vars, spoofaction->getResponseConfig()); checkAllParametersConsumed("SpoofRawAction", vars); return ret; }); @@ -2688,8 +2725,8 @@ void setupLuaActions(LuaContext& luaCtx) qtypes.insert(boost::get(types)); } else if (types.type() == typeid(LuaArray)) { - const auto& v = boost::get>(types); - for (const auto& tpair : v) { + const auto& typesArray = boost::get>(types); + for (const auto& tpair : typesArray) { qtypes.insert(tpair.second); } } @@ -2699,7 +2736,7 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("RCodeAction", [](uint8_t rcode, boost::optional vars) { auto ret = std::shared_ptr(new RCodeAction(rcode)); auto rca = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, rca->d_responseConfig); + parseResponseConfig(vars, rca->getResponseConfig()); checkAllParametersConsumed("RCodeAction", vars); return ret; }); @@ -2707,7 +2744,7 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("ERCodeAction", [](uint8_t rcode, boost::optional vars) { auto ret = std::shared_ptr(new ERCodeAction(rcode)); auto erca = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, erca->d_responseConfig); + parseResponseConfig(vars, erca->getResponseConfig()); checkAllParametersConsumed("ERCodeAction", vars); return ret; }); @@ -2738,12 +2775,12 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("LuaResponseAction", [](LuaResponseAction::func_t func) { setLuaSideEffect(); - return std::shared_ptr(new LuaResponseAction(func)); + return std::shared_ptr(new LuaResponseAction(std::move(func))); }); luaCtx.writeFunction("LuaFFIResponseAction", [](LuaFFIResponseAction::func_t func) { setLuaSideEffect(); - return std::shared_ptr(new LuaFFIResponseAction(func)); + return std::shared_ptr(new LuaFFIResponseAction(std::move(func))); }); luaCtx.writeFunction("LuaFFIPerThreadResponseAction", [](const std::string& code) { @@ -2755,8 +2792,8 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("RemoteLogAction", [](std::shared_ptr logger, boost::optional> alterFunc, boost::optional> vars, boost::optional> metas) { if (logger) { // avoids potentially-evaluated-expression warning with clang. - RemoteLoggerInterface& rl = *logger.get(); - if (typeid(rl) != typeid(RemoteLogger)) { + RemoteLoggerInterface& remoteLoggerRef = *logger; + if (typeid(remoteLoggerRef) != typeid(RemoteLogger)) { // We could let the user do what he wants, but wrapping PowerDNS Protobuf inside a FrameStream tagged as dnstap is logically wrong. throw std::runtime_error(std::string("RemoteLogAction only takes RemoteLogger. For other types, please look at DnstapLogAction.")); } @@ -2772,7 +2809,7 @@ void setupLuaActions(LuaContext& luaCtx) if (metas) { for (const auto& [key, value] : *metas) { - config.metas.push_back({key, ProtoBufMetaKey(value)}); + config.metas.emplace_back(key, ProtoBufMetaKey(value)); } } @@ -2795,8 +2832,8 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("RemoteLogResponseAction", [](std::shared_ptr logger, boost::optional> alterFunc, boost::optional includeCNAME, boost::optional> vars, boost::optional> metas) { if (logger) { // avoids potentially-evaluated-expression warning with clang. - RemoteLoggerInterface& rl = *logger.get(); - if (typeid(rl) != typeid(RemoteLogger)) { + RemoteLoggerInterface& remoteLoggerRef = *logger; + if (typeid(remoteLoggerRef) != typeid(RemoteLogger)) { // We could let the user do what he wants, but wrapping PowerDNS Protobuf inside a FrameStream tagged as dnstap is logically wrong. throw std::runtime_error("RemoteLogResponseAction only takes RemoteLogger. For other types, please look at DnstapLogResponseAction."); } @@ -2805,7 +2842,7 @@ void setupLuaActions(LuaContext& luaCtx) std::string tags; RemoteLogActionConfiguration config; config.logger = std::move(logger); - config.alterResponseFunc = alterFunc; + config.alterResponseFunc = std::move(alterFunc); config.includeCNAME = includeCNAME ? *includeCNAME : false; getOptionalValue(vars, "serverID", config.serverID); getOptionalValue(vars, "ipEncryptKey", config.ipEncryptKey); @@ -2814,7 +2851,7 @@ void setupLuaActions(LuaContext& luaCtx) if (metas) { for (const auto& [key, value] : *metas) { - config.metas.push_back({key, ProtoBufMetaKey(value)}); + config.metas.emplace_back(key, ProtoBufMetaKey(value)); } } @@ -2864,11 +2901,11 @@ void setupLuaActions(LuaContext& luaCtx) return std::shared_ptr(new SetDisableECSAction()); }); - luaCtx.writeFunction("SetECSAction", [](const std::string& v4, boost::optional v6) { - if (v6) { - return std::shared_ptr(new SetECSAction(Netmask(v4), Netmask(*v6))); + luaCtx.writeFunction("SetECSAction", [](const std::string& v4Netmask, boost::optional v6Netmask) { + if (v6Netmask) { + return std::shared_ptr(new SetECSAction(Netmask(v4Netmask), Netmask(*v6Netmask))); } - return std::shared_ptr(new SetECSAction(Netmask(v4))); + return std::shared_ptr(new SetECSAction(Netmask(v4Netmask))); }); #ifdef HAVE_NET_SNMP @@ -2897,7 +2934,7 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("HTTPStatusAction", [](uint16_t status, std::string body, boost::optional contentType, boost::optional vars) { auto ret = std::shared_ptr(new HTTPStatusAction(status, PacketBuffer(body.begin(), body.end()), contentType ? *contentType : "")); auto hsa = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, hsa->d_responseConfig); + parseResponseConfig(vars, hsa->getResponseConfig()); checkAllParametersConsumed("HTTPStatusAction", vars); return ret; }); @@ -2916,9 +2953,15 @@ void setupLuaActions(LuaContext& luaCtx) luaCtx.writeFunction("NegativeAndSOAAction", [](bool nxd, const std::string& zone, uint32_t ttl, const std::string& mname, const std::string& rname, uint32_t serial, uint32_t refresh, uint32_t retry, uint32_t expire, uint32_t minimum, boost::optional vars) { bool soaInAuthoritySection = false; getOptionalValue(vars, "soaInAuthoritySection", soaInAuthoritySection); - auto ret = std::shared_ptr(new NegativeAndSOAAction(nxd, DNSName(zone), ttl, DNSName(mname), DNSName(rname), serial, refresh, retry, expire, minimum, soaInAuthoritySection)); + NegativeAndSOAAction::SOAParams params{ + .serial = serial, + .refresh = refresh, + .retry = retry, + .expire = expire, + .minimum = minimum}; + auto ret = std::shared_ptr(new NegativeAndSOAAction(nxd, DNSName(zone), ttl, DNSName(mname), DNSName(rname), params, soaInAuthoritySection)); auto action = std::dynamic_pointer_cast(ret); - parseResponseConfig(vars, action->d_responseConfig); + parseResponseConfig(vars, action->getResponseConfig()); checkAllParametersConsumed("NegativeAndSOAAction", vars); return ret; }); diff --git a/pdns/dnsdist-lua.hh b/pdns/dnsdist-lua.hh index 0037d6b9f0..5c35c3fb9d 100644 --- a/pdns/dnsdist-lua.hh +++ b/pdns/dnsdist-lua.hh @@ -33,7 +33,7 @@ struct ResponseConfig boost::optional setRA{boost::none}; uint32_t ttl{60}; }; -void setResponseHeadersFromConfig(dnsheader& dh, const ResponseConfig& config); +void setResponseHeadersFromConfig(dnsheader& dnsheader, const ResponseConfig& config); class SpoofAction : public DNSAction { @@ -66,7 +66,7 @@ public: { } - DNSAction::Action operator()(DNSQuestion* dq, string* ruleresult) const override; + DNSAction::Action operator()(DNSQuestion* dnsquestion, string* ruleresult) const override; string toString() const override { @@ -84,9 +84,13 @@ public: return ret; } + [[nodiscard]] ResponseConfig& getResponseConfig() + { + return d_responseConfig; + } - ResponseConfig d_responseConfig; private: + ResponseConfig d_responseConfig; static thread_local std::default_random_engine t_randomEngine; std::vector d_addrs; std::unordered_set d_types; diff --git a/pdns/dnsdist-protobuf.cc b/pdns/dnsdist-protobuf.cc index 031816f030..30445ed24f 100644 --- a/pdns/dnsdist-protobuf.cc +++ b/pdns/dnsdist-protobuf.cc @@ -27,13 +27,13 @@ #include "dnsdist-protobuf.hh" #include "protozero.hh" -DNSDistProtoBufMessage::DNSDistProtoBufMessage(const DNSQuestion& dq) : - d_dq(dq), d_type(pdns::ProtoZero::Message::MessageType::DNSQueryType) +DNSDistProtoBufMessage::DNSDistProtoBufMessage(const DNSQuestion& dnsquestion) : + d_dq(dnsquestion) { } -DNSDistProtoBufMessage::DNSDistProtoBufMessage(const DNSResponse& dr, bool includeCNAME) : - d_dq(dr), d_dr(&dr), d_type(pdns::ProtoZero::Message::MessageType::DNSResponseType), d_includeCNAME(includeCNAME) +DNSDistProtoBufMessage::DNSDistProtoBufMessage(const DNSResponse& dnsresponse, bool includeCNAME) : + d_dq(dnsresponse), d_dr(&dnsresponse), d_type(pdns::ProtoZero::Message::MessageType::DNSResponseType), d_includeCNAME(includeCNAME) { } @@ -268,14 +268,14 @@ ProtoBufMetaKey::ProtoBufMetaKey(const std::string& key) throw std::runtime_error("Invalid ProtoBuf key '" + key + "'"); } -std::vector ProtoBufMetaKey::getValues(const DNSQuestion& dq) const +std::vector ProtoBufMetaKey::getValues(const DNSQuestion& dnsquestion) const { auto& idx = s_types.get(); auto it = idx.find(d_type); if (it == idx.end()) { throw std::runtime_error("Trying to get the values of an unsupported type: " + std::to_string(static_cast(d_type))); } - return (it->d_func)(dq, d_subKey, d_numericSubKey); + return (it->d_func)(dnsquestion, d_subKey, d_numericSubKey); } const std::string& ProtoBufMetaKey::getName() const @@ -289,56 +289,56 @@ const std::string& ProtoBufMetaKey::getName() const } const ProtoBufMetaKey::TypeContainer ProtoBufMetaKey::s_types = { - ProtoBufMetaKey::KeyTypeDescription{"sni", Type::SNI, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { return {dq.sni}; }, false}, - ProtoBufMetaKey::KeyTypeDescription{"pool", Type::Pool, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { return {dq.ids.poolName}; }, false}, - ProtoBufMetaKey::KeyTypeDescription{"b64-content", Type::B64Content, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { const auto& data = dq.getData(); return {Base64Encode(std::string(data.begin(), data.end()))}; }, false}, + ProtoBufMetaKey::KeyTypeDescription{"sni", Type::SNI, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { return {dnsquestion.sni}; }, false}, + ProtoBufMetaKey::KeyTypeDescription{"pool", Type::Pool, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { return {dnsquestion.ids.poolName}; }, false}, + ProtoBufMetaKey::KeyTypeDescription{"b64-content", Type::B64Content, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { const auto& data = dnsquestion.getData(); return {Base64Encode(std::string(data.begin(), data.end()))}; }, false}, #ifdef HAVE_DNS_OVER_HTTPS - ProtoBufMetaKey::KeyTypeDescription{"doh-header", Type::DoHHeader, [](const DNSQuestion& dq, const std::string& name, uint8_t) -> std::vector { - if (!dq.ids.du) { + ProtoBufMetaKey::KeyTypeDescription{"doh-header", Type::DoHHeader, [](const DNSQuestion& dnsquestion, const std::string& name, uint8_t) -> std::vector { + if (!dnsquestion.ids.du) { return {}; } - auto headers = dq.ids.du->getHTTPHeaders(); - auto it = headers.find(name); - if (it != headers.end()) { - return {it->second}; + auto headers = dnsquestion.ids.du->getHTTPHeaders(); + auto iter = headers.find(name); + if (iter != headers.end()) { + return {iter->second}; } return {}; }, true, false}, - ProtoBufMetaKey::KeyTypeDescription{"doh-host", Type::DoHHost, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { - if (dq.ids.du) { - return {dq.ids.du->getHTTPHost()}; + ProtoBufMetaKey::KeyTypeDescription{"doh-host", Type::DoHHost, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { + if (dnsquestion.ids.du) { + return {dnsquestion.ids.du->getHTTPHost()}; } return {}; }, true, false}, - ProtoBufMetaKey::KeyTypeDescription{"doh-path", Type::DoHPath, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { - if (dq.ids.du) { - return {dq.ids.du->getHTTPPath()}; + ProtoBufMetaKey::KeyTypeDescription{"doh-path", Type::DoHPath, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { + if (dnsquestion.ids.du) { + return {dnsquestion.ids.du->getHTTPPath()}; } return {}; }, false}, - ProtoBufMetaKey::KeyTypeDescription{"doh-query-string", Type::DoHQueryString, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { - if (dq.ids.du) { - return {dq.ids.du->getHTTPQueryString()}; + ProtoBufMetaKey::KeyTypeDescription{"doh-query-string", Type::DoHQueryString, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { + if (dnsquestion.ids.du) { + return {dnsquestion.ids.du->getHTTPQueryString()}; } return {}; }, false}, - ProtoBufMetaKey::KeyTypeDescription{"doh-scheme", Type::DoHScheme, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { - if (dq.ids.du) { - return {dq.ids.du->getHTTPScheme()}; + ProtoBufMetaKey::KeyTypeDescription{"doh-scheme", Type::DoHScheme, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { + if (dnsquestion.ids.du) { + return {dnsquestion.ids.du->getHTTPScheme()}; } return {}; }, false, false}, #endif // HAVE_DNS_OVER_HTTPS - ProtoBufMetaKey::KeyTypeDescription{"proxy-protocol-value", Type::ProxyProtocolValue, [](const DNSQuestion& dq, const std::string&, uint8_t numericSubKey) -> std::vector { - if (!dq.proxyProtocolValues) { + ProtoBufMetaKey::KeyTypeDescription{"proxy-protocol-value", Type::ProxyProtocolValue, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t numericSubKey) -> std::vector { + if (!dnsquestion.proxyProtocolValues) { return {}; } - for (const auto& value : *dq.proxyProtocolValues) { + for (const auto& value : *dnsquestion.proxyProtocolValues) { if (value.type == numericSubKey) { return {value.content}; } @@ -346,21 +346,21 @@ const ProtoBufMetaKey::TypeContainer ProtoBufMetaKey::s_types = { return {}; }, true, false, true}, - ProtoBufMetaKey::KeyTypeDescription{"proxy-protocol-values", Type::ProxyProtocolValues, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { + ProtoBufMetaKey::KeyTypeDescription{"proxy-protocol-values", Type::ProxyProtocolValues, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { std::vector result; - if (!dq.proxyProtocolValues) { + if (!dnsquestion.proxyProtocolValues) { return result; } - for (const auto& value : *dq.proxyProtocolValues) { + for (const auto& value : *dnsquestion.proxyProtocolValues) { result.push_back(std::to_string(value.type) + ":" + value.content); } return result; }}, - ProtoBufMetaKey::KeyTypeDescription{"tag", Type::Tag, [](const DNSQuestion& dq, const std::string& subKey, uint8_t) -> std::vector { - if (!dq.ids.qTag) { + ProtoBufMetaKey::KeyTypeDescription{"tag", Type::Tag, [](const DNSQuestion& dnsquestion, const std::string& subKey, uint8_t) -> std::vector { + if (!dnsquestion.ids.qTag) { return {}; } - for (const auto& [key, value] : *dq.ids.qTag) { + for (const auto& [key, value] : *dnsquestion.ids.qTag) { if (key == subKey) { return {value}; } @@ -368,18 +368,21 @@ const ProtoBufMetaKey::TypeContainer ProtoBufMetaKey::s_types = { return {}; }, true, true}, - ProtoBufMetaKey::KeyTypeDescription{"tags", Type::Tags, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector { + ProtoBufMetaKey::KeyTypeDescription{"tags", Type::Tags, [](const DNSQuestion& dnsquestion, const std::string&, uint8_t) -> std::vector { std::vector result; - if (!dq.ids.qTag) { + if (!dnsquestion.ids.qTag) { return result; } - for (const auto& [key, value] : *dq.ids.qTag) { + for (const auto& [key, value] : *dnsquestion.ids.qTag) { if (value.empty()) { /* avoids a spurious ':' when the value is empty */ result.push_back(key); } else { - result.push_back(key + ":" + value); + auto tag = key; + tag.append(":"); + tag.append(value); + result.push_back(tag); } } return result; diff --git a/pdns/dnsdist-protobuf.hh b/pdns/dnsdist-protobuf.hh index 8c00e011d1..86ec100331 100644 --- a/pdns/dnsdist-protobuf.hh +++ b/pdns/dnsdist-protobuf.hh @@ -30,8 +30,10 @@ class DNSDistProtoBufMessage { public: - DNSDistProtoBufMessage(const DNSQuestion& dq); - DNSDistProtoBufMessage(const DNSResponse& dr, bool includeCNAME); + DNSDistProtoBufMessage(const DNSQuestion& dnsquestion); + DNSDistProtoBufMessage(const DNSResponse& dnsresponse, bool includeCNAME); + DNSDistProtoBufMessage(const DNSQuestion&&) = delete; + DNSDistProtoBufMessage(const DNSResponse&&, bool) = delete; void setServerIdentity(const std::string& serverId); void setRequestor(const ComboAddress& requestor); @@ -45,7 +47,7 @@ public: void setTime(time_t sec, uint32_t usec); void setQueryTime(time_t sec, uint32_t usec); void setQuestion(const DNSName& name, uint16_t qtype, uint16_t qclass); - void setEDNSSubnet(const Netmask& nm); + void setEDNSSubnet(const Netmask& netmask); void addTag(const std::string& strValue); void addMeta(const std::string& key, std::vector&& strValues, const std::vector& intValues); @@ -53,7 +55,7 @@ public: void serialize(std::string& data) const; - std::string toDebugString() const; + [[nodiscard]] std::string toDebugString() const; private: struct PBRecord @@ -66,8 +68,8 @@ private: }; struct PBQuestion { - PBQuestion(const DNSName& name, uint16_t type, uint16_t class_) : - d_name(name), d_type(type), d_class(class_) + PBQuestion(DNSName name, uint16_t type, uint16_t class_) : + d_name(std::move(name)), d_type(type), d_class(class_) { } @@ -85,6 +87,8 @@ private: }; std::unordered_map d_metaTags; + // FIXME wondering if the cost of moving to a shared_ptr would be that bad + // NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members) const DNSQuestion& d_dq; const DNSResponse* d_dr{nullptr}; const std::string* d_ServerIdentityRef{nullptr}; @@ -123,9 +127,9 @@ class ProtoBufMetaKey struct KeyTypeDescription { - const std::string d_name; - const Type d_type; - const std::function(const DNSQuestion&, const std::string&, uint8_t)> d_func; + std::string d_name; + Type d_type; + std::function(const DNSQuestion&, const std::string&, uint8_t)> d_func; bool d_prefix{false}; bool d_caseSensitive{true}; bool d_numeric{false}; @@ -138,20 +142,19 @@ class ProtoBufMetaKey { }; - typedef boost::multi_index_container< + using TypeContainer = boost::multi_index_container< KeyTypeDescription, indexed_by< hashed_unique, member>, - hashed_unique, member>>> - TypeContainer; + hashed_unique, member>>>; static const TypeContainer s_types; public: ProtoBufMetaKey(const std::string& key); - const std::string& getName() const; - std::vector getValues(const DNSQuestion& dq) const; + [[nodiscard]] const std::string& getName() const; + [[nodiscard]] std::vector getValues(const DNSQuestion& dnsquestion) const; private: std::string d_subKey; diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 5fe938ff1b..9d5d06f592 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -149,6 +149,13 @@ struct DNSQuestion ids.qTag->insert_or_assign(key, value); } + void setTag(const std::string& key, std::string&& value) { + if (!ids.qTag) { + ids.qTag = std::make_unique(); + } + ids.qTag->insert_or_assign(key, std::move(value)); + } + const struct timespec& getQueryRealTime() const { return ids.queryRealTime.d_start; diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc index 25a90078a8..21008f38fe 100644 --- a/pdns/dnsname.cc +++ b/pdns/dnsname.cc @@ -100,7 +100,7 @@ DNSName::DNSName(const std::string_view sw) } -DNSName::DNSName(const char* pos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, uint16_t minOffset) +DNSName::DNSName(const char* pos, size_t len, size_t offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, uint16_t minOffset) { if (offset >= len) throw std::range_error("Trying to read past the end of the buffer ("+std::to_string(offset)+ " >= "+std::to_string(len)+")"); diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh index 29c8325c5d..dbdc2ac2c3 100644 --- a/pdns/dnsname.hh +++ b/pdns/dnsname.hh @@ -100,7 +100,7 @@ public: DNSName(DNSName&& a) = default; explicit DNSName(std::string_view sw); //!< Constructs from a human formatted, escaped presentation - DNSName(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=nullptr, uint16_t* qclass=nullptr, unsigned int* consumed=nullptr, uint16_t minOffset=0); //!< Construct from a DNS Packet, taking the first question if offset=12. If supplied, consumed is set to the number of bytes consumed from the packet, which will not be equal to the wire length of the resulting name in case of compression. + DNSName(const char* pos, size_t len, size_t offset, bool uncompress, uint16_t* qtype = nullptr, uint16_t* qclass = nullptr, unsigned int* consumed = nullptr, uint16_t minOffset = 0); //!< Construct from a DNS Packet, taking the first question if offset=12. If supplied, consumed is set to the number of bytes consumed from the packet, which will not be equal to the wire length of the resulting name in case of compression. bool isPartOf(const DNSName& rhs) const; //!< Are we part of the rhs name? Note that name.isPartOf(name). inline bool operator==(const DNSName& rhs) const; //!< DNS-native comparison (case insensitive) - empty compares to empty diff --git a/pdns/dnstap.cc b/pdns/dnstap.cc index 577b3637f7..beb7ac5e8d 100644 --- a/pdns/dnstap.cc +++ b/pdns/dnstap.cc @@ -57,7 +57,12 @@ enum : protozero::pbf_tag_type }; } -DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, DnstapMessage::ProtocolType protocol, const char* packet, const size_t len, const struct timespec* queryTime, const struct timespec* responseTime, boost::optional auth) : +std::string&& DnstapMessage::getBuffer() +{ + return std::move(d_buffer); +} + +DnstapMessage::DnstapMessage(std::string&& buffer, DnstapMessage::MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, DnstapMessage::ProtocolType protocol, const char* packet, const size_t len, const struct timespec* queryTime, const struct timespec* responseTime, const boost::optional& auth) : d_buffer(buffer) { protozero::pbf_writer pbf{d_buffer}; @@ -68,7 +73,9 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ protozero::pbf_writer pbf_message{pbf, DnstapBaseFields::message}; + // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) pbf_message.add_enum(DnstapMessageFields::type, static_cast(type)); + // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) pbf_message.add_enum(DnstapMessageFields::socket_protocol, static_cast(protocol)); if (requestor != nullptr) { @@ -80,9 +87,11 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ if (requestor != nullptr) { if (requestor->sin4.sin_family == AF_INET) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) pbf_message.add_bytes(DnstapMessageFields::query_address, reinterpret_cast(&requestor->sin4.sin_addr.s_addr), sizeof(requestor->sin4.sin_addr.s_addr)); } else if (requestor->sin4.sin_family == AF_INET6) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) pbf_message.add_bytes(DnstapMessageFields::query_address, reinterpret_cast(&requestor->sin6.sin6_addr.s6_addr), sizeof(requestor->sin6.sin6_addr.s6_addr)); } pbf_message.add_uint32(DnstapMessageFields::query_port, ntohs(requestor->sin4.sin_port)); @@ -90,9 +99,11 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ if (responder != nullptr) { if (responder->sin4.sin_family == AF_INET) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) pbf_message.add_bytes(DnstapMessageFields::response_address, reinterpret_cast(&responder->sin4.sin_addr.s_addr), sizeof(responder->sin4.sin_addr.s_addr)); } else if (responder->sin4.sin_family == AF_INET6) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) pbf_message.add_bytes(DnstapMessageFields::response_address, reinterpret_cast(&responder->sin6.sin6_addr.s6_addr), sizeof(responder->sin6.sin6_addr.s6_addr)); } pbf_message.add_uint32(DnstapMessageFields::response_port, ntohs(responder->sin4.sin_port)); @@ -109,8 +120,8 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ } if (packet != nullptr && len >= sizeof(dnsheader)) { - const dnsheader_aligned dh(packet); - if (!dh->qr) { + const dnsheader_aligned dnsheader(packet); + if (!dnsheader->qr) { pbf_message.add_bytes(DnstapMessageFields::query_message, packet, len); } else { diff --git a/pdns/dnstap.hh b/pdns/dnstap.hh index bcfb07ce0f..357319b9a5 100644 --- a/pdns/dnstap.hh +++ b/pdns/dnstap.hh @@ -61,12 +61,13 @@ public: DoQ = 7 }; - DnstapMessage(std::string& buffer, MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, ProtocolType protocol, const char* packet, const size_t len, const struct timespec* queryTime, const struct timespec* responseTime, boost::optional auth = boost::none); + DnstapMessage(std::string&& buffer, MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, ProtocolType protocol, const char* packet, size_t len, const struct timespec* queryTime, const struct timespec* responseTime, const boost::optional& auth = boost::none); void setExtra(const std::string& extra); + std::string&& getBuffer(); -protected: - std::string& d_buffer; +private: + std::string d_buffer; }; #endif /* DISABLE_PROTOBUF */ diff --git a/pdns/recursordist/lwres.cc b/pdns/recursordist/lwres.cc index 7f840ca890..96425df3d5 100644 --- a/pdns/recursordist/lwres.cc +++ b/pdns/recursordist/lwres.cc @@ -112,7 +112,9 @@ static void logFstreamQuery(const std::shared_ptr(&*packet.begin()), packet.size(), &ts, nullptr, auth); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + DnstapMessage message(std::move(str), DnstapMessage::MessageType::resolver_query, SyncRes::s_serverID, &localip, &ip, protocol, reinterpret_cast(packet.data()), packet.size(), &ts, nullptr, auth); + str = message.getBuffer(); for (auto& logger : *fstreamLoggers) { remoteLoggerQueueData(*logger, str); @@ -141,7 +143,9 @@ static void logFstreamResponse(const std::shared_ptr(packet.data()), packet.size(), &ts1, &ts2, auth); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + DnstapMessage message(std::move(str), DnstapMessage::MessageType::resolver_response, SyncRes::s_serverID, &localip, &ip, protocol, reinterpret_cast(packet.data()), packet.size(), &ts1, &ts2, auth); + str = message.getBuffer(); for (auto& logger : *fstreamLoggers) { remoteLoggerQueueData(*logger, str); diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index fa8cc10537..165c3865c3 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -1507,8 +1507,8 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi else { TIMEVAL_TO_TIMESPEC(&comboWriter->d_now, &timeSpec); // NOLINT } - DnstapMessage message(str, DnstapMessage::MessageType::resolver_response, SyncRes::s_serverID, &comboWriter->d_source, &comboWriter->d_destination, comboWriter->d_tcp ? DnstapMessage::ProtocolType::DoTCP : DnstapMessage::ProtocolType::DoUDP, reinterpret_cast(&*packet.begin()), packet.size(), &timeSpec, nullptr, comboWriter->d_mdp.d_qname); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) - + DnstapMessage message(std::move(str), DnstapMessage::MessageType::resolver_response, SyncRes::s_serverID, &comboWriter->d_source, &comboWriter->d_destination, comboWriter->d_tcp ? DnstapMessage::ProtocolType::DoTCP : DnstapMessage::ProtocolType::DoUDP, reinterpret_cast(&*packet.begin()), packet.size(), &timeSpec, nullptr, comboWriter->d_mdp.d_qname); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + str = message.getBuffer(); for (auto& logger : *(t_nodFrameStreamServersInfo.servers)) { if (logger->logUDRs()) { remoteLoggerQueueData(*logger, str); @@ -1666,7 +1666,8 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi else { TIMEVAL_TO_TIMESPEC(&comboWriter->d_now, &timeSpec); // NOLINT } - DnstapMessage message(str, DnstapMessage::MessageType::client_query, SyncRes::s_serverID, &comboWriter->d_source, &comboWriter->d_destination, comboWriter->d_tcp ? DnstapMessage::ProtocolType::DoTCP : DnstapMessage::ProtocolType::DoUDP, nullptr, 0, &timeSpec, nullptr, comboWriter->d_mdp.d_qname); + DnstapMessage message(std::move(str), DnstapMessage::MessageType::client_query, SyncRes::s_serverID, &comboWriter->d_source, &comboWriter->d_destination, comboWriter->d_tcp ? DnstapMessage::ProtocolType::DoTCP : DnstapMessage::ProtocolType::DoUDP, nullptr, 0, &timeSpec, nullptr, comboWriter->d_mdp.d_qname); + str = message.getBuffer(); for (auto& logger : *(t_nodFrameStreamServersInfo.servers)) { if (logger->logNODs()) {