Use new fields in dnsmessage.proto

diff --git a/contrib/ProtobufLogger.py b/contrib/ProtobufLogger.py
index 9cd614342266aaf71724ba4b613722fe0e220d1d..3538bc3249d8af6f453dce033fedbe7be37b0e6d 100644 (file)
--- a/contrib/ProtobufLogger.py
+++ b/contrib/ProtobufLogger.py
@@ -257,27 +257,38 @@ class PDNSPBConnHandler(object):
         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 0b95ea30178a896c0cb538d5af1895cc0054fad5..ed97a22d6d6c92e8ea4fb5714647868dc0998abc 100644 (file)
--- 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 7727a35cc75524024dbf5cfe447b9f0923a8c7c6..67ed5c57d12f083bf71ccc0cd7120b88357b77f7 100644 (file)
--- a/pdns/dnswriter.cc
+++ b/pdns/dnswriter.cc
@@ -95,7 +95,7 @@ template <typename Container> void GenericDNSPacketWriter<Container>::startRecor
   d_sor=d_content.size(); // this will remind us where to stuff the record size
 }
 
-template <typename Container> uint32_t GenericDNSPacketWriter<Container>::addOpt(const uint16_t udpsize, const uint16_t extRCode, const uint16_t ednsFlags, const optvect_t& options, const uint8_t version)
+template <typename Container> void GenericDNSPacketWriter<Container>::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 <typename Container> uint32_t GenericDNSPacketWriter<Container>::addOpt
     xfr16BitInt(option.second.length());
     xfrBlob(option.second);
   }
-  return ttl;
 }
 
 template <typename Container> void GenericDNSPacketWriter<Container>::xfr48BitInt(uint64_t val)

diff --git a/pdns/dnswriter.hh b/pdns/dnswriter.hh
index fa0540faacf49d5968b472446d1c53e7671f6353..4d6d28618242b27aea11e79d21e3b6826a371ebc 100644 (file)
--- 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<pair<uint16_t,std::string> >;
-  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 b2c96c2184235012e4c0de9d0285950c0ce1d002..baac7c87f9065b5788ecb0dd8c2e15e43429708d 100644 (file)
--- 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<protozero::pbf_tag_type>(Field::response)};

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 59a543ceb942f2568245b3959047147ecf6846c6..2291986a58c78f786fdd75a2c59403f0547ef83f 100644 (file)
--- 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<unsigned>(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 806786ee3d08ee36fbd49a2b30c2b25ea7c7b81c..10d47c3e4243ba0a0eed43f1cf6774036530ab79 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -562,12 +562,11 @@ void protobufLogQuery(LocalStateHolder<LuaConfigItems>& 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<LuaCon
   for (const auto& metaItem : meta) {
     pbMessage.setMeta(metaItem.first, metaItem.second.stringVal, metaItem.second.intVal);
   }
-  pbMessage.setMeta("_pdnsRFlags", {}, {htons(*getFlagsFromDNSHeader(header))});
 #ifdef NOD_ENABLED
   if (g_nodEnabled) {
     pbMessage.setNewlyObservedDomain(false);