From 55cbc60d46a71563a0dff5ba7c63683a3f3d6c7a Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Mon, 8 Feb 2016 16:41:36 +0100 Subject: [PATCH] rgw: Keystone token parsing doesn't need to know API version. This patch targets an issue with S3Extension in Keystone: requested tokens always conform to Keystone API v2 - regardless of the version used to make the request. Previous implementation of KeystoneToken::parse() as well as KeystoneToken::decode_json() had to know the API version explicitly. Thus, they might be affected in the future by changes in S3-compatibility middleware of Keystone. Signed-off-by: Radoslaw Zarzynski --- src/rgw/Makefile.am | 3 ++- src/rgw/rgw_json_enc.cc | 57 ++++++++++++++++++++++++++++++----------- src/rgw/rgw_keystone.cc | 19 +++++++++++--- src/rgw/rgw_keystone.h | 6 +---- src/rgw/rgw_rest_s3.h | 6 +---- src/rgw/rgw_swift.cc | 4 +-- 6 files changed, 63 insertions(+), 32 deletions(-) diff --git a/src/rgw/Makefile.am b/src/rgw/Makefile.am index 76afdfa070206..9eefc1abbb46a 100644 --- a/src/rgw/Makefile.am +++ b/src/rgw/Makefile.am @@ -7,7 +7,8 @@ DENCODER_SOURCES += \ rgw/rgw_basic_types.cc \ rgw/rgw_common.cc \ rgw/rgw_env.cc \ - rgw/rgw_json_enc.cc + rgw/rgw_json_enc.cc \ + rgw/rgw_keystone.cc DENCODER_DEPS += -lcurl -lexpat \ libcls_version_client.la \ diff --git a/src/rgw/rgw_json_enc.cc b/src/rgw/rgw_json_enc.cc index 5acc3eb653ad6..dabe282e9b461 100644 --- a/src/rgw/rgw_json_enc.cc +++ b/src/rgw/rgw_json_enc.cc @@ -1096,25 +1096,52 @@ void KeystoneToken::decode_json(JSONObj *root_obj) { JSONDecoder::decode_json("user", user, root_obj, true); - if (version == KeystoneApiVersion::VER_2) { - JSONDecoder::decode_json("token", token, root_obj, true); + const auto version = KeystoneService::get_api_version(); - roles = user.roles_v2; - project = token.tenant_v2; - } else if (version == KeystoneApiVersion::VER_3) { + if (version == KeystoneApiVersion::VER_3) { string expires_iso8601; - struct tm t; - - JSONDecoder::decode_json("expires_at", expires_iso8601, root_obj, true); - - if (parse_iso8601(expires_iso8601.c_str(), &t)) { - token.expires = timegm(&t); + if (JSONDecoder::decode_json("expires_at", expires_iso8601, root_obj)) { + /* VER_3 */ + /* Presence of "expires_at" suggests we are dealing with OpenStack + * Identity API v3 (aka Keystone API v3) token. */ + struct tm t; + + if (parse_iso8601(expires_iso8601.c_str(), &t)) { + token.expires = timegm(&t); + } else { + token.expires = 0; + throw JSONDecoder::err("Failed to parse ISO8601 expiration date" + "from Keystone response."); + } + JSONDecoder::decode_json("roles", roles, root_obj, true); + JSONDecoder::decode_json("project", project, root_obj, true); } else { - token.expires = 0; - throw JSONDecoder::err("Failed to parse ISO8601 expiration date from Keystone response."); + /* fallback: VER_2 */ + JSONDecoder::decode_json("token", token, root_obj, true); + roles = user.roles_v2; + project = token.tenant_v2; + } + } else if (version == KeystoneApiVersion::VER_2) { + if (JSONDecoder::decode_json("token", token, root_obj)) { + /* VER_2 */ + roles = user.roles_v2; + project = token.tenant_v2; + } else { + /* fallback: VER_3 */ + string expires_iso8601; + JSONDecoder::decode_json("expires_at", expires_iso8601, root_obj, true); + + struct tm t; + if (parse_iso8601(expires_iso8601.c_str(), &t)) { + token.expires = timegm(&t); + } else { + token.expires = 0; + throw JSONDecoder::err("Failed to parse ISO8601 expiration date" + "from Keystone response."); + } + JSONDecoder::decode_json("roles", roles, root_obj, true); + JSONDecoder::decode_json("project", project, root_obj, true); } - JSONDecoder::decode_json("roles", roles, root_obj, true); - JSONDecoder::decode_json("project", project, root_obj, true); } } diff --git a/src/rgw/rgw_keystone.cc b/src/rgw/rgw_keystone.cc index c8ab2b9b91cde..ba18d34965129 100644 --- a/src/rgw/rgw_keystone.cc +++ b/src/rgw/rgw_keystone.cc @@ -49,10 +49,21 @@ int KeystoneToken::parse(CephContext *cct, bufferlist& bl) } try { - if (version == KeystoneApiVersion::VER_2) { - JSONDecoder::decode_json("access", *this, &parser, true); - } else if (version == KeystoneApiVersion::VER_3) { - JSONDecoder::decode_json("token", *this, &parser, true); + const auto version = KeystoneService::get_api_version(); + + if (version == KeystoneApiVersion::VER_3) { + if (!JSONDecoder::decode_json("access", *this, &parser)) { + /* Token structure doesn't follow Identity API v2, so the token + * must be in v3. Otherwise we can assume it's wrongly formatted. */ + JSONDecoder::decode_json("token", *this, &parser, true); + } + } else if (version == KeystoneApiVersion::VER_2) { + if (!JSONDecoder::decode_json("token", *this, &parser)) { + /* If the token cannot be parsed according to V2, try V3. */ + JSONDecoder::decode_json("access", *this, &parser, true); + } + } else { + return -ENOTSUP; } } catch (JSONDecoder::err& err) { ldout(cct, 0) << "Keystone token parse error: " << err.message << dendl; diff --git a/src/rgw/rgw_keystone.h b/src/rgw/rgw_keystone.h index af829e335411a..ba3f609ec22d9 100644 --- a/src/rgw/rgw_keystone.h +++ b/src/rgw/rgw_keystone.h @@ -17,9 +17,6 @@ public: }; class KeystoneToken { -protected: - KeystoneApiVersion version; - public: class Domain { public: @@ -67,8 +64,7 @@ public: public: // FIXME: default ctor needs to be eradicated here - KeystoneToken() : version(KeystoneApiVersion::VER_2) {}; - KeystoneToken(KeystoneApiVersion _version) : version(_version) {}; + KeystoneToken() = default; time_t get_expires() { return token.expires; } string get_domain_id() {return project.domain.id;}; string get_domain_name() {return project.domain.name;}; diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h index 2132da204f168..a97b82fe1cddf 100644 --- a/src/rgw/rgw_rest_s3.h +++ b/src/rgw/rgw_rest_s3.h @@ -356,11 +356,7 @@ private: public: explicit RGW_Auth_S3_Keystone_ValidateToken(CephContext *_cct) - : RGWHTTPClient(_cct), - /* This is really crazy but S3Extension in Keystone always - * returns token conforming to v2 - regardless whether you - * have requested v3 or not. */ - response(KeystoneToken(KeystoneApiVersion::VER_2)) { + : RGWHTTPClient(_cct) { get_str_list(cct->_conf->rgw_keystone_accepted_roles, roles_list); } diff --git a/src/rgw/rgw_swift.cc b/src/rgw/rgw_swift.cc index 487a93ca128e5..831fd5e52a7ba 100644 --- a/src/rgw/rgw_swift.cc +++ b/src/rgw/rgw_swift.cc @@ -304,7 +304,7 @@ int RGWSwift::get_keystone_admin_token(CephContext * const cct, int ret = token_req.process("POST", token_url.c_str()); if (ret < 0) return ret; - KeystoneToken t = KeystoneToken(keystone_version); + KeystoneToken t; if (t.parse(cct, token_bl) != 0) return -EINVAL; token = t.token.id; @@ -521,7 +521,7 @@ static bool decode_pki_token(CephContext *cct, const string& token, bufferlist& int RGWSwift::validate_keystone_token(RGWRados *store, const string& token, struct rgw_swift_auth_info *info, RGWUserInfo& rgw_user) { - KeystoneToken t(KeystoneService::get_api_version()); + KeystoneToken t; string token_id; get_token_id(token, token_id); -- 2.39.5