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; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c78f2732d..714d6a768 100644 --- a/test/CMakeLists.txt +++ b/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 diff --git a/test/base-utility.cpp b/test/base-utility.cpp new file mode 100644 index 000000000..fdc1a17d2 --- /dev/null +++ b/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 +#include + +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(duration1).count() / (double)duration_cast(duration2).count(); + BOOST_CHECK(0.9 <= diff && diff <= 1.1); +} + +BOOST_AUTO_TEST_SUITE_END()