Make the hashing of plaintext credentials optional

diff --git a/docs/settings.rst b/docs/settings.rst
index e6b568022f0030c6d7f74ad77914388512e0062e..b8a7c0beda09261193adea47721e0f06f711d981 100644 (file)
--- a/docs/settings.rst
+++ b/docs/settings.rst
@@ -1774,6 +1774,17 @@ IP Address for webserver/API to listen on.
 
 Webserver/API access is only allowed from these subnets.
 
+.. _setting-webserver-hash-plaintext-credentials:
+
+``webserver-hash-plaintext-credentials``
+----------------------------------------
+..versionadded:: 4.6.0
+
+-  Boolean
+-  Default: no
+
+Whether passwords and API keys supplied as plaintext should be hashed during startup, to prevent the plaintext versions from staying in memory. Doing so increases significantly the cost of verifying credentials.
+
 .. _setting-webserver-loglevel:
 
 ``webserver-loglevel``

diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc
index afc9b0b685d5ec83c574d42d523fcdf3f0c34e19..042647f3c6fd97b0cfe587d43df255bdd368b99a 100644 (file)
--- a/pdns/common_startup.cc
+++ b/pdns/common_startup.cc
@@ -169,6 +169,7 @@ void declareArguments()
   ::arg().set("webserver-allow-from","Webserver/API access is only allowed from these subnets")="127.0.0.1,::1";
   ::arg().set("webserver-loglevel", "Amount of logging in the webserver (none, normal, detailed)") = "normal";
   ::arg().set("webserver-max-bodysize","Webserver/API maximum request/response body size in megabytes")="2";
+  ::arg().setSwitch("webserver-hash-plaintext-credentials","Whether to hash passwords and api keys supplied in plaintext, to prevent keeping the plaintext version in memory at runtime")="no";
 
   ::arg().setSwitch("query-logging","Hint backends that queries should be logged")="no";
 

diff --git a/pdns/credentials.cc b/pdns/credentials.cc
index e17111058d9f5a83a1b7a78a4c5a42af6c29523c..139e6337ee720306b338c72d2aa3ed106e34997d 100644 (file)
--- a/pdns/credentials.cc
+++ b/pdns/credentials.cc
@@ -92,21 +92,26 @@ bool isPasswordHashed(const std::string& password)
 
 /* if the password is in cleartext and hashing is available,
    the hashed form will be kept in memory */
