From 35b073b3a6e4388bb14df8a8178d5b07711fab54 Mon Sep 17 00:00:00 2001 From: buzhimingyonghu <1512049108@qq.com> Date: Thu, 31 Oct 2024 07:06:48 +0000 Subject: [PATCH 1/5] fix authrequired bug --- conf/pika.conf | 6 ++--- include/acl.h | 7 +++--- include/pika_client_conn.h | 23 ++++++------------- src/acl.cc | 17 ++++++-------- src/pika_client_conn.cc | 46 +++++++++++++++++++++----------------- 5 files changed, 45 insertions(+), 54 deletions(-) diff --git a/conf/pika.conf b/conf/pika.conf index 59aef75fa0..30e34db922 100644 --- a/conf/pika.conf +++ b/conf/pika.conf @@ -103,7 +103,7 @@ timeout : 60 # the value of userpass will be ignored and all users are considered as administrators, # in this scenario, users are not subject to the restrictions imposed by the userblacklist. # PS: "user password" refers to value of the parameter below: userpass. -requirepass : +requirepass : 123 # Password for replication verify, used for authentication when a slave # connects to a master to request replication. @@ -115,14 +115,14 @@ masterauth : # 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. # PS: "admin password" refers to value of the parameter above: requirepass. -# userpass : +userpass :12345 # The blacklist of commands for users that logged in by userpass, # the commands that added to this list will not be available for users except for administrator. # [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 :FLUSHALL # 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 5aa83d408d..77bd5ba8a3 100644 --- a/include/acl.h +++ b/include/acl.h @@ -129,7 +129,7 @@ class AclSelector { friend User; public: - explicit AclSelector() : AclSelector(0){}; + explicit AclSelector() : AclSelector(0) {}; explicit AclSelector(uint32_t flag); explicit AclSelector(const AclSelector& selector); ~AclSelector() = default; @@ -138,7 +138,7 @@ class AclSelector { inline bool HasFlags(uint32_t flag) const { return flags_ & flag; }; inline void AddFlags(uint32_t flag) { flags_ |= flag; }; inline void DecFlags(uint32_t flag) { flags_ &= ~flag; }; - bool EqualChannel(const std::vector &allChannel); + bool EqualChannel(const std::vector& allChannel); private: pstd::Status SetSelector(const std::string& op); @@ -224,8 +224,7 @@ class User { ~User() = default; std::string Name() const; - - // inline uint32_t Flags() const { return flags_; }; + // inline uint32_t Flags() const { return flags_; }; inline bool HasFlags(uint32_t flag) const { return flags_ & flag; }; inline void AddFlags(uint32_t flag) { flags_ |= flag; }; inline void DecFlags(uint32_t flag) { flags_ &= ~flag; }; diff --git a/include/pika_client_conn.h b/include/pika_client_conn.h index 5b912592ab..3124d2036c 100644 --- a/include/pika_client_conn.h +++ b/include/pika_client_conn.h @@ -22,25 +22,15 @@ struct TimeStat { before_queue_ts_ = 0; } - uint64_t start_ts() const { - return enqueue_ts_; - } + uint64_t start_ts() const { return enqueue_ts_; } - uint64_t total_time() const { - return process_done_ts_ > enqueue_ts_ ? process_done_ts_ - enqueue_ts_ : 0; - } + uint64_t total_time() const { return process_done_ts_ > enqueue_ts_ ? process_done_ts_ - enqueue_ts_ : 0; } - uint64_t queue_time() const { - return dequeue_ts_ > enqueue_ts_ ? dequeue_ts_ - enqueue_ts_ : 0; - } + uint64_t queue_time() const { return dequeue_ts_ > enqueue_ts_ ? dequeue_ts_ - enqueue_ts_ : 0; } - uint64_t process_time() const { - return process_done_ts_ > dequeue_ts_ ? process_done_ts_ - dequeue_ts_ : 0; - } + uint64_t process_time() const { return process_done_ts_ > dequeue_ts_ ? process_done_ts_ - dequeue_ts_ : 0; } - uint64_t before_queue_time() const { - return process_done_ts_ > dequeue_ts_ ? before_queue_ts_ - enqueue_ts_ : 0; - } + uint64_t before_queue_time() const { return process_done_ts_ > dequeue_ts_ ? before_queue_ts_ - enqueue_ts_ : 0; } uint64_t enqueue_ts_; uint64_t dequeue_ts_; @@ -94,7 +84,7 @@ class PikaClientConn : public net::RedisConn { void UnAuth(const std::shared_ptr& user); bool IsAuthed() const; - + void InitUser(); bool AuthRequired() const; std::string UserName() const; @@ -123,6 +113,7 @@ class PikaClientConn : public net::RedisConn { std::vector> resp_array; std::shared_ptr time_stat_; + private: net::ServerThread* const server_thread_; std::string current_db_; diff --git a/src/acl.cc b/src/acl.cc index bd6862aa81..dea532b724 100644 --- a/src/acl.cc +++ b/src/acl.cc @@ -489,31 +489,28 @@ void Acl::InitLimitUser(const std::string& bl, bool limit_exist) { auto u = GetUser(DefaultLimitUser); if (limit_exist) { if (!bl.empty()) { - for(auto& cmd : blacklist) { + 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) { + for (auto& cmd : blacklist) { cmd = pstd::StringTrim(cmd, " "); u->SetUser("-" + cmd); } } + if (pass.empty()) { + u->SetUser("nopass"); + } else { + u->SetUser(">" + pass); + } } // bool Acl::CheckUserCanExec(const std::shared_ptr& cmd, const PikaCmdArgsType& argv) { cmd->name(); } diff --git a/src/pika_client_conn.cc b/src/pika_client_conn.cc index 37a47bc371..2643cd30af 100644 --- a/src/pika_client_conn.cc +++ b/src/pika_client_conn.cc @@ -30,8 +30,7 @@ PikaClientConn::PikaClientConn(int fd, const std::string& ip_port, net::Thread* : RedisConn(fd, ip_port, thread, mpx, handle_type, max_conn_rbuf_size), server_thread_(reinterpret_cast(thread)), current_db_(g_pika_conf->default_db()) { - // client init, set client user is default, and authenticated = false - UnAuth(g_pika_server->Acl()->GetUserLock(Acl::DefaultUser)); + InitUser(); time_stat_.reset(new TimeStat()); } @@ -259,7 +258,7 @@ void PikaClientConn::ProcessMonitor(const PikaCmdArgsType& argv) { } bool PikaClientConn::IsInterceptedByRTC(std::string& opt) { - //currently we only Intercept: Get, HGet + // currently we only Intercept: Get, HGet if (opt == kCmdNameGet && g_pika_conf->GetCacheString()) { return true; } @@ -289,11 +288,9 @@ void PikaClientConn::ProcessRedisCmds(const std::vector& bool is_slow_cmd = g_pika_conf->is_slow_cmd(opt); bool is_admin_cmd = g_pika_conf->is_admin_cmd(opt); - //we don't intercept pipeline batch (argvs.size() > 1) - if (g_pika_conf->rtc_cache_read_enabled() && - argvs.size() == 1 && IsInterceptedByRTC(opt) && - PIKA_CACHE_NONE != g_pika_conf->cache_mode() && - !IsInTxn()) { + // we don't intercept pipeline batch (argvs.size() > 1) + if (g_pika_conf->rtc_cache_read_enabled() && argvs.size() == 1 && IsInterceptedByRTC(opt) && + PIKA_CACHE_NONE != g_pika_conf->cache_mode() && !IsInTxn()) { // read in cache if (ReadCmdInCache(argvs[0], opt)) { delete arg; @@ -358,21 +355,17 @@ bool PikaClientConn::ReadCmdInCache(const net::RedisCmdArgsType& argv, const std resp_num--; return false; } - //acl check + // acl check int8_t subCmdIndex = -1; std::string errKey; auto checkRes = user_->CheckUserPermission(c_ptr, argv, subCmdIndex, &errKey); std::string object; - if (checkRes == AclDeniedCmd::CMD || - checkRes == AclDeniedCmd::KEY || - checkRes == AclDeniedCmd::CHANNEL || - checkRes == AclDeniedCmd::NO_SUB_CMD || - checkRes == AclDeniedCmd::NO_AUTH - ) { - //acl check failed + if (checkRes == AclDeniedCmd::CMD || checkRes == AclDeniedCmd::KEY || checkRes == AclDeniedCmd::CHANNEL || + checkRes == AclDeniedCmd::NO_SUB_CMD || checkRes == AclDeniedCmd::NO_AUTH) { + // acl check failed return false; } - //only read command(Get, HGet) will reach here, no need of record lock + // only read command(Get, HGet) will reach here, no need of record lock bool read_status = c_ptr->DoReadCommandInCache(); auto cmdstat_map = g_pika_cmd_table_manager->GetCommandStatMap(); resp_num--; @@ -547,13 +540,24 @@ void PikaClientConn::UnAuth(const std::shared_ptr& user) { } bool PikaClientConn::IsAuthed() const { return authenticated_; } - +void PikaClientConn::InitUser() { + if (g_pika_conf->requirepass().empty()) { + user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultUser); + } + user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultLimitUser); + authenticated_ = user_->HasFlags(static_cast(AclUserFlag::NO_PASS)) && + !user_->HasFlags(static_cast(AclUserFlag::DISABLED)); +} bool PikaClientConn::AuthRequired() const { // If the user does not have a password, and the user is valid, then the user does not need authentication // Otherwise, you need to determine whether go has been authenticated - return (!user_->HasFlags(static_cast(AclUserFlag::NO_PASS)) || - user_->HasFlags(static_cast(AclUserFlag::DISABLED))) && - !IsAuthed(); + if (user_->HasFlags(static_cast(AclUserFlag::DISABLED))) { + return true; + } + if (user_->HasFlags(static_cast(AclUserFlag::NO_PASS))) { + return false; + } + return !IsAuthed(); } std::string PikaClientConn::UserName() const { return user_->Name(); } From 0054e94b72b8c5af67bf13bfdd09f67f0eec0283 Mon Sep 17 00:00:00 2001 From: buzhimingyonghu <1512049108@qq.com> Date: Thu, 31 Oct 2024 07:35:10 +0000 Subject: [PATCH 2/5] init pika.conf --- conf/pika.conf | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/conf/pika.conf b/conf/pika.conf index 30e34db922..d4f0efb011 100644 --- a/conf/pika.conf +++ b/conf/pika.conf @@ -103,7 +103,7 @@ timeout : 60 # the value of userpass will be ignored and all users are considered as administrators, # in this scenario, users are not subject to the restrictions imposed by the userblacklist. # PS: "user password" refers to value of the parameter below: userpass. -requirepass : 123 +requirepass : # Password for replication verify, used for authentication when a slave # connects to a master to request replication. @@ -115,14 +115,14 @@ masterauth : # 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. # PS: "admin password" refers to value of the parameter above: requirepass. -userpass :12345 +# userpass : # The blacklist of commands for users that logged in by userpass, # the commands that added to this list will not be available for users except for administrator. # [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 :FLUSHALL +# 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". @@ -691,4 +691,4 @@ dont-compact-sst-created-in-seconds : 20 # For OBD_Compact # According to the number of sst files in rocksdb, # compact every `compact-every-num-of-files` file. -best-delete-min-ratio : 10 +best-delete-min-ratio : 10 \ No newline at end of file From 62754faedf1e5d205d654e37b6ea5e8aa2e4c3a4 Mon Sep 17 00:00:00 2001 From: buzhimingyonghu <1512049108@qq.com> Date: Thu, 31 Oct 2024 09:25:39 +0000 Subject: [PATCH 3/5] fix InitUser --- src/pika_client_conn.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pika_client_conn.cc b/src/pika_client_conn.cc index 2643cd30af..35e71e7529 100644 --- a/src/pika_client_conn.cc +++ b/src/pika_client_conn.cc @@ -543,8 +543,9 @@ bool PikaClientConn::IsAuthed() const { return authenticated_; } void PikaClientConn::InitUser() { if (g_pika_conf->requirepass().empty()) { user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultUser); + } else { + user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultLimitUser); } - user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultLimitUser); authenticated_ = user_->HasFlags(static_cast(AclUserFlag::NO_PASS)) && !user_->HasFlags(static_cast(AclUserFlag::DISABLED)); } From e643c87c82624370dad489fa7c3be3467647a3d2 Mon Sep 17 00:00:00 2001 From: buzhimingyonghu <1512049108@qq.com> Date: Thu, 7 Nov 2024 10:01:49 +0000 Subject: [PATCH 4/5] fix InitUser --- src/acl.cc | 13 ++++++++----- src/pika_client_conn.cc | 11 +++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/acl.cc b/src/acl.cc index dea532b724..04226d811c 100644 --- a/src/acl.cc +++ b/src/acl.cc @@ -495,7 +495,15 @@ void Acl::InitLimitUser(const std::string& bl, bool limit_exist) { } 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("~*"); @@ -506,11 +514,6 @@ void Acl::InitLimitUser(const std::string& bl, bool limit_exist) { u->SetUser("-" + cmd); } } - if (pass.empty()) { - u->SetUser("nopass"); - } else { - u->SetUser(">" + pass); - } } // bool Acl::CheckUserCanExec(const std::shared_ptr& cmd, const PikaCmdArgsType& argv) { cmd->name(); } diff --git a/src/pika_client_conn.cc b/src/pika_client_conn.cc index 35e71e7529..768cb6d5ad 100644 --- a/src/pika_client_conn.cc +++ b/src/pika_client_conn.cc @@ -541,10 +541,10 @@ void PikaClientConn::UnAuth(const std::shared_ptr& user) { bool PikaClientConn::IsAuthed() const { return authenticated_; } void PikaClientConn::InitUser() { - if (g_pika_conf->requirepass().empty()) { - user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultUser); - } else { + if (!g_pika_conf->GetUserBlackList().empty()) { user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultLimitUser); + } else { + user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultUser); } authenticated_ = user_->HasFlags(static_cast(AclUserFlag::NO_PASS)) && !user_->HasFlags(static_cast(AclUserFlag::DISABLED)); @@ -552,13 +552,16 @@ void PikaClientConn::InitUser() { bool PikaClientConn::AuthRequired() const { // If the user does not have a password, and the user is valid, then the user does not need authentication // Otherwise, you need to determine whether go has been authenticated + if (IsAuthed()) { + return false; + } if (user_->HasFlags(static_cast(AclUserFlag::DISABLED))) { return true; } if (user_->HasFlags(static_cast(AclUserFlag::NO_PASS))) { return false; } - return !IsAuthed(); + return true; } std::string PikaClientConn::UserName() const { return user_->Name(); } From 19c1c8b97632344ec93bfd4667f8c460cce68c3e Mon Sep 17 00:00:00 2001 From: buzhimingyonghu <1512049108@qq.com> Date: Thu, 7 Nov 2024 12:00:06 +0000 Subject: [PATCH 5/5] fix no userblacklist --- src/acl.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/acl.cc b/src/acl.cc index 04226d811c..dad50f73e6 100644 --- a/src/acl.cc +++ b/src/acl.cc @@ -477,7 +477,11 @@ void Acl::UpdateDefaultUserPassword(const std::string& pass) { if (pass.empty()) { u->SetUser("nopass"); } else { - u->SetUser(">" + pass); + if (g_pika_conf->userpass().empty()) { + u->SetUser("nopass"); + } else { + u->SetUser(">" + pass); + } } }