From: Pieter Lexis Date: Fri, 12 Dec 2025 15:04:40 +0000 (+0100) Subject: feat(dnsdist): Allow stripping TRACEPARENT EDNS option in SetTraceAction X-Git-Tag: rec-5.4.0-beta1~65^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=de5523cd4b256fe54c6c93cf355506c3965f583c;p=thirdparty%2Fpdns.git feat(dnsdist): Allow stripping TRACEPARENT EDNS option in SetTraceAction --- diff --git a/pdns/dnsdistdist/dnsdist-actions-definitions.yml b/pdns/dnsdistdist/dnsdist-actions-definitions.yml index e80e3f990f..371e2fa99b 100644 --- a/pdns/dnsdistdist/dnsdist-actions-definitions.yml +++ b/pdns/dnsdistdist/dnsdist-actions-definitions.yml @@ -422,6 +422,10 @@ are processed after this action" type: "u16" default: 65500 description: "The EDNS Option code to take the Trace ID from" + - name: "strip_incoming_traceid" + type: "bool" + default: "false" + description: "Remove any existing TRACEPARENT EDNS option from the query. When use_incoming_traceid is set to true, the ID will be picked set before removing the option" - name: "SNMPTrap" description: "Send an SNMP trap, adding the message string as the query description. Subsequent rules are processed after this action" parameters: diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.cc b/pdns/dnsdistdist/dnsdist-actions-factory.cc index cf4c5cbdf1..4e0c303b37 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.cc +++ b/pdns/dnsdistdist/dnsdist-actions-factory.cc @@ -1683,7 +1683,7 @@ class SetTraceAction : public DNSAction { public: SetTraceAction(SetTraceActionConfiguration& config) : - d_value{config.value}, d_loggers(config.remote_loggers), d_useIncomingTraceID(config.use_incoming_traceid), d_incomingTraceIDOptionCode(config.trace_edns_option) {}; + d_value{config.value}, d_loggers(config.remote_loggers), d_useIncomingTraceID(config.use_incoming_traceid), d_incomingTraceIDOptionCode(config.trace_edns_option), d_stripIncomingTraceID(config.strip_incoming_traceid) {}; DNSAction::Action operator()([[maybe_unused]] DNSQuestion* dnsquestion, [[maybe_unused]] std::string* ruleresult) const override { @@ -1692,33 +1692,70 @@ public: vinfolog("SetTraceAction called, but OpenTelemetry tracing is globally disabled. Did you forget to call setOpenTelemetryTracing?"); return Action::None; } + dnsquestion->ids.tracingEnabled = d_value; - if (d_value) { - tracer->setRootSpanAttribute("query.qname", AnyValue{dnsquestion->ids.qname.toStringNoDot()}); - tracer->setRootSpanAttribute("query.qtype", AnyValue{QType(dnsquestion->ids.qtype).toString()}); - tracer->setRootSpanAttribute("query.remote.address", AnyValue{dnsquestion->ids.origRemote.toString()}); - tracer->setRootSpanAttribute("query.remote.port", AnyValue{dnsquestion->ids.origRemote.getPort()}); - if (d_useIncomingTraceID.value_or(false)) { - if (dnsquestion->ednsOptions == nullptr && !parseEDNSOptions(*dnsquestion)) { - // Maybe parsed, but no EDNS found - return Action::None; - } - if (dnsquestion->ednsOptions == nullptr) { - // Parsing failed, log a warning and return - vinfolog("parsing EDNS options failed while looking for OpenTelemetry Trace ID"); - return Action::None; - } - pdns::trace::TraceID traceID; - pdns::trace::SpanID spanID; - if (pdns::trace::extractOTraceIDs(*(dnsquestion->ednsOptions), EDNSOptionCode::EDNSOptionCodeEnum(d_incomingTraceIDOptionCode.value_or(EDNSOptionCode::OTTRACEIDS)), traceID, spanID)) { - tracer->setTraceID(traceID); - if (spanID != pdns::trace::s_emptySpanID) { - tracer->setRootSpanID(spanID); - } + + if (!d_value) { + // Tracing has been turned off or should not be enabled, clean up + dnsquestion->ids.ottraceLoggers.clear(); + return Action::None; + } + + dnsquestion->ids.ottraceLoggers = d_loggers; + tracer->setRootSpanAttribute("query.qname", AnyValue{dnsquestion->ids.qname.toStringNoDot()}); + tracer->setRootSpanAttribute("query.qtype", AnyValue{QType(dnsquestion->ids.qtype).toString()}); + tracer->setRootSpanAttribute("query.remote.address", AnyValue{dnsquestion->ids.origRemote.toString()}); + tracer->setRootSpanAttribute("query.remote.port", AnyValue{dnsquestion->ids.origRemote.getPort()}); + + if (!d_useIncomingTraceID.value_or(false) && !d_stripIncomingTraceID.value_or(false)) { + // No need to check EDNS + return Action::None; + } + + // We need to check if EDNS Options exist + if (dnsquestion->ednsOptions == nullptr && !parseEDNSOptions(*dnsquestion)) { + // Maybe parsed, but no EDNS found + return Action::None; + } + if (dnsquestion->ednsOptions == nullptr) { + // Parsing failed, log a warning and return + vinfolog("parsing EDNS options failed while looking for OpenTelemetry Trace ID"); + return Action::None; + } + + if (d_useIncomingTraceID.value_or(false)) { + pdns::trace::TraceID traceID; + pdns::trace::SpanID spanID; + if (pdns::trace::extractOTraceIDs(*(dnsquestion->ednsOptions), EDNSOptionCode::EDNSOptionCodeEnum(d_incomingTraceIDOptionCode.value_or(EDNSOptionCode::OTTRACEIDS)), traceID, spanID)) { + tracer->setTraceID(traceID); + if (spanID != pdns::trace::s_emptySpanID) { + tracer->setRootSpanID(spanID); } } - dnsquestion->ids.ottraceLoggers = d_loggers; } + + if (d_stripIncomingTraceID.value_or(false)) { + uint16_t optStart; + size_t optLen; + bool last; + + if (locateEDNSOptRR(dnsquestion->getData(), &optStart, &optLen, &last) != 0) { + // Can't find the EDNS OPT RR, can't happen + return Action::None; + } + + if (!last) { + vinfolog("SetTraceAction: EDNS options is not the last RR in the packet, not removing TRACEPARENT"); + return Action::None; + } + + size_t existingOptLen = optLen; + removeEDNSOptionFromOPT(reinterpret_cast(&dnsquestion->getMutableData().at(optStart)), &optLen, d_incomingTraceIDOptionCode.value_or(EDNSOptionCode::OTTRACEIDS)); + dnsquestion->getMutableData().resize(dnsquestion->getData().size() - (existingOptLen - optLen)); + // Ensure the EDNS Option View is not out of date + dnsquestion->ednsOptions.reset(); + } + return Action::None; } @@ -1734,6 +1771,7 @@ private: std::optional d_useIncomingTraceID; std::optional d_incomingTraceIDOptionCode; + std::optional d_stripIncomingTraceID; }; #endif diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.hh b/pdns/dnsdistdist/dnsdist-actions-factory.hh index 041d9c7fe1..a75e9300dd 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.hh +++ b/pdns/dnsdistdist/dnsdist-actions-factory.hh @@ -129,6 +129,7 @@ struct SetTraceActionConfiguration std::vector> remote_loggers; bool use_incoming_traceid = false; std::uint16_t trace_edns_option = 0; + bool strip_incoming_traceid = false; }; std::shared_ptr getSetTraceAction(SetTraceActionConfiguration& config); #endif /* DISABLE_PROTOBUF */ diff --git a/pdns/dnsdistdist/dnsdist-configuration-yaml.cc b/pdns/dnsdistdist/dnsdist-configuration-yaml.cc index b156353254..3ffc2ea17e 100644 --- a/pdns/dnsdistdist/dnsdist-configuration-yaml.cc +++ b/pdns/dnsdistdist/dnsdist-configuration-yaml.cc @@ -1708,6 +1708,7 @@ std::shared_ptr getSetTraceAction(const SetTraceActionConfigur .remote_loggers = std::move(loggers), .use_incoming_traceid = config.use_incoming_traceid, .trace_edns_option = config.trace_edns_option, + .strip_incoming_traceid = config.strip_incoming_traceid, }; auto action = dnsdist::actions::getSetTraceAction(actionConfig); diff --git a/pdns/dnsdistdist/dnsdist-lua-actions.cc b/pdns/dnsdistdist/dnsdist-lua-actions.cc index 3aeb391743..b5da5a1094 100644 --- a/pdns/dnsdistdist/dnsdist-lua-actions.cc +++ b/pdns/dnsdistdist/dnsdist-lua-actions.cc @@ -238,7 +238,7 @@ void setupLuaActions(LuaContext& luaCtx) }); #ifndef DISABLE_PROTOBUF - luaCtx.writeFunction("SetTraceAction", [](bool value, std::optional>> remote_loggers, std::optional use_incoming_traceid, std::optional trace_edns_option) { + luaCtx.writeFunction("SetTraceAction", [](bool value, std::optional>> remote_loggers, std::optional use_incoming_traceid, std::optional trace_edns_option, std::optional strip_incoming_traceid) { dnsdist::actions::SetTraceActionConfiguration config; if (remote_loggers) { @@ -259,6 +259,7 @@ void setupLuaActions(LuaContext& luaCtx) config.value = value; config.trace_edns_option = trace_edns_option.value_or(65500); config.use_incoming_traceid = use_incoming_traceid.value_or(false); + config.strip_incoming_traceid = strip_incoming_traceid.value_or(false); return dnsdist::actions::getSetTraceAction(config); }); diff --git a/pdns/dnsdistdist/docs/reference/ottrace.rst b/pdns/dnsdistdist/docs/reference/ottrace.rst index 9424c0b5a1..879b8f0844 100644 --- a/pdns/dnsdistdist/docs/reference/ottrace.rst +++ b/pdns/dnsdistdist/docs/reference/ottrace.rst @@ -139,3 +139,7 @@ Should there be no ID in the incoming query, a random ID will be generated. type: SetTrace value: true use_incoming_traceid: true + +As :program:`dnsdist` keeps EDNS existing options in the query, the Trace ID option is passed as-is to the backend, which might not be desirable. +Using the ``strip_incoming_traceid`` boolean option, the EDNS option will be removed from the query. +Note that this will only happen when ``value`` is set to ``true``. diff --git a/regression-tests.dnsdist/test_OpenTelemetryTracing.py b/regression-tests.dnsdist/test_OpenTelemetryTracing.py index 6a5964fa19..4f6a13285a 100644 --- a/regression-tests.dnsdist/test_OpenTelemetryTracing.py +++ b/regression-tests.dnsdist/test_OpenTelemetryTracing.py @@ -4,10 +4,12 @@ import base64 import binascii import dns.message import dns.rrset +import dns.rcode import dns.rdataclass import dns.rdatatype import dns.edns import time +import threading import opentelemetry.proto.trace.v1.trace_pb2 import google.protobuf.json_format @@ -745,3 +747,93 @@ query_rules: def testTCP(self): self.doTest(useTCP=True, extraFunctions={"queueResponse"}) + + +def servfailOnTraceParent(request: dns.message.Message): + response = dns.message.make_response(request) + if any(opt.otype == 65500 for opt in request.options): + response.set_rcode(dns.rcode.SERVFAIL) + return response.to_wire() + + +class TestOpenTelemetryTracingStripIncomingTraceParent( + DNSDistOpenTelemetryProtobufTest +): + _yaml_config_params = [ + "_testServerPort", + ] + _yaml_config_template = """--- +logging: + open_telemetry_tracing: true + +backends: + - address: 127.0.0.1:%d + protocol: Do53 + +query_rules: + - name: Enable tracing + selector: + type: All + action: + type: SetTrace + value: true + strip_incoming_traceid: true +""" + + @classmethod + def startResponders(cls): + print("Launching responders..") + + cls._UDPResponder = threading.Thread( + name="UDP Responder", + target=cls.UDPResponder, + args=[ + cls._testServerPort, + cls._toResponderQueue, + cls._fromResponderQueue, + False, + servfailOnTraceParent, + ], + ) + cls._UDPResponder.daemon = True + cls._UDPResponder.start() + + cls._TCPResponder = threading.Thread( + name="TCP Responder", + target=cls.TCPResponder, + args=[ + cls._testServerPort, + cls._toResponderQueue, + cls._fromResponderQueue, + False, + False, + servfailOnTraceParent, + ], + ) + cls._TCPResponder.daemon = True + cls._TCPResponder.start() + + def doQuery(self, useTCP=False): + name = "query.ot.tests.powerdns.com." + + ottrace = dns.edns.GenericOption(str(65500), "\x00\x00") + ottrace.data += binascii.a2b_hex("12345678901234567890123456789012") + ottrace.data += binascii.a2b_hex("1234567890123456") + query = dns.message.make_query( + name, "A", "IN", use_edns=True, options=[ottrace] + ) + + if useTCP: + (_, receivedResponse) = self.sendTCPQuery(query, response=None) + else: + (_, receivedResponse) = self.sendUDPQuery(query, response=None) + + self.assertIsNotNone(receivedResponse) + # If we stripped the OpenTelemetry Trace ID from the query, we should not get a SERVFAIL + self.assertEqual(receivedResponse.rcode(), dns.rcode.NOERROR) + + def testStripIncomingTraceIDUDP(self): + self.doQuery() + + def testStripIncomingTraceIDTCP(self): + self.doQuery(True)