From 7186d1cbbbb77e03a756b231ff5e097d52c3581c Mon Sep 17 00:00:00 2001
From: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Date: Wed, 10 Feb 2016 15:32:12 +0100
Subject: [PATCH 1/3] rgw: improve debugs around S3/Keystone auth mechanism.

Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
---
 src/rgw/rgw_rest_s3.cc | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index 105ad8711b8..d59123bc174 100644
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -2798,7 +2798,7 @@ int RGW_Auth_S3_Keystone_ValidateToken::validate_s3token(
   int ret = process("POST", keystone_url.c_str());
   if (ret < 0) {
     dout(2) << "s3 keystone: token validation ERROR: " << rx_buffer.c_str()
-	    << dendl;
+            << dendl;
     return -EPERM;
   }
 
@@ -2817,8 +2817,9 @@ int RGW_Auth_S3_Keystone_ValidateToken::validate_s3token(
   }
 
   if (!found) {
-    ldout(cct, 5) << "s3 keystone: user does not hold a matching role; required roles: "
-		  << cct->_conf->rgw_keystone_accepted_roles << dendl;
+    ldout(cct, 5) << "s3 keystone: user does not hold a matching role;"
+                     " required roles: "
+                  << cct->_conf->rgw_keystone_accepted_roles << dendl;
     return -EPERM;
   }
 
@@ -3481,14 +3482,17 @@ int RGW_Auth_S3::authorize_v2(RGWRados *store, struct req_state *s)
 
 	if ((req_sec < now - RGW_AUTH_GRACE_MINS * 60 ||
 	     req_sec > now + RGW_AUTH_GRACE_MINS * 60) && !qsr) {
-	  dout(10) << "req_sec=" << req_sec << " now=" << now
-		   << "; now - RGW_AUTH_GRACE_MINS="
-		   << now - RGW_AUTH_GRACE_MINS * 60
-		   << "; now + RGW_AUTH_GRACE_MINS="
-		   << now + RGW_AUTH_GRACE_MINS * 60 << dendl;
-	  dout(0) << "NOTICE: request time skew too big now="
-		  << utime_t(now, 0) << " req_time="
-		  << s->header_time << dendl;
+	  ldout(s->cct, 10) << "req_sec=" << req_sec << " now=" << now
+                            << "; now - RGW_AUTH_GRACE_MINS="
+                            << now - RGW_AUTH_GRACE_MINS * 60
+                            << "; now + RGW_AUTH_GRACE_MINS="
+                            << now + RGW_AUTH_GRACE_MINS * 60
+                            << dendl;
+
+	  ldout(s->cct, 0)  << "NOTICE: request time skew too big now="
+                            << utime_t(now, 0)
+                            << " req_time=" << s->header_time
+                            << dendl;
 	  return -ERR_REQUEST_TIME_SKEWED;
 	}
 
@@ -3520,7 +3524,7 @@ int RGW_Auth_S3::authorize_v2(RGWRados *store, struct req_state *s)
     /* get the user info */
     if (rgw_get_user_info_by_access_key(store, auth_id, *(s->user)) < 0) {
       dout(5) << "error reading user info, uid=" << auth_id
-	      << " can't authenticate" << dendl;
+              << " can't authenticate" << dendl;
       return -ERR_INVALID_ACCESS_KEY;
     }
 
@@ -3535,14 +3539,14 @@ int RGW_Auth_S3::authorize_v2(RGWRados *store, struct req_state *s)
 
     time_t req_sec = s->header_time.sec();
     if ((req_sec < now - RGW_AUTH_GRACE_MINS * 60 ||
-	 req_sec > now + RGW_AUTH_GRACE_MINS * 60) && !qsr) {
+        req_sec > now + RGW_AUTH_GRACE_MINS * 60) && !qsr) {
       dout(10) << "req_sec=" << req_sec << " now=" << now
-	       << "; now - RGW_AUTH_GRACE_MINS="
-	       << now - RGW_AUTH_GRACE_MINS * 60
-	       << "; now + RGW_AUTH_GRACE_MINS="
-	       << now + RGW_AUTH_GRACE_MINS * 60 << dendl;
-      dout(0) << "NOTICE: request time skew too big now=" << utime_t(now, 0)
-	      << " req_time=" << s->header_time << dendl;
+               << "; now - RGW_AUTH_GRACE_MINS=" << now - RGW_AUTH_GRACE_MINS * 60
+               << "; now + RGW_AUTH_GRACE_MINS=" << now + RGW_AUTH_GRACE_MINS * 60
+               << dendl;
+      dout(0)  << "NOTICE: request time skew too big now=" << utime_t(now, 0)
+               << " req_time=" << s->header_time
+               << dendl;
       return -ERR_REQUEST_TIME_SKEWED;
     }
 

From 19843ce2b5c20e3820548d0b3f73ad9e4f2bc315 Mon Sep 17 00:00:00 2001
From: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Date: Wed, 10 Feb 2016 15:36:26 +0100
Subject: [PATCH 2/3] rgw: enable users of RGWHTTPClient to get HTTP status
 code.

Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
---
 src/rgw/rgw_http_client.cc |  1 +
 src/rgw/rgw_http_client.h  | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/rgw/rgw_http_client.cc b/src/rgw/rgw_http_client.cc
