From: Chris Hofstaedtler <chris.hofstaedtler@deduktiva.com>
Date: Mon, 5 Mar 2018 13:10:32 +0000 (+0100)
Subject: dnsdist: create RemoteLoggers in client mode, but avoid connecting
X-Git-Tag: dnsdist-1.3.0~65^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6b44773a79bb2a799366ffefaeaef092f83c6c2b;p=thirdparty%2Fpdns.git

dnsdist: create RemoteLoggers in client mode, but avoid connecting

Fixes a nullptr deref under --check-config.
---

diff --git a/pdns/dnsdist-lua-bindings.cc b/pdns/dnsdist-lua-bindings.cc
index e8cc99f5a4..bb1415c7a4 100644
--- a/pdns/dnsdist-lua-bindings.cc
+++ b/pdns/dnsdist-lua-bindings.cc
@@ -266,29 +266,20 @@ void setupLuaBindings(bool client)
 
   /* RemoteLogger */
   g_lua.writeFunction("newRemoteLogger", [client](const std::string& remote, boost::optional<uint16_t> timeout, boost::optional<uint64_t> maxQueuedEntries, boost::optional<uint8_t> reconnectWaitTime) {
-      if (client) {
-        return std::shared_ptr<RemoteLoggerInterface>();
-      }
-      return std::shared_ptr<RemoteLoggerInterface>(new RemoteLogger(ComboAddress(remote), timeout ? *timeout : 2, maxQueuedEntries ? *maxQueuedEntries : 100, reconnectWaitTime ? *reconnectWaitTime : 1));
+      return std::shared_ptr<RemoteLoggerInterface>(new RemoteLogger(ComboAddress(remote), timeout ? *timeout : 2, maxQueuedEntries ? *maxQueuedEntries : 100, reconnectWaitTime ? *reconnectWaitTime : 1, client));
     });
 
   g_lua.writeFunction("newFrameStreamUnixLogger", [client](const std::string& address) {
-      if (client) {
-        return std::shared_ptr<RemoteLoggerInterface>();
-      }
 #ifdef HAVE_FSTRM
-      return std::shared_ptr<RemoteLoggerInterface>(new FrameStreamLogger(AF_UNIX, address));
+      return std::shared_ptr<RemoteLoggerInterface>(new FrameStreamLogger(AF_UNIX, address, !client));
 #else
       throw std::runtime_error("fstrm support is required to build an AF_UNIX FrameStreamLogger");
 #endif /* HAVE_FSTRM */
     });
 
   g_lua.writeFunction("newFrameStreamTcpLogger", [client](const std::string& address) {
-      if (client) {
-        return std::shared_ptr<RemoteLoggerInterface>();
-      }
 #if defined(HAVE_FSTRM) && defined(HAVE_FSTRM_TCP_WRITER_INIT)
-      return std::shared_ptr<RemoteLoggerInterface>(new FrameStreamLogger(AF_INET, address));
+      return std::shared_ptr<RemoteLoggerInterface>(new FrameStreamLogger(AF_INET, address, !client));
 #else
       throw std::runtime_error("fstrm with TCP support is required to build an AF_INET FrameStreamLogger");
 #endif /* HAVE_FSTRM */
diff --git a/pdns/fstrm_logger.cc b/pdns/fstrm_logger.cc
index fb232b3900..617aa73d5c 100644
--- a/pdns/fstrm_logger.cc
+++ b/pdns/fstrm_logger.cc
@@ -9,7 +9,7 @@
 
 #ifdef HAVE_FSTRM
 
-FrameStreamLogger::FrameStreamLogger(const int family, const std::string& address): d_family(family), d_address(address)
+FrameStreamLogger::FrameStreamLogger(const int family, const std::string& address, bool connect): d_family(family), d_address(address)
 {
   fstrm_res res;
 
@@ -78,14 +78,16 @@ FrameStreamLogger::FrameStreamLogger(const int family, const std::string& addres
       throw std::runtime_error("FrameStreamLogger: fstrm_iothr_options_set_queue_model failed: " + std::to_string(res));
     }
 
-    d_iothr = fstrm_iothr_init(d_iothropt, &d_writer);
-    if (!d_iothr) {
-      throw std::runtime_error("FrameStreamLogger: fstrm_iothr_init() failed.");
-    }
+    if (connect) {
+      d_iothr = fstrm_iothr_init(d_iothropt, &d_writer);
+      if (!d_iothr) {
+        throw std::runtime_error("FrameStreamLogger: fstrm_iothr_init() failed.");
+      }
 
-    d_ioqueue = fstrm_iothr_get_input_queue(d_iothr);
-    if (!d_ioqueue) {
-      throw std::runtime_error("FrameStreamLogger: fstrm_iothr_get_input_queue() failed.");
+      d_ioqueue = fstrm_iothr_get_input_queue(d_iothr);
+      if (!d_ioqueue) {
+        throw std::runtime_error("FrameStreamLogger: fstrm_iothr_get_input_queue() failed.");
+      }
     }
   } catch (std::runtime_error &e) {
     this->cleanup();
@@ -130,6 +132,9 @@ FrameStreamLogger::~FrameStreamLogger()
 
 void FrameStreamLogger::queueData(const std::string& data)
 {
+  if (!d_ioqueue || !d_iothr) {
+    return;
+  }
   uint8_t *frame = (uint8_t*)malloc(data.length());
   if (!frame) {
     warnlog("FrameStreamLogger: cannot allocate memory for stream.");
diff --git a/pdns/fstrm_logger.hh b/pdns/fstrm_logger.hh
index f661e719e4..6f4c40a1ec 100644
--- a/pdns/fstrm_logger.hh
+++ b/pdns/fstrm_logger.hh
@@ -35,7 +35,7 @@
 class FrameStreamLogger : public RemoteLoggerInterface, boost::noncopyable
 {
 public:
-  FrameStreamLogger(int family, const std::string& address);
+  FrameStreamLogger(int family, const std::string& address, bool connect);
   virtual ~FrameStreamLogger();
   virtual void queueData(const std::string& data) override;
   virtual std::string toString() override
diff --git a/pdns/remote_logger.cc b/pdns/remote_logger.cc
index 9844cc132a..6c472e7745 100644
--- a/pdns/remote_logger.cc
+++ b/pdns/remote_logger.cc
@@ -41,10 +41,6 @@ void RemoteLogger::busyReconnectLoop()
 
 void RemoteLogger::worker()
 {
-  if (d_asyncConnect) {
-    busyReconnectLoop();
-  }
-
   while(true) {
     std::string data;
     {
diff --git a/regression-tests.dnsdist/dnsdisttests.py b/regression-tests.dnsdist/dnsdisttests.py
index eaea70b598..5dfec19f5b 100644
--- a/regression-tests.dnsdist/dnsdisttests.py
+++ b/regression-tests.dnsdist/dnsdisttests.py
@@ -70,6 +70,12 @@ class DNSDistTest(unittest.TestCase):
             dnsdistcmd.extend(['--acl', acl])
         print(' '.join(dnsdistcmd))
 
+        # validate config with --check-config, which sets client=true, possibly exposing bugs.
+        testcmd = dnsdistcmd + ['--check-config']
+        output = subprocess.check_output(testcmd, close_fds=True)
+        if output != b'Configuration \'dnsdist_test.conf\' OK!\n':
+            raise AssertionError('dnsdist --check-config failed: %s' % output)
+
         if shutUp:
             with open(os.devnull, 'w') as fdDevNull:
                 cls._dnsdist = subprocess.Popen(dnsdistcmd, close_fds=True, stdout=fdDevNull)