Fix pre-signed url issues

Signed-off-by: Romy <[email protected]>
noobaa · Oct 31, 2024 · a4156b1 · a4156b1
1 parent b27e63b
commit a4156b1
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 5 deletions.
diff --git a/src/endpoint/s3/s3_errors.js b/src/endpoint/s3/s3_errors.js
@@ -534,6 +534,21 @@ S3Error.InvalidEncodingType = Object.freeze({
     message: 'Invalid Encoding Method specified in Request',
     http_code: 400,
 });
+S3Error.AuthorizationQueryParametersError = Object.freeze({
+    code: 'AuthorizationQueryParametersError',
+    message: 'X-Amz-Expires must be less than a week (in seconds); that is, the given X-Amz-Expires must be less than 604800 seconds',
+    http_code: 400,
+});
+S3Error.RequestExpired = Object.freeze({
+    code: 'AccessDenied',
+    message: 'Request has expired',
+    http_code: 403,
+});
+S3Error.RequestNotValidYet = Object.freeze({
+    code: 'AccessDenied',
+    message: 'request is no valid yet',
+    http_code: 403,
+});
 
 ////////////////////////////////////////////////////////////////
 // S3 Select                                                  //

diff --git a/src/util/http_utils.js b/src/util/http_utils.js
@@ -597,8 +597,15 @@ function check_headers(req, options) {
     if (isNaN(req_time) && !req.query.Expires && is_not_anonymous_req) {
         throw new options.ErrorClass(options.error_access_denied);
     }
-
-    if (Math.abs(Date.now() - req_time) > config.AMZ_DATE_MAX_TIME_SKEW_MILLIS) {
+    // future request should throw AccessDenied with request is no valid yet message
+    // we add a grace period of one second
+    if (req_time > Date.now() - 1000) {
+        throw new S3Error(S3Error.RequestNotValidYet);
+    }
+    const is_presigned_url = req.query.Expires || (req.query['X-Amz-Date'] && req.query['X-Amz-Expires']);
+    // on regular requests the skew limit is 15 minutes
+    // on presigned url requests we don't need to check skew
+    if (!is_presigned_url && (Math.abs(Date.now() - req_time) > config.AMZ_DATE_MAX_TIME_SKEW_MILLIS)) {
         throw new options.ErrorClass(options.error_request_time_too_skewed);
     }
 }

diff --git a/src/util/signature_utils.js b/src/util/signature_utils.js
@@ -9,6 +9,7 @@ const path = require('path');
 const crypto = require('crypto');
 const S3Error = require('../endpoint/s3/s3_errors').S3Error;
 const http_utils = require('./http_utils');
+const time_utils = require('./time_utils');
 const { RpcError } = require('../rpc');
 
 
@@ -126,9 +127,9 @@ function _string_to_sign_v4(req, signed_headers, xamzdate, region, service) {
 
 function _check_expiry_query_v4(request_date, expires_seconds) {
     const now = Date.now();
-    const expires = (new Date(request_date).getTime()) + (Number(expires_seconds) * 1000);
+    const expires = (new Date(time_utils.parse_amz_date(request_date)).getTime()) + (Number(expires_seconds) * 1000);
     if (now > expires) {
-        throw new Error('Authentication Expired (V4)');
+        throw new S3Error(S3Error.RequestExpired);
     }
 }
 
@@ -180,7 +181,7 @@ function _check_expiry_query_s3(expires_epoch) {
     const now = Date.now();
     const expires = Number(expires_epoch) * 1000;
     if (now > expires) {
-        throw new Error('Authentication Expired (S3)');
+        throw new S3Error(S3Error.RequestExpired);
     }
 }
 
@@ -285,12 +286,20 @@ function make_auth_token_from_request(req) {
  */
 function check_request_expiry(req) {
     if (req.query['X-Amz-Date'] && req.query['X-Amz-Expires']) {
+        _check_expiry_limit(req.query['X-Amz-Expires']);
         _check_expiry_query_v4(req.query['X-Amz-Date'], req.query['X-Amz-Expires']);
     } else if (req.query.Expires) {
+        _check_expiry_limit(req.query.Expires);
         _check_expiry_query_s3(req.query.Expires);
     }
 }
 
+// expiry_seconds limit is 7 days
+function _check_expiry_limit(expiry_seconds) {
+    if (Number(expiry_seconds) > 7 * 24 * 60 * 60 * 1000) {
+        throw new S3Error(S3Error.AuthorizationQueryParametersError);
+    }
+}
 
 /**
  *