]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Refuse console connection without a proper key set
authorRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 5 Jun 2018 21:28:31 +0000 (23:28 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 5 Jun 2018 21:28:31 +0000 (23:28 +0200)
pdns/dnsdist-console.cc
pdns/dnsdist-console.hh
pdns/dnsdist-lua.cc
pdns/dnsdist.cc
pdns/dolog.hh
pdns/sodcrypto.cc
pdns/sodcrypto.hh
pdns/test-dnscrypt_cc.cc
pdns/test-dnsdist_cc.cc
regression-tests.dnsdist/dnsdisttests.py
regression-tests.dnsdist/test_Console.py

index f8134fff48df0db5256ba917a7cb5ddcce2d8e5f..4405065b555b3b579a01d5d295be357fdd950e07 100644 (file)
@@ -42,6 +42,7 @@ GlobalStateHolder<NetmaskGroup> g_consoleACL;
 vector<pair<struct timeval, string> > g_confDelta;
 std::string g_consoleKey;
 bool g_logConsoleConnections{true};
+bool g_consoleEnabled{false};
 
 // MUST BE CALLED UNDER A LOCK - right now the LuaLock
 static void feedConfigDelta(const std::string& line)
@@ -73,10 +74,56 @@ static string historyFile(const bool &ignoreHOME = false)
   return ret;
 }
 