index c233d93a1d7..c04a47d9a4e 100644
--- a/src/rgw/rgw_http_client.cc
+++ b/src/rgw/rgw_http_client.cc
@@ -118,6 +118,7 @@ int RGWHTTPClient::process(const char *method, const char *url)
     dout(0) << "curl_easy_perform returned error: " << error_buf << dendl;
     ret = -EINVAL;
   }
+  curl_easy_getinfo(curl_handle, CURLINFO_RESPONSE_CODE, &http_status);
   curl_easy_cleanup(curl_handle);
   curl_slist_free_all(h);
 
diff --git a/src/rgw/rgw_http_client.h b/src/rgw/rgw_http_client.h
index b390911136b..3235fe11540 100644
--- a/src/rgw/rgw_http_client.h
+++ b/src/rgw/rgw_http_client.h
@@ -19,6 +19,7 @@ class RGWHTTPClient
   bufferlist::iterator send_iter;
   size_t send_len;
   bool has_send_len;
+  long http_status;
 
   rgw_http_req_data *req_data;
 
@@ -33,8 +34,18 @@ protected:
   list<pair<string, string> > headers;
   int init_request(const char *method, const char *url, rgw_http_req_data *req_data);
 public:
-  explicit RGWHTTPClient(CephContext *_cct): send_len (0), has_send_len(false), req_data(NULL), user_info(NULL), cct(_cct) {}
+  static const long HTTP_STATUS_NOSTATUS     = 0;
+  static const long HTTP_STATUS_UNAUTHORIZED = 401;
+
   virtual ~RGWHTTPClient();
+  explicit RGWHTTPClient(CephContext *_cct)
+    : send_len(0),
+      has_send_len(false),
+      http_status(HTTP_STATUS_NOSTATUS),
+      req_data(nullptr),
+      user_info(nullptr),
+      cct(_cct) {
+  }
 
   void set_user_info(void *info) {
     user_info = info;
@@ -57,6 +68,10 @@ public:
     has_send_len = true;
   }
 
+  long get_http_status() const {
+    return http_status;
+  }
+
   int process(const char *method, const char *url);
   int process(const char *url) { return process("GET", url); }
 

From a7a5cf2b919dbdee79307d7cb4a0fc597bf9da76 Mon Sep 17 00:00:00 2001
From: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Date: Wed, 10 Feb 2016 15:33:25 +0100
Subject: [PATCH 3/3] rgw: return proper error codes in S3/Keystone auth.

Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
---
 src/rgw/rgw_rest_s3.cc | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index d59123bc174..81ccc1db201 100644
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -2802,6 +2802,11 @@ int RGW_Auth_S3_Keystone_ValidateToken::validate_s3token(
     return -EPERM;
   }
 
+  /* if the supplied signature is wrong, we will get 401 from Keystone */
+  if (get_http_status() == HTTP_STATUS_UNAUTHORIZED) {
+    return -ERR_SIGNATURE_NO_MATCH;
+  }
+
   /* now parse response */
   if (response.parse(cct, rx_buffer) < 0) {
     dout(2) << "s3 keystone: token parsing failed" << dendl;
@@ -2820,7 +2825,7 @@ int RGW_Auth_S3_Keystone_ValidateToken::validate_s3token(
     ldout(cct, 5) << "s3 keystone: user does not hold a matching role;"
                      " required roles: "
                   << cct->_conf->rgw_keystone_accepted_roles << dendl;
-    return -EPERM;
+    return -ERR_INVALID_ACCESS_KEY;
   }
 
   /* everything seems fine, continue with this user */
@@ -3462,7 +3467,7 @@ int RGW_Auth_S3::authorize_v2(RGWRados *store, struct req_state *s)
   }
 
   /* try keystone auth first */
-  int keystone_result = -EINVAL;
+  int keystone_result = -ERR_INVALID_ACCESS_KEY;;
   if (store->ctx()->_conf->rgw_s3_auth_use_keystone
       && !store->ctx()->_conf->rgw_keystone_url.empty()) {
     dout(20) << "s3 keystone: trying keystone auth" << dendl;
@@ -3471,8 +3476,9 @@ int RGW_Auth_S3::authorize_v2(RGWRados *store, struct req_state *s)
     string token;
 
     if (!rgw_create_s3_canonical_header(s->info,
-					&s->header_time, token, qsr)) {
+                                        &s->header_time, token, qsr)) {
       dout(10) << "failed to create auth header\n" << token << dendl;
+      keystone_result = -EPERM;
     } else {
       keystone_result = keystone_validator.validate_s3token(auth_id, token,
 							    auth_sign);
@@ -3514,18 +3520,17 @@ int RGW_Auth_S3::authorize_v2(RGWRados *store, struct req_state *s)
     }
   }
 
-  /* keystone failed (or not enabled); check if we want to use rados backend */
-  if (!store->ctx()->_conf->rgw_s3_auth_use_rados
-      && keystone_result < 0)
-    return keystone_result;
-
-  /* now try rados backend, but only if keystone did not succeed */
   if (keystone_result < 0) {
+    if (!store->ctx()->_conf->rgw_s3_auth_use_rados) {
+      /* No other auth option possible. Terminate request. */
+      return keystone_result;
+    }
+
     /* get the user info */
     if (rgw_get_user_info_by_access_key(store, auth_id, *(s->user)) < 0) {
       dout(5) << "error reading user info, uid=" << auth_id
               << " can't authenticate" << dendl;
-      return -ERR_INVALID_ACCESS_KEY;
+      return keystone_result;
     }
 
     /* now verify signature */