]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw: fix http error checks in keystone/barbican/vault clients 53763/head
authorCasey Bodley <cbodley@redhat.com>
Thu, 5 Oct 2023 15:59:52 +0000 (11:59 -0400)
committerCasey Bodley <cbodley@redhat.com>
Wed, 11 Oct 2023 16:22:30 +0000 (12:22 -0400)
when RGWHTTPManager encounters an http error, it uses
rgw_http_error_to_errno() to map that to a negative posix error code.
RGWHTTPClient::process() returns that mapped error code, and exposes the
original http error via get_http_status()

the http client code for keystone, barbican, and vault were returning
early on the errors from process(), so weren't getting to the http error
checks

these clients now check for specific http errors before testing the
result of process()

Fixes: https://tracker.ceph.com/issues/62989
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 36622e19c7fb217f32f74aada0d224785344f565)

Conflicts:
src/rgw/rgw_auth_keystone.cc null_yield
src/rgw/rgw_kms.cc null_yield

src/rgw/rgw_auth_keystone.cc
src/rgw/rgw_kms.cc

index ee16e38dda52fc6b9a1a8f0d9aacdfeb638f07c9..6bc4f372768f18bac9fd5489ac7ea9cb40ec0216 100644 (file)
@@ -75,9 +75,6 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp, const std::string&
   validate.set_url(url);
 
   int ret = validate.process(null_yield);
-  if (ret < 0) {
-    throw ret;
-  }
 
   /* NULL terminate for debug output. */
   token_body_bl.append(static_cast<char>(0));
@@ -96,6 +93,10 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp, const std::string&
                   << validate.get_http_status() << dendl;
     return boost::none;
   }
+  // throw any other http or connection errors
+  if (ret < 0) {
+    throw ret;
+  }
 
   ldpp_dout(dpp, 20) << "received response status=" << validate.get_http_status()
                  << ", body=" << token_body_bl.c_str() << dendl;
@@ -328,11 +329,6 @@ EC2Engine::get_from_keystone(const DoutPrefixProvider* dpp, const std::string_vi
 
   /* send request */
   ret = validate.process(null_yield);
-  if (ret < 0) {
-    ldpp_dout(dpp, 2) << "s3 keystone: token validation ERROR: "
-                  << token_body_bl.c_str() << dendl;
-    throw ret;
-  }
 
   /* if the supplied signature is wrong, we will get 401 from Keystone */
   if (validate.get_http_status() ==
@@ -342,6 +338,12 @@ EC2Engine::get_from_keystone(const DoutPrefixProvider* dpp, const std::string_vi
           decltype(validate)::HTTP_STATUS_NOTFOUND) {
     return std::make_pair(boost::none, -ERR_INVALID_ACCESS_KEY);
   }
+  // throw any other http or connection errors
+  if (ret < 0) {
+    ldpp_dout(dpp, 2) << "s3 keystone: token validation ERROR: "
+                  << token_body_bl.c_str() << dendl;
+    throw ret;
+  }
 
   /* now parse response */
   rgw::keystone::TokenEnvelope token_envelope;
@@ -404,18 +406,19 @@ std::pair<boost::optional<std::string>, int> EC2Engine::get_secret_from_keystone
 
   /* send request */
   ret = secret.process(null_yield);
+
+  /* if the supplied access key isn't found, we will get 404 from Keystone */
+  if (secret.get_http_status() ==
+          decltype(secret)::HTTP_STATUS_NOTFOUND) {
+    return make_pair(boost::none, -ERR_INVALID_ACCESS_KEY);
+  }
+  // return any other http or connection errors
   if (ret < 0) {
     ldpp_dout(dpp, 2) << "s3 keystone: secret fetching error: "
                   << token_body_bl.c_str() << dendl;
     return make_pair(boost::none, ret);
   }
 
-  /* if the supplied signature is wrong, we will get 401 from Keystone */
-  if (secret.get_http_status() ==
-          decltype(secret)::HTTP_STATUS_NOTFOUND) {
-    return make_pair(boost::none, -EINVAL);
-  }
-
   /* now parse response */
 
   JSONParser parser;
index bfea30989b536c4ac752a56fbf36d5d4e4f39e92..8e5f686d352ff4386be60ca3fd85cd9757a3c7fd 100644 (file)
@@ -306,16 +306,17 @@ protected:
     }
 
     res = secret_req.process(null_yield);
-    if (res < 0) {
-      ldpp_dout(dpp, 0) << "ERROR: Request to Vault failed with error " << res << dendl;
-      return res;
-    }
 
+    // map 401 to EACCES instead of EPERM
     if (secret_req.get_http_status() ==
         RGWHTTPTransceiver::HTTP_STATUS_UNAUTHORIZED) {
       ldpp_dout(dpp, 0) << "ERROR: Vault request failed authorization" << dendl;
       return -EACCES;
     }
+    if (res < 0) {
+      ldpp_dout(dpp, 0) << "ERROR: Request to Vault failed with error " << res << dendl;
+      return res;
+    }
 
     ldpp_dout(dpp, 20) << "Request to Vault returned " << res << " and HTTP status "
       << secret_req.get_http_status() << dendl;
@@ -925,13 +926,14 @@ static int request_key_from_barbican(const DoutPrefixProvider *dpp,
   secret_req.append_header("X-Auth-Token", barbican_token);
 
   res = secret_req.process(null_yield);
-  if (res < 0) {
-    return res;
-  }
+  // map 401 to EACCES instead of EPERM
   if (secret_req.get_http_status() ==
       RGWHTTPTransceiver::HTTP_STATUS_UNAUTHORIZED) {
     return -EACCES;
   }
+  if (res < 0) {
+    return res;
+  }
 
   if (secret_req.get_http_status() >=200 &&
       secret_req.get_http_status() < 300 &&