-CredentialsHolder::CredentialsHolder(std::string&& password)
+CredentialsHolder::CredentialsHolder(std::string&& password, bool hashPlaintext)
 {
   bool locked = false;
 
   if (isHashingAvailable()) {
     if (!isPasswordHashed(password)) {
-      d_credentials = hashPassword(password);
-      locked = true;
+      if (hashPlaintext) {
+        d_credentials = hashPassword(password);
+        locked = true;
+        d_isHashed = true;
+      }
     }
     else {
       d_wasHashed = true;
+      d_isHashed = true;
       d_credentials = std::move(password);
     }
   }
-  else {
+
+  if (!d_isHashed) {
     d_fallbackHashPerturb = random();
     d_fallbackHash = burtle(reinterpret_cast<const unsigned char*>(password.data()), password.size(), d_fallbackHashPerturb);
     d_credentials = std::move(password);
@@ -130,7 +135,7 @@ CredentialsHolder::~CredentialsHolder()
 
 bool CredentialsHolder::matches(const std::string& password) const
 {
-  if (isHashingAvailable()) {
+  if (d_isHashed) {
     return verifyPassword(d_credentials, password);
   }
   else {

diff --git a/pdns/credentials.hh b/pdns/credentials.hh
index 970aa3fbcd1f6d406ebb90e78f018d86db1702ae..9d42669d6b3288f741f1b39c6e3156dd6f0a1e8a 100644 (file)
--- a/pdns/credentials.hh
+++ b/pdns/credentials.hh
@@ -32,7 +32,7 @@ class CredentialsHolder
 public:
   /* if the password is in cleartext and hashing is available,
      the hashed form will be kept in memory */
-  CredentialsHolder(std::string&& password);
+  CredentialsHolder(std::string&& password, bool hashPlaintext);
   ~CredentialsHolder();
 
   CredentialsHolder(const CredentialsHolder&) = delete;
@@ -44,6 +44,11 @@ public:
   {
     return d_wasHashed;
   }
+  /* whether it is hashed in memory */
+  bool isHashed() const
+  {
+    return d_isHashed;
+  }
 
   static bool isHashingAvailable();
 
@@ -53,4 +58,6 @@ private:
   uint32_t d_fallbackHash{0};
   /* whether it was constructed from a hashed and salted string */
   bool d_wasHashed{false};
+  /* whether it is hashed in memory */
+  bool d_isHashed{false};
 };

diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc
index 79d019f1daec8ee9d0bdaffa648592c403df45b8..b09f4ba076019d0ce93e754ba729f960a835ff11 100644 (file)
--- a/pdns/dnsdist-lua.cc
+++ b/pdns/dnsdist-lua.cc
@@ -944,7 +944,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
         SListen(sock, 5);
         auto launch=[sock, local, password, apiKey, customHeaders, acl]() {
           if (password) {
-            auto holder = make_unique<CredentialsHolder>(std::string(*password));
+            auto holder = make_unique<CredentialsHolder>(std::string(*password), false);
             if (!holder->wasHashed() && holder->isHashingAvailable()) {
               warnlog("Passing a plain-text password to 'webserver()' is deprecated, please use 'setWebserverConfig()' with a hashed password instead.");
             }
@@ -953,7 +953,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
           }
 
           if (apiKey) {
-            auto holder = make_unique<CredentialsHolder>(std::string(*apiKey));
+            auto holder = make_unique<CredentialsHolder>(std::string(*apiKey), false);
             setWebserverAPIKey(std::move(holder));
           }
 
@@ -990,9 +990,14 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
         return ;
       }
 
+      bool hashPlaintextCredentials = false;
+      if (vars->count("hashPlaintextCredentials")) {
+        hashPlaintextCredentials = boost::get<bool>(vars->at("hashPlaintextCredentials"));
+      }
+
       if (vars->count("password")) {
         std::string password = boost::get<std::string>(vars->at("password"));
-        auto holder = make_unique<CredentialsHolder>(std::move(password));
+        auto holder = make_unique<CredentialsHolder>(std::move(password), hashPlaintextCredentials);
         if (!holder->wasHashed() && holder->isHashingAvailable()) {
           warnlog("Passing a plain-text password via the 'password' parameter to 'setWebserverConfig()' is deprecated, please generate a hashed one using 'hashPassword()' instead.");
         }
@@ -1002,7 +1007,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
 
       if (vars->count("apiKey")) {
         std::string apiKey = boost::get<std::string>(vars->at("apiKey"));
-        auto holder = make_unique<CredentialsHolder>(std::move(apiKey));
+        auto holder = make_unique<CredentialsHolder>(std::move(apiKey), hashPlaintextCredentials);
         if (!holder->wasHashed() && holder->isHashingAvailable()) {
           warnlog("Passing a plain-text API key via the 'apiKey' parameter to 'setWebserverConfig()' is deprecated, please generate a hashed one using 'hashPassword()' instead.");
         }

diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst
index 815d93d2ed2ba9415eed75c5108fe63d91bd8244..296c2397774506c940aaa6621e2b3ce75e1adea3 100644 (file)
--- a/pdns/dnsdistdist/docs/reference/config.rst
+++ b/pdns/dnsdistdist/docs/reference/config.rst
@@ -335,6 +335,7 @@ Webserver configuration
 
   .. versionchanged:: 1.7.0
     The optional ``password`` and ``apiKey`` parameters now accept hashed passwords.
+    The optional ``hashPlaintextCredentials`` parameter has been added.
 
   Setup webserver configuration. See :func:`webserver`.
 
@@ -348,6 +349,7 @@ Webserver configuration
   * ``acl=newACL``: string - List of IP addresses, as a string, that are allowed to open a connection to the web server. Defaults to "127.0.0.1, ::1".
   * ``statsRequireAuthentication``: bool - Whether access to the statistics (/metrics and /jsonstat endpoints) require a valid password or API key. Defaults to true.
   * ``maxConcurrentConnections``: int - The maximum number of concurrent web connections, or 0 which means an unlimited number. Defaults to 100.
+  * ``hashPlaintextCredentials``: bool - Whether passwords and API keys provided in plaintext should be hashed during startup, to prevent the plaintext versions from staying in memory. Doing so increases significantly the cost of verifying credentials. Defaults to false.
 
 .. function:: registerWebHandler(path, handler)
 

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index a8228e74ccec72855a06807243b4c08dbda38867..66dd782918ef9f22a24e2a2b3a9b9ecf68d64244 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -5746,6 +5746,7 @@ int main(int argc, char **argv)
     ::arg().set("webserver-password", "Password required for accessing the webserver") = "";
     ::arg().set("webserver-allow-from","Webserver access is only allowed from these subnets")="127.0.0.1,::1";
     ::arg().set("webserver-loglevel", "Amount of logging in the webserver (none, normal, detailed)") = "normal";
+    ::arg().setSwitch("webserver-hash-plaintext-credentials","Whether to hash passwords and api keys supplied in plaintext, to prevent keeping the plaintext version in memory at runtime")="no";
     ::arg().set("carbon-ourname", "If set, overrides our reported hostname for carbon stats")="";
     ::arg().set("carbon-server", "If set, send metrics in carbon (graphite) format to this server IP address")="";
     ::arg().set("carbon-interval", "Number of seconds between carbon (graphite) updates")="30";

diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst
index 8fb965af7376b158d1498be19e31166a1bb946f2..6e16c30d12265712f0f873d4e0136253665d2a02 100644 (file)
--- a/pdns/recursordist/docs/settings.rst
+++ b/pdns/recursordist/docs/settings.rst
@@ -2110,6 +2110,17 @@ These IPs and subnets are allowed to access the webserver. Note that
 specifying an IP address without a netmask uses an implicit netmask
 of /32 or /128.
 
+.. _setting-webserver-hash-plaintext-credentials:
+
+``webserver-hash-plaintext-credentials``
+----------------------------------------
+..versionadded:: 4.6.0
+
+-  Boolean
+-  Default: no
+
+Whether passwords and API keys supplied as plaintext should be hashed during startup, to prevent the plaintext versions from staying in memory. Doing so increases significantly the cost of verifying credentials.
+
 .. _setting-webserver-loglevel:
 
 ``webserver-loglevel``

diff --git a/pdns/test-credentials_cc.cc b/pdns/test-credentials_cc.cc
index a1853a82ab1b368838f4d5dea4277c42978ecef9..145ab311b567a17fe72b249dbdeb5604e4ab226d 100644 (file)
--- a/pdns/test-credentials_cc.cc
+++ b/pdns/test-credentials_cc.cc
@@ -35,20 +35,28 @@ BOOST_AUTO_TEST_CASE(test_CredentialsHolder)
 {
   const std::string plaintext("test");
 
-  auto holder = CredentialsHolder(std::string(plaintext));
+  auto holder = CredentialsHolder(std::string(plaintext), false);
 
   BOOST_CHECK(holder.matches(plaintext));
   BOOST_CHECK(!holder.matches("not test"));
   BOOST_CHECK(!holder.wasHashed());
+  BOOST_CHECK(!holder.isHashed());
 
 #ifdef HAVE_CRYPTO_PWHASH_STR
   BOOST_CHECK(CredentialsHolder::isHashingAvailable());
   const std::string sampleHash("$argon2id$v=19$m=65536,t=2,p=1$ndQKu3+ZsWedqRrlNFUaNw$tnb0MJVe5C2hlqkDt0Ln3R6VKCYkfMYdxDy+puXes3s");
 
-  auto fromHashedHolder = CredentialsHolder(std::string(sampleHash));
+  auto fromHashedHolder = CredentialsHolder(std::string(sampleHash), true);
   BOOST_CHECK(fromHashedHolder.wasHashed());
+  BOOST_CHECK(fromHashedHolder.isHashed());
   BOOST_CHECK(fromHashedHolder.matches(plaintext));
   BOOST_CHECK(!fromHashedHolder.matches("not test"));
+
+  auto fromPlaintextHolder = CredentialsHolder(std::string(plaintext), true);
+  BOOST_CHECK(!fromPlaintextHolder.wasHashed());
+  BOOST_CHECK(fromPlaintextHolder.isHashed());
+  BOOST_CHECK(fromPlaintextHolder.matches(plaintext));
+  BOOST_CHECK(!fromPlaintextHolder.matches("not test"));
 #else
   BOOST_CHECK(!CredentialsHolder::isHashingAvailable());
 #endif

diff --git a/pdns/webserver.hh b/pdns/webserver.hh
index 2fff9c5009c02ea40fdc277295ed3529e0a53a1c..d8707e09149a3c7dcc5b66904b896f015163f04a 100644 (file)
--- a/pdns/webserver.hh
+++ b/pdns/webserver.hh
@@ -163,18 +163,18 @@ public:
   WebServer(string listenaddress, int port);
   virtual ~WebServer() { };
 
-  void setApiKey(const string &apikey) {
+  void setApiKey(const string &apikey, bool hashPlaintext) {
     if (!apikey.empty()) {
-      d_apikey = make_unique<CredentialsHolder>(std::string(apikey));
+      d_apikey = make_unique<CredentialsHolder>(std::string(apikey), hashPlaintext);
     }
     else {
       d_apikey.reset();
     }
   }
 
-  void setPassword(const string &password) {
+  void setPassword(const string &password, bool hashPlaintext) {
     if (!password.empty()) {
-      d_webserverPassword = make_unique<CredentialsHolder>(std::string(password));
+      d_webserverPassword = make_unique<CredentialsHolder>(std::string(password), hashPlaintext);
     }
     else {
       d_webserverPassword.reset();

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index b1a39a86ded64a1a34f15302d61ab4a1bb7db424..2aae0c300cecc784dc1a2fce72d607ff6b5b3b4e 100644 (file)
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -65,10 +65,10 @@ AuthWebServer::AuthWebServer() :
   d_min5(0),
   d_min1(0)
 {
-  if(arg().mustDo("webserver") || arg().mustDo("api")) {
+  if (arg().mustDo("webserver") || arg().mustDo("api")) {
     d_ws = unique_ptr<WebServer>(new WebServer(arg()["webserver-address"], arg().asNum("webserver-port")));
-    d_ws->setApiKey(arg()["api-key"]);
-    d_ws->setPassword(arg()["webserver-password"]);
+    d_ws->setApiKey(arg()["api-key"], arg().mustDo("webserver-hash-plaintext-credentials"));
+    d_ws->setPassword(arg()["webserver-password"], arg().mustDo("webserver-hash-plaintext-credentials"));
     d_ws->setLogLevel(arg()["webserver-loglevel"]);
 
     NetmaskGroup acl;

diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index 58d310e4692738a7ebe25bfdd11a8d9792549981..2b0a87068d55814d14333537d469a3efe37f3a30 100644 (file)
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -1122,8 +1122,8 @@ RecursorWebServer::RecursorWebServer(FDMultiplexer* fdm)
 
   d_ws = make_unique<AsyncWebServer>(fdm, arg()["webserver-address"], arg().asNum("webserver-port"));
 
-  d_ws->setApiKey(arg()["api-key"]);
-  d_ws->setPassword(arg()["webserver-password"]);
+  d_ws->setApiKey(arg()["api-key"], arg().mustDo("webserver-hash-plaintext-credentials"));
+  d_ws->setPassword(arg()["webserver-password"], arg().mustDo("webserver-hash-plaintext-credentials"));
   d_ws->setLogLevel(arg()["webserver-loglevel"]);
 
   NetmaskGroup acl;