rgw/user: add 'active' flag to RGWAccessKey

inactive keys are removed from the key pool so can't be used to
authenticate. the 'key create' admin op now takes an 'active' option

Fixes: https://tracker.ceph.com/issues/59186

Signed-off-by: Casey Bodley <cbodley@redhat.com>
This commit is contained in:
Casey Bodley 2023-12-20 11:25:03 -05:00
parent 72be1f4a8a
commit 463f463d50
9 changed files with 108 additions and 58 deletions

View File

@ -1163,6 +1163,13 @@ Request Parameters
:Example: True [True]
:Required: No
``active``
:Description: Activate or deactivate a key.
:Type: Boolean
:Example: True [True]
:Required: No
Response Entities
~~~~~~~~~~~~~~~~~

View File

@ -651,7 +651,9 @@ void RGWOp_Key_Create::execute(optional_yield y)
std::string secret_key;
std::string key_type_str;
bool gen_key;
bool gen_key = true;
bool active = true;
bool active_specified = false;
RGWUserAdminOpState op_state(driver);
@ -662,12 +664,16 @@ void RGWOp_Key_Create::execute(optional_yield y)
RESTArgs::get_string(s, "access-key", access_key, &access_key);
RESTArgs::get_string(s, "secret-key", secret_key, &secret_key);
RESTArgs::get_string(s, "key-type", key_type_str, &key_type_str);
RESTArgs::get_bool(s, "generate-key", true, &gen_key);
RESTArgs::get_bool(s, "generate-key", gen_key, &gen_key);
RESTArgs::get_bool(s, "active", active, &active, &active_specified);
op_state.set_user_id(uid);
op_state.set_subuser(subuser);
op_state.set_access_key(access_key);
op_state.set_secret_key(secret_key);
if (active_specified) {
op_state.access_key_active = active;
}
if (gen_key)
op_state.set_generate_key();

View File

