From 0900a8888444435b51ae47f69bd2bbf0ae451784 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Wed, 16 Jul 2025 13:15:54 +0300 Subject: [PATCH] Merge 'auth: move passwords::check call to alien thread' from Andrzej Jackowski MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Analysis of customer stalls revealed that the function `detail::hash_with_salt` (invoked by `passwords::check`) often blocks the reactor. Internally, this function uses the external `crypt_r` function to compute password hashes, which is CPU-intensive. This PR addresses the issue in two ways: 1) `sha-512` is now the only password hashing scheme for new passwords (it was already the common-case). 2) `passwords::check` is moved to a dedicated alien thread. Regarding point 1: before this change, the following hashing schemes were supported by `identify_best_supported_scheme()`: bcrypt_y, bcrypt_a, SHA-512, SHA-256, and MD5. The reason for this was that the `crypt_r` function used for password hashing comes from an external library (currently `libxcrypt`), and the supported hashing algorithms vary depending on the library in use. However: - The bcrypt schemes never worked properly because their prefixes lack the required round count (e.g. `$2y$` instead of `$2y$05$`). Moreover, bcrypt is slower than SHA-512, so it not good idea to fix or use it. - SHA-256 and SHA-512 both belong to the SHA-2 family. Libraries that support one almost always support the other, so it’s very unlikely to find SHA-256 without SHA-512. - MD5 is no longer considered secure for password hashing. Regarding point 2: the `passwords::check` call now runs on a shared alien thread created at database startup. An `std::mutex` synchronizes that thread with the shards. In theory this could introduce a frequent lock contention, but in practice each shard handles only a few hundred new connections per second—even during storms. There is already `_conns_cpu_concurrency_semaphore` in `generic_server` limits the number of concurrent connection handlers. Fixes https://github.com/scylladb/scylladb/issues/24524 Backport not needed, as it is a new feature. Closes scylladb/scylladb#24924 * github.com:scylladb/scylladb: main: utils: add thread names to alien workers auth: move passwords::check call to alien thread test: wait for 3 clients with given username in test_service_level_api auth: refactor password checking in password_authenticator auth: make SHA-512 the only password hashing scheme for new passwords auth: whitespace change in identify_best_supported_scheme() auth: require scheme as parameter for `generate_salt` auth: check password hashing scheme support on authenticator start (cherry picked from commit c762425ea74d06d71ffd87b9c8f57c363c8cab85) --- auth/allow_all_authenticator.cc | 4 +++- auth/allow_all_authenticator.hh | 3 ++- auth/certificate_authenticator.cc | 5 +++-- auth/certificate_authenticator.hh | 3 ++- auth/password_authenticator.cc | 29 +++++++++++++++++++--------- auth/password_authenticator.hh | 7 ++++++- auth/passwords.cc | 14 +++++--------- auth/passwords.hh | 20 +++++++++---------- auth/saslauthd_authenticator.cc | 5 +++-- auth/saslauthd_authenticator.hh | 3 ++- auth/service.cc | 5 +++-- auth/service.hh | 4 +++- auth/transitional.cc | 7 ++++--- main.cc | 11 +++++++---- test/boost/auth_passwords_test.cc | 6 +++--- test/cqlpy/test_service_level_api.py | 13 +++++++++++++ test/lib/cql_test_env.cc | 6 +++++- utils/alien_worker.cc | 18 +++++++++++++---- utils/alien_worker.hh | 4 ++-- 19 files changed, 110 insertions(+), 57 deletions(-) diff --git a/auth/allow_all_authenticator.cc b/auth/allow_all_authenticator.cc index fb78d3de9a..3579acd69c 100644 --- a/auth/allow_all_authenticator.cc +++ b/auth/allow_all_authenticator.cc @@ -9,6 +9,7 @@ #include "auth/allow_all_authenticator.hh" #include "service/migration_manager.hh" +#include "utils/alien_worker.hh" #include "utils/class_registrator.hh" namespace auth { @@ -21,6 +22,7 @@ static const class_registrator< allow_all_authenticator, cql3::query_processor&, ::service::raft_group0_client&, - ::service::migration_manager&> registration("org.apache.cassandra.auth.AllowAllAuthenticator"); + ::service::migration_manager&, + utils::alien_worker&> registration("org.apache.cassandra.auth.AllowAllAuthenticator"); } diff --git a/auth/allow_all_authenticator.hh b/auth/allow_all_authenticator.hh index 8f43f62ad5..18f7f61a64 100644 --- a/auth/allow_all_authenticator.hh +++ b/auth/allow_all_authenticator.hh @@ -13,6 +13,7 @@ #include "auth/authenticated_user.hh" #include "auth/authenticator.hh" #include "auth/common.hh" +#include "utils/alien_worker.hh" namespace cql3 { class query_processor; @@ -28,7 +29,7 @@ extern const std::string_view allow_all_authenticator_name; class allow_all_authenticator final : public authenticator { public: - allow_all_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&) { + allow_all_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, utils::alien_worker&) { } virtual future<> start() override { diff --git a/auth/certificate_authenticator.cc b/auth/certificate_authenticator.cc index 0bcebec9c5..c9139d170e 100644 --- a/auth/certificate_authenticator.cc +++ b/auth/certificate_authenticator.cc @@ -33,13 +33,14 @@ static const class_registrator cert_auth_reg(CERT_AUTH_NAME); + , ::service::migration_manager& + , utils::alien_worker&> cert_auth_reg(CERT_AUTH_NAME); enum class auth::certificate_authenticator::query_source { subject, altname }; -auth::certificate_authenticator::certificate_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&) +auth::certificate_authenticator::certificate_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&, utils::alien_worker&) : _queries([&] { auto& conf = qp.db().get_config(); auto queries = conf.auth_certificate_role_queries(); diff --git a/auth/certificate_authenticator.hh b/auth/certificate_authenticator.hh index 134f725315..229f819d40 100644 --- a/auth/certificate_authenticator.hh +++ b/auth/certificate_authenticator.hh @@ -10,6 +10,7 @@ #pragma once #include "auth/authenticator.hh" +#include "utils/alien_worker.hh" #include // IWYU pragma: keep namespace cql3 { @@ -31,7 +32,7 @@ class certificate_authenticator : public authenticator { enum class query_source; std::vector> _queries; public: - certificate_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&); + certificate_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, utils::alien_worker&); ~certificate_authenticator(); future<> start() override; diff --git a/auth/password_authenticator.cc b/auth/password_authenticator.cc index 0f636a11e5..38de5cb247 100644 --- a/auth/password_authenticator.cc +++ b/auth/password_authenticator.cc @@ -48,14 +48,14 @@ static const class_registrator< password_authenticator, cql3::query_processor&, ::service::raft_group0_client&, - ::service::migration_manager&> password_auth_reg("org.apache.cassandra.auth.PasswordAuthenticator"); + ::service::migration_manager&, + utils::alien_worker&> password_auth_reg("org.apache.cassandra.auth.PasswordAuthenticator"); static thread_local auto rng_for_salt = std::default_random_engine(std::random_device{}()); static std::string_view get_config_value(std::string_view value, std::string_view def) { return value.empty() ? def : value; } - std::string password_authenticator::default_superuser(const db::config& cfg) { return std::string(get_config_value(cfg.auth_superuser_name(), DEFAULT_USER_NAME)); } @@ -63,12 +63,13 @@ std::string password_authenticator::default_superuser(const db::config& cfg) { password_authenticator::~password_authenticator() { } -password_authenticator::password_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm) +password_authenticator::password_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, utils::alien_worker& hashing_worker) : _qp(qp) , _group0_client(g0) , _migration_manager(mm) , _stopped(make_ready_future<>()) , _superuser(default_superuser(qp.db().get_config())) + , _hashing_worker(hashing_worker) {} static bool has_salted_hash(const cql3::untyped_result_set_row& row) { @@ -125,7 +126,7 @@ future<> password_authenticator::legacy_create_default_if_missing() { } std::string salted_pwd(get_config_value(_qp.db().get_config().auth_superuser_salted_password(), "")); if (salted_pwd.empty()) { - salted_pwd = passwords::hash(DEFAULT_USER_PASSWORD, rng_for_salt); + salted_pwd = passwords::hash(DEFAULT_USER_PASSWORD, rng_for_salt, _scheme); } const auto query = update_row_query(); co_await _qp.execute_internal( @@ -172,7 +173,7 @@ future<> password_authenticator::maybe_create_default_password() { // Set default superuser's password. std::string salted_pwd(get_config_value(_qp.db().get_config().auth_superuser_salted_password(), "")); if (salted_pwd.empty()) { - salted_pwd = passwords::hash(DEFAULT_USER_PASSWORD, rng_for_salt); + salted_pwd = passwords::hash(DEFAULT_USER_PASSWORD, rng_for_salt, _scheme); } const auto update_query = update_row_query(); co_await collect_mutations(_qp, batch, update_query, {salted_pwd, _superuser}); @@ -202,6 +203,10 @@ future<> password_authenticator::maybe_create_default_password_with_retries() { future<> password_authenticator::start() { return once_among_shards([this] { + // Verify that at least one hashing scheme is supported. + passwords::detail::verify_scheme(_scheme); + plogger.info("Using password hashing scheme: {}", passwords::detail::prefix_for_scheme(_scheme)); + _stopped = do_after_system_ready(_as, [this] { return async([this] { if (legacy_mode(_qp)) { @@ -289,7 +294,13 @@ future password_authenticator::authenticate( try { const std::optional salted_hash = co_await get_password_hash(username); - if (!salted_hash || !passwords::check(password, *salted_hash)) { + if (!salted_hash) { + throw exceptions::authentication_exception("Username and/or password are incorrect"); + } + const bool password_match = co_await _hashing_worker.submit([password = std::move(password), salted_hash = std::move(salted_hash)]{ + return passwords::check(password, *salted_hash); + }); + if (!password_match) { throw exceptions::authentication_exception("Username and/or password are incorrect"); } co_return username; @@ -313,7 +324,7 @@ future<> password_authenticator::create(std::string_view role_name, const authen auto maybe_hash = options.credentials.transform([&] (const auto& creds) -> sstring { return std::visit(make_visitor( [&] (const password_option& opt) { - return passwords::hash(opt.password, rng_for_salt); + return passwords::hash(opt.password, rng_for_salt, _scheme); }, [] (const hashed_password_option& opt) { return opt.hashed_password; @@ -356,11 +367,11 @@ future<> password_authenticator::alter(std::string_view role_name, const authent query, consistency_for_user(role_name), internal_distributed_query_state(), - {passwords::hash(password, rng_for_salt), sstring(role_name)}, + {passwords::hash(password, rng_for_salt, _scheme), sstring(role_name)}, cql3::query_processor::cache_internal::no).discard_result(); } else { co_await collect_mutations(_qp, mc, query, - {passwords::hash(password, rng_for_salt), sstring(role_name)}); + {passwords::hash(password, rng_for_salt, _scheme), sstring(role_name)}); } } diff --git a/auth/password_authenticator.hh b/auth/password_authenticator.hh index 152a368352..3b4846f6c2 100644 --- a/auth/password_authenticator.hh +++ b/auth/password_authenticator.hh @@ -15,7 +15,9 @@ #include "db/consistency_level_type.hh" #include "auth/authenticator.hh" +#include "auth/passwords.hh" #include "service/raft/raft_group0_client.hh" +#include "utils/alien_worker.hh" namespace db { class config; @@ -43,12 +45,15 @@ class password_authenticator : public authenticator { abort_source _as; std::string _superuser; // default superuser name from the config (may or may not be present in roles table) shared_promise<> _superuser_created_promise; + // We used to also support bcrypt, SHA-256, and MD5 (ref. scylladb#24524). + constexpr static auth::passwords::scheme _scheme = passwords::scheme::sha_512; + utils::alien_worker& _hashing_worker; public: static db::consistency_level consistency_for_user(std::string_view role_name); static std::string default_superuser(const db::config&); - password_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&); + password_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, utils::alien_worker&); ~password_authenticator(); diff --git a/auth/passwords.cc b/auth/passwords.cc index 6bd49a3173..954ca2de8b 100644 --- a/auth/passwords.cc +++ b/auth/passwords.cc @@ -21,18 +21,14 @@ static thread_local crypt_data tlcrypt = {}; namespace detail { -scheme identify_best_supported_scheme() { - const auto all_schemes = { scheme::bcrypt_y, scheme::bcrypt_a, scheme::sha_512, scheme::sha_256, scheme::md5 }; - // "Random", for testing schemes. +void verify_scheme(scheme scheme) { const sstring random_part_of_salt = "aaaabbbbccccdddd"; - for (scheme c : all_schemes) { - const sstring salt = sstring(prefix_for_scheme(c)) + random_part_of_salt; - const char* e = crypt_r("fisk", salt.c_str(), &tlcrypt); + const sstring salt = sstring(prefix_for_scheme(scheme)) + random_part_of_salt; + const char* e = crypt_r("fisk", salt.c_str(), &tlcrypt); - if (e && (e[0] != '*')) { - return c; - } + if (e && (e[0] != '*')) { + return; } throw no_supported_schemes(); diff --git a/auth/passwords.hh b/auth/passwords.hh index 88ce9af75a..617f55ac13 100644 --- a/auth/passwords.hh +++ b/auth/passwords.hh @@ -21,10 +21,11 @@ class no_supported_schemes : public std::runtime_error { public: no_supported_schemes(); }; - /// -/// Apache Cassandra uses a library to provide the bcrypt scheme. Many Linux implementations do not support bcrypt, so -/// we support alternatives. The cost is loss of direct compatibility with Apache Cassandra system tables. +/// Apache Cassandra uses a library to provide the bcrypt scheme. In ScyllaDB, we use SHA-512 +/// instead of bcrypt for performance and for historical reasons (see scylladb#24524). +/// Currently, SHA-512 is always chosen as the hashing scheme for new passwords, but the other +/// algorithms remain supported for CREATE ROLE WITH HASHED PASSWORD and backward compatibility. /// enum class scheme { bcrypt_y, @@ -51,11 +52,11 @@ sstring generate_random_salt_bytes(RandomNumberEngine& g) { } /// -/// Test each allowed hashing scheme and report the best supported one on the current system. +/// Test given hashing scheme on the current system. /// -/// \throws \ref no_supported_schemes when none of the known schemes is supported. +/// \throws \ref no_supported_schemes when scheme is unsupported. /// -scheme identify_best_supported_scheme(); +void verify_scheme(scheme scheme); std::string_view prefix_for_scheme(scheme) noexcept; @@ -67,8 +68,7 @@ std::string_view prefix_for_scheme(scheme) noexcept; /// \throws \ref no_supported_schemes when no known hashing schemes are supported on the system. /// template -sstring generate_salt(RandomNumberEngine& g) { - static const scheme scheme = identify_best_supported_scheme(); +sstring generate_salt(RandomNumberEngine& g, scheme scheme) { static const sstring prefix = sstring(prefix_for_scheme(scheme)); return prefix + generate_random_salt_bytes(g); } @@ -93,8 +93,8 @@ sstring hash_with_salt(const sstring& pass, const sstring& salt); /// \throws \ref std::system_error when the implementation-specific implementation fails to hash the cleartext. /// template -sstring hash(const sstring& pass, RandomNumberEngine& g) { - return detail::hash_with_salt(pass, detail::generate_salt(g)); +sstring hash(const sstring& pass, RandomNumberEngine& g, scheme scheme) { + return detail::hash_with_salt(pass, detail::generate_salt(g, scheme)); } /// diff --git a/auth/saslauthd_authenticator.cc b/auth/saslauthd_authenticator.cc index c0ddb2ebd8..3d7bbf19e4 100644 --- a/auth/saslauthd_authenticator.cc +++ b/auth/saslauthd_authenticator.cc @@ -34,9 +34,10 @@ static const class_registrator< saslauthd_authenticator, cql3::query_processor&, ::service::raft_group0_client&, - ::service::migration_manager&> saslauthd_auth_reg("com.scylladb.auth.SaslauthdAuthenticator"); + ::service::migration_manager&, + utils::alien_worker&> saslauthd_auth_reg("com.scylladb.auth.SaslauthdAuthenticator"); -saslauthd_authenticator::saslauthd_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&) +saslauthd_authenticator::saslauthd_authenticator(cql3::query_processor& qp, ::service::raft_group0_client&, ::service::migration_manager&, utils::alien_worker&) : _socket_path(qp.db().get_config().saslauthd_socket_path()) {} diff --git a/auth/saslauthd_authenticator.hh b/auth/saslauthd_authenticator.hh index 7c2b1118f6..a661dae876 100644 --- a/auth/saslauthd_authenticator.hh +++ b/auth/saslauthd_authenticator.hh @@ -11,6 +11,7 @@ #pragma once #include "auth/authenticator.hh" +#include "utils/alien_worker.hh" namespace cql3 { class query_processor; @@ -28,7 +29,7 @@ namespace auth { class saslauthd_authenticator : public authenticator { sstring _socket_path; ///< Path to the domain socket on which saslauthd is listening. public: - saslauthd_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&); + saslauthd_authenticator(cql3::query_processor&, ::service::raft_group0_client&, ::service::migration_manager&, utils::alien_worker&); future<> start() override; diff --git a/auth/service.cc b/auth/service.cc index ea3bc1abf7..a661415944 100644 --- a/auth/service.cc +++ b/auth/service.cc @@ -187,14 +187,15 @@ service::service( ::service::migration_notifier& mn, ::service::migration_manager& mm, const service_config& sc, - maintenance_socket_enabled used_by_maintenance_socket) + maintenance_socket_enabled used_by_maintenance_socket, + utils::alien_worker& hashing_worker) : service( std::move(c), qp, g0, mn, create_object(sc.authorizer_java_name, qp, g0, mm), - create_object(sc.authenticator_java_name, qp, g0, mm), + create_object(sc.authenticator_java_name, qp, g0, mm, hashing_worker), create_object(sc.role_manager_java_name, qp, g0, mm), used_by_maintenance_socket) { } diff --git a/auth/service.hh b/auth/service.hh index eac2f61d21..9554e0c540 100644 --- a/auth/service.hh +++ b/auth/service.hh @@ -26,6 +26,7 @@ #include "cql3/description.hh" #include "seastarx.hh" #include "service/raft/raft_group0_client.hh" +#include "utils/alien_worker.hh" #include "utils/observable.hh" #include "utils/serialized_action.hh" #include "service/maintenance_mode.hh" @@ -126,7 +127,8 @@ public: ::service::migration_notifier&, ::service::migration_manager&, const service_config&, - maintenance_socket_enabled); + maintenance_socket_enabled, + utils::alien_worker&); future<> start(::service::migration_manager&, db::system_keyspace&); diff --git a/auth/transitional.cc b/auth/transitional.cc index d2a5135d77..cbbefbbdbc 100644 --- a/auth/transitional.cc +++ b/auth/transitional.cc @@ -37,8 +37,8 @@ class transitional_authenticator : public authenticator { public: static const sstring PASSWORD_AUTHENTICATOR_NAME; - transitional_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm) - : transitional_authenticator(std::make_unique(qp, g0, mm)) { + transitional_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, utils::alien_worker& hashing_worker) + : transitional_authenticator(std::make_unique(qp, g0, mm, hashing_worker)) { } transitional_authenticator(std::unique_ptr a) : _authenticator(std::move(a)) { @@ -239,7 +239,8 @@ static const class_registrator< auth::transitional_authenticator, cql3::query_processor&, ::service::raft_group0_client&, - ::service::migration_manager&> transitional_authenticator_reg(auth::PACKAGE_NAME + "TransitionalAuthenticator"); + ::service::migration_manager&, + utils::alien_worker&> transitional_authenticator_reg(auth::PACKAGE_NAME + "TransitionalAuthenticator"); static const class_registrator< auth::authorizer, diff --git a/main.cc b/main.cc index 37282202b9..5406978d3a 100644 --- a/main.cc +++ b/main.cc @@ -749,7 +749,9 @@ sharded token_metadata; // Note: we are creating this thread before app.run so that it doesn't // inherit Seastar's CPU affinity masks. We want this thread to be free // to migrate between CPUs; we think that's what makes the most sense. - auto rpc_dict_training_worker = utils::alien_worker(startlog, 19); + auto rpc_dict_training_worker = utils::alien_worker(startlog, 19, "rpc-dict"); + // niceness=10 is ~10% of normal process time + auto hashing_worker = utils::alien_worker(startlog, 10, "pwd-hash"); return app.run(ac, av, [&] () -> future { @@ -779,7 +781,8 @@ sharded token_metadata; return seastar::async([&app, cfg, ext, &disk_space_monitor_shard0, &cm, &sstm, &db, &qp, &bm, &proxy, &mapreduce_service, &mm, &mm_notifier, &ctx, &opts, &dirs, &prometheus_server, &cf_cache_hitrate_calculator, &load_meter, &feature_service, &gossiper, &snitch, &token_metadata, &erm_factory, &snapshot_ctl, &messaging, &sst_dir_semaphore, &raft_gr, &service_memory_limiter, - &repair, &sst_loader, &ss, &lifecycle_notifier, &stream_manager, &task_manager, &rpc_dict_training_worker] { + &repair, &sst_loader, &ss, &lifecycle_notifier, &stream_manager, &task_manager, &rpc_dict_training_worker, + &hashing_worker] { try { if (opts.contains("relabel-config-file") && !opts["relabel-config-file"].as().empty()) { // calling update_relabel_config_from_file can cause an exception that would stop startup @@ -2038,7 +2041,7 @@ sharded token_metadata; maintenance_auth_config.authenticator_java_name = sstring{auth::allow_all_authenticator_name}; maintenance_auth_config.role_manager_java_name = sstring{auth::maintenance_socket_role_manager_name}; - maintenance_auth_service.start(perm_cache_config, std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), maintenance_auth_config, maintenance_socket_enabled::yes).get(); + maintenance_auth_service.start(perm_cache_config, std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), maintenance_auth_config, maintenance_socket_enabled::yes, std::ref(hashing_worker)).get(); cql_maintenance_server_ctl.emplace(maintenance_auth_service, mm_notifier, gossiper, qp, service_memory_limiter, sl_controller, lifecycle_notifier, *cfg, maintenance_cql_sg_stats_key, maintenance_socket_enabled::yes, dbcfg.statement_scheduling_group); @@ -2259,7 +2262,7 @@ sharded token_metadata; auth_config.authenticator_java_name = qualified_authenticator_name; auth_config.role_manager_java_name = qualified_role_manager_name; - auth_service.start(std::move(perm_cache_config), std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), auth_config, maintenance_socket_enabled::no).get(); + auth_service.start(std::move(perm_cache_config), std::ref(qp), std::ref(group0_client), std::ref(mm_notifier), std::ref(mm), auth_config, maintenance_socket_enabled::no, std::ref(hashing_worker)).get(); std::any stop_auth_service; // Has to be called after node joined the cluster (join_cluster()) diff --git a/test/boost/auth_passwords_test.cc b/test/boost/auth_passwords_test.cc index 3307125184..69e92eab41 100644 --- a/test/boost/auth_passwords_test.cc +++ b/test/boost/auth_passwords_test.cc @@ -29,7 +29,7 @@ BOOST_AUTO_TEST_CASE(passwords_are_salted) { std::unordered_set observed_passwords{}; for (int i = 0; i < 10; ++i) { - const sstring e = auth::passwords::hash(cleartext, rng_for_salt); + const sstring e = auth::passwords::hash(cleartext, rng_for_salt, auth::passwords::scheme::sha_512); BOOST_REQUIRE(!observed_passwords.contains(e)); observed_passwords.insert(e); } @@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE(correct_passwords_authenticate) { }; for (const char* p : passwords) { - BOOST_REQUIRE(auth::passwords::check(p, auth::passwords::hash(p, rng_for_salt))); + BOOST_REQUIRE(auth::passwords::check(p, auth::passwords::hash(p, rng_for_salt, auth::passwords::scheme::sha_512))); } } @@ -55,6 +55,6 @@ BOOST_AUTO_TEST_CASE(correct_passwords_authenticate) { // A hashed password that does not match the password in cleartext does not authenticate. // BOOST_AUTO_TEST_CASE(incorrect_passwords_do_not_authenticate) { - const sstring hashed_password = auth::passwords::hash("actual_password", rng_for_salt); + const sstring hashed_password = auth::passwords::hash("actual_password", rng_for_salt,auth::passwords::scheme::sha_512); BOOST_REQUIRE(!auth::passwords::check("password_guess", hashed_password)); } diff --git a/test/cqlpy/test_service_level_api.py b/test/cqlpy/test_service_level_api.py index c723d13f32..9ed6f8b7a0 100644 --- a/test/cqlpy/test_service_level_api.py +++ b/test/cqlpy/test_service_level_api.py @@ -48,6 +48,17 @@ def count_opened_connections_from_table(cql): return result +def wait_for_clients(cql, username, clients_num, wait_s = 1, timeout_s = 30): + start_time = time.time() + while time.time() - start_time < timeout_s: + result = cql.execute(f"SELECT COUNT(*) FROM system.clients WHERE username='{username}' ALLOW FILTERING") + if result.one()[0] == clients_num: + return + else: + time.sleep(wait_s) + + raise RuntimeError(f"Awaiting for {clients_num} clients timed out.") + def wait_until_all_connections_authenticated(cql, wait_s = 1, timeout_s = 30): start_time = time.time() while time.time() - start_time < timeout_s: @@ -85,6 +96,7 @@ def test_count_opened_cql_connections(cql): try: with new_session(cql, user): + wait_for_clients(cql, user, 3) # 3 from smp=2 + control connection wait_until_all_connections_authenticated(cql) verify_scheduling_group_assignment(cql, user, sl, get_shard_count(cql)) @@ -120,6 +132,7 @@ def test_switch_tenants(cql): try: with new_session(cql, user) as user_session: + wait_for_clients(cql, user, 3) # 3 from smp=2 + control connection wait_until_all_connections_authenticated(cql) verify_scheduling_group_assignment(cql, user, sl1, shard_count) diff --git a/test/lib/cql_test_env.cc b/test/lib/cql_test_env.cc index 26cfb5d5d0..8fdb40a21d 100644 --- a/test/lib/cql_test_env.cc +++ b/test/lib/cql_test_env.cc @@ -1079,7 +1079,11 @@ private: auth_config.authenticator_java_name = qualified_authenticator_name; auth_config.role_manager_java_name = qualified_role_manager_name; - _auth_service.start(perm_cache_config, std::ref(_qp), std::ref(group0_client), std::ref(_mnotifier), std::ref(_mm), auth_config, maintenance_socket_enabled::no).get(); + + + const uint64_t niceness = 19; + auto hashing_worker = utils::alien_worker(startlog, niceness, "pwd-hash"); + _auth_service.start(perm_cache_config, std::ref(_qp), std::ref(group0_client), std::ref(_mnotifier), std::ref(_mm), auth_config, maintenance_socket_enabled::no, std::ref(hashing_worker)).get(); _auth_service.invoke_on_all([this] (auth::service& auth) { return auth.start(_mm.local(), _sys_ks.local()); }).get(); diff --git a/utils/alien_worker.cc b/utils/alien_worker.cc index bdfccda29b..fc229f6200 100644 --- a/utils/alien_worker.cc +++ b/utils/alien_worker.cc @@ -13,14 +13,24 @@ using namespace seastar; namespace utils { -std::thread alien_worker::spawn(seastar::logger& log, int niceness) { +std::thread alien_worker::spawn(seastar::logger& log, int niceness, const seastar::sstring& name_suffix) { sigset_t newset; sigset_t oldset; sigfillset(&newset); auto r = ::pthread_sigmask(SIG_SETMASK, &newset, &oldset); assert(r == 0); - auto thread = std::thread([this, &log, niceness] () noexcept { + auto thread_name = fmt::format("alien-{}", name_suffix); + if (thread_name.size() > 15) { + log.warn("Thread name '{}' is longer than 15 characters, truncating to fit", thread_name); + thread_name.resize(15); // pthread_setname_np requires name to be <= 15 characters + } + auto thread = std::thread([this, &log, niceness, thread_name] () noexcept { errno = 0; + int setname_value = pthread_setname_np(pthread_self(), thread_name.c_str()); + if (setname_value != 0) { + log.error("Unable to set worker thread name '{}', setname_value={}", thread_name, setname_value); + std::abort(); + } int nice_value = nice(niceness); if (nice_value == -1 && errno != 0) { log.warn("Unable to renice worker thread (system error number {}); the thread will compete with reactor, which can cause latency spikes. Try adding CAP_SYS_NICE", errno); @@ -43,8 +53,8 @@ std::thread alien_worker::spawn(seastar::logger& log, int niceness) { return thread; } -alien_worker::alien_worker(seastar::logger& log, int niceness) - : _thread(spawn(log, niceness)) +alien_worker::alien_worker(seastar::logger& log, int niceness, const seastar::sstring& name_suffix) + : _thread(spawn(log, niceness, name_suffix)) {} alien_worker::~alien_worker() { diff --git a/utils/alien_worker.hh b/utils/alien_worker.hh index 3591a28bd7..f7c6278b98 100644 --- a/utils/alien_worker.hh +++ b/utils/alien_worker.hh @@ -29,9 +29,9 @@ class alien_worker { // Note: initialization of _thread uses other fields, so it must be performed last. std::thread _thread; - std::thread spawn(seastar::logger&, int niceness); + std::thread spawn(seastar::logger&, int niceness, const seastar::sstring& name_suffix); public: - alien_worker(seastar::logger&, int niceness); + alien_worker(seastar::logger&, int niceness, const seastar::sstring& name_suffix); ~alien_worker(); // The worker captures `this`, so `this` must have a stable address. alien_worker(const alien_worker&) = delete;