Merge pull request #6968 from Icinga/bugfix/timing-attack

Secure ApiUser::GetByAuthHeader() against timing attacks

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;
+}

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);

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;

test/CMakeLists.txt

@@ -36,6 +36,7 @@ set(base_test_SOURCES
   base-string.cpp
   base-timer.cpp
   base-type.cpp
+  base-utility.cpp
   base-value.cpp
   config-ops.cpp
   icinga-checkresult.cpp
@@ -120,6 +121,8 @@ add_boost_test(base
     base_type/assign
     base_type/byname
     base_type/instantiate
+    base_utility/comparepasswords_works
+    base_utility/comparepasswords_issafe
     base_value/scalar
     base_value/convert
     base_value/format

test/base-utility.cpp

@@ -0,0 +1,70 @@
+/******************************************************************************
+ * Icinga 2                                                                   *
+ * Copyright (C) 2012-2018 Icinga Development Team (https://icinga.com/)      *
+ *                                                                            *
+ * This program is free software; you can redistribute it and/or              *
+ * modify it under the terms of the GNU General Public License                *
+ * as published by the Free Software Foundation; either version 2             *
+ * of the License, or (at your option) any later version.                     *
+ *                                                                            *
+ * This program is distributed in the hope that it will be useful,            *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of             *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the              *
+ * GNU General Public License for more details.                               *
+ *                                                                            *
+ * You should have received a copy of the GNU General Public License          *
+ * along with this program; if not, write to the Free Software Foundation     *
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.             *
+ ******************************************************************************/
+
+#include "base/utility.hpp"
+#include <chrono>
+#include <BoostTestTargetConfig.h>
+
+using namespace icinga;
+
+BOOST_AUTO_TEST_SUITE(base_utility)
+
+BOOST_AUTO_TEST_CASE(comparepasswords_works)
+{
+	BOOST_CHECK(Utility::ComparePasswords("", ""));
+
+	BOOST_CHECK(!Utility::ComparePasswords("x", ""));
+	BOOST_CHECK(!Utility::ComparePasswords("", "x"));
+
+	BOOST_CHECK(Utility::ComparePasswords("x", "x"));
+	BOOST_CHECK(!Utility::ComparePasswords("x", "y"));
+
+	BOOST_CHECK(Utility::ComparePasswords("abcd", "abcd"));
+	BOOST_CHECK(!Utility::ComparePasswords("abc", "abcd"));
+	BOOST_CHECK(!Utility::ComparePasswords("abcde", "abcd"));
+}
+
+BOOST_AUTO_TEST_CASE(comparepasswords_issafe)
+{
+	using std::chrono::duration_cast;
+	using std::chrono::microseconds;
+	using std::chrono::steady_clock;
+
+	String a, b;
+
+	a.Append(200000001, 'a');
+	b.Append(200000002, 'b');
+
+	auto start1 (steady_clock::now());
+
+	Utility::ComparePasswords(a, a);
+
+	auto duration1 (steady_clock::now() - start1);
+
+	auto start2 (steady_clock::now());
+
+	Utility::ComparePasswords(a, b);
+
+	auto duration2 (steady_clock::now() - start2);
+
+	double diff = (double)duration_cast<microseconds>(duration1).count() / (double)duration_cast<microseconds>(duration2).count();
+	BOOST_CHECK(0.9 <= diff && diff <= 1.1);
+}
+
+BOOST_AUTO_TEST_SUITE_END()