From: krionbsd Date: Mon, 6 Apr 2020 10:28:34 +0000 (+0200) Subject: Utilize std::shuffle and introduce pdns::dns_random_engine X-Git-Tag: dnsdist-1.5.0-rc2~17^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbec1961a3d3e79b5e0a50477a7bf33812310b67;p=thirdparty%2Fpdns.git Utilize std::shuffle and introduce pdns::dns_random_engine std::random_shuffle is deprecated and removed starting from C++17, so we might better use std::shuffle which uses URBG: https://en.cppreference.com/w/cpp/named_req/UniformRandomBitGenerator under the hood for a better random generation. --- diff --git a/pdns/Makefile.am b/pdns/Makefile.am index 524110a07b..3aa748fd6f 100644 --- a/pdns/Makefile.am +++ b/pdns/Makefile.am @@ -213,6 +213,7 @@ pdns_server_SOURCES = \ secpoll-auth.cc secpoll-auth.hh \ serialtweaker.cc \ sha.hh \ + shuffle.cc shuffle.hh \ signingpipe.cc signingpipe.hh \ sillyrecords.cc \ slavecommunicator.cc \ @@ -324,6 +325,7 @@ pdnsutil_SOURCES = \ qtype.cc \ rcpgenerator.cc rcpgenerator.hh \ serialtweaker.cc \ + shuffle.cc shuffle.hh \ signingpipe.cc \ sillyrecords.cc \ sstuff.hh \ @@ -1298,6 +1300,7 @@ testrunner_SOURCES = \ rcpgenerator.cc \ responsestats.cc \ responsestats-auth.cc \ + shuffle.cc shuffle.hh \ sillyrecords.cc \ statbag.cc \ test-arguments_cc.cc \ diff --git a/pdns/dns_random.hh b/pdns/dns_random.hh index 8afda61e60..b968b0766b 100644 --- a/pdns/dns_random.hh +++ b/pdns/dns_random.hh @@ -21,7 +21,31 @@ */ #pragma once #include +#include void dns_random_init(const std::string& data = "", bool force_reinit = false); uint32_t dns_random(uint32_t n); uint16_t dns_random_uint16(); + +namespace pdns { + struct dns_random_engine { + + typedef uint32_t result_type; + + static constexpr result_type min() + { + return 0; + } + + static constexpr result_type max() + { + return std::numeric_limits::max() - 1; + } + + result_type operator()() + { + return dns_random(std::numeric_limits::max()); + } + }; +} + diff --git a/pdns/dnspacket.cc b/pdns/dnspacket.cc index 97a372be32..fa193d4834 100644 --- a/pdns/dnspacket.cc +++ b/pdns/dnspacket.cc @@ -50,6 +50,7 @@ #include "ednssubnet.hh" #include "gss_context.hh" #include "dns_random.hh" +#include "shuffle.hh" bool DNSPacket::s_doEDNSSubnetProcessing; uint16_t DNSPacket::s_udpTruncationThreshold; @@ -226,7 +227,7 @@ void DNSPacket::wrapup() static bool mustNotShuffle = ::arg().mustDo("no-shuffle"); if(!d_tcp && !mustNotShuffle) { - shuffle(d_rrs); + pdns::shuffle(d_rrs); } d_wrapped=true; diff --git a/pdns/misc.cc b/pdns/misc.cc index 279151116a..72ea5f05d1 100644 --- a/pdns/misc.cc +++ b/pdns/misc.cc @@ -585,83 +585,6 @@ string makeHexDump(const string& str) return ret; } -// shuffle, maintaining some semblance of order -void shuffle(vector& rrs) -{ - vector::iterator first, second; - for(first=rrs.begin();first!=rrs.end();++first) - if(first->dr.d_place==DNSResourceRecord::ANSWER && first->dr.d_type != QType::CNAME) // CNAME must come first - break; - for(second=first;second!=rrs.end();++second) - if(second->dr.d_place!=DNSResourceRecord::ANSWER) - break; - - if(second-first > 1) - random_shuffle(first,second); - - // now shuffle the additional records - for(first=second;first!=rrs.end();++first) - if(first->dr.d_place==DNSResourceRecord::ADDITIONAL && first->dr.d_type != QType::CNAME) // CNAME must come first - break; - for(second=first;second!=rrs.end();++second) - if(second->dr.d_place!=DNSResourceRecord::ADDITIONAL) - break; - - if(second-first>1) - random_shuffle(first,second); - - // we don't shuffle the rest -} - - -// shuffle, maintaining some semblance of order -void shuffle(vector& rrs) -{ - vector::iterator first, second; - for(first=rrs.begin();first!=rrs.end();++first) - if(first->d_place==DNSResourceRecord::ANSWER && first->d_type != QType::CNAME) // CNAME must come first - break; - for(second=first;second!=rrs.end();++second) - if(second->d_place!=DNSResourceRecord::ANSWER || second->d_type == QType::RRSIG) // leave RRSIGs at the end - break; - - if(second-first>1) - random_shuffle(first,second); - - // now shuffle the additional records - for(first=second;first!=rrs.end();++first) - if(first->d_place==DNSResourceRecord::ADDITIONAL && first->d_type != QType::CNAME) // CNAME must come first - break; - for(second=first; second!=rrs.end(); ++second) - if(second->d_place!=DNSResourceRecord::ADDITIONAL) - break; - - if(second-first>1) - random_shuffle(first,second); - - // we don't shuffle the rest -} - -static uint16_t mapTypesToOrder(uint16_t type) -{ - if(type == QType::CNAME) - return 0; - if(type == QType::RRSIG) - return 65535; - else - return 1; -} - -// make sure rrs is sorted in d_place order to avoid surprises later -// then shuffle the parts that desire shuffling -void orderAndShuffle(vector& rrs) -{ - std::stable_sort(rrs.begin(), rrs.end(), [](const DNSRecord&a, const DNSRecord& b) { - return std::make_tuple(a.d_place, mapTypesToOrder(a.d_type)) < std::make_tuple(b.d_place, mapTypesToOrder(b.d_type)); - }); - shuffle(rrs); -} - void normalizeTV(struct timeval& tv) { if(tv.tv_usec > 1000000) { diff --git a/pdns/misc.hh b/pdns/misc.hh index b4924c4274..503992510f 100644 --- a/pdns/misc.hh +++ b/pdns/misc.hh @@ -287,12 +287,6 @@ inline void unixDie(const string &why) } string makeHexDump(const string& str); -struct DNSRecord; -struct DNSZoneRecord; -void shuffle(vector& rrs); -void shuffle(vector& rrs); - -void orderAndShuffle(vector& rrs); void normalizeTV(struct timeval& tv); const struct timeval operator+(const struct timeval& lhs, const struct timeval& rhs); diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 722631a00b..473dec1714 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -90,6 +90,7 @@ #include "gettime.hh" #include "proxy-protocol.hh" #include "pubsuffix.hh" +#include "shuffle.hh" #ifdef NOD_ENABLED #include "nod.hh" #endif /* NOD_ENABLED */ @@ -1557,7 +1558,7 @@ static void startDoResolve(void *p) } if(ret.size()) { - orderAndShuffle(ret); + pdns::orderAndShuffle(ret); if(auto sl = luaconfsLocal->sortlist.getOrderCmp(dc->d_source)) { stable_sort(ret.begin(), ret.end(), *sl); variableAnswer=true; diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am index 657ea7eec6..6842f7d248 100644 --- a/pdns/recursordist/Makefile.am +++ b/pdns/recursordist/Makefile.am @@ -168,6 +168,7 @@ pdns_recursor_SOURCES = \ secpoll-recursor.cc secpoll-recursor.hh \ secpoll.cc secpoll.hh \ sholder.hh \ + shuffle.cc shuffle.hh \ sillyrecords.cc \ snmp-agent.hh snmp-agent.cc \ sortlist.cc sortlist.hh \ diff --git a/pdns/recursordist/shuffle.cc b/pdns/recursordist/shuffle.cc new file mode 120000 index 0000000000..eaf754bf9c --- /dev/null +++ b/pdns/recursordist/shuffle.cc @@ -0,0 +1 @@ +../shuffle.cc \ No newline at end of file diff --git a/pdns/recursordist/shuffle.hh b/pdns/recursordist/shuffle.hh new file mode 120000 index 0000000000..90ed1c6a8d --- /dev/null +++ b/pdns/recursordist/shuffle.hh @@ -0,0 +1 @@ +../shuffle.hh \ No newline at end of file diff --git a/pdns/shuffle.cc b/pdns/shuffle.cc new file mode 100644 index 0000000000..6440d77c66 --- /dev/null +++ b/pdns/shuffle.cc @@ -0,0 +1,109 @@ +/* + * 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. + */ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include + +#include "shuffle.hh" +#include "dns_random.hh" +#include "dnsparser.hh" + +// shuffle, maintaining some semblance of order +void pdns::shuffle(std::vector& rrs) +{ + std::vector::iterator first, second; + for(first=rrs.begin();first!=rrs.end();++first) + if(first->dr.d_place==DNSResourceRecord::ANSWER && first->dr.d_type != QType::CNAME) // CNAME must come first + break; + for(second=first;second!=rrs.end();++second) + if(second->dr.d_place!=DNSResourceRecord::ANSWER) + break; + + dns_random_engine r; + if(second-first > 1) + shuffle(first, second, r); + + // now shuffle the additional records + for(first=second;first!=rrs.end();++first) + if(first->dr.d_place==DNSResourceRecord::ADDITIONAL && first->dr.d_type != QType::CNAME) // CNAME must come first + break; + for(second=first;second!=rrs.end();++second) + if(second->dr.d_place!=DNSResourceRecord::ADDITIONAL) + break; + + if(second-first>1) + shuffle(first, second, r); + + // we don't shuffle the rest +} + + +// shuffle, maintaining some semblance of order +void pdns::shuffle(std::vector& rrs) +{ + std::vector::iterator first, second; + for(first=rrs.begin();first!=rrs.end();++first) + if(first->d_place==DNSResourceRecord::ANSWER && first->d_type != QType::CNAME) // CNAME must come first + break; + for(second=first;second!=rrs.end();++second) + if(second->d_place!=DNSResourceRecord::ANSWER || second->d_type == QType::RRSIG) // leave RRSIGs at the end + break; + + dns_random_engine r; + if(second-first>1) + shuffle(first, second, r); + + // now shuffle the additional records + for(first=second;first!=rrs.end();++first) + if(first->d_place==DNSResourceRecord::ADDITIONAL && first->d_type != QType::CNAME) // CNAME must come first + break; + for(second=first; second!=rrs.end(); ++second) + if(second->d_place!=DNSResourceRecord::ADDITIONAL) + break; + + if(second-first>1) + shuffle(first, second, r); + + // we don't shuffle the rest +} + +static uint16_t mapTypesToOrder(uint16_t type) +{ + if (type == QType::CNAME) + return 0; + if (type == QType::RRSIG) + return 65535; + else + return 1; +} + +// make sure rrs is sorted in d_place order to avoid surprises later +// then shuffle the parts that desire shuffling +void pdns::orderAndShuffle(vector& rrs) +{ + std::stable_sort(rrs.begin(), rrs.end(), [](const DNSRecord&a, const DNSRecord& b) { + return std::make_tuple(a.d_place, mapTypesToOrder(a.d_type)) < std::make_tuple(b.d_place, mapTypesToOrder(b.d_type)); + }); + shuffle(rrs); +} diff --git a/pdns/shuffle.hh b/pdns/shuffle.hh new file mode 100644 index 0000000000..c42e400720 --- /dev/null +++ b/pdns/shuffle.hh @@ -0,0 +1,33 @@ +/* + * 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 + +struct DNSRecord; +struct DNSZoneRecord; + +namespace pdns { + void shuffle(std::vector& rrs); + void shuffle(std::vector& rrs); + void orderAndShuffle(std::vector& rrs); +} + diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 93f97d3821..50ac112d10 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -992,7 +992,7 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, t_sstorage.nsSpeeds[qname].purge(speeds); if(ret.size() > 1) { - random_shuffle(ret.begin(), ret.end()); + shuffle(ret.begin(), ret.end(), pdns::dns_random_engine()); speedOrderCA so(speeds); stable_sort(ret.begin(), ret.end(), so); @@ -1657,6 +1657,8 @@ inline std::vector> SyncRes::shuffleInSpeedOrder(NsSet return rnameservers; } + // Using shuffle(rnameservers.begin(), rnameservers.end(), pdsn:dns_ramndom_engine()) causes a boost assert, + // to be investigated random_shuffle(rnameservers.begin(),rnameservers.end()); speedOrder so; stable_sort(rnameservers.begin(),rnameservers.end(), so); @@ -1688,7 +1690,7 @@ inline vector SyncRes::shuffleForwardSpeed(const vector