diff --git a/lib/base/utility.hpp b/lib/base/utility.hpp index c2d1a0857..9ddb82dc4 100644 --- a/lib/base/utility.hpp +++ b/lib/base/utility.hpp @@ -7,7 +7,9 @@ #include "base/string.hpp" #include "base/array.hpp" #include "base/threadpool.hpp" +#include "base/tlsutility.hpp" #include +#include #include #include #include @@ -146,6 +148,40 @@ public: static void IncrementTime(double); #endif /* I2_DEBUG */ + /** + * TruncateUsingHash truncates a given string to an allowed maximum length while avoiding collisions in the output + * using a hash function (SHA1). + * + * For inputs shorter than the maximum output length, the output will be the same as the input. If the input has at + * least the maximum output length, it is hashed used SHA1 and the output has the format "A...B" where A is a prefix + * of the input and B is the hex-encoded SHA1 hash of the input. The length of A is chosen so that the result has + * the maximum allowed output length. + * + * @tparam maxLength Maximum length of the output string (must be at least 44) + * @param in String to truncate + * @return A truncated string derived from in of at most length maxLength + */ + template + static String TruncateUsingHash(const String &in) { + /* + * Note: be careful when changing this function as it is used to derive file names that should not change + * between versions or would need special handling if they do (/var/lib/icinga2/api/packages/_api). + */ + + const size_t sha1HexLength = SHA_DIGEST_LENGTH*2; + static_assert(maxLength >= 1 + 3 + sha1HexLength, + "maxLength must be at least 44 to hold one character, '...', and a hex-encoded SHA1 hash"); + + /* If the input is shorter than the limit, no truncation is needed */ + if (in.GetLength() < maxLength) { + return in; + } + + const char *trunc = "..."; + + return in.SubStr(0, maxLength - sha1HexLength - strlen(trunc)) + trunc + SHA1(in); + } + private: Utility(); diff --git a/lib/remote/configobjectutility.cpp b/lib/remote/configobjectutility.cpp index 6ba0e0ecc..02e81a19f 100644 --- a/lib/remote/configobjectutility.cpp +++ b/lib/remote/configobjectutility.cpp @@ -35,15 +35,29 @@ String ConfigObjectUtility::GetObjectConfigPath(const Type::Ptr& type, const Str boost::algorithm::to_lower(typeDir); /* This may throw an exception the caller above must handle. */ - String prefix = GetConfigDir(); + String prefix = GetConfigDir() + "/conf.d/" + type->GetPluralName().ToLower() + "/"; - auto old (prefix + "/conf.d/" + typeDir + "/" + EscapeName(fullName) + ".conf"); + String escapedName = EscapeName(fullName); - if (fullName.GetLength() <= 80u + 3u /* "..." */ + 40u /* hex SHA1 */ || Utility::PathExists(old)) { - return std::move(old); + String longPath = prefix + escapedName + ".conf"; + + /* + * The long path may cause trouble due to exceeding the allowed filename length of the filesystem. Therefore, the + * preferred solution would be to use the truncated and hashed version as returned at the end of this function. + * However, for compatibility reasons, we have to keep the old long version in some cases. Notably, this could lead + * to the creation of objects that can't be synced to child nodes if they are running an older version. Thus, for + * now, the fix is only enabled for comments and downtimes, as these are the object types for which the issue is + * most likely triggered but can't be worked around easily (you'd have to rename the host and/or service in order to + * be able to schedule a downtime or add an acknowledgement, which is not feasible) and the impact of not syncing + * these objects through the whole cluster is limited. For other object types, we currently prefer to fail the + * creation early so that configuration inconsistencies throughout the cluster are avoided. + */ + if ((type->GetName() != "Comment" && type->GetName() != "Downtime") || Utility::PathExists(longPath)) { + return std::move(longPath); } - return prefix + "/conf.d/" + typeDir + "/" + fullName.SubStr(0, 80) + "..." + SHA1(fullName) + ".conf"; + /* Maximum length 80 bytes object name + 3 bytes "..." + 40 bytes SHA1 (hex-encoded) */ + return prefix + Utility::TruncateUsingHash<80+3+40>(escapedName) + ".conf"; } void ConfigObjectUtility::RepairPackage(const String& package) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c5e508154..6a5a4bb27 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -118,6 +118,7 @@ add_boost_test(base base_utility/comparepasswords_issafe base_utility/validateutf8 base_utility/EscapeCreateProcessArg + base_utility/TruncateUsingHash base_value/scalar base_value/convert base_value/format diff --git a/test/base-utility.cpp b/test/base-utility.cpp index 1da0a623b..65222e1fd 100644 --- a/test/base-utility.cpp +++ b/test/base-utility.cpp @@ -111,4 +111,28 @@ BOOST_AUTO_TEST_CASE(EscapeCreateProcessArg) #endif /* _WIN32 */ } +BOOST_AUTO_TEST_CASE(TruncateUsingHash) +{ + /* + * Note: be careful when changing the output of TruncateUsingHash as it is used to derive file names that should not + * change between versions or would need special handling if they do (/var/lib/icinga2/api/packages/_api). + */ + + /* minimum allowed value for maxLength template parameter */ + BOOST_CHECK_EQUAL(Utility::TruncateUsingHash<44>(std::string(64, 'a')), + "a...0098ba824b5c16427bd7a1122a5a442a25ec644d"); + + BOOST_CHECK_EQUAL(Utility::TruncateUsingHash<80>(std::string(100, 'a')), + std::string(37, 'a') + "...7f9000257a4918d7072655ea468540cdcbd42e0c"); + + /* short enough values should not be truncated */ + BOOST_CHECK_EQUAL(Utility::TruncateUsingHash<80>(""), ""); + BOOST_CHECK_EQUAL(Utility::TruncateUsingHash<80>(std::string(60, 'a')), std::string(60, 'a')); + BOOST_CHECK_EQUAL(Utility::TruncateUsingHash<80>(std::string(79, 'a')), std::string(79, 'a')); + + /* inputs of maxLength are hashed to avoid collisions */ + BOOST_CHECK_EQUAL(Utility::TruncateUsingHash<80>(std::string(80, 'a')), + std::string(37, 'a') + "...86f33652fcffd7fa1443e246dd34fe5d00e25ffd"); +} + BOOST_AUTO_TEST_SUITE_END()