From: Remi Gacogne Date: Thu, 16 May 2024 15:07:27 +0000 (+0200) Subject: dnsdist: Remove XPF support X-Git-Tag: rec-5.1.0-beta1~33^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F14184%2Fhead;p=thirdparty%2Fpdns.git dnsdist: Remove XPF support --- diff --git a/builder-support/debian/authoritative/debian-buster/control b/builder-support/debian/authoritative/debian-buster/control index 3148aa707f..9f1458a9f4 100644 --- a/builder-support/debian/authoritative/debian-buster/control +++ b/builder-support/debian/authoritative/debian-buster/control @@ -81,7 +81,7 @@ Description: Tools for DNS debugging by PowerDNS * nsec3dig: Calculate the correctness of NSEC3 proofs * pdns_notify: Simple tool for sending DNS notifies * saxfr: AXFR zones and show extra information - * sdig: dig-like tool supporting DoH, DoT, PROXY-protocol and XPF + * sdig: dig-like tool supporting DoH, DoT and PROXY-protocol * stubquery: Stub resolver query tool Package: pdns-ixfrdist diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index e5255a67f7..5fb7fa5a8a 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -208,7 +208,6 @@ dnsdist_SOURCES = \ dnsdist-tcp-upstream.hh \ dnsdist-tcp.cc dnsdist-tcp.hh \ dnsdist-web.cc dnsdist-web.hh \ - dnsdist-xpf.cc dnsdist-xpf.hh \ dnsdist-xsk.cc dnsdist-xsk.hh \ dnsdist.cc dnsdist.hh \ dnslabeltext.cc \ @@ -258,7 +257,6 @@ dnsdist_SOURCES = \ threadname.hh threadname.cc \ uuid-utils.hh uuid-utils.cc \ views.hh \ - xpf.cc xpf.hh \ xsk.cc xsk.hh testrunner_SOURCES = \ @@ -309,7 +307,6 @@ testrunner_SOURCES = \ dnsdist-svc.cc dnsdist-svc.hh \ dnsdist-tcp-downstream.cc \ dnsdist-tcp.cc dnsdist-tcp.hh \ - dnsdist-xpf.cc dnsdist-xpf.hh \ dnsdist-xsk.cc dnsdist-xsk.hh \ dnsdist.hh \ dnslabeltext.cc \ @@ -368,7 +365,6 @@ testrunner_SOURCES = \ testrunner.cc \ threadname.hh threadname.cc \ uuid-utils.hh uuid-utils.cc \ - xpf.cc xpf.hh \ xsk.cc xsk.hh dnsdist_LDFLAGS = \ diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h b/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h index 14d6fabd5e..a13d64bffc 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h +++ b/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h @@ -79,7 +79,6 @@ bool dnsdist_ffi_dnsquestion_get_tcp(const dnsdist_ffi_dnsquestion_t* dq) __attr dnsdist_ffi_protocol_type dnsdist_ffi_dnsquestion_get_protocol(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); bool dnsdist_ffi_dnsquestion_get_skip_cache(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); bool dnsdist_ffi_dnsquestion_get_use_ecs(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); -bool dnsdist_ffi_dnsquestion_get_add_xpf(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); bool dnsdist_ffi_dnsquestion_get_ecs_override(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); uint16_t dnsdist_ffi_dnsquestion_get_ecs_prefix_length(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); bool dnsdist_ffi_dnsquestion_is_temp_failure_ttl_set(const dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index 6c08cfc1bc..ebeb99f8f2 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -209,11 +209,6 @@ bool dnsdist_ffi_dnsquestion_get_use_ecs(const dnsdist_ffi_dnsquestion_t* dq) return dq->dq->useECS; } -bool dnsdist_ffi_dnsquestion_get_add_xpf(const dnsdist_ffi_dnsquestion_t* dq) -{ - return dq->dq->addXPF; -} - bool dnsdist_ffi_dnsquestion_get_ecs_override(const dnsdist_ffi_dnsquestion_t* dq) { return dq->dq->ecsOverride; diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 55075c8efc..6d62d20acb 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -534,8 +534,6 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) getOptionalValue(vars, "disableZeroScope", config.disableZeroScope); getOptionalValue(vars, "ipBindAddrNoPort", config.ipBindAddrNoPort); - getOptionalIntegerValue("newServer", vars, "addXPF", config.xpfRRCode); - getOptionalValue(vars, "reconnectOnUp", config.reconnectOnUp); LuaArray cpuMap; diff --git a/pdns/dnsdistdist/dnsdist-tcp.cc b/pdns/dnsdistdist/dnsdist-tcp.cc index 53d149e11a..240f4fbf02 100644 --- a/pdns/dnsdistdist/dnsdist-tcp.cc +++ b/pdns/dnsdistdist/dnsdist-tcp.cc @@ -36,7 +36,6 @@ #include "dnsdist-tcp-downstream.hh" #include "dnsdist-downstream-connection.hh" #include "dnsdist-tcp-upstream.hh" -#include "dnsdist-xpf.hh" #include "dnsparser.hh" #include "dolog.hh" #include "gettime.hh" diff --git a/pdns/dnsdistdist/dnsdist-xpf.cc b/pdns/dnsdistdist/dnsdist-xpf.cc deleted file mode 100644 index df560ac38d..0000000000 --- a/pdns/dnsdistdist/dnsdist-xpf.cc +++ /dev/null @@ -1,67 +0,0 @@ -/* - * This file is part of PowerDNS or dnsdist. - * Copyright -- PowerDNS.COM B.V. and its contributors - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * In addition, for the avoidance of any doubt, permission is granted to - * link this program with OpenSSL and to (re)distribute the binaries - * produced as the result of such linking. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#include "dnsdist-xpf.hh" - -#include "dnsdist-dnsparser.hh" -#include "dnsparser.hh" -#include "xpf.hh" - -bool addXPF(DNSQuestion& dnsQuestion, uint16_t optionCode) -{ - std::string payload = generateXPFPayload(dnsQuestion.overTCP(), dnsQuestion.ids.origRemote, dnsQuestion.ids.origDest); - uint8_t root = '\0'; - dnsrecordheader drh{}; - drh.d_type = htons(optionCode); - drh.d_class = htons(QClass::IN); - drh.d_ttl = 0; - drh.d_clen = htons(payload.size()); - size_t recordHeaderLen = sizeof(root) + sizeof(drh); - - if (!dnsQuestion.hasRoomFor(payload.size() + recordHeaderLen)) { - return false; - } - - size_t xpfSize = sizeof(root) + sizeof(drh) + payload.size(); - auto& data = dnsQuestion.getMutableData(); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - uint32_t realPacketLen = getDNSPacketLength(reinterpret_cast(data.data()), data.size()); - data.resize(realPacketLen + xpfSize); - - size_t pos = realPacketLen; - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - memcpy(reinterpret_cast(&data.at(pos)), &root, sizeof(root)); - pos += sizeof(root); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - memcpy(reinterpret_cast(&data.at(pos)), &drh, sizeof(drh)); - pos += sizeof(drh); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - memcpy(reinterpret_cast(&data.at(pos)), payload.data(), payload.size()); - pos += payload.size(); - (void)pos; - - dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsQuestion.getMutableData(), [](dnsheader& header) { - header.arcount = htons(ntohs(header.arcount) + 1); - return true; - }); - return true; -} diff --git a/pdns/dnsdistdist/dnsdist-xpf.hh b/pdns/dnsdistdist/dnsdist-xpf.hh deleted file mode 100644 index a3de71029e..0000000000 --- a/pdns/dnsdistdist/dnsdist-xpf.hh +++ /dev/null @@ -1,26 +0,0 @@ -/* - * This file is part of PowerDNS or dnsdist. - * Copyright -- PowerDNS.COM B.V. and its contributors - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * In addition, for the avoidance of any doubt, permission is granted to - * link this program with OpenSSL and to (re)distribute the binaries - * produced as the result of such linking. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ -#pragma once - -#include "dnsdist.hh" - -bool addXPF(DNSQuestion& dnsQuestion, uint16_t optionCode); diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc index d411b0b66c..2b53a81b24 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -69,7 +69,6 @@ #include "dnsdist-secpoll.hh" #include "dnsdist-tcp.hh" #include "dnsdist-web.hh" -#include "dnsdist-xpf.hh" #include "dnsdist-xsk.hh" #include "base64.hh" @@ -1572,10 +1571,6 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, LocalHolders /* save the DNS flags as sent to the backend so we can cache the answer with the right flags later */ dnsQuestion.ids.cacheFlags = *getFlagsFromDNSHeader(dnsQuestion.getHeader().get()); - if (dnsQuestion.addXPF && selectedBackend->d_config.xpfRRCode != 0) { - addXPF(dnsQuestion, selectedBackend->d_config.xpfRRCode); - } - if (selectedBackend->d_config.useProxyProtocol && dnsQuestion.getProtocol().isEncrypted() && selectedBackend->d_config.d_proxyProtocolAdvertiseTLS) { if (!dnsQuestion.proxyProtocolValues) { dnsQuestion.proxyProtocolValues = std::make_unique>(); diff --git a/pdns/dnsdistdist/dnsdist.hh b/pdns/dnsdistdist/dnsdist.hh index 561e8f68c1..278751dbb0 100644 --- a/pdns/dnsdistdist/dnsdist.hh +++ b/pdns/dnsdistdist/dnsdist.hh @@ -194,7 +194,6 @@ public: uint8_t ednsRCode{0}; bool ecsOverride; bool useECS{true}; - bool addXPF{true}; bool asynchronous{false}; }; @@ -775,7 +774,6 @@ struct DownstreamState : public std::enable_shared_from_this QType checkType{QType::A}; uint16_t checkClass{QClass::IN}; uint16_t d_retries{5}; - uint16_t xpfRRCode{0}; uint16_t checkTimeout{1000}; /* in milliseconds */ uint16_t d_lazyHealthCheckSampleSize{100}; uint16_t d_lazyHealthCheckMinSampleCount{1}; diff --git a/pdns/dnsdistdist/docs/advanced/passing-source-address.rst b/pdns/dnsdistdist/docs/advanced/passing-source-address.rst index 4e8445fd21..62b54c5ec9 100644 --- a/pdns/dnsdistdist/docs/advanced/passing-source-address.rst +++ b/pdns/dnsdistdist/docs/advanced/passing-source-address.rst @@ -26,7 +26,7 @@ In addition to the global settings, rules and Lua bindings can alter this behavi In effect this means that for the EDNS Client Subnet option to be added to the request, ``useClientSubnet`` should be set to ``true`` for the backend used (default to ``false``) and ECS should not have been disabled by calling :func:`SetDisableECSAction` or setting ``dq.useECS`` to ``false`` (default to true). -Note that any trailing data present in the incoming query is removed when an OPT (or XPF) record has to be inserted. +Note that any trailing data present in the incoming query is removed when an OPT record has to be inserted. In addition to the drawback that it can only pass the source IP address, and the fact that it needs to override any existing ECS option, adding that option requires parsing and editing the query, as well as parsing and editing the response in most cases. @@ -46,24 +46,6 @@ In addition to the drawback that it can only pass the source IP address, and the | Response, EDNS with ECS | remove or edit the ECS option if needed | +----------------------------+-------------------------------------------------+ -X-Proxied-For -------------- - -.. note:: - This is a deprecated feature that will be removed in the near future. - -The experimental XPF record (from `draft-bellis-dnsop-xpf `_) is an alternative to the use of EDNS Client Subnet which has the advantages of preserving any existing EDNS Client Subnet value sent by the client, and of passing along the original destination address, as well as the initial source and destination ports. - -In order to provide the downstream server with the address of the real client, or at least the one talking to dnsdist, the ``addXPF`` parameter can be used when creating a :func:`new server `. -This parameter indicates whether an XPF record shall be added to the query. Since that record is experimental, there is currently no option code assigned to it, and therefore one needs to be specified as an argument to the ``addXPF`` parameter. - -If the incoming request already contains a XPF record, it will not be overwritten. Instead a new one will be added to the query and the existing one will be preserved. -That might be an issue by allowing clients to spoof their source address by adding a forged XPF record to their query. That can be prevented by using a rule to drop incoming queries containing a XPF record (in that example the 65280 option code has been assigned to XPF): - -.. code-block:: lua - - addAction(RecordsTypeCountRule(DNSSection.Additional, 65280, 1, 65535), DropAction()) - Proxy Protocol -------------- @@ -85,6 +67,24 @@ dnsdist 1.5.0 only supports outgoing Proxy Protocol. Support for parsing incomin Both the PowerDNS Authoritative Server and the Recursor can parse PROXYv2 headers, if configured to do so with their `proxy-protocol-from` setting. +X-Proxied-For +------------- + +.. note:: + XPF support has been removed in 2.0.0. + +The experimental XPF record (from `draft-bellis-dnsop-xpf `_) is an alternative to the use of EDNS Client Subnet which has the advantages of preserving any existing EDNS Client Subnet value sent by the client, and of passing along the original destination address, as well as the initial source and destination ports. + +In order to provide the downstream server with the address of the real client, or at least the one talking to dnsdist, the ``addXPF`` parameter can be used when creating a :func:`new server `. +This parameter indicates whether an XPF record shall be added to the query. Since that record is experimental, there is currently no option code assigned to it, and therefore one needs to be specified as an argument to the ``addXPF`` parameter. + +If the incoming request already contains a XPF record, it will not be overwritten. Instead a new one will be added to the query and the existing one will be preserved. +That might be an issue by allowing clients to spoof their source address by adding a forged XPF record to their query. That can be prevented by using a rule to drop incoming queries containing a XPF record (in that example the 65280 option code has been assigned to XPF): + +.. code-block:: lua + + addAction(RecordsTypeCountRule(DNSSection.Additional, 65280, 1, 65535), DropAction()) + Influence on caching -------------------- @@ -97,7 +97,7 @@ For that feature to work, dnsdist will look up twice into the packet cache when That feature is enabled by setting ``disableZeroScope=false`` on :func:`newServer` (default) and ``parseECS=true`` on :func:`newPacketCache` (not the default). -Things are different for XPF and the proxy protocol, because dnsdist then does the cache lookup **before** adding the payload. It means that caching can still be enabled as long as the response is not source-dependent, but should be disabled otherwise. +Things are different for the proxy protocol, because dnsdist then does the cache lookup **before** adding the payload. It means that caching can still be enabled as long as the response is not source-dependent, but should be disabled otherwise. +------------------+----------+---------------------+----------------+------------------------+ | Protocol | Standard | Require DNS parsing | Contains ports | Caching | @@ -106,7 +106,5 @@ Things are different for XPF and the proxy protocol, because dnsdist then does t +------------------+----------+---------------------+----------------+------------------------+ | ECS (zero-scope) | Yes | Query and response | No | Yes | +------------------+----------+---------------------+----------------+------------------------+ -| XPF | No | Query | Yes | Depends on the backend | -+------------------+----------+---------------------+----------------+------------------------+ | Proxy Protocol | No | No | Yes | Depends on the backend | +------------------+----------+---------------------+----------------+------------------------+ diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index 0484a2e09a..8967187a07 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -638,6 +638,9 @@ Servers .. versionchanged:: 1.9.0 Added ``MACAddr``, ``proxyProtocolAdvertiseTLS`` and ``xskSockets`` to server_table. + .. versionchanged:: 2.0.0 + Removed ``addXPF`` from server_table. + :param str server_string: A simple IP:PORT string. :param table server_table: A table with at least an ``address`` key @@ -688,7 +691,6 @@ Servers - address, e.g. ``""192.0.2.2""`` - interface name, e.g. ``""eth0""`` - address@interface, e.g. ``""192.0.2.2@eth0""`` " - ``addXPF`` ``number`` "Add the client's IP address and port to the query, along with the original destination address and port, using the experimental XPF record from `draft-bellis-dnsop-xpf `_ and the specified option code. Default is disabled (0). This is a deprecated feature that will be removed in the near future." ``sockets`` ``number`` "Number of UDP sockets (and thus source ports) used toward the backend server, defaults to a single one. Note that for backends which are multithreaded, this setting will have an effect on the number of cores that will be used to process traffic from dnsdist. For example you may want to set 'sockets' to a number somewhat higher than the number of worker threads configured in the backend, particularly if the Linux kernel is being used to distribute traffic to multiple threads listening on the same socket (via `reuseport`). See also :func:`setRandomizedOutgoingSockets`." ``disableZeroScope`` ``bool`` "Disable the EDNS Client Subnet 'zero scope' feature, which does a cache lookup for an answer valid for all subnets (ECS scope of 0) before adding ECS information to the query and doing the regular lookup. This requires the ``parseECS`` option of the corresponding cache to be set to true" ``rise`` ``number`` "Require ``number`` consecutive successful checks before declaring the backend up, default: 1" diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 6bdd576471..f75cc10383 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -30,7 +30,6 @@ #include "dnsdist-metrics.hh" #include "dnsdist-proxy-protocol.hh" #include "dnsdist-rules.hh" -#include "dnsdist-xpf.hh" #include "libssl.hh" #include "threadname.hh" diff --git a/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc b/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc index 79c9ef9677..df8f4d87fa 100644 --- a/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc @@ -214,10 +214,6 @@ BOOST_AUTO_TEST_CASE(test_Query) BOOST_CHECK_EQUAL(dnsdist_ffi_dnsquestion_get_use_ecs(&lightDQ), false); } - { - BOOST_CHECK_EQUAL(dnsdist_ffi_dnsquestion_get_add_xpf(&lightDQ), true); - } - { BOOST_CHECK_EQUAL(dnsdist_ffi_dnsquestion_get_ecs_override(&lightDQ), false); dnsdist_ffi_dnsquestion_set_ecs_override(&lightDQ, true); diff --git a/pdns/dnsdistdist/test-dnsdist_cc.cc b/pdns/dnsdistdist/test-dnsdist_cc.cc index f9704f9f45..03811f295a 100644 --- a/pdns/dnsdistdist/test-dnsdist_cc.cc +++ b/pdns/dnsdistdist/test-dnsdist_cc.cc @@ -32,7 +32,6 @@ #include "dnsdist-ecs.hh" #include "dnsdist-internal-queries.hh" #include "dnsdist-tcp.hh" -#include "dnsdist-xpf.hh" #include "dnsdist-xsk.hh" #include "dolog.hh" @@ -99,7 +98,7 @@ BOOST_AUTO_TEST_SUITE(test_dnsdist_cc) static const uint16_t ECSSourcePrefixV4 = 24; static const uint16_t ECSSourcePrefixV6 = 56; -static void validateQuery(const PacketBuffer& packet, bool hasEdns = true, bool hasXPF = false, uint16_t additionals = 0, uint16_t answers = 0, uint16_t authorities = 0) +static void validateQuery(const PacketBuffer& packet, bool hasEdns = true, uint16_t additionals = 0, uint16_t answers = 0, uint16_t authorities = 0) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) MOADNSParser mdp(true, reinterpret_cast(packet.data()), packet.size()); @@ -109,7 +108,7 @@ static void validateQuery(const PacketBuffer& packet, bool hasEdns = true, bool BOOST_CHECK_EQUAL(mdp.d_header.qdcount, 1U); BOOST_CHECK_EQUAL(mdp.d_header.ancount, answers); BOOST_CHECK_EQUAL(mdp.d_header.nscount, authorities); - uint16_t expectedARCount = additionals + (hasEdns ? 1U : 0U) + (hasXPF ? 1U : 0U); + uint16_t expectedARCount = additionals + (hasEdns ? 1U : 0U); BOOST_CHECK_EQUAL(mdp.d_header.arcount, expectedARCount); } @@ -149,82 +148,6 @@ static void validateResponse(const PacketBuffer& packet, bool hasEdns, uint8_t a BOOST_CHECK_EQUAL(mdp.d_header.arcount, (hasEdns ? 1U : 0U) + additionalCount); } -BOOST_AUTO_TEST_CASE(test_addXPF) -{ - static const uint16_t xpfOptionCode = 65422; - - DNSName name("www.powerdns.com."); - InternalQueryState ids; - ids.protocol = dnsdist::Protocol::DoUDP; - ids.origRemote = ComboAddress("::1"); - ids.origDest = ComboAddress("::1"); - - PacketBuffer query; - GenericDNSPacketWriter packetWriter(query, name, QType::A, QClass::IN, 0); - packetWriter.getHeader()->rd = 1; - PacketBuffer queryWithXPF; - - { - PacketBuffer packet = query; - - /* large enough packet */ - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - ids.qname = DNSName(reinterpret_cast(packet.data()), packet.size(), sizeof(dnsheader), false, &ids.qtype, &ids.qclass); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) - DNSQuestion dnsQuestion(ids, const_cast(packet)); - BOOST_CHECK_EQUAL(ids.qname, name); - BOOST_CHECK(ids.qtype == QType::A); - - BOOST_CHECK(addXPF(dnsQuestion, xpfOptionCode)); - BOOST_CHECK(packet.size() > query.size()); - validateQuery(packet, false, true); - queryWithXPF = packet; - } - - { - PacketBuffer packet = query; - - /* packet is already too large for the 4096 limit over UDP */ - packet.resize(4096); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - ids.qname = DNSName(reinterpret_cast(packet.data()), packet.size(), sizeof(dnsheader), false, &ids.qtype, &ids.qclass); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) - DNSQuestion dnsQuestion(ids, const_cast(packet)); - BOOST_CHECK_EQUAL(ids.qname, name); - BOOST_CHECK(ids.qtype == QType::A); - - BOOST_REQUIRE(!addXPF(dnsQuestion, xpfOptionCode)); - BOOST_CHECK_EQUAL(packet.size(), 4096U); - packet.resize(query.size()); - validateQuery(packet, false, false); - } - - { - PacketBuffer packet = query; - - /* packet with trailing data (overriding it) */ - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - ids.qname = DNSName(reinterpret_cast(packet.data()), packet.size(), sizeof(dnsheader), false, &ids.qtype, &ids.qclass); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) - DNSQuestion dnsQuestion(ids, const_cast(packet)); - BOOST_CHECK_EQUAL(ids.qname, name); - BOOST_CHECK(ids.qtype == QType::A); - - /* add trailing data */ - const size_t trailingDataSize = 10; - /* Making sure we have enough room to allow for fake trailing data */ - packet.resize(packet.size() + trailingDataSize); - for (size_t idx = 0; idx < trailingDataSize; idx++) { - packet.push_back('A'); - } - - BOOST_CHECK(addXPF(dnsQuestion, xpfOptionCode)); - BOOST_CHECK_EQUAL(packet.size(), queryWithXPF.size()); - BOOST_CHECK_EQUAL(memcmp(queryWithXPF.data(), packet.data(), queryWithXPF.size()), 0); - validateQuery(packet, false, true); - } -} - BOOST_AUTO_TEST_CASE(addECSWithoutEDNS) { bool ednsAdded = false; @@ -334,7 +257,7 @@ BOOST_AUTO_TEST_CASE(addECSWithoutEDNSButWithAnswer) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, true); BOOST_CHECK_EQUAL(ecsAdded, true); - validateQuery(packet, true, false, 0, 1); + validateQuery(packet, true, 0, 1); validateECS(packet, remote); PacketBuffer queryWithEDNS = packet; @@ -353,7 +276,7 @@ BOOST_AUTO_TEST_CASE(addECSWithoutEDNSButWithAnswer) BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); packet.resize(query.size()); - validateQuery(packet, false, false, 0, 1); + validateQuery(packet, false, 0, 1); /* packet with trailing data (overriding it) */ packet = query; @@ -377,7 +300,7 @@ BOOST_AUTO_TEST_CASE(addECSWithoutEDNSButWithAnswer) BOOST_CHECK_EQUAL(memcmp(queryWithEDNS.data(), packet.data(), queryWithEDNS.size()), 0); BOOST_CHECK_EQUAL(ednsAdded, true); BOOST_CHECK_EQUAL(ecsAdded, true); - validateQuery(packet, true, false, 0, 1); + validateQuery(packet, true, 0, 1); } BOOST_AUTO_TEST_CASE(addECSWithoutEDNSAlreadyParsed) @@ -756,7 +679,7 @@ BOOST_AUTO_TEST_CASE(replaceECSFollowedByTSIG) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 1); + validateQuery(packet, true, 1); validateECS(packet, remote); /* not large enough packet */ @@ -774,7 +697,7 @@ BOOST_AUTO_TEST_CASE(replaceECSFollowedByTSIG) BOOST_CHECK_EQUAL(packet.size(), query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 1); + validateQuery(packet, true, 1); } BOOST_AUTO_TEST_CASE(replaceECSAfterAN) @@ -814,7 +737,7 @@ BOOST_AUTO_TEST_CASE(replaceECSAfterAN) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 0, 1, 0); + validateQuery(packet, true, 0, 1, 0); validateECS(packet, remote); /* not large enough packet */ @@ -832,7 +755,7 @@ BOOST_AUTO_TEST_CASE(replaceECSAfterAN) BOOST_CHECK_EQUAL(packet.size(), query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 0, 1, 0); + validateQuery(packet, true, 0, 1, 0); } BOOST_AUTO_TEST_CASE(replaceECSAfterAuth) @@ -872,7 +795,7 @@ BOOST_AUTO_TEST_CASE(replaceECSAfterAuth) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 0, 0, 1); + validateQuery(packet, true, 0, 0, 1); validateECS(packet, remote); /* not large enough packet */ @@ -890,7 +813,7 @@ BOOST_AUTO_TEST_CASE(replaceECSAfterAuth) BOOST_CHECK_EQUAL(packet.size(), query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 0, 0, 1); + validateQuery(packet, true, 0, 0, 1); } BOOST_AUTO_TEST_CASE(replaceECSBetweenTwoRecords) @@ -931,7 +854,7 @@ BOOST_AUTO_TEST_CASE(replaceECSBetweenTwoRecords) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 2); + validateQuery(packet, true, 2); validateECS(packet, remote); /* not large enough packet */ @@ -949,7 +872,7 @@ BOOST_AUTO_TEST_CASE(replaceECSBetweenTwoRecords) BOOST_CHECK_EQUAL(packet.size(), query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 2); + validateQuery(packet, true, 2); } BOOST_AUTO_TEST_CASE(insertECSInEDNSBetweenTwoRecords) @@ -985,7 +908,7 @@ BOOST_AUTO_TEST_CASE(insertECSInEDNSBetweenTwoRecords) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, true); - validateQuery(packet, true, false, 2); + validateQuery(packet, true, 2); validateECS(packet, remote); /* not large enough packet */ @@ -1003,7 +926,7 @@ BOOST_AUTO_TEST_CASE(insertECSInEDNSBetweenTwoRecords) BOOST_CHECK_EQUAL(packet.size(), query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false, 2); + validateQuery(packet, true, 2); } BOOST_AUTO_TEST_CASE(insertECSAfterTSIG) @@ -1036,8 +959,8 @@ BOOST_AUTO_TEST_CASE(insertECSAfterTSIG) BOOST_CHECK(packet.size() > query.size()); BOOST_CHECK_EQUAL(ednsAdded, true); BOOST_CHECK_EQUAL(ecsAdded, true); - /* the MOADNSParser does not allow anything except XPF after a TSIG */ - BOOST_CHECK_THROW(validateQuery(packet, true, false, 1), MOADNSException); + /* the MOADNSParser does not allow anything after a TSIG */ + BOOST_CHECK_THROW(validateQuery(packet, true, 1), MOADNSException); validateECS(packet, remote); /* not large enough packet */ @@ -1055,7 +978,7 @@ BOOST_AUTO_TEST_CASE(insertECSAfterTSIG) BOOST_CHECK_EQUAL(packet.size(), query.size()); BOOST_CHECK_EQUAL(ednsAdded, false); BOOST_CHECK_EQUAL(ecsAdded, false); - validateQuery(packet, true, false); + validateQuery(packet, true); } BOOST_AUTO_TEST_CASE(removeEDNSWhenFirst) diff --git a/pdns/dnsdistdist/xpf.cc b/pdns/dnsdistdist/xpf.cc deleted file mode 120000 index 98ab7228cd..0000000000 --- a/pdns/dnsdistdist/xpf.cc +++ /dev/null @@ -1 +0,0 @@ -../xpf.cc \ No newline at end of file diff --git a/pdns/dnsdistdist/xpf.hh b/pdns/dnsdistdist/xpf.hh deleted file mode 120000 index 023c8c4796..0000000000 --- a/pdns/dnsdistdist/xpf.hh +++ /dev/null @@ -1 +0,0 @@ -../xpf.hh \ No newline at end of file diff --git a/pdns/dnspacket.cc b/pdns/dnspacket.cc index 63a98041ea..b18da82c7b 100644 --- a/pdns/dnspacket.cc +++ b/pdns/dnspacket.cc @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include #include @@ -57,7 +57,7 @@ bool DNSPacket::s_doEDNSSubnetProcessing; bool DNSPacket::s_doEDNSCookieProcessing; string DNSPacket::s_EDNSCookieKey; uint16_t DNSPacket::s_udpTruncationThreshold; - + DNSPacket::DNSPacket(bool isQuery): d_isQuery(isQuery) { memset(&d, 0, sizeof(d)); @@ -132,7 +132,7 @@ void DNSPacket::setAnswer(bool b) if(b) { d_rawpacket.assign(12,(char)0); memset((void *)&d,0,sizeof(d)); - + d.qr=b; } } @@ -295,12 +295,12 @@ void DNSPacket::wrapup(bool throwsOnTruncation) pw.getHeader()->id=d.id; pw.getHeader()->rd=d.rd; pw.getHeader()->tc=d.tc; - + DNSPacketWriter::optvect_t opts; /* optsize is expected to hold an upper bound of data that will be - added after actual record data - i.e. OPT, TSIG, perhaps one day - XPF. Because of the way `pw` incrementally writes the packet, we + added after actual record data - i.e. OPT, TSIG. + Because of the way `pw` incrementally writes the packet, we cannot easily 'go back' and remove a few records. So, to prevent going over our maximum size, we keep our (potential) extra data in mind. @@ -369,13 +369,13 @@ void DNSPacket::wrapup(bool throwsOnTruncation) if(!d_rrs.empty()) pw.commit(); noCommit:; - + if(d_haveednssubnet) { EDNSSubnetOpts eso = d_eso; // use the scopeMask from the resolver, if it is greater - issue #5469 maxScopeMask = max(maxScopeMask, eso.scope.getBits()); eso.scope = Netmask(eso.source.getNetwork(), maxScopeMask); - + string opt = makeEDNSSubnetOptsString(eso); opts.emplace_back(8, opt); // 'EDNS SUBNET' } @@ -396,10 +396,10 @@ void DNSPacket::wrapup(bool throwsOnTruncation) throw; } } - + if(d_trc.d_algoName.countLabels()) addTSIG(pw, d_trc, d_tsigkeyname, d_tsigsecret, d_tsigprevious, d_tsigtimersonly); - + d_rawpacket.assign((char*)&packet[0], packet.size()); // XXX we could do this natively on a vector.. // copy RR counts so they can be read later @@ -433,7 +433,7 @@ std::unique_ptr DNSPacket::replyPacket() const r->setAnswer(true); // this implies the allocation of the header r->setA(true); // and we are authoritative r->setRA(false); // no recursion available - r->setRD(d.rd); // if you wanted to recurse, answer will say you wanted it + r->setRD(d.rd); // if you wanted to recurse, answer will say you wanted it r->setID(d.id); r->setOpcode(d.opcode); @@ -469,7 +469,7 @@ std::unique_ptr DNSPacket::replyPacket() const void DNSPacket::spoofQuestion(const DNSPacket& qd) { d_wrapped=true; // if we do this, don't later on wrapup - + int labellen; string::size_type i=sizeof(d); @@ -484,8 +484,8 @@ void DNSPacket::spoofQuestion(const DNSPacket& qd) int DNSPacket::noparse(const char *mesg, size_t length) { - d_rawpacket.assign(mesg,length); - if(length < 12) { + d_rawpacket.assign(mesg,length); + if(length < 12) { g_log << Logger::Debug << "Ignoring packet: too short ("<(&version), sizeof(version)); - ret.append(reinterpret_cast(&protocol), sizeof(protocol)); - - // We already established source and destination sin_family equivalence - if (source.isIPv4()) { - assert(addrSize == sizeof(source.sin4.sin_addr.s_addr)); - ret.append(reinterpret_cast(&source.sin4.sin_addr.s_addr), addrSize); - assert(addrSize == sizeof(destination.sin4.sin_addr.s_addr)); - ret.append(reinterpret_cast(&destination.sin4.sin_addr.s_addr), addrSize); - } - else { - assert(addrSize == sizeof(source.sin6.sin6_addr.s6_addr)); - ret.append(reinterpret_cast(&source.sin6.sin6_addr.s6_addr), addrSize); - assert(addrSize == sizeof(destination.sin6.sin6_addr.s6_addr)); - ret.append(reinterpret_cast(&destination.sin6.sin6_addr.s6_addr), addrSize); - } - - ret.append(reinterpret_cast(&sourcePort), sizeof(sourcePort)); - ret.append(reinterpret_cast(&destinationPort), sizeof(destinationPort)); - - return ret; -} - -bool parseXPFPayload(const char* payload, size_t len, ComboAddress& source, ComboAddress* destination) -{ - static const size_t addr4Size = sizeof(source.sin4.sin_addr.s_addr); - static const size_t addr6Size = sizeof(source.sin6.sin6_addr.s6_addr); - uint8_t version; - uint8_t protocol; - uint16_t sourcePort; - uint16_t destinationPort; - - if (len != (sizeof(version) + sizeof(protocol) + (addr4Size * 2) + sizeof(sourcePort) + sizeof(destinationPort)) && - len != (sizeof(version) + sizeof(protocol) + (addr6Size * 2) + sizeof(sourcePort) + sizeof(destinationPort))) { - return false; - } - - size_t pos = 0; - - memcpy(&version, payload + pos, sizeof(version)); - pos += sizeof(version); - - if (version != 4 && version != 6) { - return false; - } - - memcpy(&protocol, payload + pos, sizeof(protocol)); - pos += sizeof(protocol); - - if (protocol != 6 && protocol != 17) { - return false; - } - - const size_t addrSize = version == 4 ? sizeof(source.sin4.sin_addr.s_addr) : sizeof(source.sin6.sin6_addr.s6_addr); - if (len - pos != ((addrSize * 2) + sizeof(sourcePort) + sizeof(destinationPort))) { - return false; - } - - source = makeComboAddressFromRaw(version, payload + pos, addrSize); - pos += addrSize; - if (destination != nullptr) { - *destination = makeComboAddressFromRaw(version, payload + pos, addrSize); - } - pos += addrSize; - - memcpy(&sourcePort, payload + pos, sizeof(sourcePort)); - pos += sizeof(sourcePort); - source.sin4.sin_port = sourcePort; - - memcpy(&destinationPort, payload + pos, sizeof(destinationPort)); - pos += sizeof(destinationPort); - (void) pos; - - if (destination != nullptr) { - destination->sin4.sin_port = destinationPort; - } - - return true; -} diff --git a/pdns/xpf.hh b/pdns/xpf.hh deleted file mode 100644 index 4e8f466793..0000000000 --- a/pdns/xpf.hh +++ /dev/null @@ -1,29 +0,0 @@ -/* - * This file is part of PowerDNS or dnsdist. - * Copyright -- PowerDNS.COM B.V. and its contributors - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * In addition, for the avoidance of any doubt, permission is granted to - * link this program with OpenSSL and to (re)distribute the binaries - * produced as the result of such linking. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#pragma once - -#include - -std::string generateXPFPayload(bool tcp, const ComboAddress& source, const ComboAddress& destination); -bool parseXPFPayload(const char* payload, size_t len, ComboAddress& source, ComboAddress* destination); - diff --git a/regression-tests.dnsdist/test_XPF.py b/regression-tests.dnsdist/test_XPF.py deleted file mode 100644 index 934ad50d94..0000000000 --- a/regression-tests.dnsdist/test_XPF.py +++ /dev/null @@ -1,75 +0,0 @@ -#!/usr/bin/env python - -import dns -from dnsdisttests import DNSDistTest - -class XPFTest(DNSDistTest): - """ - dnsdist is configured to add XPF to the query - """ - - _xpfCode = 65422 - _config_template = """ - newServer{address="127.0.0.1:%d", addXPF=%d} - """ - _config_params = ['_testServerPort', '_xpfCode'] - - def checkMessageHasXPF(self, msg, expectedValue): - self.assertGreaterEqual(len(msg.additional), 1) - - found = False - for add in msg.additional: - if add.rdtype == self._xpfCode: - found = True - self.assertEqual(add.rdclass, dns.rdataclass.IN) - self.assertEqual(add.ttl, 0) - xpfData = add.to_rdataset()[0].to_text() - # skip the ports - self.assertEqual(xpfData[:26], expectedValue[:26]) - - self.assertTrue(found) - - def testXPF(self): - """ - XPF - """ - name = 'xpf.tests.powerdns.com.' - query = dns.message.make_query(name, 'A', 'IN') - - expectedQuery = dns.message.make_query(name, 'A', 'IN') - # 0x04 is IPv4, 0x11 (17) is UDP then 127.0.0.1 as source and destination - # and finally the ports, zeroed because we have no way to know them beforehand - xpfData = "\\# 14 04117f0000017f00000100000000" - rdata = dns.rdata.from_text(dns.rdataclass.IN, self._xpfCode, xpfData) - rrset = dns.rrset.from_rdata(".", 0, rdata) - expectedQuery.additional.append(rrset) - - response = dns.message.make_response(expectedQuery) - - (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) - self.assertTrue(receivedQuery) - self.assertTrue(receivedResponse) - receivedQuery.id = expectedQuery.id - receivedResponse.id = response.id - - self.checkMessageHasXPF(receivedQuery, xpfData) - self.assertEqual(response, receivedResponse) - - expectedQuery = dns.message.make_query(name, 'A', 'IN') - # 0x04 is IPv4, 0x06 (6) is TCP then 127.0.0.1 as source and destination - # and finally the ports, zeroed because we have no way to know them beforehand - xpfData = "\\# 14 04067f0000017f00000100000000" - rdata = dns.rdata.from_text(dns.rdataclass.IN, self._xpfCode, xpfData) - rrset = dns.rrset.from_rdata(".", 0, rdata) - expectedQuery.additional.append(rrset) - - response = dns.message.make_response(expectedQuery) - - (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response) - self.assertTrue(receivedQuery) - self.assertTrue(receivedResponse) - receivedQuery.id = expectedQuery.id - receivedResponse.id = response.id - - self.checkMessageHasXPF(receivedQuery, xpfData) - self.assertEqual(response, receivedResponse)