From: Remi Gacogne Date: Fri, 25 Oct 2019 14:35:37 +0000 (+0200) Subject: Add a parameter to limit the number of '$GENERATE' steps X-Git-Tag: dnsdist-1.4.0~22^2~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ba3d53d16f70c58409683d89c4446eb496cbab5f;p=thirdparty%2Fpdns.git Add a parameter to limit the number of '$GENERATE' steps --- diff --git a/docs/settings.rst b/docs/settings.rst index 54b5137c01..17e7e7f631 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -927,6 +927,21 @@ will generally suffice for most installations. Maximum number of empty non-terminals to add to a zone. This is a protection measure to avoid database explosion due to long names. +.. _setting-max-generate-steps: + +``max-generate-steps`` +---------------------- + +.. versionadded:: 4.3.0 + +- Integer +- Default: 0 + +Maximum number of steps for a '$GENERATE' directive when parsing a +zone file. This is a protection measure to prevent consuming a lot of +CPU and memory when untrusted zones are loaded. Default to 0 which +means unlimited. + .. _setting-max-nsec3-iterations: ``max-nsec3-iterations`` diff --git a/modules/bindbackend/bindbackend2.cc b/modules/bindbackend/bindbackend2.cc index 4322f220f2..634fc831d2 100644 --- a/modules/bindbackend/bindbackend2.cc +++ b/modules/bindbackend/bindbackend2.cc @@ -481,11 +481,11 @@ void Bind2Backend::parseZoneFile(BB2DomainInfo *bbd) nsec3zone=getNSEC3PARAM(bbd->d_name, &ns3pr); bbd->d_records = shared_ptr(new recordstorage_t()); - ZoneParserTNG zpt(bbd->d_filename, bbd->d_name, s_binddirectory); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; string hashed; - while(zpt.get(rr)) { + while(zpt.get(rr)) { if(rr.qtype.getCode() == QType::NSEC || rr.qtype.getCode() == QType::NSEC3 || rr.qtype.getCode() == QType::NSEC3PARAM) continue; // we synthesise NSECs on demand diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc index bce6b27ef6..1a383c71fa 100644 --- a/pdns/common_startup.cc +++ b/pdns/common_startup.cc @@ -228,6 +228,8 @@ void declareArguments() ::arg().set("tcp-fast-open", "Enable TCP Fast Open support on the listening sockets, using the supplied numerical value as the queue size")="0"; + ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; + ::arg().set("rng", "Specify the random number generator to use. Valid values are auto,sodium,openssl,getrandom,arc4random,urandom.")="auto"; } diff --git a/pdns/fuzz_zoneparsertng.cc b/pdns/fuzz_zoneparsertng.cc index a6f30d3859..442555b122 100644 --- a/pdns/fuzz_zoneparsertng.cc +++ b/pdns/fuzz_zoneparsertng.cc @@ -47,6 +47,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { boost::split(lines, tmp, boost::is_any_of("\n")); ZoneParserTNG zpt(lines, g_rootdnsname); + /* limit the number of steps for '$GENERATE' entries */ + zpt.setMaxGenerateSteps(10000); DNSResourceRecord drr; while (zpt.get(drr)) { } diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index d7c3716d2a..3f1a5166fd 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -4698,6 +4698,7 @@ int main(int argc, char **argv) ::arg().set("distribution-load-factor", "The load factor used when PowerDNS is distributing queries to worker threads")="0.0"; ::arg().setSwitch("qname-minimization", "Use Query Name Minimization")="no"; ::arg().setSwitch("nothing-below-nxdomain", "When an NXDOMAIN exists in cache for a name with fewer labels than the qname, send NXDOMAIN without doing a lookup (see RFC 8020)")="yes"; + ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; #ifdef NOD_ENABLED ::arg().set("new-domain-tracking", "Track newly observed domains (i.e. never seen before).")="no"; ::arg().set("new-domain-log", "Log newly observed domains.")="yes"; diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index c26b82aca1..d13ab9ec50 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -90,6 +90,7 @@ void loadMainConfig(const std::string& configdir) ::arg().set("max-nsec3-iterations","Limit the number of NSEC3 hash iterations")="500"; // RFC5155 10.3 ::arg().set("max-signature-cache-entries", "Maximum number of signatures cache entries")=""; ::arg().set("rng", "Specify random number generator to use. Valid values are auto,sodium,openssl,getrandom,arc4random,urandom.")="auto"; + ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; ::arg().laxFile(configname.c_str()); if(!::arg()["load-modules"].empty()) { @@ -917,6 +918,7 @@ int editZone(const DNSName &zone) { } cmdline.clear(); ZoneParserTNG zpt(tmpnam, g_rootdnsname); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord zrr; map, vector > grouped; try { @@ -1088,6 +1090,7 @@ int loadZone(DNSName zone, const string& fname) { } DNSBackend* db = di.backend; ZoneParserTNG zpt(fname, zone); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; if(!db->startTransaction(zone, di.id)) { @@ -1416,6 +1419,7 @@ void testSpeed(DNSSECKeeper& dk, const DNSName& zone, const string& remote, int void verifyCrypto(const string& zone) { ZoneParserTNG zpt(zone); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; DNSKEYRecordContent drc; RRSIGRecordContent rrc; diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index d9b65d35bf..ba7770d4bc 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -872,6 +872,20 @@ Maximum number of incoming requests handled concurrently per tcp connection. This number must be larger than 0 and smaller than 65536 and also smaller than `max-mthreads`. +.. _setting-max-generate-steps: + +``max-generate-steps`` +---------------------- + +.. versionadded:: 4.3.0 + +- Integer +- Default: 0 + +Maximum number of steps for a '$GENERATE' directive when parsing a +zone file. This is a protection measure to prevent consuming a lot of +CPU and memory when untrusted zones are loaded. Default to 0 which +means unlimited. .. _setting-max-mthreads: diff --git a/pdns/reczones.cc b/pdns/reczones.cc index 505a18402e..c811b0c989 100644 --- a/pdns/reczones.cc +++ b/pdns/reczones.cc @@ -84,6 +84,7 @@ void primeHints(void) } else { ZoneParserTNG zpt(::arg()["hint-file"]); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; while(zpt.get(rr)) { @@ -381,6 +382,7 @@ std::shared_ptr parseAuthAndForwards() ad.d_rdForward = false; g_log<filename, i->name, BP.getDirectory()); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; obj["name"] = i->name.toString(); @@ -205,6 +207,7 @@ try } else { ZoneParserTNG zpt(zonefile, DNSName(::arg()["zone-name"])); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; string zname; Json::object obj; diff --git a/pdns/zone2ldap.cc b/pdns/zone2ldap.cc index 1d760dec35..b7ebcb3c31 100644 --- a/pdns/zone2ldap.cc +++ b/pdns/zone2ldap.cc @@ -248,6 +248,7 @@ int main( int argc, char* argv[] ) args.set( "layout", "How to arrange entries in the directory (simple or as tree)" ) = "simple"; args.set( "domainid", "Domain ID of the first domain found (incremented afterwards)" ) = "1"; args.set( "metadata-dn", "DN under which to store the domain metadata" ) = ""; + args.set( "max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; args.parse( argc, argv ); @@ -316,6 +317,7 @@ int main( int argc, char* argv[] ) cerr << "Parsing file: " << i.filename << ", domain: " << i.name << endl; g_zonename = i.name; ZoneParserTNG zpt(i.filename, i.name, BP.getDirectory()); + zpt.setMaxGenerateSteps(args.asNum("max-generate-steps")); DNSResourceRecord rr; while(zpt.get(rr)) { callback(g_domainid, rr.qname, rr.qtype.getName(), encode_non_ascii(rr.content), rr.ttl); @@ -344,6 +346,7 @@ int main( int argc, char* argv[] ) g_zonename = DNSName(args["zone-name"]); ZoneParserTNG zpt(args["zone-file"], g_zonename); + zpt.setMaxGenerateSteps(args.asNum("max-generate-steps")); DNSResourceRecord rr; while(zpt.get(rr)) { callback(g_domainid, rr.qname, rr.qtype.getName(), encode_non_ascii(rr.content), rr.ttl); diff --git a/pdns/zone2sql.cc b/pdns/zone2sql.cc index 5b86fa3410..08fca465d4 100644 --- a/pdns/zone2sql.cc +++ b/pdns/zone2sql.cc @@ -236,6 +236,7 @@ try ::arg().set("soa-refresh-default","Do not change")="0"; ::arg().set("soa-retry-default","Do not change")="0"; ::arg().set("soa-expire-default","Do not change")="0"; + ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; ::arg().setCmd("help","Provide a helpful message"); ::arg().setCmd("version","Print the version"); @@ -319,6 +320,7 @@ try emitDomain(i->name, &(i->masters)); ZoneParserTNG zpt(i->filename, i->name, BP.getDirectory()); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; bool seenSOA=false; string comment; @@ -357,6 +359,7 @@ try zonename = DNSName(::arg()["zone-name"]); ZoneParserTNG zpt(zonefile, zonename); + zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); DNSResourceRecord rr; startNewTransaction(); string comment; diff --git a/pdns/zoneparser-tng.cc b/pdns/zoneparser-tng.cc index 9772e36ada..b2d88b3a94 100644 --- a/pdns/zoneparser-tng.cc +++ b/pdns/zoneparser-tng.cc @@ -328,6 +328,12 @@ bool ZoneParserTNG::get(DNSResourceRecord& rr, std::string* comment) d_templatestop < d_templatecounter) { throw exception("Invalid $GENERATE parameters"); } + if (d_maxGenerateSteps != 0) { + size_t numberOfSteps = (d_templatestop - d_templatecounter) / d_templatestep; + if (numberOfSteps > d_maxGenerateSteps) { + throw exception("The number of $GENERATE steps (" + std::to_string(numberOfSteps) + ") is too high, the maximum is set to " + std::to_string(d_maxGenerateSteps)); + } + } d_templateline=d_line; parts.pop_front(); parts.pop_front(); diff --git a/pdns/zoneparser-tng.hh b/pdns/zoneparser-tng.hh index 36d58f9b39..f20845371d 100644 --- a/pdns/zoneparser-tng.hh +++ b/pdns/zoneparser-tng.hh @@ -41,6 +41,10 @@ public: DNSName getZoneName(); string getLineOfFile(); // for error reporting purposes pair getLineNumAndFile(); // idem + void setMaxGenerateSteps(size_t max) + { + d_maxGenerateSteps = max; + } private: bool getLine(); bool getTemplateLine(); @@ -63,6 +67,7 @@ private: vector::iterator d_zonedataline; std::stack d_filestates; parts_t d_templateparts; + size_t d_maxGenerateSteps{0}; int d_defaultttl; uint32_t d_templatecounter, d_templatestop, d_templatestep; bool d_havedollarttl;