This patch series contains the following changes: - Incorporation of `crypt_sha512.c` from musl to out codebase - Conversion of `crypt_sha512.c` to C++ and coroutinization - Coroutinization of `auth::passwords::check` - Enabling use of `__crypt_sha512` orignated from `crypt_sha512.c` for computing SHA 512 passwords of length <=255 - Addition of yielding in the aforementioned hashing implementation. The alien thread was a solution for reactor stalls caused by indivisible password‑hashing tasks (https://github.com/scylladb/scylladb/issues/24524). However, because there is only one alien thread, overall hashing throughput was reduced (see, e.g., https://github.com/scylladb/scylla-enterprise/issues/5711). To address this, the alien‑thread solution is reverted, and a hashing implementation with yielding is introduced in this patch series. Before this patch series, ScyllaDB used SHA-512 hashing provided by the `crypt_r` function, which in our case meant using the implementation from the `libxcrypt` library. Adding yielding to this `libxcrypt` implementation is problematic, both due to licensing (LGPL) and because the implementation is split into many functions across multiple files. In contrast, the SHA-512 implementation from `musl libc` has a more permissive license and is concise, which makes it easier to incorporate into the ScyllaDB codebase. The performance of this solution was compared with the previous implementation that used one alien thread and the implementation after the alien thread was reverted. The results (median) of `perf-cql-raw` with `--connection-per-request 1 --smp 10` parameters are as follows: - Alien thread: 41.5 new connections/s per shard - Reverted alien thread: 244.1 new connections/s per shard - This commit (yielding in hashing): 198.4 new connections/s per shard The roughly 20% performance deterioration compared to the old implementation without the alien thread comes from the fact that the new hashing algorithm implemented in `utils/crypt_sha512.cc` performs an expensive self-verification and stack cleanup. On the other hand, with smp=10 the current implementation achieves roughly 5x higher throughput than the alien thread. In addition, due to yielding added in this commit, the algorithm is expected to provide similar protection from stalls as the alien thread did. In a test that in parallel started a cassandra-stress workload and created thousands of new connections using python-driver, the values of `scylla_reactor_stalls_count` metric were as follows: - Alien thread: 109 stalls/shard total - Reverted alien thread: 13186 stalls/shard total - This commit (yielding in hashing): 149 stalls/shard total Similarly, the `scylla_scheduler_time_spent_on_task_quota_violations_ms` values were: - Alien thread: 1087 ms/shard total - Reverted alien thread: 72839 ms/shard total - This commit (yielding in hashing): 1623 ms/shard total To summarize, yielding during hashing computations achieves similar throughput to the old solution without the alien thread but also prevents stalls similarly to the alien thread. Fixes: scylladb/scylladb#26859 Refs: scylladb/scylla-enterprise#5711 No automatic backport. After this PR is completed, the alien thread should be rather reverted from older branches (2025.2-2025.4 because on 2025.1 it's already removed). Backporting of the other commits needs further discussion. Closes scylladb/scylladb#26860 * github.com:scylladb/scylladb: test/boost: add too_long_password to auth_passwords_test test/boost: add same_hashes_as_crypt_r to auth_passwords_test auth: utils: add yielding to crypt_sha512 auth: change return type of passwords::check to future auth: remove code duplication in verify_scheme test/boost: coroutinize auth_passwords_test utils: coroutinize crypt_sha512 utils: make crypt_sha512.cc to compile utils: license: import crypt_sha512.c from musl to the project Revert "auth: move passwords::check call to alien thread"
252 lines
8.6 KiB
C++
252 lines
8.6 KiB
C++
/*
|
|
* Copyright (C) 2017-present ScyllaDB
|
|
*
|
|
* Modified by ScyllaDB
|
|
*/
|
|
|
|
/*
|
|
* SPDX-License-Identifier: (LicenseRef-ScyllaDB-Source-Available-1.0 and Apache-2.0)
|
|
*/
|
|
|
|
#include "auth/authenticated_user.hh"
|
|
#include "auth/authenticator.hh"
|
|
#include "auth/authorizer.hh"
|
|
#include "auth/default_authorizer.hh"
|
|
#include "auth/password_authenticator.hh"
|
|
#include "auth/cache.hh"
|
|
#include "auth/permission.hh"
|
|
#include "service/raft/raft_group0_client.hh"
|
|
#include "utils/class_registrator.hh"
|
|
|
|
namespace auth {
|
|
|
|
static const sstring PACKAGE_NAME("com.scylladb.auth.");
|
|
|
|
static const sstring& transitional_authenticator_name() {
|
|
static const sstring name = PACKAGE_NAME + "TransitionalAuthenticator";
|
|
return name;
|
|
}
|
|
|
|
static const sstring& transitional_authorizer_name() {
|
|
static const sstring name = PACKAGE_NAME + "TransitionalAuthorizer";
|
|
return name;
|
|
}
|
|
|
|
class transitional_authenticator : public authenticator {
|
|
std::unique_ptr<authenticator> _authenticator;
|
|
|
|
public:
|
|
static const sstring PASSWORD_AUTHENTICATOR_NAME;
|
|
|
|
transitional_authenticator(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm, cache& cache)
|
|
: transitional_authenticator(std::make_unique<password_authenticator>(qp, g0, mm, cache)) {
|
|
}
|
|
transitional_authenticator(std::unique_ptr<authenticator> a)
|
|
: _authenticator(std::move(a)) {
|
|
}
|
|
|
|
virtual future<> start() override {
|
|
return _authenticator->start();
|
|
}
|
|
|
|
virtual future<> stop() override {
|
|
return _authenticator->stop();
|
|
}
|
|
|
|
virtual std::string_view qualified_java_name() const override {
|
|
return transitional_authenticator_name();
|
|
}
|
|
|
|
virtual bool require_authentication() const override {
|
|
return true;
|
|
}
|
|
|
|
virtual authentication_option_set supported_options() const override {
|
|
return _authenticator->supported_options();
|
|
}
|
|
|
|
virtual authentication_option_set alterable_options() const override {
|
|
return _authenticator->alterable_options();
|
|
}
|
|
|
|
virtual future<authenticated_user> authenticate(const credentials_map& credentials) const override {
|
|
auto i = credentials.find(authenticator::USERNAME_KEY);
|
|
if ((i == credentials.end() || i->second.empty())
|
|
&& (!credentials.contains(PASSWORD_KEY) || credentials.at(PASSWORD_KEY).empty())) {
|
|
// return anon user
|
|
return make_ready_future<authenticated_user>(anonymous_user());
|
|
}
|
|
return make_ready_future().then([this, &credentials] {
|
|
return _authenticator->authenticate(credentials);
|
|
}).handle_exception([](auto ep) {
|
|
try {
|
|
std::rethrow_exception(ep);
|
|
} catch (const exceptions::authentication_exception&) {
|
|
// return anon user
|
|
return make_ready_future<authenticated_user>(anonymous_user());
|
|
}
|
|
});
|
|
}
|
|
|
|
virtual future<> create(std::string_view role_name, const authentication_options& options, ::service::group0_batch& mc) override {
|
|
return _authenticator->create(role_name, options, mc);
|
|
}
|
|
|
|
virtual future<> alter(std::string_view role_name, const authentication_options& options, ::service::group0_batch& mc) override {
|
|
return _authenticator->alter(role_name, options, mc);
|
|
}
|
|
|
|
virtual future<> drop(std::string_view role_name, ::service::group0_batch& mc) override {
|
|
return _authenticator->drop(role_name, mc);
|
|
}
|
|
|
|
virtual future<custom_options> query_custom_options(std::string_view role_name) const override {
|
|
return _authenticator->query_custom_options(role_name);
|
|
}
|
|
|
|
virtual bool uses_password_hashes() const override {
|
|
return _authenticator->uses_password_hashes();
|
|
}
|
|
|
|
virtual future<std::optional<sstring>> get_password_hash(std::string_view role_name) const override {
|
|
return _authenticator->get_password_hash(role_name);
|
|
}
|
|
|
|
virtual const resource_set& protected_resources() const override {
|
|
return _authenticator->protected_resources();
|
|
}
|
|
|
|
virtual ::shared_ptr<sasl_challenge> new_sasl_challenge() const override {
|
|
class sasl_wrapper : public sasl_challenge {
|
|
public:
|
|
sasl_wrapper(::shared_ptr<sasl_challenge> sasl)
|
|
: _sasl(std::move(sasl)) {
|
|
}
|
|
|
|
virtual bytes evaluate_response(bytes_view client_response) override {
|
|
try {
|
|
return _sasl->evaluate_response(client_response);
|
|
} catch (const exceptions::authentication_exception&) {
|
|
_complete = true;
|
|
return {};
|
|
}
|
|
}
|
|
|
|
virtual bool is_complete() const override {
|
|
return _complete || _sasl->is_complete();
|
|
}
|
|
|
|
virtual future<authenticated_user> get_authenticated_user() const override {
|
|
return futurize_invoke([this] {
|
|
return _sasl->get_authenticated_user().handle_exception([](auto ep) {
|
|
try {
|
|
std::rethrow_exception(ep);
|
|
} catch (const exceptions::authentication_exception&) {
|
|
// return anon user
|
|
return make_ready_future<authenticated_user>(anonymous_user());
|
|
}
|
|
});
|
|
});
|
|
}
|
|
|
|
const sstring& get_username() const override {
|
|
return _sasl->get_username();
|
|
}
|
|
|
|
private:
|
|
::shared_ptr<sasl_challenge> _sasl;
|
|
|
|
bool _complete = false;
|
|
};
|
|
return ::make_shared<sasl_wrapper>(_authenticator->new_sasl_challenge());
|
|
}
|
|
|
|
virtual future<> ensure_superuser_is_created() const override {
|
|
return _authenticator->ensure_superuser_is_created();
|
|
}
|
|
};
|
|
|
|
class transitional_authorizer : public authorizer {
|
|
std::unique_ptr<authorizer> _authorizer;
|
|
|
|
public:
|
|
transitional_authorizer(cql3::query_processor& qp, ::service::raft_group0_client& g0, ::service::migration_manager& mm)
|
|
: transitional_authorizer(std::make_unique<default_authorizer>(qp, g0, mm)) {
|
|
}
|
|
transitional_authorizer(std::unique_ptr<authorizer> a)
|
|
: _authorizer(std::move(a)) {
|
|
}
|
|
|
|
~transitional_authorizer() {
|
|
}
|
|
|
|
virtual future<> start() override {
|
|
return _authorizer->start();
|
|
}
|
|
|
|
virtual future<> stop() override {
|
|
return _authorizer->stop();
|
|
}
|
|
|
|
virtual std::string_view qualified_java_name() const override {
|
|
return transitional_authorizer_name();
|
|
}
|
|
|
|
virtual future<permission_set> authorize(const role_or_anonymous&, const resource&) const override {
|
|
static const permission_set transitional_permissions =
|
|
permission_set::of<
|
|
permission::CREATE,
|
|
permission::ALTER,
|
|
permission::DROP,
|
|
permission::SELECT,
|
|
permission::MODIFY>();
|
|
|
|
return make_ready_future<permission_set>(transitional_permissions);
|
|
}
|
|
|
|
virtual future<> grant(std::string_view s, permission_set ps, const resource& r, ::service::group0_batch& mc) override {
|
|
return _authorizer->grant(s, std::move(ps), r, mc);
|
|
}
|
|
|
|
virtual future<> revoke(std::string_view s, permission_set ps, const resource& r, ::service::group0_batch& mc) override {
|
|
return _authorizer->revoke(s, std::move(ps), r, mc);
|
|
}
|
|
|
|
virtual future<std::vector<permission_details>> list_all() const override {
|
|
return _authorizer->list_all();
|
|
}
|
|
|
|
virtual future<> revoke_all(std::string_view s, ::service::group0_batch& mc) override {
|
|
return _authorizer->revoke_all(s, mc);
|
|
}
|
|
|
|
virtual future<> revoke_all(const resource& r, ::service::group0_batch& mc) override {
|
|
return _authorizer->revoke_all(r, mc);
|
|
}
|
|
|
|
virtual const resource_set& protected_resources() const override {
|
|
return _authorizer->protected_resources();
|
|
}
|
|
};
|
|
|
|
}
|
|
|
|
//
|
|
// To ensure correct initialization order, we unfortunately need to use string literals.
|
|
//
|
|
|
|
static const class_registrator<
|
|
auth::authenticator,
|
|
auth::transitional_authenticator,
|
|
cql3::query_processor&,
|
|
::service::raft_group0_client&,
|
|
::service::migration_manager&,
|
|
auth::cache&> transitional_authenticator_reg(auth::PACKAGE_NAME + "TransitionalAuthenticator");
|
|
|
|
static const class_registrator<
|
|
auth::authorizer,
|
|
auth::transitional_authorizer,
|
|
cql3::query_processor&,
|
|
::service::raft_group0_client&,
|
|
::service::migration_manager&> transitional_authorizer_reg(auth::PACKAGE_NAME + "TransitionalAuthorizer");
|