From: Alexander A. Klimov <alexander.klimov@icinga.com>
Date: Fri, 22 Feb 2019 10:37:07 +0000 (+0100)
Subject: Secure ApiUser::GetByAuthHeader() against timing attacks
X-Git-Tag: v2.11.0-rc1~218^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9558ebc0f46febc7692bbb65394708b78b276d46;p=icinga2

Secure ApiUser::GetByAuthHeader() against timing attacks
---

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 <cstdint>
 #include <mmatch.h>
 #include <boost/lexical_cast.hpp>
 #include <boost/thread/tss.hpp>
@@ -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;