rgw: fix return value of auth v2/v4

* The return value of auth v2/v4 in RGW is different from that in AWS:
*     1. When 'Expires' is missing in auth v2 query string request, AWS
*     returns AccessDenied while RGW returns SignatureDoesNotMatch;
*     2. When 'X-Amz-Expires' is missing in auth v4 query string
*     request, AWS returns AuthorizationQueryParametersError while RGW
*     returns RequestTimeTooSkewed;
* Changes:
*     1. When 'Expires' is missing in auth v2 query string request,
*     change RGW's return value to AccessDenied;
*     2. When 'X-Amz-Expires' is missing in auth v4 query string
*     request, change RGW's return value to AccessDenied;
*     3. remove time skew check from parse_v4_query_string;

Fixes: http://tracker.ceph.com/issues/22439
Signed-off-by: Bingyin Zhang <zhangbingyin@cloudin.cn>

diff --git a/src/rgw/rgw_auth_s3.cc b/src/rgw/rgw_auth_s3.cc
index ba137e3f59b509656f6d3318da4a85fc993d3ba0..2d03a401b0c1e6809253e14b89f665cab6a5f6d3 100644 (file)
--- a/src/rgw/rgw_auth_s3.cc
+++ b/src/rgw/rgw_auth_s3.cc
@@ -246,41 +246,24 @@ static inline int parse_v4_query_string(const req_info& info,              /* in
     return -EPERM;
   }
 
-  /* Used for pre-signatured url, We shouldn't return -ERR_REQUEST_TIME_SKEWED
-   * when current time <= X-Amz-Expires */
-  bool qsr = false;
-
-  uint64_t now_req = 0;
-  uint64_t now = ceph_clock_now();
-
   boost::string_view expires = info.args.get("X-Amz-Expires");
-  if (!expires.empty()) {
-    /* X-Amz-Expires provides the time period, in seconds, for which
-       the generated presigned URL is valid. The minimum value
-       you can set is 1, and the maximum is 604800 (seven days) */
-    time_t exp = atoll(expires.data());
-    if ((exp < 1) || (exp > 7*24*60*60)) {
-      dout(10) << "NOTICE: exp out of range, exp = " << exp << dendl;
-      return -EPERM;
-    }
-    /* handle expiration in epoch time */
-    now_req = (uint64_t)internal_timegm(&date_t);
-    if (now >= now_req + exp) {
-      dout(10) << "NOTICE: now = " << now << ", now_req = " << now_req << ", exp = " << exp << dendl;
-      return -EPERM;
-    }
-    qsr = true;
+  if (expires.empty()) {
+    return -EPERM;
   }
-
-  if ((now_req < now - RGW_AUTH_GRACE_MINS * 60 ||
-     now_req > now + RGW_AUTH_GRACE_MINS * 60) && !qsr) {
-    dout(10) << "NOTICE: request time skew too big." << dendl;
-    dout(10) << "now_req = " << now_req << " 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;
-    return -ERR_REQUEST_TIME_SKEWED;
+  /* X-Amz-Expires provides the time period, in seconds, for which
+     the generated presigned URL is valid. The minimum value
+     you can set is 1, and the maximum is 604800 (seven days) */
+  time_t exp = atoll(expires.data());
+  if ((exp < 1) || (exp > 7*24*60*60)) {
+    dout(10) << "NOTICE: exp out of range, exp = " << exp << dendl;
+    return -EPERM;
+  }
+  /* handle expiration in epoch time */
+  uint64_t req_sec = (uint64_t)internal_timegm(&date_t);
+  uint64_t now = ceph_clock_now();
+  if (now >= req_sec + exp) {
+    dout(10) << "NOTICE: now = " << now << ", req_sec = " << req_sec << ", exp = " << exp << dendl;
+    return -EPERM;
   }
 
   signedheaders = info.args.get("X-Amz-SignedHeaders");

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index bc0c4ac18715bc36f768b470f61c031fbcffb2f5..fc1bfe57e65ee0aa0bac4528575445734bcd1c74 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -3923,16 +3923,18 @@ AWSGeneralAbstractor::get_auth_data_v2(const req_state* const s) const
     qsr = true;
 
     boost::string_view expires = s->info.args.get("Expires");
-    if (! expires.empty()) {
-      /* It looks we have the guarantee that expires is a null-terminated,
-       * and thus string_view::data() can be safely used. */
-      const time_t exp = atoll(expires.data());
-      time_t now;
-      time(&now);
-
-      if (now >= exp) {
-        throw -EPERM;
-      }
+    if (expires.empty()) {
+      throw -EPERM;
+    }
+
+    /* It looks we have the guarantee that expires is a null-terminated,
+     * and thus string_view::data() can be safely used. */
+    const time_t exp = atoll(expires.data());
+    time_t now;
+    time(&now);
+
+    if (now >= exp) {
+      throw -EPERM;
     }
   } else {
     /* The "Authorization" HTTP header is being used. */