From fcd32fe879c6d9673a92b5d2485d87b4953b6758 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Mon, 23 May 2022 12:12:05 +0200 Subject: [PATCH] Remove XPF support from recursor --- pdns/pdns_recursor.cc | 24 +--- pdns/recursordist/Makefile.am | 3 - pdns/recursordist/docs/settings.rst | 12 +- pdns/recursordist/rec-main.cc | 9 +- pdns/recursordist/rec-main.hh | 5 +- pdns/recursordist/rec-tcp.cc | 7 +- pdns/recursordist/test-xpf_cc.cc | 176 ---------------------------- pdns/recursordist/xpf.cc | 1 - pdns/recursordist/xpf.hh | 1 - 9 files changed, 18 insertions(+), 220 deletions(-) delete mode 100644 pdns/recursordist/test-xpf_cc.cc delete mode 120000 pdns/recursordist/xpf.cc delete mode 120000 pdns/recursordist/xpf.hh diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index b9e05657f8..f7fea424c6 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -31,7 +31,6 @@ #include "responsestats.hh" #include "shuffle.hh" #include "validate-recursor.hh" -#include "xpf.hh" #ifdef HAVE_SYSTEMD #include @@ -64,7 +63,6 @@ typedef map listenSocketsAddresses_t; // is shared across all static listenSocketsAddresses_t g_listenSocketsAddresses; // is shared across all threads right now static set g_fromtosockets; // listen sockets that use 'sendfromto()' mechanism (without actually using sendfromto()) -NetmaskGroup g_XPFAcl; NetmaskGroup g_paddingFrom; size_t g_proxyProtocolMaximumSize; size_t g_maxUDPQueriesPerRound; @@ -1726,10 +1724,8 @@ void startDoResolve(void* p) } void getQNameAndSubnet(const std::string& question, DNSName* dnsname, uint16_t* qtype, uint16_t* qclass, - bool& foundECS, EDNSSubnetOpts* ednssubnet, EDNSOptionViewMap* options, - bool& foundXPF, ComboAddress* xpfSource, ComboAddress* xpfDest) + bool& foundECS, EDNSSubnetOpts* ednssubnet, EDNSOptionViewMap* options) { - const bool lookForXPF = xpfSource != nullptr && g_xpfRRCode != 0; const bool lookForECS = ednssubnet != nullptr; const dnsheader_aligned dnshead(question.data()); const dnsheader* dh = dnshead.get(); @@ -1741,9 +1737,9 @@ void getQNameAndSubnet(const std::string& question, DNSName* dnsname, uint16_t* const size_t headerSize = /* root */ 1 + sizeof(dnsrecordheader); const uint16_t arcount = ntohs(dh->arcount); - for (uint16_t arpos = 0; arpos < arcount && questionLen > (pos + headerSize) && ((lookForECS && !foundECS) || (lookForXPF && !foundXPF)); arpos++) { + for (uint16_t arpos = 0; arpos < arcount && questionLen > (pos + headerSize) && (lookForECS && !foundECS); arpos++) { if (question.at(pos) != 0) { - /* not an OPT or a XPF, bye. */ + /* not an OPT, bye. */ return; } @@ -1785,13 +1781,6 @@ void getQNameAndSubnet(const std::string& question, DNSName* dnsname, uint16_t* } } } - else if (lookForXPF && ntohs(drh->d_type) == g_xpfRRCode && ntohs(drh->d_class) == QClass::IN && drh->d_ttl == 0) { - if ((questionLen - pos) < ntohs(drh->d_clen)) { - return; - } - - foundXPF = parseXPFPayload(reinterpret_cast(&question.at(pos)), ntohs(drh->d_clen), *xpfSource, xpfDest); - } pos += ntohs(drh->d_clen); } @@ -1903,7 +1892,6 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr unsigned int ctag = 0; uint32_t qhash = 0; bool needECS = false; - bool needXPF = g_XPFAcl.match(fromaddr); std::unordered_set policyTags; std::map meta; LuaContext::LuaObject data; @@ -1956,16 +1944,14 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr #endif // We do not have a SyncRes specific Lua context at this point yet, so ok to use t_pdl - if (needECS || needXPF || (t_pdl && (t_pdl->d_gettag || t_pdl->d_gettag_ffi)) || dh->opcode == Opcode::Notify) { + if (needECS || (t_pdl && (t_pdl->d_gettag || t_pdl->d_gettag_ffi)) || dh->opcode == Opcode::Notify) { try { EDNSOptionViewMap ednsOptions; - bool xpfFound = false; ecsFound = false; getQNameAndSubnet(question, &qname, &qtype, &qclass, - ecsFound, &ednssubnet, g_gettagNeedsEDNSOptions ? &ednsOptions : nullptr, - xpfFound, needXPF ? &source : nullptr, needXPF ? &destination : nullptr); + ecsFound, &ednssubnet, g_gettagNeedsEDNSOptions ? &ednsOptions : nullptr); qnameParsed = true; ecsParsed = true; diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am index 00ad4cba67..f7d02e7460 100644 --- a/pdns/recursordist/Makefile.am +++ b/pdns/recursordist/Makefile.am @@ -212,7 +212,6 @@ pdns_recursor_SOURCES = \ webserver.cc webserver.hh \ ws-api.cc ws-api.hh \ ws-recursor.cc ws-recursor.hh \ - xpf.cc xpf.hh \ zonemd.cc zonemd.hh \ zoneparser-tng.cc zoneparser-tng.hh @@ -350,14 +349,12 @@ testrunner_SOURCES = \ test-syncres_cc8.cc \ test-syncres_cc9.cc \ test-tsig.cc \ - test-xpf_cc.cc \ testrunner.cc \ threadname.hh threadname.cc \ tsigverifier.cc tsigverifier.hh \ unix_utility.cc \ validate-recursor.cc validate-recursor.hh \ validate.cc validate.hh \ - xpf.cc xpf.hh \ zonemd.cc zonemd.hh \ zoneparser-tng.cc zoneparser-tng.hh diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index 529d0b4d82..b956df0731 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -689,7 +689,7 @@ Lower this if you experience timeouts. - Comma separated list of netmasks - Default: (none) -List of netmasks (proxy IP in case of XPF or proxy-protocol presence, client IP otherwise) for which EDNS padding will be enabled in responses, provided that `edns-padding-mode`_ applies. +List of netmasks (proxy IP in case of proxy-protocol presence, client IP otherwise) for which EDNS padding will be enabled in responses, provided that `edns-padding-mode`_ applies. .. _setting-edns-padding-mode: @@ -2412,12 +2412,15 @@ If a PID file should be written to `socket-dir`_ ------------------ .. versionadded:: 4.2.0 +.. versionchanged:: 4.8.0 + This setting was removed. + - IP addresses or netmasks, separated by commas - Default: empty .. note:: This is an experimental implementation of `draft-bellis-dnsop-xpf `_. - This is a deprecated feature that will be removed in the near future. + This is a deprecated feature that was removed starting with version 4.8.0. The server will trust XPF records found in queries sent from those netmasks (both IPv4 and IPv6), and will adjust queries' source and destination accordingly. This is especially useful when the recursor @@ -2431,12 +2434,15 @@ should be done on the proxy. --------------- .. versionadded:: 4.2.0 +.. versionchanged:: 4.8.0 + This setting was removed. + - Integer - Default: 0 .. note:: This is an experimental implementation of `draft-bellis-dnsop-xpf `_. - This is a deprecated feature that will be removed in the near future. + This is a deprecated feature that was removed starting with version 4.8.0. This option sets the resource record code to use for XPF records, as long as an official code has not been assigned to it. 0 means that XPF is disabled. diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index c5c77e10a8..a879977c3e 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -82,7 +82,6 @@ thread_local std::shared_ptr t_udrDBp; std::atomic statsWanted; uint32_t g_disthashseed; bool g_useIncomingECS; -uint16_t g_xpfRRCode{0}; NetmaskGroup g_proxyProtocolACL; boost::optional g_dns64Prefix{boost::none}; DNSName g_dns64PrefixReverse; @@ -1572,9 +1571,6 @@ static int serviceMain(int argc, char* argv[], Logr::log_t log) SyncRes::parseEDNSSubnetAddFor(::arg()["ecs-add-for"]); g_useIncomingECS = ::arg().mustDo("use-incoming-edns-subnet"); - g_XPFAcl.toMasks(::arg()["xpf-allow-from"]); - g_xpfRRCode = ::arg().asNum("xpf-rr-code"); - g_proxyProtocolACL.toMasks(::arg()["proxy-protocol-from"]); g_proxyProtocolMaximumSize = ::arg().asNum("proxy-protocol-maximum-size"); @@ -2725,9 +2721,6 @@ int main(int argc, char** argv) ::arg().setSwitch("log-rpz-changes", "Log additions and removals to RPZ zones at Info level") = "no"; - ::arg().set("xpf-allow-from", "XPF information is only processed from these subnets") = ""; - ::arg().set("xpf-rr-code", "XPF option code to use") = "0"; - ::arg().set("proxy-protocol-from", "A Proxy Protocol header is only allowed from these subnets") = ""; ::arg().set("proxy-protocol-maximum-size", "The maximum size of a proxy protocol payload, including the TLV values") = "512"; @@ -2769,7 +2762,7 @@ int main(int argc, char** argv) ::arg().set("aggressive-nsec-cache-size", "The number of records to cache in the aggressive cache. If set to a value greater than 0, and DNSSEC processing or validation is enabled, the recursor will cache NSEC and NSEC3 records to generate negative answers, as defined in rfc8198") = "100000"; - ::arg().set("edns-padding-from", "List of netmasks (proxy IP in case of XPF or proxy-protocol presence, client IP otherwise) for which EDNS padding will be enabled in responses, provided that 'edns-padding-mode' applies") = ""; + ::arg().set("edns-padding-from", "List of netmasks (proxy IP in case of proxy-protocol presence, client IP otherwise) for which EDNS padding will be enabled in responses, provided that 'edns-padding-mode' applies") = ""; ::arg().set("edns-padding-mode", "Whether to add EDNS padding to all responses ('always') or only to responses for queries containing the EDNS padding option ('padded-queries-only', the default). In both modes, padding will only be added to responses for queries coming from `edns-padding-from`_ sources") = "padded-queries-only"; ::arg().set("edns-padding-tag", "Packetcache tag associated to responses sent with EDNS padding, to prevent sending these to clients for which padding is not enabled.") = "7830"; diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh index 56e8ea72af..52d5b3fe6d 100644 --- a/pdns/recursordist/rec-main.hh +++ b/pdns/recursordist/rec-main.hh @@ -191,7 +191,6 @@ extern thread_local std::unique_ptr t_packetCache; extern bool g_logCommonErrors; extern size_t g_proxyProtocolMaximumSize; extern std::atomic g_quiet; -extern NetmaskGroup g_XPFAcl; extern thread_local std::shared_ptr>> t_protobufServers; extern thread_local std::shared_ptr t_pdl; extern bool g_gettagNeedsEDNSOptions; @@ -218,7 +217,6 @@ extern boost::optional g_dns64Prefix; extern DNSName g_dns64PrefixReverse; extern uint64_t g_latencyStatSize; extern bool g_addExtendedResolutionDNSErrors; -extern uint16_t g_xpfRRCode; extern NetmaskGroup g_proxyProtocolACL; extern std::atomic g_statsWanted; extern uint32_t g_disthashseed; @@ -505,8 +503,7 @@ bool checkProtobufExport(LocalStateHolder& luaconfsLocal); bool checkOutgoingProtobufExport(LocalStateHolder& luaconfsLocal); bool checkFrameStreamExport(LocalStateHolder& luaconfsLocal); void getQNameAndSubnet(const std::string& question, DNSName* dnsname, uint16_t* qtype, uint16_t* qclass, - bool& foundECS, EDNSSubnetOpts* ednssubnet, EDNSOptionViewMap* options, - bool& foundXPF, ComboAddress* xpfSource, ComboAddress* xpfDest); + bool& foundECS, EDNSSubnetOpts* ednssubnet, EDNSOptionViewMap* options); void protobufLogQuery(LocalStateHolder& luaconfsLocal, const boost::uuids::uuid& uniqueId, const ComboAddress& remote, const ComboAddress& local, const ComboAddress& mappedSource, const Netmask& ednssubnet, bool tcp, uint16_t id, size_t len, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::unordered_set& policyTags, const std::string& requestorId, const std::string& deviceId, const std::string& deviceName, const std::map& meta); bool isAllowNotifyForZone(DNSName qname); bool checkForCacheHit(bool qnameParsed, unsigned int tag, const string& data, diff --git a/pdns/recursordist/rec-tcp.cc b/pdns/recursordist/rec-tcp.cc index 0747664f4a..0140e472fe 100644 --- a/pdns/recursordist/rec-tcp.cc +++ b/pdns/recursordist/rec-tcp.cc @@ -387,7 +387,6 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var) uint16_t qtype = 0; uint16_t qclass = 0; bool needECS = false; - bool needXPF = g_XPFAcl.match(conn->d_remote); string requestorId; string deviceId; string deviceName; @@ -407,16 +406,14 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var) checkFrameStreamExport(luaconfsLocal); #endif - if (needECS || needXPF || (t_pdl && (t_pdl->d_gettag_ffi || t_pdl->d_gettag)) || dc->d_mdp.d_header.opcode == Opcode::Notify) { + if (needECS || (t_pdl && (t_pdl->d_gettag_ffi || t_pdl->d_gettag)) || dc->d_mdp.d_header.opcode == Opcode::Notify) { try { EDNSOptionViewMap ednsOptions; - bool xpfFound = false; dc->d_ecsParsed = true; dc->d_ecsFound = false; getQNameAndSubnet(conn->data, &qname, &qtype, &qclass, - dc->d_ecsFound, &dc->d_ednssubnet, g_gettagNeedsEDNSOptions ? &ednsOptions : nullptr, - xpfFound, needXPF ? &dc->d_source : nullptr, needXPF ? &dc->d_destination : nullptr); + dc->d_ecsFound, &dc->d_ednssubnet, g_gettagNeedsEDNSOptions ? &ednsOptions : nullptr); qnameParsed = true; if (t_pdl) { diff --git a/pdns/recursordist/test-xpf_cc.cc b/pdns/recursordist/test-xpf_cc.cc deleted file mode 100644 index 02abbad37a..0000000000 --- a/pdns/recursordist/test-xpf_cc.cc +++ /dev/null @@ -1,176 +0,0 @@ -#define BOOST_TEST_DYN_LINK -#define BOOST_TEST_NO_MAIN - -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif -#include - -#include "xpf.hh" - -BOOST_AUTO_TEST_SUITE(xpf_cc) - -BOOST_AUTO_TEST_CASE(test_generateXPFPayload) -{ - - /* Mixing v4 with v6 should throw */ - BOOST_CHECK_THROW(generateXPFPayload(false, ComboAddress("192.0.2.1"), ComboAddress("2001:db8::1")), std::runtime_error); - BOOST_CHECK_THROW(generateXPFPayload(false, ComboAddress("2001:db8::1"), ComboAddress("192.0.2.1")), std::runtime_error); - - { - /* v4 payload over UDP */ - ComboAddress source("192.0.2.1:53"); - ComboAddress destination("192.0.2.2:65535"); - - auto payload = generateXPFPayload(false, source, destination); - BOOST_CHECK_EQUAL(payload.size(), 14U); - BOOST_CHECK_EQUAL(payload.at(0), 4); - BOOST_CHECK_EQUAL(payload.at(1), 17); - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination)); - BOOST_CHECK_EQUAL(parsedSource.toStringWithPort(), source.toStringWithPort()); - BOOST_CHECK_EQUAL(parsedDestination.toStringWithPort(), destination.toStringWithPort()); - } - - { - /* v4 payload over TCP */ - ComboAddress source("192.0.2.1:53"); - ComboAddress destination("192.0.2.2:65535"); - - auto payload = generateXPFPayload(true, source, destination); - BOOST_CHECK_EQUAL(payload.size(), 14U); - BOOST_CHECK_EQUAL(payload.at(0), 4); - BOOST_CHECK_EQUAL(payload.at(1), 6); - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination)); - BOOST_CHECK_EQUAL(parsedSource.toStringWithPort(), source.toStringWithPort()); - BOOST_CHECK_EQUAL(parsedDestination.toStringWithPort(), destination.toStringWithPort()); - } - - { - /* v6 payload over UDP */ - ComboAddress source("[2001:db8::1]:42"); - ComboAddress destination("[::1]:65535"); - - auto payload = generateXPFPayload(false, source, destination); - BOOST_CHECK_EQUAL(payload.size(), 38U); - BOOST_CHECK_EQUAL(payload.at(0), 6); - BOOST_CHECK_EQUAL(payload.at(1), 17); - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination)); - BOOST_CHECK_EQUAL(parsedSource.toStringWithPort(), source.toStringWithPort()); - BOOST_CHECK_EQUAL(parsedDestination.toStringWithPort(), destination.toStringWithPort()); - } - - { - /* v6 payload over TCP */ - ComboAddress source("[2001:db8::1]:42"); - ComboAddress destination("[::1]:65535"); - - auto payload = generateXPFPayload(true, source, destination); - BOOST_CHECK_EQUAL(payload.size(), 38U); - BOOST_CHECK_EQUAL(payload.at(0), 6); - BOOST_CHECK_EQUAL(payload.at(1), 6); - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination)); - BOOST_CHECK_EQUAL(parsedSource.toStringWithPort(), source.toStringWithPort()); - BOOST_CHECK_EQUAL(parsedDestination.toStringWithPort(), destination.toStringWithPort()); - } -} - -BOOST_AUTO_TEST_CASE(test_parseXPFPayload) -{ - - /* invalid sizes */ - { - ComboAddress source; - ComboAddress destination; - - BOOST_CHECK_EQUAL(parseXPFPayload(nullptr, 0, source, &destination), false); - BOOST_CHECK_EQUAL(parseXPFPayload(nullptr, 13, source, &destination), false); - BOOST_CHECK_EQUAL(parseXPFPayload(nullptr, 15, source, &destination), false); - BOOST_CHECK_EQUAL(parseXPFPayload(nullptr, 37, source, &destination), false); - BOOST_CHECK_EQUAL(parseXPFPayload(nullptr, 39, source, &destination), false); - } - - { - /* invalid protocol */ - ComboAddress source("[2001:db8::1]:42"); - ComboAddress destination("[::1]:65535"); - - auto payload = generateXPFPayload(true, source, destination); - /* set protocol to 0 */ - payload.at(1) = 0; - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK_EQUAL(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination), false); - } - - { - /* invalid version */ - ComboAddress source("[2001:db8::1]:42"); - ComboAddress destination("[::1]:65535"); - - auto payload = generateXPFPayload(true, source, destination); - /* set version to 0 */ - payload.at(0) = 0; - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK_EQUAL(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination), false); - } - - { - /* payload too short (v6 size with v4 payload) */ - ComboAddress source("192.0.2.1:53"); - ComboAddress destination("192.0.2.2:65535"); - - auto payload = generateXPFPayload(true, source, destination); - /* set version to 6 */ - payload.at(0) = 6; - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK_EQUAL(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination), false); - } - - { - /* payload too long (v6 size with v4 payload) */ - ComboAddress source("[2001:db8::1]:42"); - ComboAddress destination("[::1]:65535"); - - auto payload = generateXPFPayload(true, source, destination); - /* set version to 4 */ - payload.at(0) = 4; - - ComboAddress parsedSource; - ComboAddress parsedDestination; - BOOST_CHECK_EQUAL(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, &parsedDestination), false); - } - - { - /* v4 payload over UDP */ - ComboAddress source("192.0.2.1:53"); - ComboAddress destination("192.0.2.2:65535"); - - auto payload = generateXPFPayload(false, source, destination); - BOOST_CHECK_EQUAL(payload.size(), 14U); - BOOST_CHECK_EQUAL(payload.at(0), 4); - BOOST_CHECK_EQUAL(payload.at(1), 17); - - ComboAddress parsedSource; - BOOST_CHECK(parseXPFPayload(payload.c_str(), payload.size(), parsedSource, nullptr)); - BOOST_CHECK_EQUAL(parsedSource.toStringWithPort(), source.toStringWithPort()); - } -} - -BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/recursordist/xpf.cc b/pdns/recursordist/xpf.cc deleted file mode 120000 index 98ab7228cd..0000000000 --- a/pdns/recursordist/xpf.cc +++ /dev/null @@ -1 +0,0 @@ -../xpf.cc \ No newline at end of file diff --git a/pdns/recursordist/xpf.hh b/pdns/recursordist/xpf.hh deleted file mode 120000 index 023c8c4796..0000000000 --- a/pdns/recursordist/xpf.hh +++ /dev/null @@ -1 +0,0 @@ -../xpf.hh \ No newline at end of file -- 2.47.2