From d9f37d6f7ab0ad5c3d03570d030a91297c1c1fb2 Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Wed, 2 Jun 2021 15:35:03 +0200 Subject: [PATCH] Cookies: use constant time comparison --- pdns/Makefile.am | 1 + pdns/dnsdistdist/Makefile.am | 1 + pdns/dnsdistdist/string_compare.hh | 1 + pdns/ednscookies.cc | 5 ++++- pdns/recursordist/Makefile.am | 1 + pdns/recursordist/string_compare.hh | 1 + pdns/string_compare.hh | 5 ++--- 7 files changed, 11 insertions(+), 4 deletions(-) create mode 120000 pdns/dnsdistdist/string_compare.hh create mode 120000 pdns/recursordist/string_compare.hh diff --git a/pdns/Makefile.am b/pdns/Makefile.am index 97cd1a281a..4bedb0a1f4 100644 --- a/pdns/Makefile.am +++ b/pdns/Makefile.am @@ -53,6 +53,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 index a9f4433f20..26371f6bf3 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -103,6 +103,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 new file mode 120000 index 0000000000..7c3ecf51f4 --- /dev/null +++ b/pdns/dnsdistdist/string_compare.hh @@ -0,0 +1 @@ +../string_compare.hh \ No newline at end of file diff --git a/pdns/ednscookies.cc b/pdns/ednscookies.cc index 8c232406d1..29f60fcdb2 100644 --- a/pdns/ednscookies.cc +++ b/pdns/ednscookies.cc @@ -19,9 +19,12 @@ * 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 @@ -106,7 +109,7 @@ bool EDNSCookiesOpt::isValid(const string& secret, const ComboAddress& source) reinterpret_cast(&toHash[0]), toHash.length(), reinterpret_cast(&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 index b80d554f50..6ceec19979 100644 --- 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 new file mode 120000 index 0000000000..7c3ecf51f4 --- /dev/null +++ b/pdns/recursordist/string_compare.hh @@ -0,0 +1 @@ +../string_compare.hh \ No newline at end of file diff --git a/pdns/string_compare.hh b/pdns/string_compare.hh index ad4712f3bc..65d51c6749 100644 --- 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 } - -- 2.47.2