From df23e4b2ea4f8647271a9ce541a1fdbc4d9fe4a6 Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Thu, 18 Jan 2024 09:29:05 +0000 Subject: [PATCH] rgw: invalidate and retry keystone admin token We validate client tokens against the Keystone API by sending our own "admin token" that is allowed to lookup client tokens. This "admin token" is cached and upon checking the cache we verify the expiration on the token before using it but we have no logic to invalidate the cache if the response from the Keystone API says that the "admin token" is invalid. Since we don't invalidate it and it still has not expired it will stay in our cache and continue to cause Swift API requests for clients to be dropped because of the invalid admin token, until service is restarted, admin token is expired (which it can already be) or until the whole cache is dropped or TokenCache::invalidate() called on the admin token. There is probably multiple places in Keystone where it invalidates tokens, but one example where the "admin token" would be invalidated and return HTTP 401 status code is if the user that is configured in rgw_keystone_admin_user has it's password changed (even if it's the same password as the current one) then Keystone will invalidate it's cache and invalidated existing tokens even if they have not expired yet. Fixes: https://tracker.ceph.com/issues/64094 Signed-off-by: Tobias Urdin --- src/rgw/rgw_auth_keystone.cc | 44 +++++++++++++++++++++++++++--------- src/rgw/rgw_keystone.cc | 9 +++++++- src/rgw/rgw_keystone.h | 4 +++- 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/rgw/rgw_auth_keystone.cc b/src/rgw/rgw_auth_keystone.cc index 552159823bb4b..7b24306a15b0e 100644 --- a/src/rgw/rgw_auth_keystone.cc +++ b/src/rgw/rgw_auth_keystone.cc @@ -48,6 +48,10 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp, using RGWValidateKeystoneToken = \ rgw::keystone::Service::RGWValidateKeystoneToken; + bool admin_token_retried = false; + +admin_token_retry: + /* The container for plain response obtained from Keystone. It will be * parsed token_envelope_t::parse method. */ ceph::bufferlist token_body_bl; @@ -72,8 +76,10 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp, } std::string admin_token; - if (rgw::keystone::Service::get_admin_token(dpp, token_cache, config, - y, admin_token) < 0) { + bool admin_token_cached = false; + int ret = rgw::keystone::Service::get_admin_token(dpp, token_cache, config, + y, admin_token, admin_token_cached); + if (ret < 0) { throw -EINVAL; } @@ -82,7 +88,7 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp, validate.set_url(url); - int ret = validate.process(y); + ret = validate.process(y); /* NULL terminate for debug output. */ token_body_bl.append(static_cast(0)); @@ -91,12 +97,26 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp, * Although failure at the parsing phase doesn't impose a threat, * this allows to return proper error code (EACCESS instead of EINVAL * or similar) and thus improves logging. */ - if (validate.get_http_status() == - /* Most likely: wrong admin credentials or admin token. */ - RGWValidateKeystoneToken::HTTP_STATUS_UNAUTHORIZED || - validate.get_http_status() == - /* Most likely: non-existent token supplied by the client. */ - RGWValidateKeystoneToken::HTTP_STATUS_NOTFOUND) { + + /* If admin token is invalid we should expire it from the cache and + try one last time without the cache. */ + bool admin_token_unauthorized = (validate.get_http_status() == + RGWValidateKeystoneToken::HTTP_STATUS_UNAUTHORIZED); + + if (admin_token_unauthorized && admin_token_cached) { + ldpp_dout(dpp, 20) << "invalidating admin_token cache due to 401" << dendl; + token_cache.invalidate_admin(dpp); + + if (!admin_token_retried) { + ldpp_dout(dpp, 20) << "retrying with uncached admin_token" << dendl; + admin_token_retried = true; + goto admin_token_retry; + } + } + + /* If admin token is invalid or token supplied by client is non-existent. */ + if (admin_token_unauthorized || validate.get_http_status() == + RGWValidateKeystoneToken::HTTP_STATUS_NOTFOUND) { ldpp_dout(dpp, 5) << "Failed keystone auth from " << url << " with " << validate.get_http_status() << dendl; return boost::none; @@ -404,8 +424,9 @@ EC2Engine::get_from_keystone(const DoutPrefixProvider* dpp, const std::string_vi /* get authentication token for Keystone. */ std::string admin_token; + bool admin_token_cached = false; int ret = rgw::keystone::Service::get_admin_token(dpp, token_cache, config, - y, admin_token); + y, admin_token, admin_token_cached); if (ret < 0) { ldpp_dout(dpp, 2) << "s3 keystone: cannot get token for keystone access" << dendl; @@ -500,8 +521,9 @@ auto EC2Engine::get_secret_from_keystone(const DoutPrefixProvider* dpp, /* get authentication token for Keystone. */ std::string admin_token; + bool admin_token_cached = false; int ret = rgw::keystone::Service::get_admin_token(dpp, token_cache, config, - y, admin_token); + y, admin_token, admin_token_cached); if (ret < 0) { ldpp_dout(dpp, 2) << "s3 keystone: cannot get token for keystone access" << dendl; diff --git a/src/rgw/rgw_keystone.cc b/src/rgw/rgw_keystone.cc index 7d5264f980eee..7713d3f2a50a6 100644 --- a/src/rgw/rgw_keystone.cc +++ b/src/rgw/rgw_keystone.cc @@ -140,7 +140,8 @@ int Service::get_admin_token(const DoutPrefixProvider *dpp, TokenCache& token_cache, const Config& config, optional_yield y, - std::string& token) + std::string& token, + bool& token_cached) { /* Let's check whether someone uses the deprecated "admin token" feature * based on a shared secret from keystone.conf file. */ @@ -156,6 +157,7 @@ int Service::get_admin_token(const DoutPrefixProvider *dpp, if (token_cache.find_admin(t)) { ldpp_dout(dpp, 20) << "found cached admin token" << dendl; token = t.token.id; + token_cached = true; return 0; } @@ -521,6 +523,11 @@ void TokenCache::invalidate(const DoutPrefixProvider *dpp, const std::string& to tokens.erase(iter); } +void TokenCache::invalidate_admin(const DoutPrefixProvider *dpp) +{ + invalidate(dpp, admin_token_id); +} + bool TokenCache::going_down() const { return down_flag; diff --git a/src/rgw/rgw_keystone.h b/src/rgw/rgw_keystone.h index f800830767d3a..7b18b1c546a32 100644 --- a/src/rgw/rgw_keystone.h +++ b/src/rgw/rgw_keystone.h @@ -123,7 +123,8 @@ public: TokenCache& token_cache, const Config& config, optional_yield y, - std::string& token); + std::string& token, + bool& token_cached); static int issue_admin_token_request(const DoutPrefixProvider *dpp, const Config& config, optional_yield y, @@ -284,6 +285,7 @@ public: void add_admin(const TokenEnvelope& token); void add_barbican(const TokenEnvelope& token); void invalidate(const DoutPrefixProvider *dpp, const std::string& token_id); + void invalidate_admin(const DoutPrefixProvider *dpp); bool going_down() const; private: void add_locked(const std::string& token_id, const TokenEnvelope& token, -- 2.39.5