+static bool sendMessageToServer(int fd, const std::string& line, SodiumNonce& readingNonce, SodiumNonce& writingNonce, const bool outputEmptyLine)
+{
+  string msg = sodEncryptSym(line, g_consoleKey, writingNonce);
+  const auto msgLen = msg.length();
+  if (msgLen > std::numeric_limits<uint32_t>::max()) {
+    cout << "Encrypted essage is too long to be sent to the server, "<< std::to_string(msgLen) << " > " << std::numeric_limits<uint32_t>::max() << endl;
+    return true;
+  }
+
+  putMsgLen32(fd, static_cast<uint32_t>(msgLen));
+
+  if (!msg.empty()) {
+    cerr << "sending message of size " << msgLen << endl;
+    writen2(fd, msg);
+  }
+
+  uint32_t len;
+  if(!getMsgLen32(fd, &len)) {
+    cout << "Connection closed by the server." << endl;
+    return false;
+  }
+
+  if (len == 0) {
+    if (outputEmptyLine) {
+      cout << endl;
+    }
+
+    return true;
+  }
+
+  boost::scoped_array<char> resp(new char[len]);
+  readn2(fd, resp.get(), len);
+  msg.assign(resp.get(), len);
+  msg = sodDecryptSym(msg, g_consoleKey, readingNonce);
+  cout << msg;
+  cout.flush();
+
+  return true;
+}
+
 void doClient(ComboAddress server, const std::string& command)
 {
-  if(g_verbose)
+  if (!sodIsValidKey(g_consoleKey)) {
+    cerr << "The currently configured console key is not valid, please configure a valid key using the setKey() directive" << endl;
+    return;
+  }
+
+  if(g_verbose) {
     cout<<"Connecting to "<<server.toStringWithPort()<<endl;
+  }
 
   int fd=socket(server.sin4.sin_family, SOCK_STREAM, 0);
   if (fd < 0) {
@@ -93,25 +140,18 @@ void doClient(ComboAddress server, const std::string& command)
   readingNonce.merge(ours, theirs);
   writingNonce.merge(theirs, ours);
 
-  if(!command.empty()) {
-    string msg=sodEncryptSym(command, g_consoleKey, writingNonce);
-    putMsgLen32(fd, (uint32_t) msg.length());
-    if(!msg.empty())
-      writen2(fd, msg);
-    uint32_t len;
-    if(getMsgLen32(fd, &len)) {
-      if (len > 0) {
-        boost::scoped_array<char> resp(new char[len]);
-        readn2(fd, resp.get(), len);
-        msg.assign(resp.get(), len);
-        msg=sodDecryptSym(msg, g_consoleKey, readingNonce);
-        cout<<msg;
-        cout.flush();
-      }
-    }
-    else {
-      cout << "Connection closed by the server." << endl;
-    }
+  /* try sending an empty message, the server should send an empty
+     one back. If it closes the connection instead, we are probably
+     having a key mismatch issue. */
+  if (!sendMessageToServer(fd, "", readingNonce, writingNonce, false)) {
+    cerr<<"The server closed the connection right away, likely indicating a key mismatch. Please check your setKey() directive."<<endl;
+    close(fd);
+    return;
+  }
+
+  if (!command.empty()) {
+    sendMessageToServer(fd, command, readingNonce, writingNonce, false);
+
     close(fd);
     return; 
   }
@@ -150,26 +190,9 @@ void doClient(ComboAddress server, const std::string& command)
     if(line.empty())
       continue;
 
-    string msg=sodEncryptSym(line, g_consoleKey, writingNonce);
-    putMsgLen32(fd, (uint32_t) msg.length());
-    writen2(fd, msg);
-    uint32_t len;
-    if(!getMsgLen32(fd, &len)) {
-      cout << "Connection closed by the server." << endl;
+    if (!sendMessageToServer(fd, line, readingNonce, writingNonce, true)) {
       break;
     }
-
-    if (len > 0) {
-      boost::scoped_array<char> resp(new char[len]);
-      readn2(fd, resp.get(), len);
-      msg.assign(resp.get(), len);
-      msg=sodDecryptSym(msg, g_consoleKey, readingNonce);
-      cout<<msg;
-      cout.flush();
-    }
-    else {
-      cout<<endl;
-    }
   }
   close(fd);
 }
@@ -529,8 +552,9 @@ try
 
     boost::scoped_array<char> msg(new char[len]);
     readn2(fd, msg.get(), len);
-    
+
     string line(msg.get(), len);
+
     line = sodDecryptSym(line, g_consoleKey, readingNonce);
     //    cerr<<"Have decrypted line: "<<line<<endl;
     string response;
@@ -643,6 +667,12 @@ try
 
   while ((sock = SAccept(fd, client)) >= 0) {
 
+    if (!sodIsValidKey(g_consoleKey)) {
+      vinfolog("Control connection from %s dropped because we don't have a valid key configured, please configure one using setKey()", client.toStringWithPort());
+      close(sock);
+      continue;
+    }
+
     if (!localACL->match(client)) {
       vinfolog("Control connection from %s dropped because of ACL", client.toStringWithPort());
       close(sock);
index 01eb5d48b1f897dea25edf8b95dde91f9c3a1d3c..7de8b68454f8bb3c10f67e015e3e60c9f810af61 100644 (file)
@@ -42,6 +42,7 @@ extern GlobalStateHolder<NetmaskGroup> g_consoleACL;
 extern const std::vector<ConsoleKeyword> g_consoleKeywords;
 extern std::string g_consoleKey; // in theory needs locking
 extern bool g_logConsoleConnections;
+extern bool g_consoleEnabled;
 
 void doClient(ComboAddress server, const std::string& command);
 void doConsole();
index e35f0c0f3e2b0f9dc8ce7bcf922e4a148cfabee1..efa4f105fccfb002a7d7cedf0c7a943e6e80771a 100644 (file)
@@ -611,6 +611,13 @@ void setupLuaConfig(bool client)
        return;
       }
 
+      g_consoleEnabled = true;
+#ifdef HAVE_LIBSODIUM
+      if (g_configurationDone && g_consoleKey.empty()) {
+        warnlog("Warning, the console has been enabled via 'controlSocket()' but no key has been set with 'setKey()' so all connections will fail until a key has been set");
+      }
+#endif
+
       try {
        int sock = SSocket(local.sin4.sin_family, SOCK_STREAM, 0);
        SSetsockopt(sock, SOL_SOCKET, SO_REUSEADDR, 1);
index eb2873c624d6343825be380da2bc3aeb54f86a7c..c03e5aff90c9d0b0e48a019e670eb72e52edc776 100644 (file)
@@ -81,7 +81,6 @@ bool g_verbose;
 
 struct DNSDistStats g_stats;
 uint16_t g_maxOutstanding{10240};
-bool g_console;
 bool g_verboseHealthChecks{false};
 uint32_t g_staleCacheEntriesTTL{0};
 bool g_syslog{true};
@@ -2062,7 +2061,6 @@ try
   signal(SIGPIPE, SIG_IGN);
   signal(SIGCHLD, SIG_IGN);
   openlog("dnsdist", LOG_PID, LOG_DAEMON);
-  g_console=true;
 
 #ifdef HAVE_LIBSODIUM
   if (sodium_init() == -1) {
@@ -2549,6 +2547,7 @@ try
   }
 
   warnlog("dnsdist %s comes with ABSOLUTELY NO WARRANTY. This is free software, and you are welcome to redistribute it according to the terms of the GPL version 2", VERSION);
+
   vector<string> vec;
   std::string acls;
   g_ACL.getLocal()->toStringVector(&vec);
@@ -2569,6 +2568,12 @@ try
   }
   infolog("Console ACL allowing connections from: %s", acls.c_str());
 
+#ifdef HAVE_LIBSODIUM
+  if (g_consoleEnabled && g_consoleKey.empty()) {
+    warnlog("Warning, the console has been enabled via 'controlSocket()' but no key has been set with 'setKey()' so all connections will fail until a key has been set");
+  }
+#endif
+
   uid_t newgid=0;
   gid_t newuid=0;
 
index 19b4f16adb7a0cafeec0f5f2f70f875e853eeb7f..0d5bffe0d93040f5be3cf539bfcefe592f40a984 100644 (file)
@@ -36,7 +36,7 @@
           warnlog("Query took %d milliseconds", 1232.4); // yes, %d
           errlog("Unable to bind to %s: %s", ca.toStringWithPort(), strerr(errno));
 
-   If bool g_console is true, will log to stdout. Will syslog in any case with LOG_INFO,
+   Will log to stdout. Will syslog in any case with LOG_INFO,
    LOG_WARNING, LOG_ERR respectively. If g_verbose=false, vinfolog is a noop.
    More generically, dolog(someiostream, "Hello %s", stream) will log to someiostream
 
@@ -67,7 +67,6 @@ void dolog(std::ostream& os, const char* s, T value, Args... args)
   }    
 }
 
-extern bool g_console;
 extern bool g_verbose;
 extern bool g_syslog;
 
@@ -78,8 +77,7 @@ void genlog(int level, const char* s, Args... args)
   dolog(str, s, args...);
   if(g_syslog)
     syslog(level, "%s", str.str().c_str());
-  if(g_console) 
-    std::cout<<str.str()<<std::endl;
+  std::cout<<str.str()<<std::endl;
 }
 
 
