diff --git a/conf/lms.conf b/conf/lms.conf index 3da3e6cc..56822c3f 100644 --- a/conf/lms.conf +++ b/conf/lms.conf @@ -60,6 +60,9 @@ acousticbrainz-api-base-url = "https://acousticbrainz.org"; # Authentication # Available backends: "internal", "PAM", "http-headers" authentication-backend = "internal"; +# The number of bcrypt rounds to be used when backend is set to "internal". The higher the more secure +internal-password-bcrypt-round = 12; +# The header to be used to read the authentication user when backend is set to "http-headers" http-headers-login-field = "X-Forwarded-User"; # Max entries in the login throttler (1 entry per IP address. For IPv6, the whole /64 block is used) diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp index 911dedd6..44378136 100644 --- a/src/libs/database/impl/Migration.cpp +++ b/src/libs/database/impl/Migration.cpp @@ -35,7 +35,7 @@ namespace lms::db { namespace { - static constexpr Version LMS_DATABASE_VERSION{ 75 }; + static constexpr Version LMS_DATABASE_VERSION{ 76 }; } VersionInfo::VersionInfo() @@ -962,6 +962,12 @@ SELECT utils::executeCommand(*session.getDboSession(), "DROP INDEX IF EXISTS auth_token_value_idx"); } + void migrateFromV75(Session& session) + { + // Added a new option to set the bcrypt count to be use to hash user's passwords + utils::executeCommand(*session.getDboSession(), "ALTER TABLE user ADD bcrypt_round_count INTEGER NOT NULL DEFAULT(7)"); + } + bool doDbMigration(Session& session) { constexpr std::string_view outdatedMsg{ "Outdated database, please rebuild it (delete the .db file and restart)" }; @@ -1013,6 +1019,7 @@ SELECT { 72, migrateFromV72 }, { 73, migrateFromV73 }, { 74, migrateFromV74 }, + { 75, migrateFromV75 }, }; bool migrationPerformed{}; @@ -1044,7 +1051,9 @@ SELECT LMS_LOG(DB, INFO, "Migrating database from version " << version << " to " << version + 1 << "..."); auto itMigrationFunc{ migrationFunctions.find(version) }; - assert(itMigrationFunc != std::cend(migrationFunctions)); + if (itMigrationFunc == std::cend(migrationFunctions)) + throw core::LmsException{ "No code found to upgrade database!" }; + itMigrationFunc->second(session); VersionInfo::get(session).modify()->setVersion(++version); diff --git a/src/libs/database/include/database/User.hpp b/src/libs/database/include/database/User.hpp index 24d4a1b7..0ec076b2 100644 --- a/src/libs/database/include/database/User.hpp +++ b/src/libs/database/include/database/User.hpp @@ -42,6 +42,7 @@ namespace lms::db public: struct PasswordHash { + std::size_t bcryptRoundCount; std::string salt; std::string hash; }; @@ -91,7 +92,7 @@ namespace lms::db // accessors const std::string& getLoginName() const { return _loginName; } - PasswordHash getPasswordHash() const { return PasswordHash{ _passwordSalt, _passwordHash }; } + PasswordHash getPasswordHash() const { return PasswordHash{ .bcryptRoundCount = static_cast(_bcryptRoundCount), .salt = _passwordSalt, .hash = _passwordHash }; } const Wt::WDateTime& getLastLogin() const { return _lastLogin; } std::size_t getAuthTokensCount() const { return _authTokens.size(); } @@ -99,6 +100,7 @@ namespace lms::db void setLastLogin(const Wt::WDateTime& dateTime) { _lastLogin = dateTime; } void setPasswordHash(const PasswordHash& passwordHash) { + _bcryptRoundCount = passwordHash.bcryptRoundCount; _passwordSalt = passwordHash.salt; _passwordHash = passwordHash.hash; } @@ -132,6 +134,7 @@ namespace lms::db { Wt::Dbo::field(a, _type, "type"); Wt::Dbo::field(a, _loginName, "login_name"); + Wt::Dbo::field(a, _bcryptRoundCount, "bcrypt_round_count"); Wt::Dbo::field(a, _passwordSalt, "password_salt"); Wt::Dbo::field(a, _passwordHash, "password_hash"); Wt::Dbo::field(a, _lastLogin, "last_login"); @@ -155,6 +158,7 @@ namespace lms::db static pointer create(Session& session, std::string_view loginName); std::string _loginName; + int _bcryptRoundCount{}; std::string _passwordSalt; std::string _passwordHash; Wt::WDateTime _lastLogin; diff --git a/src/libs/services/auth/impl/PasswordServiceBase.cpp b/src/libs/services/auth/impl/PasswordServiceBase.cpp index d8afa0a4..4a7a8b91 100644 --- a/src/libs/services/auth/impl/PasswordServiceBase.cpp +++ b/src/libs/services/auth/impl/PasswordServiceBase.cpp @@ -54,8 +54,7 @@ namespace lms::auth { } - PasswordServiceBase::CheckResult - PasswordServiceBase::checkUserPassword(const boost::asio::ip::address& clientAddress, std::string_view loginName, std::string_view password) + PasswordServiceBase::CheckResult PasswordServiceBase::checkUserPassword(const boost::asio::ip::address& clientAddress, std::string_view loginName, std::string_view password) { LMS_LOG(AUTH, DEBUG, "Checking password for user '" << loginName << "'"); diff --git a/src/libs/services/auth/impl/internal/InternalPasswordService.cpp b/src/libs/services/auth/impl/internal/InternalPasswordService.cpp index 93e9ed17..013923e0 100644 --- a/src/libs/services/auth/impl/internal/InternalPasswordService.cpp +++ b/src/libs/services/auth/impl/internal/InternalPasswordService.cpp @@ -21,7 +21,7 @@ #include -#include "core/Exception.hpp" +#include "core/IConfig.hpp" #include "core/ILogger.hpp" #include "database/Session.hpp" #include "database/User.hpp" @@ -31,7 +31,11 @@ namespace lms::auth { InternalPasswordService::InternalPasswordService(db::Db& db, std::size_t maxThrottlerEntries) : PasswordServiceBase{ db, maxThrottlerEntries } + , _bcryptRoundCount{ static_cast(core::Service::get()->getULong("internal-password-bcrypt-round", 12)) } { + if (_bcryptRoundCount < 7 || _bcryptRoundCount > 31) + throw Exception{ "\"internal-password-bcrypt-round\" must be in range 7-31" }; + _validator.setMinimumLength(Wt::Auth::PasswordStrengthType::OneCharClass, 4); _validator.setMinimumLength(Wt::Auth::PasswordStrengthType::TwoCharClass, 4); _validator.setMinimumLength(Wt::Auth::PasswordStrengthType::PassPhrase, 4); @@ -54,7 +58,7 @@ namespace lms::auth if (!user) { LMS_LOG(AUTH, DEBUG, "hashing random stuff"); - // hash random stuff here to waste some time + // hash random stuff here to waste some time, don't give clue the user does not exist hashRandomPassword(); return false; } @@ -63,13 +67,29 @@ namespace lms::auth passwordHash = user->getPasswordHash(); if (passwordHash.salt.empty() || passwordHash.hash.empty()) { - // hash random stuff here to waste some time + // hash random stuff here to waste some time, don't give clue the user has no password set hashRandomPassword(); return false; } } - return _hashFunc.verify(std::string{ password }, std::string{ passwordHash.salt }, std::string{ passwordHash.hash }); + // Note: the round count set in the actual hash is used to verify, not the one used to construct _hashFunc + bool passwordMatched{ _hashFunc.verify(std::string{ password }, std::string{ passwordHash.salt }, std::string{ passwordHash.hash }) }; + if (passwordMatched && passwordHash.bcryptRoundCount != _bcryptRoundCount) + { + LMS_LOG(AUTH, INFO, "Updating password hash for user '" << loginName << "' to match new bcrypt round count: previously " << passwordHash.bcryptRoundCount << " rounds, now " << _bcryptRoundCount << " rounds"); + const db::User::PasswordHash updatedPasswordHash{ hashPassword(password) }; + + { + db::Session& session{ getDbSession() }; + auto transaction{ session.createWriteTransaction() }; + + if (db::User::pointer user{ db::User::find(session, loginName) }) + user.modify()->setPasswordHash(updatedPasswordHash); + } + } + + return passwordMatched; } bool InternalPasswordService::canSetPasswords() const @@ -95,37 +115,37 @@ namespace lms::auth { const db::User::PasswordHash passwordHash{ hashPassword(newPassword) }; - db::Session& session{ getDbSession() }; - auto transaction{ session.createWriteTransaction() }; - - db::User::pointer user{ db::User::find(session, userId) }; - if (!user) - throw Exception{ "User not found!" }; - - switch (checkPasswordAcceptability(newPassword, PasswordValidationContext{ user->getLoginName(), user->getType() })) { - case PasswordAcceptabilityResult::OK: - break; - case PasswordAcceptabilityResult::TooWeak: - throw PasswordTooWeakException{}; - case PasswordAcceptabilityResult::MustMatchLoginName: - throw PasswordMustMatchLoginNameException{}; - } + db::Session& session{ getDbSession() }; + auto transaction{ session.createWriteTransaction() }; - user.modify()->setPasswordHash(passwordHash); + db::User::pointer user{ db::User::find(session, userId) }; + if (!user) + throw Exception{ "User not found!" }; + + switch (checkPasswordAcceptability(newPassword, PasswordValidationContext{ user->getLoginName(), user->getType() })) + { + case PasswordAcceptabilityResult::OK: + break; + case PasswordAcceptabilityResult::TooWeak: + throw PasswordTooWeakException{}; + case PasswordAcceptabilityResult::MustMatchLoginName: + throw PasswordMustMatchLoginNameException{}; + } + + user.modify()->setPasswordHash(passwordHash); + } } db::User::PasswordHash InternalPasswordService::hashPassword(std::string_view password) const { const std::string salt{ Wt::WRandom::generateId(32) }; - return db::User::PasswordHash{ .salt = salt, .hash = _hashFunc.compute(std::string{ password }, salt) }; + return db::User::PasswordHash{ .bcryptRoundCount = _bcryptRoundCount, .salt = salt, .hash = _hashFunc.compute(std::string{ password }, salt) }; } - void - InternalPasswordService::hashRandomPassword() const + void InternalPasswordService::hashRandomPassword() const { hashPassword(Wt::WRandom::generateId(32)); } - } // namespace lms::auth diff --git a/src/libs/services/auth/impl/internal/InternalPasswordService.hpp b/src/libs/services/auth/impl/internal/InternalPasswordService.hpp index 17ccbf6f..3f37f2a2 100644 --- a/src/libs/services/auth/impl/internal/InternalPasswordService.hpp +++ b/src/libs/services/auth/impl/internal/InternalPasswordService.hpp @@ -44,8 +44,8 @@ namespace lms::auth db::User::PasswordHash hashPassword(std::string_view password) const; void hashRandomPassword() const; - const Wt::Auth::BCryptHashFunction _hashFunc{ 7 }; // TODO parametrize this + const unsigned _bcryptRoundCount; + const Wt::Auth::BCryptHashFunction _hashFunc{ static_cast(_bcryptRoundCount) }; Wt::Auth::PasswordStrengthValidator _validator; }; - } // namespace lms::auth