From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 30 Mar 2021 17:25:11 +0000 (+0200)
Subject: Attempt at constant-time credentials verification without sodium
X-Git-Tag: dnsdist-1.7.0-alpha1~12^2~33
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b2504b293adf87992f10b932369f320b22e50f69;p=thirdparty%2Fpdns.git

Attempt at constant-time credentials verification without sodium
---

diff --git a/pdns/Makefile.am b/pdns/Makefile.am
index 6e14a2effa..5982b70572 100644
--- a/pdns/Makefile.am
+++ b/pdns/Makefile.am
@@ -17,6 +17,7 @@ AM_CXXFLAGS = \
 
 AM_LDFLAGS = \
 	$(PROGRAM_LDFLAGS) \
+	$(LIBCRYPTO_LIBS) \
 	$(THREADFLAGS)
 
 AM_LFLAGS = -i
diff --git a/pdns/credentials.cc b/pdns/credentials.cc
index c25ed3c195..e829a41288 100644
--- a/pdns/credentials.cc
+++ b/pdns/credentials.cc
@@ -28,6 +28,7 @@
 #endif
 
 #include "credentials.hh"
+#include "misc.hh"
 
 std::string hashPassword(const std::string& password)
 {
@@ -106,6 +107,8 @@ CredentialsHolder::CredentialsHolder(std::string&& password)
     }
   }
   else {
+    d_fallbackHashPerturb = random();
+    d_fallbackHash = burtle(reinterpret_cast<const unsigned char*>(password.data()), password.size(), d_fallbackHashPerturb);
     d_credentials = std::move(password);
   }
 
@@ -122,6 +125,8 @@ CredentialsHolder::~CredentialsHolder()
 #ifdef HAVE_LIBSODIUM
   sodium_munlock(d_credentials.data(), d_credentials.size());
 #endif
+  d_fallbackHashPerturb = 0;
+  d_fallbackHash = 0;
 }
 
 bool CredentialsHolder::matches(const std::string& password) const
@@ -130,8 +135,12 @@ bool CredentialsHolder::matches(const std::string& password) const
     return verifyPassword(d_credentials, password);
   }
   else {
-#warning FIXME: would be better to do a poor-man hashing using burtle and a random seed first
-    return password == d_credentials;
+    uint32_t fallback = burtle(reinterpret_cast<const unsigned char*>(password.data()), password.size(), d_fallbackHashPerturb);
+    if (fallback != d_fallbackHash) {
+      return false;
+    }
+
+    return constantTimeStringEquals(password, d_credentials);
   }
 }
 
diff --git a/pdns/credentials.hh b/pdns/credentials.hh
index 1b5a67f679..03978d1655 100644
--- a/pdns/credentials.hh
+++ b/pdns/credentials.hh
@@ -48,5 +48,7 @@ public:
 
 private:
   std::string d_credentials;
+  uint32_t d_fallbackHashPerturb;
+  uint32_t d_fallbackHash{0};
   bool d_hashed{false};
 };
diff --git a/pdns/dns_random.hh b/pdns/dns_random.hh
index b968b0766b..52f2812314 100644
--- a/pdns/dns_random.hh
+++ b/pdns/dns_random.hh
@@ -48,4 +48,3 @@ namespace pdns {
     }
   };
 }
-
diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am
index ab7f661490..277b1a9b98 100644
--- a/pdns/dnsdistdist/Makefile.am
+++ b/pdns/dnsdistdist/Makefile.am
@@ -351,6 +351,7 @@ endif
 
 if HAVE_LIBCRYPTO
 dnsdist_LDADD += $(LIBCRYPTO_LDFLAGS) $(LIBCRYPTO_LIBS)
+testrunner_LDADD += $(LIBCRYPTO_LDFLAGS) $(LIBCRYPTO_LIBS)
 dnsdist_SOURCES += ipcipher.cc ipcipher.hh
 endif
 
diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc
index e2db27f07c..e4f77e795f 100644
--- a/pdns/dnssecinfra.cc
+++ b/pdns/dnssecinfra.cc
@@ -632,27 +632,6 @@ static string calculateHMAC(const std::string& key, const std::string& text, TSI
   return string((char*) hash, outlen);
 }
 
-static bool constantTimeStringEquals(const std::string& a, const std::string& b)
-{
-  if (a.size() != b.size()) {
-    return false;
-  }
-  const size_t size = a.size();
-#ifdef HAVE_CRYPTO_MEMCMP
-  return CRYPTO_memcmp(a.c_str(), b.c_str(), size) == 0;
-#else
-  const volatile unsigned char *_a = (const volatile unsigned char *) a.c_str();
-  const volatile unsigned char *_b = (const volatile unsigned char *) b.c_str();
-  unsigned char res = 0;
-
-  for (size_t idx = 0; idx < size; idx++) {
-    res |= _a[idx] ^ _b[idx];
-  }
-
-  return res == 0;
-#endif
-}
-
 static string makeTSIGPayload(const string& previous, const char* packetBegin, size_t packetSize, const DNSName& tsigKeyName, const TSIGRecordContent& trc, bool timersonly)
 {
   string message;
diff --git a/pdns/misc.cc b/pdns/misc.cc
index af667bf668..ca480c8f54 100644
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -1645,3 +1645,29 @@ size_t parseSVCBValueList(const std::string &in, vector<std::string> &val) {
   parseSVCBValueListFromParsedRFC1035CharString(parsed, val);
   return ret;
 };
+
+#ifdef HAVE_CRYPTO_MEMCMP
+#include <openssl/crypto.h>
+#endif
+
+bool constantTimeStringEquals(const std::string& a, const std::string& b)
+{
+  if (a.size() != b.size()) {
+    return false;
+  }
+  const size_t size = a.size();
+#ifdef HAVE_CRYPTO_MEMCMP
+  return CRYPTO_memcmp(a.c_str(), b.c_str(), size) == 0;
+#else
+  const volatile unsigned char *_a = (const volatile unsigned char *) a.c_str();
+  const volatile unsigned char *_b = (const volatile unsigned char *) b.c_str();
+  unsigned char res = 0;
+
+  for (size_t idx = 0; idx < size; idx++) {
+    res |= _a[idx] ^ _b[idx];
+  }
+
+  return res == 0;
+#endif
+}
+
diff --git a/pdns/misc.hh b/pdns/misc.hh
index 3c54329e0e..6307293f55 100644
--- a/pdns/misc.hh
+++ b/pdns/misc.hh
@@ -633,6 +633,8 @@ size_t parseSVCBValueList(const std::string &in, vector<std::string> &val);
 
 std::string makeLuaString(const std::string& in);
 
+bool constantTimeStringEquals(const std::string& a, const std::string& b);
+
 // Used in NID and L64 records
 struct NodeOrLocatorID { uint8_t content[8]; };
 
diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am
index 39a5fb3c24..d4d49da8cc 100644
--- a/pdns/recursordist/Makefile.am
+++ b/pdns/recursordist/Makefile.am
@@ -367,7 +367,8 @@ pdns_recursor_SOURCES += \
 	sodiumsigners.cc
 pdns_recursor_LDADD += $(LIBSODIUM_LIBS)
 
-rec_control_LDADD = $(LIBSODIUM_LIBS)
+rec_control_LDADD = $(LIBSODIUM_LIBS) \
+	$(LIBCRYPTO_LIBS)
 
 testrunner_SOURCES += \
 	sodiumsigners.cc