From 59e62735ce47d2f4787f414cb78c5688971c6a14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=81=E5=B0=8F=E5=B8=85?= <56024577+dingxiaoshuai123@users.noreply.github.com> Date: Fri, 8 Mar 2024 20:36:31 +0800 Subject: [PATCH] fix: acl forward compatible (#2459) * fix: ACL user authentication errors * blacklist instead of acl user * add rename command (#2462) * support config get userblacklist --------- --- conf/pika.conf | 4 +-- include/acl.h | 3 +++ include/pika_conf.h | 11 ++++++++ src/acl.cc | 45 ++++++++++++++++++++++++++++++++ src/pika_acl.cc | 4 +++ src/pika_admin.cc | 21 ++++++++++++--- src/pika_conf.cc | 8 +++--- tests/integration/server_test.go | 2 +- tests/unit/acl.tcl | 2 +- 9 files changed, 89 insertions(+), 11 deletions(-) diff --git a/conf/pika.conf b/conf/pika.conf index 398f99eb8b..eabd8a285b 100644 --- a/conf/pika.conf +++ b/conf/pika.conf @@ -79,7 +79,7 @@ requirepass : # [NOTICE] The value of this parameter must match the "requirepass" setting on the master. masterauth : -# The [password of user], which is empty by default.(Deprecated) +# The [password of user], which is empty by default. # [NOTICE] If this user password is the same as admin password (including both being empty), # the value of this parameter will be ignored and all users are considered as administrators, # in this scenario, users are not subject to the restrictions imposed by the userblacklist. @@ -91,7 +91,7 @@ masterauth : # [Advice] It's recommended to add high-risk commands to this list. # [Format] Commands should be separated by ",". For example: FLUSHALL, SHUTDOWN, KEYS, CONFIG # By default, this list is empty. -userblacklist : +# userblacklist : # Running Mode of Pika, The current version only supports running in "classic mode". # If set to 'classic', Pika will create multiple DBs whose number is the value of configure item "databases". diff --git a/include/acl.h b/include/acl.h index 1363732352..5aa83d408d 100644 --- a/include/acl.h +++ b/include/acl.h @@ -365,6 +365,8 @@ class Acl { void UpdateDefaultUserPassword(const std::string& pass); + void InitLimitUser(const std::string& bl, bool limit_exist); + // After the user channel is modified, determine whether the current channel needs to be disconnected void KillPubsubClientsIfNeeded(const std::shared_ptr& origin, const std::shared_ptr& newUser); @@ -380,6 +382,7 @@ class Acl { static std::vector GetAllCategoryName(); static const std::string DefaultUser; + static const std::string DefaultLimitUser; static const int64_t LogGroupingMaxTimeDelta; // Adds a new entry in the ACL log, making sure to delete the old entry diff --git a/include/pika_conf.h b/include/pika_conf.h index 0bf1d3972f..37a88920ff 100644 --- a/include/pika_conf.h +++ b/include/pika_conf.h @@ -178,6 +178,10 @@ class PikaConf : public pstd::BaseConf { std::shared_lock l(rwlock_); return masterauth_; } + std::string userpass() { + std::shared_lock l(rwlock_); + return userpass_; + } std::string bgsave_path() { std::shared_lock l(rwlock_); return bgsave_path_; @@ -377,6 +381,11 @@ class PikaConf : public pstd::BaseConf { return pstd::Set2String(slow_cmd_set_, ','); } + const std::string GetUserBlackList() { + std::shared_lock l(rwlock_); + return userblacklist_; + } + bool is_slow_cmd(const std::string& cmd) { std::shared_lock l(rwlock_); return slow_cmd_set_.find(cmd) != slow_cmd_set_.end(); @@ -709,6 +718,7 @@ class PikaConf : public pstd::BaseConf { std::string replication_id_; std::string requirepass_; std::string masterauth_; + std::string userpass_; std::atomic classic_mode_; int databases_ = 0; int default_slot_num_ = 1; @@ -762,6 +772,7 @@ class PikaConf : public pstd::BaseConf { std::string network_interface_; + std::string userblacklist_; std::vector users_; // acl user rules std::string aclFile_; diff --git a/src/acl.cc b/src/acl.cc index 3c78adf0e8..bf0f4758f2 100644 --- a/src/acl.cc +++ b/src/acl.cc @@ -294,7 +294,16 @@ std::vector User::AllChannelKey() { pstd::Status Acl::Initialization() { AddUser(CreateDefaultUser()); UpdateDefaultUserPassword(g_pika_conf->requirepass()); + auto status = LoadUsersAtStartup(); + auto u = GetUser(DefaultLimitUser); + bool limit_exist = true; + if (nullptr == u) { + AddUser(CreatedUser(DefaultLimitUser)); + limit_exist = false; + } + InitLimitUser(g_pika_conf->GetUserBlackList(), limit_exist); + if (!status.ok()) { return status; } @@ -472,6 +481,41 @@ void Acl::UpdateDefaultUserPassword(const std::string& pass) { } } +void Acl::InitLimitUser(const std::string& bl, bool limit_exist) { + auto pass = g_pika_conf->userpass(); + std::vector blacklist; + pstd::StringSplit(bl, ',', blacklist); + std::unique_lock wl(mutex_); + auto u = GetUser(DefaultLimitUser); + if (limit_exist) { + if (!bl.empty()) { + u->SetUser("+@all"); + for(auto& cmd : blacklist) { + cmd = pstd::StringTrim(cmd, " "); + u->SetUser("-" + cmd); + } + u->SetUser("on"); + if (!pass.empty()) { + u->SetUser(">"+pass); + } + } + } else { + if (pass.empty()) { + u->SetUser("nopass"); + } else { + u->SetUser(">"+pass); + } + u->SetUser("on"); + u->SetUser("+@all"); + u->SetUser("~*"); + u->SetUser("&*"); + + for(auto& cmd : blacklist) { + cmd = pstd::StringTrim(cmd, " "); + u->SetUser("-" + cmd); + } + } +} // bool Acl::CheckUserCanExec(const std::shared_ptr& cmd, const PikaCmdArgsType& argv) { cmd->name(); } std::shared_ptr Acl::CreateDefaultUser() { @@ -725,6 +769,7 @@ std::array, 3> Acl::SelectorFlags = {{ }}; const std::string Acl::DefaultUser = "default"; +const std::string Acl::DefaultLimitUser = "limit"; const int64_t Acl::LogGroupingMaxTimeDelta = 60000; void Acl::AddLogEntry(int32_t reason, int32_t context, const std::string& username, const std::string& object, diff --git a/src/pika_acl.cc b/src/pika_acl.cc index 296cbe4206..b6fe3375b7 100644 --- a/src/pika_acl.cc +++ b/src/pika_acl.cc @@ -106,6 +106,10 @@ void PikaAclCmd::DelUser() { res().SetRes(CmdRes::kErrOther, "The 'default' user cannot be removed"); return; } + if (it->data() == Acl::DefaultLimitUser) { + res().SetRes(CmdRes::kErrOther, "The 'limit' user cannot be removed"); + return; + } } std::vector userNames(argv_.begin() + 2, argv_.end()); diff --git a/src/pika_admin.cc b/src/pika_admin.cc index 5ee92c6e11..d41d6d5943 100644 --- a/src/pika_admin.cc +++ b/src/pika_admin.cc @@ -270,15 +270,24 @@ void AuthCmd::Do() { std::string pwd = ""; bool defaultAuth = false; if (argv_.size() == 2) { - userName = Acl::DefaultUser; pwd = argv_[1]; - defaultAuth = true; +// defaultAuth = true; } else { userName = argv_[1]; pwd = argv_[2]; } - auto authResult = AuthenticateUser(name(), userName, pwd, conn, defaultAuth); + AuthResult authResult; + if (userName == "") { + // default + authResult = AuthenticateUser(name(), Acl::DefaultUser, pwd, conn, true); + if (authResult != AuthResult::OK && authResult != AuthResult::NO_REQUIRE_PASS) { + // Limit + authResult = AuthenticateUser(name(), Acl::DefaultLimitUser, pwd, conn, defaultAuth); + } + } else { + authResult = AuthenticateUser(name(), userName, pwd, conn, defaultAuth); + } switch (authResult) { case AuthResult::INVALID_CONN: @@ -1569,7 +1578,11 @@ void ConfigCmd::ConfigGet(std::string& ret) { EncodeString(&config_body, "slow-cmd-thread-pool-size"); EncodeNumber(&config_body, g_pika_conf->slow_cmd_thread_pool_size()); } - + if (pstd::stringmatch(pattern.data(), "userblacklist", 1) != 0) { + elements += 2; + EncodeString(&config_body, "userblacklist"); + EncodeString(&config_body, g_pika_conf -> GetUserBlackList()); + } if (pstd::stringmatch(pattern.data(), "slow-cmd-list", 1) != 0) { elements += 2; EncodeString(&config_body, "slow-cmd-list"); diff --git a/src/pika_conf.cc b/src/pika_conf.cc index 2c5ac2ef7f..f1fe76020f 100644 --- a/src/pika_conf.cc +++ b/src/pika_conf.cc @@ -47,7 +47,7 @@ int PikaConf::Load() { GetConfStr("replication-id", &replication_id_); GetConfStr("requirepass", &requirepass_); GetConfStr("masterauth", &masterauth_); - // GetConfStr("userpass", &userpass_); + GetConfStr("userpass", &userpass_); GetConfInt("maxclients", &maxclients_); if (maxclients_ <= 0) { maxclients_ = 20000; @@ -463,6 +463,8 @@ int PikaConf::Load() { network_interface_ = ""; GetConfStr("network-interface", &network_interface_); + // userblacklist + GetConfStr("userblacklist", &userblacklist_); // acl users GetConfStrMulti("user", &users_); @@ -643,8 +645,8 @@ int PikaConf::ConfigRewrite() { SetConfInt("timeout", timeout_); SetConfStr("requirepass", requirepass_); SetConfStr("masterauth", masterauth_); - // SetConfStr("userpass", userpass_); - // SetConfStr("userblacklist", userblacklist); + SetConfStr("userpass", userpass_); + SetConfStr("userblacklist", userblacklist_); SetConfStr("dump-prefix", bgsave_prefix_); SetConfInt("maxclients", maxclients_); SetConfInt("dump-expire", expire_dump_days_); diff --git a/tests/integration/server_test.go b/tests/integration/server_test.go index 95685d2fdf..c836ff1c55 100644 --- a/tests/integration/server_test.go +++ b/tests/integration/server_test.go @@ -49,7 +49,7 @@ var _ = Describe("Server", func() { r = client.Do(ctx, "config", "set", "requirepass", "foobar") Expect(r.Val()).To(Equal("OK")) - r = client.Do(ctx, "AUTH", "wrong!") + r = client.Do(ctx, "AUTH", "default", "wrong!") Expect(r.Err()).To(MatchError("WRONGPASS invalid username-password pair or user is disabled.")) // r = client.Do(ctx, "AUTH", "foo", "bar") diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl index 20900905fc..b7e3c51cab 100644 --- a/tests/unit/acl.tcl +++ b/tests/unit/acl.tcl @@ -9,7 +9,7 @@ start_server {tags {"acl external:skip"}} { test {Coverage: ACL USERS} { r ACL USERS - } {default newuser} + } {default limit newuser} test {Usernames can not contain spaces or null characters} { catch {r ACL setuser "a a"} err