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 <rzarzynski@mirantis.com>

diff --git a/src/rgw/Makefile.am b/src/rgw/Makefile.am
index 76afdfa0702066ffbf350f280fe620eeccb0db88..9eefc1abbb46a0d7e2e20151803c17e637590cab 100644 (file)
--- 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 5acc3eb653ad6c3c62e6b1ddb0db5dd14c72ba58..dabe282e9b4611f71ed6da50c61ae7fed486bf5a 100644 (file)
--- 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 c8ab2b9b91cde40be37c2ef370f8c8dfb5a60a79..ba18d34965129b6d0bf690c87bba1e2b487b6ed5 100644 (file)
--- 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 af829e335411aa144219e20af4698358202ccc6a..ba3f609ec22d9420bb77960210e5a2714ee098f9 100644 (file)
--- 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 2132da204f168a4540fab3aa677c5a96b8d0f1a0..a97b82fe1cddfd3403c9ff45d1161619a38f00d0 100644 (file)
--- 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 487a93ca128e53206f4bdab87d05822c8746e9a1..831fd5e52a7ba02af22b5937094ccc726a087aab 100644 (file)
--- 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);