]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw: fix http error checks in keystone/barbican/vault clients 53846/head
authorCasey Bodley <cbodley@redhat.com>
Thu, 5 Oct 2023 15:59:52 +0000 (11:59 -0400)
committerCasey Bodley <cbodley@redhat.com>
Thu, 5 Oct 2023 15:59:56 +0000 (11:59 -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>
src/rgw/rgw_auth_keystone.cc
src/rgw/rgw_kms.cc

index c414e3627e419634828f07fa92a032f13f2ba90b..a1d76c3aaf323a36e58ecf3bce621d28bc455c3a 100644 (file)
@@ -83,9 +83,6 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp,
   validate.set_url(url);
 
   int ret = validate.process(y);
-  if (ret < 0) {
-    throw ret;
-  }
 
   /* NULL terminate for debug output. */
   token_body_bl.append(static_cast<char>(0));
@@ -104,6 +101,10 @@ TokenEngine::get_from_keystone(const DoutPrefixProvider* dpp,
                   << 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;
@@ -443,11 +444,6 @@ EC2Engine::get_from_keystone(const DoutPrefixProvider* dpp, const std::string_vi
 
   /* send request */
   ret = validate.process(y);
-  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() ==
@@ -457,6 +453,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;
@@ -521,18 +523,19 @@ auto EC2Engine::get_secret_from_keystone(const DoutPrefixProvider* dpp,
 
   /* send request */
   ret = secret.process(y);
+
+  /* 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 0249c8aebb04d3bf93d746e6e31d4208ab162d5d..ea30ff868fd97f1cb6edc78b8d5459e09c7df7cc 100644 (file)
@@ -307,16 +307,17 @@ protected:
     }
 
     res = secret_req.process(y);
-    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;
@@ -937,13 +938,14 @@ static int request_key_from_barbican(const DoutPrefixProvider *dpp,
   secret_req.append_header("X-Auth-Token", barbican_token);
 
   res = secret_req.process(y);
-  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 &&