index f83e19aa72f883032e54455b8bdd8f676cd85cab..567db8674a031d6851fdd14c8d23dfdd01dc97df 100644 (file)
@@ -38,8 +38,17 @@ string newKey()
   return "\""+Base64Encode(key)+"\"";
 }
 
+bool sodIsValidKey(const std::string& key)
+{
+  return key.size() == crypto_secretbox_KEYBYTES;
+}
+
 std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce)
 {
+  if (!sodIsValidKey(key)) {
+    throw std::runtime_error("Invalid encryption key of size " + std::to_string(key.size()) + ", use setKey() to set a valid key");
+  }
+
   std::string ciphertext;
   ciphertext.resize(msg.length() + crypto_secretbox_MACBYTES);
   crypto_secretbox_easy(reinterpret_cast<unsigned char*>(&ciphertext.at(0)),
@@ -60,6 +69,10 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium
     throw std::runtime_error("Could not decrypt message of size " + std::to_string(msg.length()));
   }
 
+  if (!sodIsValidKey(key)) {
+    throw std::runtime_error("Invalid decryption key of size " + std::to_string(key.size()) + ", use setKey() to set a valid key");
+  }
+
   decrypted.resize(msg.length() - crypto_secretbox_MACBYTES);
 
   if (crypto_secretbox_open_easy(reinterpret_cast<unsigned char*>(const_cast<char *>(decrypted.data())),
@@ -67,7 +80,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium
                                  msg.length(),
                                  nonce.value,
                                  reinterpret_cast<const unsigned char*>(key.c_str())) != 0) {
-    throw std::runtime_error("Could not decrypt message");
+    throw std::runtime_error("Could not decrypt message, please check that the key configured with setKey() is correct");
   }
 
   nonce.increment();
@@ -88,6 +101,10 @@ string newKey()
   return "\"plaintext\"";
 }
 
