Handle the new flag field addition to the EDNS option record

While there, fix an out of bounds access for an incomplete spanID.

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

diff --git a/pdns/protozero-trace.hh b/pdns/protozero-trace.hh
index 2e53a34076f1e51a5d0e2a989c0863f6de8eca2f..3e8353e2fa720ebb6fe2e18d9408081051c67d87 100644 (file)
--- a/pdns/protozero-trace.hh
+++ b/pdns/protozero-trace.hh
@@ -773,11 +773,11 @@ inline KeyValueList KeyValueList::decode(protozero::pbf_reader& reader)
 
 struct EDNSOTTraceRecord
 {
-  // 1 byte version, 1 byte reserved/alignment, 16 bytes traceid, optional 8 bytes spanid
-  static constexpr size_t fullSize = 1 + 1 + 16 + 8;
-  static constexpr size_t sizeNoSpanID = 1 + 1 + 16;
+  // 1 byte version, 1 byte reserved, 16 bytes traceid, 8 bytes spanid, 1 byte flags
+  static constexpr size_t fullSize = 1 + 1 + 16 + 8 + 1;
   static constexpr size_t traceIDOffset = 1 + 1;
-  static constexpr size_t spanIDOffset = 1 + 1 + 16;
+  static constexpr size_t spanIDOffset = traceIDOffset + 16;
+  static constexpr size_t flagsOffset = spanIDOffset + 8;
 
   EDNSOTTraceRecord(uint8_t* arg) :
     data(arg) {}
@@ -794,6 +794,10 @@ struct EDNSOTTraceRecord
   {
     std::copy(spanid.begin(), spanid.end(), &data[spanIDOffset]);
   }
+  void setFlags(const uint8_t flags)
+  {
+    data[flagsOffset] = flags;
+  }
   // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
 private:
   uint8_t* data;
@@ -815,7 +819,7 @@ struct EDNSOTTraceRecordView
   }
   [[nodiscard]] bool getTraceID(TraceID& traceid) const
   {
-    if (size >= pdns::trace::EDNSOTTraceRecord::sizeNoSpanID) {
+    if (size == pdns::trace::EDNSOTTraceRecord::fullSize) {
       std::copy(&data[EDNSOTTraceRecord::traceIDOffset], &data[EDNSOTTraceRecord::traceIDOffset + traceid.size()], traceid.begin());
       return true;
     }
@@ -824,11 +828,20 @@ struct EDNSOTTraceRecordView
   [[nodiscard]] bool getSpanID(SpanID& spanid) const
   {
     if (size == pdns::trace::EDNSOTTraceRecord::fullSize) {
+      std::string x((char*)&data[EDNSOTTraceRecord::spanIDOffset], spanid.size());
       std::copy(&data[EDNSOTTraceRecord::spanIDOffset], &data[EDNSOTTraceRecord::spanIDOffset + spanid.size()], spanid.begin());
       return true;
     }
     return false;
   }
+  [[nodiscard]] bool getFlags(uint8_t& flags) const
+  {
+    if (size == pdns::trace::EDNSOTTraceRecord::fullSize) {
+      flags = data[EDNSOTTraceRecord::flagsOffset];
+      return true;
+    }
+    return false;
+  }
   // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
 private:
   const uint8_t* const data;

diff --git a/pdns/sdig.cc b/pdns/sdig.cc
index 44b6cabb82dd217f9e37b446776580cb0f697212..cbdd0f94b73bb002f9aa2b6a70d8342349f7c9fd 100644 (file)
--- a/pdns/sdig.cc
+++ b/pdns/sdig.cc
@@ -57,7 +57,7 @@ static const string nameForClass(QClass qclass, uint16_t qtype)
   return qclass.toString();
 }
 
-using OpenTelemetryData = std::optional<std::pair<pdns::trace::TraceID, pdns::trace::SpanID>>;
+using OpenTelemetryData = std::optional<std::tuple<pdns::trace::TraceID, pdns::trace::SpanID, uint8_t>>;
 
 static std::unordered_set<uint16_t> s_expectedIDs;
 
@@ -95,13 +95,15 @@ static void fillPacket(vector<uint8_t>& packet, const string& q, const string& t
       opts.emplace_back(EDNSOptionCode::COOKIE, cookieOpt.makeOptString());
     }
     if (otids) {
-      const auto traceid = otids->first;
-      const auto spanid = otids->second;
+      const auto traceid = std::get<0>(*otids);
+      const auto spanid = std::get<1>(*otids);
+      const auto flags = std::get<2>(*otids);
       std::array<uint8_t, pdns::trace::EDNSOTTraceRecord::fullSize> data{};
       pdns::trace::EDNSOTTraceRecord record{data.data()};
       record.setVersion(0);
       record.setTraceID(traceid);
       record.setSpanID(spanid);
+      record.setFlags(flags);
       opts.emplace_back(EDNSOptionCode::OTTRACEIDS, std::string_view(reinterpret_cast<const char*>(data.data()), data.size())); // NOLINT
     }
     pw.addOpt(bufsize, 0, dnssec ? EDNSOpts::DNSSECOK : 0, opts);
@@ -372,6 +374,8 @@ try {
         auto traceIDArg = std::string(argv[++i]);
         pdns::trace::TraceID traceid;
         pdns::trace::SpanID spanid;
+        uint8_t traceflags = 0;
+        // Cannot set flags yet.
         if (traceIDArg == "-") {
           traceid.makeRandom();
           spanid.makeRandom();
@@ -379,10 +383,14 @@ try {
         else {
           auto traceIDStr = makeBytesFromHex(traceIDArg);
           if (traceIDStr.size() > traceid.size() + spanid.size()) {
-            cerr << "Maximum length of traceid plus spanid is " << traceid.size() + spanid.size()<< " bytes" << endl;
+            cerr << "Maximum length of traceid plus spanid is " << traceid.size() + spanid.size() << " bytes" << endl;
             exit(EXIT_FAILURE);
           }
-          std::string spanidStr(traceIDStr.begin() + traceid.size(), traceIDStr.end());
+
+          std::string spanidStr;
+          if (traceIDStr.size() > traceid.size()) {
+            spanidStr = std::string(traceIDStr.begin() + traceid.size(), traceIDStr.end());
+          }
           traceIDStr.resize(traceid.size());
           pdns::trace::fill(traceid, traceIDStr);
           if (spanidStr.empty()) {
@@ -394,7 +402,7 @@ try {
             pdns::trace::fill(spanid, spanidStr);
           }
         }
-        otdata = std::make_pair(traceid, spanid);
+        otdata = std::make_tuple(traceid, spanid, traceflags);
       }
       else {
         cerr << argv[i] << ": unknown argument" << endl;