@ -102,6 +102,7 @@ static void dump_access_keys_info(Formatter *f, RGWUserInfo &info)
f->dump_format("user", "%s%s%s", s.c_str(), sep, subuser);
f->dump_string("access_key", k.id);
f->dump_string("secret_key", k.key);
f->dump_bool("active", k.active);
f->close_section();
}
f->close_section();
@ -120,6 +121,7 @@ static void dump_swift_keys_info(Formatter *f, RGWUserInfo &info)
info.user_id.to_str(s);
f->dump_format("user", "%s%s%s", s.c_str(), sep, subuser);
f->dump_string("secret_key", k.key);
f->dump_bool("active", k.active);
f->close_section();
}
f->close_section();
@ -600,11 +602,6 @@ int RGWAccessKeyPool::modify_key(RGWUserAdminOpState& op_state, std::string *err
std::string key = op_state.get_secret_key();
int key_type = op_state.get_key_type();
RGWAccessKey modify_key;
pair<string, RGWAccessKey> key_pair;
map<std::string, RGWAccessKey>::iterator kiter;
switch (key_type) {
case KEY_TYPE_S3:
id = op_state.get_access_key();
@ -630,8 +627,8 @@ int RGWAccessKeyPool::modify_key(RGWUserAdminOpState& op_state, std::string *err
return -ERR_INVALID_ACCESS_KEY;
}
key_pair.first = id;
RGWAccessKey modify_key;
map<std::string, RGWAccessKey>::iterator kiter;
if (key_type == KEY_TYPE_SWIFT) {
modify_key.id = id;
modify_key.subuser = op_state.get_subuser();
@ -649,16 +646,13 @@ int RGWAccessKeyPool::modify_key(RGWUserAdminOpState& op_state, std::string *err
key = secret_key_buf;
}
if (key.empty()) {
set_err_msg(err_msg, "empty secret key");
return -ERR_INVALID_SECRET_KEY;
if (!key.empty()) {
// update the access key with the new secret key
modify_key.key = key;
}
if (op_state.access_key_active) {
modify_key.active = *op_state.access_key_active;
}
// update the access key with the new secret key
modify_key.key = key;
key_pair.second = modify_key;
if (key_type == KEY_TYPE_S3) {
(*access_keys)[id] = modify_key;

View File

@ -135,6 +135,7 @@ struct RGWUserAdminOpState {
std::map<std::string, RGWAccessKey> op_access_keys;
int32_t key_type{-1};
bool access_key_exist = false;
std::optional<bool> access_key_active;
std::set<std::string> mfa_ids;
@ -275,6 +276,10 @@ struct RGWUserAdminOpState {
access_key_exist = true;
}
void set_access_key_active(bool active) {
access_key_active = active;
}
void set_suspension(__u8 is_suspended) {
suspended = is_suspended;
suspension_op = true;

View File

@ -45,24 +45,29 @@ struct RGWAccessKey {
std::string id; // AccessKey
std::string key; // SecretKey
std::string subuser;
bool active = true;
RGWAccessKey() {}
RGWAccessKey(std::string _id, std::string _key)
: id(std::move(_id)), key(std::move(_key)) {}
void encode(bufferlist& bl) const {
ENCODE_START(2, 2, bl);
ENCODE_START(3, 2, bl);
encode(id, bl);
encode(key, bl);
encode(subuser, bl);
encode(active, bl);
ENCODE_FINISH(bl);
}
void decode(bufferlist::const_iterator& bl) {
DECODE_START_LEGACY_COMPAT_LEN_32(2, 2, 2, bl);
DECODE_START_LEGACY_COMPAT_LEN_32(3, 2, 2, bl);
decode(id, bl);
decode(key, bl);
decode(subuser, bl);
if (struct_v >= 3) {
decode(active, bl);
}
DECODE_FINISH(bl);
}
void dump(Formatter *f) const;

View File

@ -336,6 +336,7 @@ void usage()
cout << " --gen-access-key generate random access key (for S3)\n";
cout << " --gen-secret generate random secret key\n";
cout << " --key-type=<type> key type, options are: swift, s3\n";
cout << " --key-active=<bool> activate or deactivate a key\n";
cout << " --temp-url-key[-2]=<key> temp url key\n";
cout << " --access=<access> Set access permissions for sub-user, should be one\n";
cout << " of read, write, readwrite, full\n";
@ -3357,6 +3358,8 @@ int main(int argc, const char **argv)
int commit = false;
int staging = false;
int key_type = KEY_TYPE_UNDEFINED;
int key_active = true;
bool key_active_specified = false;
std::unique_ptr<rgw::sal::Bucket> bucket;
uint32_t perm_mask = 0;
RGWUserInfo info;
@ -3610,6 +3613,8 @@ int main(int argc, const char **argv)
cerr << "bad key type: " << key_type_str << std::endl;
exit(1);
}
} else if (ceph_argparse_binary_flag(args, i, &key_active, NULL, "--key-active", (char*)NULL)) {
key_active_specified = true;
} else if (ceph_argparse_witharg(args, i, &val, "--job-id", (char*)NULL)) {
job_id = val;
} else if (ceph_argparse_binary_flag(args, i, &gen_access_key, NULL, "--gen-access-key", (char*)NULL)) {
@ -6443,6 +6448,10 @@ int main(int argc, const char **argv)
if (key_type != KEY_TYPE_UNDEFINED)
user_op.set_key_type(key_type);
if (key_active_specified) {
user_op.access_key_active = key_active;
}
// set suspension operation parameters
if (opt_cmd == OPT::USER_ENABLE)
user_op.set_suspension(false);

View File

@ -2912,6 +2912,7 @@ void RGWAccessKey::dump(Formatter *f) const
encode_json("access_key", id, f);
encode_json("secret_key", key, f);
encode_json("subuser", subuser, f);
encode_json("active", active, f);
}
void RGWAccessKey::dump_plain(Formatter *f) const
@ -2932,6 +2933,7 @@ void RGWAccessKey::dump(Formatter *f, const string& user, bool swift) const
encode_json("access_key", id, f);
}
encode_json("secret_key", key, f);
encode_json("active", active, f);
}
void RGWAccessKey::decode_json(JSONObj *obj) {
@ -2945,6 +2947,7 @@ void RGWAccessKey::decode_json(JSONObj *obj) {
subuser = user.substr(pos + 1);
}
}
JSONDecoder::decode_json("active", active, obj);
}
void RGWAccessKey::decode_json(JSONObj *obj, bool swift) {
@ -2961,6 +2964,7 @@ void RGWAccessKey::decode_json(JSONObj *obj, bool swift) {
}
}
JSONDecoder::decode_json("secret_key", key, obj, true);
JSONDecoder::decode_json("active", active, obj);
}
void RGWStorageStats::dump(Formatter *f) const

View File

@ -151,6 +151,22 @@ int RGWSI_User_RADOS::read_user_info(RGWSI_MetaBackend::Context *ctx,
return 0;
}
static bool s3_key_active(const RGWUserInfo* info, const std::string& id) {
if (!info) {
return false;
}
auto i = info->access_keys.find(id);
return i != info->access_keys.end() && i->second.active;
}
static bool swift_key_active(const RGWUserInfo* info, const std::string& id) {
if (!info) {
return false;
}
auto i = info->swift_keys.find(id);
return i != info->swift_keys.end() && i->second.active;
}
class PutOperation
{
RGWSI_User_RADOS::Svc& svc;
@ -203,28 +219,28 @@ public:
}
}
for (auto iter = info.swift_keys.begin(); iter != info.swift_keys.end(); ++iter) {
if (old_info && old_info->swift_keys.count(iter->first) != 0)
for (const auto& [id, key] : info.swift_keys) {
if (!key.active || swift_key_active(old_info, id))
continue;
auto& k = iter->second;
/* check if swift mapping exists */
RGWUserInfo inf;
int r = svc.user->get_user_info_by_swift(ctx, k.id, &inf, nullptr, nullptr, y, dpp);
int r = svc.user->get_user_info_by_swift(ctx, id, &inf, nullptr, nullptr, y, dpp);
if (r >= 0 && inf.user_id != info.user_id &&
(!old_info || inf.user_id != old_info->user_id)) {
ldpp_dout(dpp, 0) << "WARNING: can't store user info, swift id (" << k.id
ldpp_dout(dpp, 0) << "WARNING: can't store user info, swift id (" << id
<< ") already mapped to another user (" << info.user_id << ")" << dendl;
return -EEXIST;
}
}
/* check if access keys already exist */
for (auto iter = info.access_keys.begin(); iter != info.access_keys.end(); ++iter) {
if (old_info && old_info->access_keys.count(iter->first) != 0)
for (const auto& [id, key] : info.access_keys) {
if (!key.active) // new key not active
continue;
if (s3_key_active(old_info, id)) // old key already active
continue;
auto& k = iter->second;
RGWUserInfo inf;
int r = svc.user->get_user_info_by_access_key(ctx, k.id, &inf, nullptr, nullptr, y, dpp);
int r = svc.user->get_user_info_by_access_key(ctx, id, &inf, nullptr, nullptr, y, dpp);
if (r >= 0 && inf.user_id != info.user_id &&
(!old_info || inf.user_id != old_info->user_id)) {
ldpp_dout(dpp, 0) << "WARNING: can't store user info, access key already mapped to another user" << dendl;
@ -266,23 +282,25 @@ public:
}
const bool renamed = old_info && old_info->user_id != info.user_id;
for (auto iter = info.access_keys.begin(); iter != info.access_keys.end(); ++iter) {
auto& k = iter->second;
if (old_info && old_info->access_keys.count(iter->first) != 0 && !renamed)
for (const auto& [id, key] : info.access_keys) {
if (!key.active)
continue;
if (s3_key_active(old_info, id) && !renamed)
continue;
ret = rgw_put_system_obj(dpp, svc.sysobj, svc.zone->get_zone_params().user_keys_pool, k.id,
ret = rgw_put_system_obj(dpp, svc.sysobj, svc.zone->get_zone_params().user_keys_pool, id,
link_bl, exclusive, NULL, real_time(), y);
if (ret < 0)
return ret;
}
for (auto siter = info.swift_keys.begin(); siter != info.swift_keys.end(); ++siter) {
auto& k = siter->second;
if (old_info && old_info->swift_keys.count(siter->first) != 0 && !renamed)
for (const auto& [id, key] : info.swift_keys) {
if (!key.active)
continue;
if (swift_key_active(old_info, id) && !renamed)
continue;
ret = rgw_put_system_obj(dpp, svc.sysobj, svc.zone->get_zone_params().user_swift_pool, k.id,
ret = rgw_put_system_obj(dpp, svc.sysobj, svc.zone->get_zone_params().user_swift_pool, id,
link_bl, exclusive, NULL, real_time(), y);
if (ret < 0)
return ret;
@ -323,23 +341,21 @@ public:
}
}
for ([[maybe_unused]] const auto& [name, access_key] : old_info.access_keys) {
if (!new_info.access_keys.count(access_key.id)) {
ret = svc.user->remove_key_index(dpp, access_key, y);
for (const auto& [id, key] : old_info.access_keys) {
if (key.active && !s3_key_active(&new_info, id)) {
ret = svc.user->remove_key_index(dpp, key, y);
if (ret < 0 && ret != -ENOENT) {
set_err_msg("ERROR: could not remove index for key " + access_key.id);
set_err_msg("ERROR: could not remove index for key " + id);
return ret;
}
}
}
for (auto old_iter = old_info.swift_keys.begin(); old_iter != old_info.swift_keys.end(); ++old_iter) {
const auto& swift_key = old_iter->second;
auto new_iter = new_info.swift_keys.find(swift_key.id);
if (new_iter == new_info.swift_keys.end()) {
ret = svc.user->remove_swift_name_index(dpp, swift_key.id, y);
for (const auto& [id, key] : old_info.swift_keys) {
if (key.active && !swift_key_active(&new_info, id)) {
ret = svc.user->remove_swift_name_index(dpp, id, y);
if (ret < 0 && ret != -ENOENT) {
set_err_msg("ERROR: could not remove index for swift_name " + swift_key.id);
set_err_msg("ERROR: could not remove index for swift_name " + id);
return ret;
}
}
@ -432,24 +448,27 @@ int RGWSI_User_RADOS::remove_user_info(RGWSI_MetaBackend::Context *ctx,
{
int ret;
auto kiter = info.access_keys.begin();
for (; kiter != info.access_keys.end(); ++kiter) {
ldpp_dout(dpp, 10) << "removing key index: " << kiter->first << dendl;
ret = remove_key_index(dpp, kiter->second, y);
for (const auto& [id, key] : info.access_keys) {
if (!key.active) {
continue;
}
ldpp_dout(dpp, 10) << "removing key index: " << id << dendl;
ret = remove_key_index(dpp, key, y);
if (ret < 0 && ret != -ENOENT) {
ldpp_dout(dpp, 0) << "ERROR: could not remove " << kiter->first << " (access key object), should be fixed (err=" << ret << ")" << dendl;
ldpp_dout(dpp, 0) << "ERROR: could not remove " << id << " (access key object), should be fixed (err=" << ret << ")" << dendl;
return ret;
}
}
auto siter = info.swift_keys.begin();
for (; siter != info.swift_keys.end(); ++siter) {
auto& k = siter->second;
ldpp_dout(dpp, 10) << "removing swift subuser index: " << k.id << dendl;
for (const auto& [id, key] : info.swift_keys) {
if (!key.active) {
continue;
}
ldpp_dout(dpp, 10) << "removing swift subuser index: " << id << dendl;
/* check if swift mapping exists */
ret = remove_swift_name_index(dpp, k.id, y);
ret = remove_swift_name_index(dpp, id, y);
if (ret < 0 && ret != -ENOENT) {
ldpp_dout(dpp, 0) << "ERROR: could not remove " << k.id << " (swift name object), should be fixed (err=" << ret << ")" << dendl;
ldpp_dout(dpp, 0) << "ERROR: could not remove " << id << " (swift name object), should be fixed (err=" << ret << ")" << dendl;
return ret;
}
}

View File

@ -204,6 +204,7 @@
--gen-access-key generate random access key (for S3)
--gen-secret generate random secret key
--key-type=<type> key type, options are: swift, s3
--key-active=<bool> activate or deactivate a key
--temp-url-key[-2]=<key> temp url key
--access=<access> Set access permissions for sub-user, should be one
of read, write, readwrite, full