+bool sodIsValidKey(const std::string& key)
+{
+  return true;
+}
 
 #endif
 
index 86d52b2f0c9cfc5d2408f4e02f5402401b750493..cfcd7eff0dddde5e505b4e82be363720aa6c1f10 100644 (file)
@@ -70,3 +70,4 @@ std::string newKeypair();
 std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce&);
 std::string sodDecryptSym(const std::string& msg, const std::string& key, SodiumNonce&);
 std::string newKey();
+bool sodIsValidKey(const std::string& key);
index c049ff03cccbc00953fc812d306eefa114493c77..f5fde4a0235758696c5708eaf7368e27586b0ee6 100644 (file)
@@ -31,7 +31,6 @@
 #include <unistd.h>
 
 bool g_verbose{true};
-bool g_console{true};
 bool g_syslog{true};
 
 BOOST_AUTO_TEST_SUITE(dnscrypt_cc)
index 7c9114f68f14f525eabd31e8884b7d4a5a4458a3..b60f063a3b8d4f7b0bb2bb7c595a0b5298c6f1c5 100644 (file)
@@ -37,7 +37,6 @@
 
 BOOST_AUTO_TEST_SUITE(dnsdist_cc)
 
-bool g_console{true};
 bool g_syslog{true};
 bool g_verbose{true};
 
index 46722a78e5a24d39037b5b1552ce6b6cddb7a258..1d3ad6e7a9d2397de2e2a6235228a1717f82e8b7 100644 (file)
@@ -458,6 +458,9 @@ class DNSDistTest(unittest.TestCase):
         sock.send(struct.pack("!I", len(msg)))
         sock.send(msg)
         data = sock.recv(4)
+        if not data:
+            raise socket.error("Got EOF while reading the response size")
+
         (responseLen,) = struct.unpack("!I", data)
         data = sock.recv(responseLen)
         response = cls._decryptConsole(data, readingNonce)
index 0ab8c8fdbe92736281b835987343797fe5f4cb7d..952cc2afa6a151a1b232512ee72a2764537cfc10 100644 (file)
@@ -41,3 +41,20 @@ class TestConsoleNotAllowed(DNSDistTest):
         Console: Not allowed by the ACL
         """
         self.assertRaises(SocketError, self.sendConsoleCommand, 'showVersion()')
+
+class TestConsoleNoKey(DNSDistTest):
+
+    _consoleKey = DNSDistTest.generateConsoleKey()
+    _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii')
+
+    _config_params = ['_consolePort', '_testServerPort']
+    _config_template = """
+    controlSocket("127.0.0.1:%s")
+    newServer{address="127.0.0.1:%d"}
+    """
+
+    def testConsoleAllowed(self):
+        """
+        Console: No key, the connection should not be allowed
+        """
+        self.assertRaises(SocketError, self.sendConsoleCommand, 'showVersion()')