From 9558ebc0f46febc7692bbb65394708b78b276d46 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 22 Feb 2019 11:37:07 +0100 Subject: [PATCH] Secure ApiUser::GetByAuthHeader() against timing attacks --- lib/base/utility.cpp | 36 ++++++++++++++++++++++++++++++++++++ lib/base/utility.hpp | 2 ++ lib/remote/apiuser.cpp | 3 ++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/base/utility.cpp b/lib/base/utility.cpp index e6582de59..e44d50f23 100644 --- a/lib/base/utility.cpp +++ b/lib/base/utility.cpp @@ -26,6 +26,7 @@ #include "base/utility.hpp" #include "base/json.hpp" #include "base/objectlock.hpp" +#include #include #include #include @@ -1948,3 +1949,38 @@ String Utility::GetFromEnvironment(const String& env) else return String(envValue); } + +/** + * Compare the password entered by a client with the actual password. + * The comparision is safe against timing attacks. + */ +bool Utility::ComparePasswords(const String& enteredPassword, const String& actualPassword) +{ + volatile const char * volatile enteredPasswordCStr = enteredPassword.CStr(); + volatile size_t enteredPasswordLen = enteredPassword.GetLength(); + + volatile const char * volatile actualPasswordCStr = actualPassword.CStr(); + volatile size_t actualPasswordLen = actualPassword.GetLength(); + + volatile uint_fast8_t result = enteredPasswordLen == actualPasswordLen; + + if (result) { + auto cStr (actualPasswordCStr); + auto len (actualPasswordLen); + + actualPasswordCStr = cStr; + actualPasswordLen = len; + } else { + auto cStr (enteredPasswordCStr); + auto len (enteredPasswordLen); + + actualPasswordCStr = cStr; + actualPasswordLen = len; + } + + for (volatile size_t i = 0; i < enteredPasswordLen; ++i) { + result &= uint_fast8_t(enteredPasswordCStr[i] == actualPasswordCStr[i]); + } + + return result; +} diff --git a/lib/base/utility.hpp b/lib/base/utility.hpp index b07ca1474..67d2a2768 100644 --- a/lib/base/utility.hpp +++ b/lib/base/utility.hpp @@ -151,6 +151,8 @@ public: static String GetFromEnvironment(const String& env); + static bool ComparePasswords(const String& enteredPassword, const String& actualPassword); + #ifdef I2_DEBUG static void SetTime(double); static void IncrementTime(double); diff --git a/lib/remote/apiuser.cpp b/lib/remote/apiuser.cpp index 346aadbef..320935a07 100644 --- a/lib/remote/apiuser.cpp +++ b/lib/remote/apiuser.cpp @@ -22,6 +22,7 @@ #include "base/configtype.hpp" #include "base/base64.hpp" #include "base/tlsutility.hpp" +#include "base/utility.hpp" using namespace icinga; @@ -63,7 +64,7 @@ ApiUser::Ptr ApiUser::GetByAuthHeader(const String& auth_header) */ if (!user || password.IsEmpty()) return nullptr; - else if (user && user->GetPassword() != password) + else if (user && !Utility::ComparePasswords(password, user->GetPassword())) return nullptr; return user; -- 2.40.0