From 9c9b49988138271cd26901643dae0514ae174321 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 5 Jun 2018 23:28:31 +0200 Subject: [PATCH] dnsdist: Refuse console connection without a proper key set --- pdns/dnsdist-console.cc | 108 +++++++++++++++-------- pdns/dnsdist-console.hh | 1 + pdns/dnsdist-lua.cc | 7 ++ pdns/dnsdist.cc | 9 +- pdns/dolog.hh | 6 +- pdns/sodcrypto.cc | 19 +++- pdns/sodcrypto.hh | 1 + pdns/test-dnscrypt_cc.cc | 1 - pdns/test-dnsdist_cc.cc | 1 - regression-tests.dnsdist/dnsdisttests.py | 3 + regression-tests.dnsdist/test_Console.py | 17 ++++ 11 files changed, 125 insertions(+), 48 deletions(-) diff --git a/pdns/dnsdist-console.cc b/pdns/dnsdist-console.cc index f8134fff4..4405065b5 100644 --- a/pdns/dnsdist-console.cc +++ b/pdns/dnsdist-console.cc @@ -42,6 +42,7 @@ GlobalStateHolder g_consoleACL; vector > 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::max()) { + cout << "Encrypted essage is too long to be sent to the server, "<< std::to_string(msgLen) << " > " << std::numeric_limits::max() << endl; + return true; + } + + putMsgLen32(fd, static_cast(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 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 "< 0) { - boost::scoped_array resp(new char[len]); - readn2(fd, resp.get(), len); - msg.assign(resp.get(), len); - msg=sodDecryptSym(msg, g_consoleKey, readingNonce); - cout< 0) { - boost::scoped_array resp(new char[len]); - readn2(fd, resp.get(), len); - msg.assign(resp.get(), len); - msg=sodDecryptSym(msg, g_consoleKey, readingNonce); - cout< msg(new char[len]); readn2(fd, msg.get(), len); - + string line(msg.get(), len); + line = sodDecryptSym(line, g_consoleKey, readingNonce); // cerr<<"Have decrypted line: "<= 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); diff --git a/pdns/dnsdist-console.hh b/pdns/dnsdist-console.hh index 01eb5d48b..7de8b6845 100644 --- a/pdns/dnsdist-console.hh +++ b/pdns/dnsdist-console.hh @@ -42,6 +42,7 @@ extern GlobalStateHolder g_consoleACL; extern const std::vector 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(); diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index e35f0c0f3..efa4f105f 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -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); diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index eb2873c62..c03e5aff9 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -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 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; diff --git a/pdns/dolog.hh b/pdns/dolog.hh index 19b4f16ad..0d5bffe0d 100644 --- a/pdns/dolog.hh +++ b/pdns/dolog.hh @@ -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<(&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(const_cast(decrypted.data())), @@ -67,7 +80,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium msg.length(), nonce.value, reinterpret_cast(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 diff --git a/pdns/sodcrypto.hh b/pdns/sodcrypto.hh index 86d52b2f0..cfcd7eff0 100644 --- a/pdns/sodcrypto.hh +++ b/pdns/sodcrypto.hh @@ -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); diff --git a/pdns/test-dnscrypt_cc.cc b/pdns/test-dnscrypt_cc.cc index c049ff03c..f5fde4a02 100644 --- a/pdns/test-dnscrypt_cc.cc +++ b/pdns/test-dnscrypt_cc.cc @@ -31,7 +31,6 @@ #include bool g_verbose{true}; -bool g_console{true}; bool g_syslog{true}; BOOST_AUTO_TEST_SUITE(dnscrypt_cc) diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc index 7c9114f68..b60f063a3 100644 --- a/pdns/test-dnsdist_cc.cc +++ b/pdns/test-dnsdist_cc.cc @@ -37,7 +37,6 @@ BOOST_AUTO_TEST_SUITE(dnsdist_cc) -bool g_console{true}; bool g_syslog{true}; bool g_verbose{true}; diff --git a/regression-tests.dnsdist/dnsdisttests.py b/regression-tests.dnsdist/dnsdisttests.py index 46722a78e..1d3ad6e7a 100644 --- a/regression-tests.dnsdist/dnsdisttests.py +++ b/regression-tests.dnsdist/dnsdisttests.py @@ -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) diff --git a/regression-tests.dnsdist/test_Console.py b/regression-tests.dnsdist/test_Console.py index 0ab8c8fdb..952cc2afa 100644 --- a/regression-tests.dnsdist/test_Console.py +++ b/regression-tests.dnsdist/test_Console.py @@ -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()') -- 2.40.0