Skip to content

Commit 9f8b097

Browse files
authored
Merge pull request #8493 from romayalon/romy-remove-skew-from-presigned-url
Fix pre-signed url issues
2 parents d37c0fb + 6bb95dc commit 9f8b097

File tree

3 files changed

+36
-5
lines changed

3 files changed

+36
-5
lines changed

src/endpoint/s3/s3_errors.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,21 @@ S3Error.InvalidEncodingType = Object.freeze({
534534
message: 'Invalid Encoding Method specified in Request',
535535
http_code: 400,
536536
});
537+
S3Error.AuthorizationQueryParametersError = Object.freeze({
538+
code: 'AuthorizationQueryParametersError',
539+
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',
540+
http_code: 400,
541+
});
542+
S3Error.RequestExpired = Object.freeze({
543+
code: 'AccessDenied',
544+
message: 'Request has expired',
545+
http_code: 403,
546+
});
547+
S3Error.RequestNotValidYet = Object.freeze({
548+
code: 'AccessDenied',
549+
message: 'request is not valid yet',
550+
http_code: 403,
551+
});
537552

538553
////////////////////////////////////////////////////////////////
539554
// S3 Select //

src/util/http_utils.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,15 @@ function check_headers(req, options) {
597597
if (isNaN(req_time) && !req.query.Expires && is_not_anonymous_req) {
598598
throw new options.ErrorClass(options.error_access_denied);
599599
}
600-
601-
if (Math.abs(Date.now() - req_time) > config.AMZ_DATE_MAX_TIME_SKEW_MILLIS) {
600+
// futureus presigned url request should throw AccessDenied with request is no valid yet message
601+
// we add a grace period of one second
602+
const is_presigned_url = req.query.Expires || (req.query['X-Amz-Date'] && req.query['X-Amz-Expires']);
603+
if (is_presigned_url && (req_time > (Date.now() + 2000))) {
604+
throw new S3Error(S3Error.RequestNotValidYet);
605+
}
606+
// on regular requests the skew limit is 15 minutes
607+
// on presigned url requests we don't need to check skew
608+
if (!is_presigned_url && (Math.abs(Date.now() - req_time) > config.AMZ_DATE_MAX_TIME_SKEW_MILLIS)) {
602609
throw new options.ErrorClass(options.error_request_time_too_skewed);
603610
}
604611
}

src/util/signature_utils.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const path = require('path');
99
const crypto = require('crypto');
1010
const S3Error = require('../endpoint/s3/s3_errors').S3Error;
1111
const http_utils = require('./http_utils');
12+
const time_utils = require('./time_utils');
1213
const { RpcError } = require('../rpc');
1314

1415

@@ -126,9 +127,9 @@ function _string_to_sign_v4(req, signed_headers, xamzdate, region, service) {
126127

127128
function _check_expiry_query_v4(request_date, expires_seconds) {
128129
const now = Date.now();
129-
const expires = (new Date(request_date).getTime()) + (Number(expires_seconds) * 1000);
130+
const expires = (new Date(time_utils.parse_amz_date(request_date)).getTime()) + (Number(expires_seconds) * 1000);
130131
if (now > expires) {
131-
throw new Error('Authentication Expired (V4)');
132+
throw new S3Error(S3Error.RequestExpired);
132133
}
133134
}
134135

@@ -180,7 +181,7 @@ function _check_expiry_query_s3(expires_epoch) {
180181
const now = Date.now();
181182
const expires = Number(expires_epoch) * 1000;
182183
if (now > expires) {
183-
throw new Error('Authentication Expired (S3)');
184+
throw new S3Error(S3Error.RequestExpired);
184185
}
185186
}
186187

@@ -285,12 +286,20 @@ function make_auth_token_from_request(req) {
285286
*/
286287
function check_request_expiry(req) {
287288
if (req.query['X-Amz-Date'] && req.query['X-Amz-Expires']) {
289+
_check_expiry_limit(req.query['X-Amz-Expires']);
288290
_check_expiry_query_v4(req.query['X-Amz-Date'], req.query['X-Amz-Expires']);
289291
} else if (req.query.Expires) {
292+
_check_expiry_limit(req.query.Expires);
290293
_check_expiry_query_s3(req.query.Expires);
291294
}
292295
}
293296

297+
// expiry_seconds limit is 7 days
298+
function _check_expiry_limit(expiry_seconds) {
299+
if (Number(expiry_seconds) > 7 * 24 * 60 * 60 * 1000) {
300+
throw new S3Error(S3Error.AuthorizationQueryParametersError);
301+
}
302+
}
294303

295304
/**
296305
*

0 commit comments

Comments
 (0)