Cookies: use constant time comparison

pieterlexis · Jul 7, 2021 · a8af6b0 · a8af6b0
1 parent 6709274
commit a8af6b0
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 4 deletions.
diff --git a/pdns/Makefile.am b/pdns/Makefile.am
@@ -52,6 +52,7 @@ EXTRA_DIST = \
 	lua-record.cc \
 	minicurl.cc \
 	minicurl.hh \
+	string_compare.hh \
 	api-swagger.yaml \
 	api-swagger.json \
 	requirements.txt \

diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am
@@ -102,6 +102,7 @@ EXTRA_DIST=COPYING \
 	   kqueuemplexer.cc \
 	   portsmplexer.cc \
 	   cdb.cc cdb.hh \
+	   string_compare.hh \
 	   ext/lmdb-safe/lmdb-safe.cc ext/lmdb-safe/lmdb-safe.hh \
 	   ext/protozero/include/* \
 	   builder-support/gen-version

diff --git a/pdns/dnsdistdist/string_compare.hh b/pdns/dnsdistdist/string_compare.hh
@@ -0,0 +1 @@
+../string_compare.hh
diff --git a/pdns/ednscookies.cc b/pdns/ednscookies.cc
@@ -19,14 +19,18 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
+#ifdef HAVE_CONFIG_H
 #include "config.h"
+#endif
 #include "ednscookies.hh"
 #include "misc.hh"
+#include "string_compare.hh"
 
 #ifdef HAVE_CRYPTO_SHORTHASH
 #include <sodium.h>
 #endif
 
+
 EDNSCookiesOpt::EDNSCookiesOpt(const std::string& option)
 {
   getEDNSCookiesOptFromString(option.c_str(), option.length());
@@ -106,7 +110,7 @@ bool EDNSCookiesOpt::isValid(const string& secret, const ComboAddress& source)
     reinterpret_cast<const unsigned char*>(&toHash[0]),
     toHash.length(),
     reinterpret_cast<const unsigned char*>(&secret[0]));
-  return server.substr(8) == hashResult;
+  return constantTimeStringEquals(server.substr(8), hashResult);
 #else
   return false;
 #endif

diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am
@@ -63,6 +63,7 @@ EXTRA_DIST = \
 	mtasker_fcontext.cc mtasker_ucontext.cc \
 	NOTICE \
 	opensslsigners.hh opensslsigners.cc \
+	string_compare.hh \
 	portsmplexer.cc \
 	dnstap.proto dnstap.cc dnstap.hh fstrm_logger.cc fstrm_logger.hh \
 	ext/protozero/include/* \

diff --git a/pdns/recursordist/string_compare.hh b/pdns/recursordist/string_compare.hh
@@ -0,0 +1 @@
+../string_compare.hh
diff --git a/pdns/string_compare.hh b/pdns/string_compare.hh
@@ -39,8 +39,8 @@ static bool constantTimeStringEquals(const std::string& a, const std::string& b)
 #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();
+  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++) {
@@ -50,4 +50,3 @@ static bool constantTimeStringEquals(const std::string& a, const std::string& b)
   return res == 0;
 #endif
 }
-