5de570c9ae
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 c762425ea7)
71 lines
2.1 KiB
C++
71 lines
2.1 KiB
C++
/*
|
|
* Copyright (C) 2024-present ScyllaDB
|
|
*/
|
|
|
|
/*
|
|
* SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0
|
|
*/
|
|
|
|
#include "utils/alien_worker.hh"
|
|
#include <seastar/util/log.hh>
|
|
|
|
using namespace seastar;
|
|
|
|
namespace utils {
|
|
|
|
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_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);
|
|
}
|
|
|
|
while (true) {
|
|
std::unique_lock lk(_mut);
|
|
_cv.wait(lk, [this] { return !_pending.empty() || !_running; });
|
|
if (!_running) {
|
|
return;
|
|
}
|
|
auto f = std::move(_pending.front());
|
|
_pending.pop();
|
|
lk.unlock();
|
|
f();
|
|
}
|
|
});
|
|
r = ::pthread_sigmask(SIG_SETMASK, &oldset, nullptr);
|
|
assert(r == 0);
|
|
return thread;
|
|
}
|
|
|
|
alien_worker::alien_worker(seastar::logger& log, int niceness, const seastar::sstring& name_suffix)
|
|
: _thread(spawn(log, niceness, name_suffix))
|
|
{}
|
|
|
|
alien_worker::~alien_worker() {
|
|
{
|
|
std::unique_lock lk(_mut);
|
|
_running = false;
|
|
}
|
|
_cv.notify_one();
|
|
_thread.join();
|
|
}
|
|
|
|
} // namespace utils
|
|
|