From deff21b4e43725698a8ee9c242ed61b0b5144f98 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Fri, 15 Nov 2024 15:18:13 +0100 Subject: [PATCH] Use new fields in dnsmessage.proto --- contrib/ProtobufLogger.py | 51 ++++++++++++++++++------------ pdns/dnsmessage.proto | 12 ++++--- pdns/dnswriter.cc | 3 +- pdns/dnswriter.hh | 2 +- pdns/protozero.hh | 14 +++++++- pdns/recursordist/pdns_recursor.cc | 9 +++--- pdns/recursordist/rec-main.cc | 8 ++--- 7 files changed, 60 insertions(+), 39 deletions(-) diff --git a/contrib/ProtobufLogger.py b/contrib/ProtobufLogger.py index 9cd614342266..3538bc3249d8 100644 --- a/contrib/ProtobufLogger.py +++ b/contrib/ProtobufLogger.py @@ -257,27 +257,38 @@ def printSummary(self, msg, typestr): if msg.HasField('outgoingQueries'): outgoingQs = str(msg.outgoingQueries) + headerFlags = "N/A" + if msg.HasField('headerFlags'): + headerFlags = "0x%04X" % msg.headerFlags + + ednsVersion = "N/A" + if msg.HasField('ednsVersion'): + ednsVersion = "0x%08X" % msg.ednsVersion + print('[%s] %s of size %d: %s%s%s -> %s%s(%s) id: %d uuid: %s%s ' - 'requestorid: %s deviceid: %s devicename: %s serverid: %s nod: %s workerId: %s pcCacheHit: %s outgoingQueries: %s' % (datestr, - typestr, - msg.inBytes, - ipfromstr, - fromportstr, - requestorstr, - iptostr, - toportstr, - protostr, - msg.id, - messageidstr, - initialrequestidstr, - requestorId, - deviceId, - deviceName, - serveridstr, - nod, - workerId, - pcCacheHit, - outgoingQs)) + 'requestorid: %s deviceid: %s devicename: %s serverid: %s nod: %s workerId: %s pcCacheHit: %s outgoingQueries: %s headerFlags: %s ednsVersion: %s' % + (datestr, + typestr, + msg.inBytes, + ipfromstr, + fromportstr, + requestorstr, + iptostr, + toportstr, + protostr, + msg.id, + messageidstr, + initialrequestidstr, + requestorId, + deviceId, + deviceName, + serveridstr, + nod, + workerId, + pcCacheHit, + outgoingQs, + headerFlags, + ednsVersion)) for mt in msg.meta: values = '' diff --git a/pdns/dnsmessage.proto b/pdns/dnsmessage.proto index 0b95ea30178a..ed97a22d6d6c 100644 --- a/pdns/dnsmessage.proto +++ b/pdns/dnsmessage.proto @@ -2,19 +2,19 @@ * This file describes the message format used by the protobuf logging feature in PowerDNS and dnsdist. * * MIT License - * + * * Copyright (c) 2016-now PowerDNS.COM B.V. and its contributors. - * + * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal * in the Software without restriction, including without limitation the rights * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell * copies of the Software, and to permit persons to whom the Software is * furnished to do so, subject to the following conditions: - * + * * The above copyright notice and this permission notice shall be included in all * copies or substantial portions of the Software. - * + * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -145,7 +145,7 @@ message PBDNSMessage { } message Meta { - required string key = 1; // MUST be unique, so if you have multiple values they must be aggregated into on Meta + required string key = 1; // MUST be unique, so if you have multiple values they must be aggregated into one Meta required MetaValue value = 2; } repeated Meta meta = 22; // Arbitrary meta-data - to be used in future rather than adding new fields all the time @@ -188,6 +188,8 @@ message PBDNSMessage { optional uint64 workerId = 25; // Thread id optional bool packetCacheHit = 26; // Was it a packet cache hit? optional uint32 outgoingQueries = 27; // Number of outgoing queries used to answer the query + optional uint32 headerFlags = 28; // Flags field in wire format + optional uint32 ednsVersion = 29; // EDNS version and flags in wire format, see https://www.rfc-editor.org/rfc/rfc6891.html#section-6.1.3 } message PBDNSMessageList { diff --git a/pdns/dnswriter.cc b/pdns/dnswriter.cc index 7727a35cc755..67ed5c57d12f 100644 --- a/pdns/dnswriter.cc +++ b/pdns/dnswriter.cc @@ -95,7 +95,7 @@ template void GenericDNSPacketWriter::startRecor d_sor=d_content.size(); // this will remind us where to stuff the record size } -template uint32_t GenericDNSPacketWriter::addOpt(const uint16_t udpsize, const uint16_t extRCode, const uint16_t ednsFlags, const optvect_t& options, const uint8_t version) +template void GenericDNSPacketWriter::addOpt(const uint16_t udpsize, const uint16_t extRCode, const uint16_t ednsFlags, const optvect_t& options, const uint8_t version) { uint32_t ttl=0; @@ -127,7 +127,6 @@ template uint32_t GenericDNSPacketWriter::addOpt xfr16BitInt(option.second.length()); xfrBlob(option.second); } - return ttl; } template void GenericDNSPacketWriter::xfr48BitInt(uint64_t val) diff --git a/pdns/dnswriter.hh b/pdns/dnswriter.hh index fa0540faacf4..4d6d28618242 100644 --- a/pdns/dnswriter.hh +++ b/pdns/dnswriter.hh @@ -70,7 +70,7 @@ public: /** Shorthand way to add an Opt-record, for example for EDNS0 purposes */ using optvect_t = vector >; - uint32_t addOpt(uint16_t udpsize, uint16_t extRCode, uint16_t ednsFlags, const optvect_t& options = {}, uint8_t version = 0); + void addOpt(const uint16_t udpsize, const uint16_t extRCode, const uint16_t ednsFlags, const optvect_t& options=optvect_t(), const uint8_t version=0); /** needs to be called after the last record is added, but can be called again and again later on. Is called internally by startRecord too. The content of the vector<> passed to the constructor is inconsistent until commit is called. diff --git a/pdns/protozero.hh b/pdns/protozero.hh index b2c96c218423..baac7c87f906 100644 --- a/pdns/protozero.hh +++ b/pdns/protozero.hh @@ -100,7 +100,9 @@ namespace ProtoZero httpVersion = 24, workerId = 25, packetCacheHit = 26, - outgoingQueries = 27 + outgoingQueries = 27, + headerFlags = 28, + ednsVersion = 29, }; enum class QuestionField : protozero::pbf_tag_type { @@ -302,6 +304,16 @@ namespace ProtoZero add_uint32(d_message, Field::outgoingQueries, num); } + void setHeaderFlags(uint16_t flags) + { + add_uint32(d_message, Field::headerFlags, flags); + } + + void setEDNSVersion(uint32_t version) + { + add_uint32(d_message, Field::ednsVersion, version); + } + void startResponse() { d_response = protozero::pbf_writer{d_message, static_cast(Field::response)}; diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 59a543ceb942..2291986a58c7 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -1718,9 +1718,8 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi OPT record. This MUST also occur when a truncated response (using the DNS header's TC bit) is returned." */ - auto ednsVersion = packetWriter.addOpt(512, ednsExtRCode, DNSSECOK ? EDNSOpts::DNSSECOK : 0, returnedEdnsOptions); + packetWriter.addOpt(512, ednsExtRCode, DNSSECOK ? EDNSOpts::DNSSECOK : 0, returnedEdnsOptions); packetWriter.commit(); - pbMessage.setMeta("_pdnsREDNS", {}, {ednsVersion}); // XXX CONDITIONAL! } t_Counters.at(rec::ResponseStats::responseStats).submitResponse(comboWriter->d_mdp.d_qtype, packet.size(), packetWriter.getHeader()->rcode); @@ -1874,7 +1873,6 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi for (const auto& metaValue : dnsQuestion.meta) { pbMessage.setMeta(metaValue.first, metaValue.second.stringVal, metaValue.second.intVal); } - pbMessage.setMeta("_pdnsRFlags", {}, {htons(*getFlagsFromDNSHeader(packetWriter.getHeader()))}); #ifdef NOD_ENABLED if (g_nodEnabled) { if (nod) { @@ -2045,6 +2043,7 @@ void getQNameAndSubnet(const std::string& question, DNSName* dnsname, uint16_t* if (pos >= questionLen) { return; } + /* OPT root label (1) followed by type (2) */ if (lookForECS && ntohs(drh->d_type) == QType::OPT) { if (options == nullptr) { @@ -2253,6 +2252,7 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr g_mtracer->clearAllocators(); */ #endif + // We do not have a SyncRes specific Lua context at this point yet, so ok to use t_pdl if (needECS || (t_pdl && (t_pdl->hasGettagFunc() || t_pdl->hasGettagFFIFunc())) || dnsheader->opcode == static_cast(Opcode::Notify)) { try { @@ -2338,8 +2338,7 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr eventTrace.add(RecEventTrace::AnswerSent); if (t_protobufServers.servers && logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || !pbData || pbData->d_tagged)) { - const dnsheader_aligned hdr(response.data()); - protobufLogResponse(hdr.get(), luaconfsLocal, pbData, tval, false, source, destination, mappedSource, ednssubnet, uniqueId, requestorId, deviceId, deviceName, meta, eventTrace, policyTags); + protobufLogResponse(dnsheader, luaconfsLocal, pbData, tval, false, source, destination, mappedSource, ednssubnet, uniqueId, requestorId, deviceId, deviceName, meta, eventTrace, policyTags); } if (eventTrace.enabled() && (SyncRes::s_event_trace_enabled & SyncRes::event_trace_to_log) != 0) { diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 806786ee3d08..10d47c3e4243 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -562,12 +562,11 @@ void protobufLogQuery(LocalStateHolder& luaconfsLocal, const boo for (const auto& mit : meta) { msg.setMeta(mit.first, mit.second.stringVal, mit.second.intVal); } - - msg.setMeta("_pdnsQFlags", {}, {htons(*getFlagsFromDNSHeader(&header))}); - + msg.setHeaderFlags(htons(*getFlagsFromDNSHeader(&header))); if (ednsVersion) { - msg.setMeta("_pdnsQEDNS", {}, {*ednsVersion}); + msg.setEDNSVersion(*ednsVersion); } + std::string strMsg(msg.finishAndMoveBuf()); for (auto& server : *t_protobufServers.servers) { remoteLoggerQueueData(*server, strMsg); @@ -646,7 +645,6 @@ void protobufLogResponse(const struct dnsheader* header, LocalStateHolder