]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw: fix http error checks in keystone/barbican/vault clients 53764/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 21:48:38 +0000 (17:48 -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, dpp

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

index 1bbabca8a97710fcf19316166f44810af0b490a7..9879d339f9cc45068797170d45cced1e43221884 100644 (file)
@@ -74,9 +74,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));
@@ -95,6 +92,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;
@@ -327,11 +328,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() ==
@@ -341,6 +337,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;
@@ -403,18 +405,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 dcdcf875115dbe5f042084cd8b631df2de045954..2fced0d7e4ea9c79fca0060640363c6106d042c4 100644 (file)
@@ -266,16 +266,17 @@ protected:
     }
 
     res = secret_req.process(null_yield);
-    if (res < 0) {
-      ldout(cct, 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) {
       ldout(cct, 0) << "ERROR: Vault request failed authorization" << dendl;
       return -EACCES;
     }
+    if (res < 0) {
+      ldout(cct, 0) << "ERROR: Request to Vault failed with error " << res << dendl;
+      return res;
+    }
 
     ldout(cct, 20) << "Request to Vault returned " << res << " and HTTP status "
       << secret_req.get_http_status() << dendl;
@@ -803,13 +804,14 @@ static int request_key_from_barbican(CephContext *cct,
   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 &&