From db8f9152168acf5d548d4f256789eae783e01667 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 7 Jul 2016 14:34:31 +0200 Subject: [PATCH] Add limits to the size of received {A,I}XFR, in megabytes This prevents memory exhaustion in case the master is sending a very large amount of data in an update. --- docs/markdown/authoritative/settings.md | 7 +++++++ docs/markdown/recursor/settings.md | 2 ++ pdns/common_startup.cc | 2 ++ pdns/ixfr.cc | 8 +++++++- pdns/ixfr.hh | 2 +- pdns/rec-lua-conf.cc | 8 ++++++-- pdns/reczones.cc | 4 ++-- pdns/resolver.cc | 15 +++++++++++---- pdns/resolver.hh | 7 +++++-- pdns/rpzloader.cc | 4 ++-- pdns/rpzloader.hh | 4 ++-- pdns/slavecommunicator.cc | 4 ++-- 12 files changed, 49 insertions(+), 18 deletions(-) diff --git a/docs/markdown/authoritative/settings.md b/docs/markdown/authoritative/settings.md index 269d4747b..2ee242779 100644 --- a/docs/markdown/authoritative/settings.md +++ b/docs/markdown/authoritative/settings.md @@ -874,3 +874,10 @@ If the webserver should print arguments. See ["Performance Monitoring"](../commo * Default: yes If a PID file should be written. Available since 4.0. + +## `xfr-max-received-mbytes` +* Integer +* Default: 100 + +Specifies the maximum number of received megabytes allowed on an incoming AXFR/IXFR update, to prevent +resource exhaustion. A value of 0 means no restriction. diff --git a/docs/markdown/recursor/settings.md b/docs/markdown/recursor/settings.md index b812d31b4..fea430269 100644 --- a/docs/markdown/recursor/settings.md +++ b/docs/markdown/recursor/settings.md @@ -472,6 +472,8 @@ In addition to those, `rpzMaster` accepts: * tsigalgo = the name of the TSIG algorithm (like 'hmac-md5') used * tsigsecret = base64 encoded TSIG secret * refresh = an integer describing the interval between checks for updates. By default, the RPZ zone's default is used +* maxReceivedMBytes = the maximum size in megabytes of an AXFR/IXFR update, to prevent resource exhaustion. +The default value of 0 means no restriction. If no settings are included, the RPZ is taken literally with no overrides applied. diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc index c783d9a74..5ef6a0053 100644 --- a/pdns/common_startup.cc +++ b/pdns/common_startup.cc @@ -187,6 +187,8 @@ void declareArguments() ::arg().setSwitch("outgoing-axfr-expand-alias", "Expand ALIAS records during outgoing AXFR")="no"; ::arg().setSwitch("8bit-dns", "Allow 8bit dns queries")="no"; + + ::arg().set("xfr-max-received-mbytes", "Maximum number of megabytes received from an incoming XFR")="100"; } static time_t s_start=time(0); diff --git a/pdns/ixfr.cc b/pdns/ixfr.cc index 06d9fe5a9..dc87f74fb 100644 --- a/pdns/ixfr.cc +++ b/pdns/ixfr.cc @@ -7,7 +7,7 @@ // Returns pairs of "remove & add" vectors. If you get an empty remove, it means you got an AXFR! vector, vector > > getIXFRDeltas(const ComboAddress& master, const DNSName& zone, const DNSRecord& oursr, - const TSIGTriplet& tt, const ComboAddress* laddr) + const TSIGTriplet& tt, const ComboAddress* laddr, size_t maxReceivedBytes) { vector, vector > > ret; vector packet; @@ -55,6 +55,7 @@ vector, vector > > getIXFRDeltas(const ComboAd // CURRENT MASTER SOA shared_ptr masterSOA; vector records; + size_t receivedBytes = 0; for(;;) { if(s.read((char*)&len, 2)!=2) break; @@ -62,8 +63,13 @@ vector, vector > > getIXFRDeltas(const ComboAd // cout<<"Got chunk of "< 0 && (maxReceivedBytes - receivedBytes) < (size_t) len) + throw std::runtime_error("Reached the maximum number of received bytes in an IXFR delta for zone '"+zone.toString()+"' from master '"+master.toStringWithPort()); + char reply[len]; readn2(s.getHandle(), reply, len); + receivedBytes += len; MOADNSParser mdp(string(reply, len)); if(mdp.d_header.rcode) throw std::runtime_error("Got an error trying to IXFR zone '"+zone.toString()+"' from master '"+master.toStringWithPort()+"': "+RCode::to_s(mdp.d_header.rcode)); diff --git a/pdns/ixfr.hh b/pdns/ixfr.hh index 543f4454a..eef225224 100644 --- a/pdns/ixfr.hh +++ b/pdns/ixfr.hh @@ -4,4 +4,4 @@ vector, vector > > getIXFRDeltas(const ComboAddress& master, const DNSName& zone, const DNSRecord& sr, const TSIGTriplet& tt=TSIGTriplet(), - const ComboAddress* laddr=0); + const ComboAddress* laddr=0, size_t maxReceivedBytes=0); diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc index 0c1474ed5..e6c07b0e3 100644 --- a/pdns/rec-lua-conf.cc +++ b/pdns/rec-lua-conf.cc @@ -130,6 +130,7 @@ void loadRecursorLuaConfig(const std::string& fname) TSIGTriplet tt; int refresh=0; std::string polName; + size_t maxReceivedXFRMBytes = 0; if(options) { auto& have = *options; if(have.count("policyName")) { @@ -163,14 +164,17 @@ void loadRecursorLuaConfig(const std::string& fname) if(have.count("refresh")) { refresh = boost::get(constGet(have,"refresh")); } + if(have.count("maxReceivedMBytes")) { + maxReceivedXFRMBytes = static_cast(boost::get(constGet(have,"maxReceivedMBytes"))); + } } ComboAddress master(master_, 53); DNSName zone(zone_); - auto sr=loadRPZFromServer(master, zone, lci.dfe, polName, defpol, 0, tt); + auto sr=loadRPZFromServer(master, zone, lci.dfe, polName, defpol, 0, tt, maxReceivedXFRMBytes * 1024 * 1024); if(refresh) sr->d_st.refresh=refresh; - std::thread t(RPZIXFRTracker, master, zone, polName, tt, sr); + std::thread t(RPZIXFRTracker, master, zone, polName, tt, sr, maxReceivedXFRMBytes * 1024 * 1024); t.detach(); } catch(std::exception& e) { diff --git a/pdns/reczones.cc b/pdns/reczones.cc index 74df7ec8e..3cd3c9e5b 100644 --- a/pdns/reczones.cc +++ b/pdns/reczones.cc @@ -311,7 +311,7 @@ string reloadAuthAndForwards() } -void RPZIXFRTracker(const ComboAddress& master, const DNSName& zone, const std::string& polName, const TSIGTriplet& tt, shared_ptr oursr) +void RPZIXFRTracker(const ComboAddress& master, const DNSName& zone, const std::string& polName, const TSIGTriplet& tt, shared_ptr oursr, size_t maxReceivedBytes) { int refresh = oursr->d_st.refresh; for(;;) { @@ -323,7 +323,7 @@ void RPZIXFRTracker(const ComboAddress& master, const DNSName& zone, const std:: L<(dr)->d_st.serial<, vector > > deltas; try { - deltas = getIXFRDeltas(master, zone, dr, tt); + deltas = getIXFRDeltas(master, zone, dr, tt, nullptr, maxReceivedBytes); } catch(std::runtime_error& e ){ L<* records) // int len=getLength(); if(len<0) throw ResolverException("EOF trying to read axfr chunk from remote TCP client"); - - timeoutReadn(len); + + if (d_maxReceivedBytes > 0 && (d_maxReceivedBytes - d_receivedBytes) < (size_t) len) + throw ResolverException("Reached the maximum number of received bytes during AXFR"); + + timeoutReadn(len); + + d_receivedBytes += (uint16_t) len; + MOADNSParser mdp(d_buf.get(), len); int err; diff --git a/pdns/resolver.hh b/pdns/resolver.hh index 24e45454d..03fb4fdcb 100644 --- a/pdns/resolver.hh +++ b/pdns/resolver.hh @@ -86,8 +86,9 @@ class AXFRRetriever : public boost::noncopyable AXFRRetriever(const ComboAddress& remote, const DNSName& zone, const TSIGTriplet& tt = TSIGTriplet(), - const ComboAddress* laddr = NULL); - ~AXFRRetriever(); + const ComboAddress* laddr = NULL, + size_t maxReceivedBytes=0); + ~AXFRRetriever(); int getChunk(Resolver::res_t &res, vector* records=0); private: @@ -104,6 +105,8 @@ class AXFRRetriever : public boost::noncopyable TSIGTriplet d_tt; string d_prevMac; // RFC2845 4.4 string d_signData; + size_t d_receivedBytes; + size_t d_maxReceivedBytes; uint32_t d_tsigPos; uint d_nonSignedMessages; // RFC2845 4.4 TSIGRecordContent d_trc; diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc index 90a2dace3..31c5e6cd3 100644 --- a/pdns/rpzloader.cc +++ b/pdns/rpzloader.cc @@ -105,14 +105,14 @@ void RPZRecordToPolicy(const DNSRecord& dr, DNSFilterEngine& target, const std:: } } -shared_ptr loadRPZFromServer(const ComboAddress& master, const DNSName& zone, DNSFilterEngine& target, const std::string& polName, boost::optional defpol, int place, const TSIGTriplet& tt) +shared_ptr loadRPZFromServer(const ComboAddress& master, const DNSName& zone, DNSFilterEngine& target, const std::string& polName, boost::optional defpol, int place, const TSIGTriplet& tt, size_t maxReceivedBytes) { L< chunk; diff --git a/pdns/rpzloader.hh b/pdns/rpzloader.hh index c61993ae4..031e82795 100644 --- a/pdns/rpzloader.hh +++ b/pdns/rpzloader.hh @@ -4,6 +4,6 @@ #include "dnsrecords.hh" int loadRPZFromFile(const std::string& fname, DNSFilterEngine& target, const std::string& policyName, boost::optional defpol, int place); -std::shared_ptr loadRPZFromServer(const ComboAddress& master, const DNSName& zone, DNSFilterEngine& target, const std::string& policyName, boost::optional defpol, int place, const TSIGTriplet& tt); +std::shared_ptr loadRPZFromServer(const ComboAddress& master, const DNSName& zone, DNSFilterEngine& target, const std::string& policyName, boost::optional defpol, int place, const TSIGTriplet& tt, size_t maxReceivedBytes); void RPZRecordToPolicy(const DNSRecord& dr, DNSFilterEngine& target, const std::string& policyName, bool addOrRemove, boost::optional defpol, int place); -void RPZIXFRTracker(const ComboAddress& master, const DNSName& zone, const std::string& policyName, const TSIGTriplet &tt, shared_ptr oursr); +void RPZIXFRTracker(const ComboAddress& master, const DNSName& zone, const std::string& policyName, const TSIGTriplet &tt, shared_ptr oursr, size_t maxReceivedBytes); diff --git a/pdns/slavecommunicator.cc b/pdns/slavecommunicator.cc index 9562c21d4..2828a7289 100644 --- a/pdns/slavecommunicator.cc +++ b/pdns/slavecommunicator.cc @@ -103,7 +103,7 @@ void CommunicatorClass::ixfrSuck(const DNSName &domain, const TSIGTriplet& tt, c DNSRecord dr; dr.d_content = std::make_shared(DNSName("."), DNSName("."), st); - auto deltas = getIXFRDeltas(remote, domain, dr, tt, laddr.sin4.sin_family ? &laddr : 0); + auto deltas = getIXFRDeltas(remote, domain, dr, tt, laddr.sin4.sin_family ? &laddr : 0, ((size_t) ::arg().asNum("xfr-max-received-mbytes")) * 1024 * 1024); zs.numDeltas=deltas.size(); // cout<<"Got "< doAxfr(const ComboAddress& raddr, const DNSName& domain, const TSIGTriplet& tt, const ComboAddress& laddr, scoped_ptr& pdl, ZoneStatus& zs) { vector rrs; - AXFRRetriever retriever(raddr, domain, tt, (laddr.sin4.sin_family == 0) ? NULL : &laddr); + AXFRRetriever retriever(raddr, domain, tt, (laddr.sin4.sin_family == 0) ? NULL : &laddr, ((size_t) ::arg().asNum("xfr-max-received-mbytes")) * 1024 * 1024); Resolver::res_t recs; bool first=true; bool firstNSEC3{true}; -- 2.50.0