]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Utilize std::shuffle and introduce pdns::dns_random_engine
authorkrionbsd <krion@FreeBSD.org>
Mon, 6 Apr 2020 10:28:34 +0000 (12:28 +0200)
committerOtto Moerbeek <otto.moerbeek@open-xchange.com>
Wed, 22 Apr 2020 07:44:47 +0000 (09:44 +0200)
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.

12 files changed:
pdns/Makefile.am
pdns/dns_random.hh
pdns/dnspacket.cc
pdns/misc.cc
pdns/misc.hh
pdns/pdns_recursor.cc
pdns/recursordist/Makefile.am
pdns/recursordist/shuffle.cc [new symlink]
pdns/recursordist/shuffle.hh [new symlink]
pdns/shuffle.cc [new file with mode: 0644]
pdns/shuffle.hh [new file with mode: 0644]
pdns/syncres.cc

index 524110a07be8a0035da66cc314e16456fd9b19b5..3aa748fd6f36344e6667891ecb918eeb50b7bf58 100644 (file)
@@ -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 \
index 8afda61e60353a6645bebefed53191e38faadc88..b968b0766b0d432422e200c8db8ccad82476dc50 100644 (file)
  */
 #pragma once
 #include <cstdint>
+#include <limits>
 
 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<result_type>::max() - 1;
+    }
+
+    result_type operator()()
+    {
+      return dns_random(std::numeric_limits<result_type>::max());
+    }
+  };
+}
+
index 97a372be32f141afd642aab116528b22490457f7..fa193d48346cba89460e909764e34715512c2edb 100644 (file)
@@ -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;
 
index 279151116a9ee6cf2d85cbd7c1d7c1f4473cc2da..72ea5f05d148364e7b117526dea72b8f09f78873 100644 (file)
@@ -585,83 +585,6 @@ string makeHexDump(const string& str)
   return ret;
 }
 
-// shuffle, maintaining some semblance of order
-void shuffle(vector<DNSZoneRecord>& rrs)
-{
-  vector<DNSZoneRecord>::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<DNSRecord>& rrs)
-{
-  vector<DNSRecord>::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<DNSRecord>& 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) {
index b4924c427468f4130f73956ccb58802ebc25f919..503992510f116d1f6618cf07e23ab87fd780cba8 100644 (file)
@@ -287,12 +287,6 @@ inline void unixDie(const string &why)
 }
 
 string makeHexDump(const string& str);
-struct DNSRecord;
-struct DNSZoneRecord;
-void shuffle(vector<DNSRecord>& rrs);
-void shuffle(vector<DNSZoneRecord>& rrs);
-
-void orderAndShuffle(vector<DNSRecord>& rrs);
 
 void normalizeTV(struct timeval& tv);
 const struct timeval operator+(const struct timeval& lhs, const struct timeval& rhs);
index 722631a00b737c6cf5b7908c9e526864ca5f8567..473dec171411d5c335d69057575d30bcc74940f1 100644 (file)
@@ -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;
index 657ea7eec6bcb4f35d98d99d675195018d60e2a5..6842f7d2487ca8001dff456d2fa82cebf1fdc9aa 100644 (file)
@@ -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 (symlink)
index 0000000..eaf754b
--- /dev/null
@@ -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 (symlink)
index 0000000..90ed1c6
--- /dev/null
@@ -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 (file)
index 0000000..6440d77
--- /dev/null
@@ -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 <string>
+
+#include "shuffle.hh"
+#include "dns_random.hh"
+#include "dnsparser.hh"
+
+// shuffle, maintaining some semblance of order
+void pdns::shuffle(std::vector<DNSZoneRecord>& rrs)
+{
+  std::vector<DNSZoneRecord>::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<DNSRecord>& rrs)
+{
+  std::vector<DNSRecord>::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<DNSRecord>& 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 (file)
index 0000000..c42e400
--- /dev/null
@@ -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 <vector>
+
+struct DNSRecord;
+struct DNSZoneRecord;
+
+namespace pdns {
+  void shuffle(std::vector<DNSRecord>& rrs);
+  void shuffle(std::vector<DNSZoneRecord>& rrs);
+  void orderAndShuffle(std::vector<DNSRecord>& rrs);
+}
+
index 93f97d38217bc075bec418f2769f5263b3f17b7e..50ac112d10cac45e81365cb023085bcab56b6887 100644 (file)
@@ -992,7 +992,7 @@ vector<ComboAddress> 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<std::pair<DNSName, float>> 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<ComboAddress> SyncRes::shuffleForwardSpeed(const vector<ComboAddre
     speed=t_sstorage.nsSpeeds[nsName].get(d_now);
     speeds[val]=speed;
   }
-  random_shuffle(nameservers.begin(),nameservers.end());
+  shuffle(nameservers.begin(),nameservers.end(), pdns::dns_random_engine());
   speedOrderCA so(speeds);
   stable_sort(nameservers.begin(),nameservers.end(), so);