Skip to content

Commit 098c340

Browse files
authored
Merge pull request #8503 from romayalon/romy-remove-skew-from-presigned-url
Presigned URL - Expiry limit from milliseconds to seconds and epoch fixes + automatic tests
2 parents 6437ba1 + 24ee56f commit 098c340

File tree

4 files changed

+141
-6
lines changed

4 files changed

+141
-6
lines changed

src/test/unit_tests/nc_coretest.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ function setup(options = {}) {
9898
});
9999

100100
// TODO - run health
101+
// wait 2 seconds before announcing nc coretes is ready
102+
await P.delay(2000);
101103
await announce(`nc coretest ready... (took ${((Date.now() - start) / 1000).toFixed(1)} sec)`);
102104
});
103105

src/test/unit_tests/test_bucketspace.js

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* Copyright (C) 2020 NooBaa */
2-
/*eslint max-lines: ["error", 2200]*/
2+
/*eslint max-lines: ["error", 2500]*/
33
/*eslint max-lines-per-function: ["error", 1300]*/
44
/*eslint max-statements: ["error", 80, { "ignoreTopLevelFunctions": true }]*/
55
'use strict';
@@ -12,8 +12,14 @@ const util = require('util');
1212
const http = require('http');
1313
const mocha = require('mocha');
1414
const assert = require('assert');
15+
const http_utils = require('../../util/http_utils');
1516
const config = require('../../../config');
1617
const fs_utils = require('../../util/fs_utils');
18+
const fetch = require('node-fetch');
19+
const P = require('../../util/promise');
20+
const cloud_utils = require('../../util/cloud_utils');
21+
const SensitiveString = require('../../util/sensitive_string');
22+
const S3Error = require('../../../src/endpoint/s3/s3_errors').S3Error;
1723
const test_utils = require('../system_tests/test_utils');
1824
const { stat, open } = require('../../util/nb_native')().fs;
1925
const { get_process_fs_context } = require('../../util/native_fs_utils');
@@ -2125,3 +2131,129 @@ async function delete_anonymous_account(accounts_dir_path, account_config_path)
21252131
console.log('Anonymous account Deleted');
21262132
}
21272133

2134+
mocha.describe('Presigned URL tests', function() {
2135+
this.timeout(50000); // eslint-disable-line no-invalid-this
2136+
const nsr = 'presigned_url_nsr';
2137+
const account_name = 'presigned_url_account';
2138+
const fs_path = path.join(TMP_PATH, 'presigned_url_tests/');
2139+
const presigned_url_bucket = 'presigned-url-bucket';
2140+
const presigned_url_object = 'presigned-url-object.txt';
2141+
const presigned_body = 'presigned_body';
2142+
let s3_client;
2143+
let access_key;
2144+
let secret_key;
2145+
CORETEST_ENDPOINT = coretest.get_http_address();
2146+
let valid_default_presigned_url;
2147+
let presigned_url_params;
2148+
2149+
mocha.before(async function() {
2150+
await fs_utils.create_fresh_path(fs_path);
2151+
await rpc_client.pool.create_namespace_resource({ name: nsr, nsfs_config: { fs_root_path: fs_path } });
2152+
const new_buckets_path = is_nc_coretest ? fs_path : '/';
2153+
const nsfs_account_config = {
2154+
uid: process.getuid(), gid: process.getgid(), new_buckets_path, nsfs_only: true
2155+
};
2156+
const account_params = { ...new_account_params, email: `${account_name}@noobaa.io`, name: account_name, default_resource: nsr, nsfs_account_config };
2157+
const res = await rpc_client.account.create_account(account_params);
2158+
access_key = res.access_keys[0].access_key;
2159+
secret_key = res.access_keys[0].secret_key;
2160+
s3_client = generate_s3_client(access_key.unwrap(), secret_key.unwrap(), CORETEST_ENDPOINT);
2161+
await s3_client.createBucket({ Bucket: presigned_url_bucket });
2162+
await s3_client.putObject({ Bucket: presigned_url_bucket, Key: presigned_url_object, Body: presigned_body });
2163+
2164+
presigned_url_params = {
2165+
bucket: new SensitiveString(presigned_url_bucket),
2166+
key: presigned_url_object,
2167+
endpoint: CORETEST_ENDPOINT,
2168+
access_key: access_key,
2169+
secret_key: secret_key
2170+
};
2171+
valid_default_presigned_url = cloud_utils.get_signed_url(presigned_url_params);
2172+
});
2173+
2174+
mocha.after(async function() {
2175+
if (!is_nc_coretest) return;
2176+
await s3_client.deleteObject({ Bucket: presigned_url_bucket, Key: presigned_url_object });
2177+
await s3_client.deleteBucket({ Bucket: presigned_url_bucket });
2178+
await rpc_client.account.delete_account({ email: `${account_name}@noobaa.io` });
2179+
await fs_utils.folder_delete(fs_path);
2180+
});
2181+
2182+
it('fetch valid presigned URL - 604800 seconds - epoch expiry - should return object data', async () => {
2183+
const data = await fetchData(valid_default_presigned_url);
2184+
assert.equal(data, presigned_body);
2185+
});
2186+
2187+
it('fetch valid presigned URL - 604800 seconds - should return object data - with valid date + expiry in seconds', async () => {
2188+
const now = new Date();
2189+
const valid_url_with_date = valid_default_presigned_url + '&X-Amz-Date=' + now.toISOString() + '&X-Amz-Expires=' + 604800;
2190+
const data = await fetchData(valid_url_with_date);
2191+
assert.equal(data, presigned_body);
2192+
});
2193+
2194+
it('fetch invalid presigned URL - 604800 seconds - epoch expiry + with future date', async () => {
2195+
const now = new Date();
2196+
// Add one hour (3600000 milliseconds)
2197+
const one_hour_in_ms = 60 * 60 * 1000;
2198+
const one_hour_from_now = new Date(now.getTime() + one_hour_in_ms);
2199+
const future_presigned_url = valid_default_presigned_url + '&X-Amz-Date=' + one_hour_from_now.toISOString();
2200+
const expected_err = new S3Error(S3Error.RequestNotValidYet);
2201+
await assert_throws_async(fetchData(future_presigned_url), expected_err.message);
2202+
});
2203+
2204+
it('fetch invalid presigned URL - 604800 expiry seconds + with future date', async () => {
2205+
const now = new Date();
2206+
// Add one hour (3600000 milliseconds)
2207+
const one_hour_in_ms = 60 * 60 * 1000;
2208+
const one_hour_from_now = new Date(now.getTime() + one_hour_in_ms);
2209+
const future_presigned_url = valid_default_presigned_url + '&X-Amz-Date=' + one_hour_from_now.toISOString() + '&X-Amz-Expires=' + 604800;
2210+
const expected_err = new S3Error(S3Error.RequestNotValidYet);
2211+
await assert_throws_async(fetchData(future_presigned_url), expected_err.message);
2212+
});
2213+
2214+
it('fetch invalid presigned URL - 604800 seconds - epoch expiry - URL expired', async () => {
2215+
const expired_presigned_url = cloud_utils.get_signed_url(presigned_url_params, 1);
2216+
// wait for 2 seconds before fetching the url
2217+
await P.delay(2000);
2218+
const expected_err = new S3Error(S3Error.RequestExpired);
2219+
await assert_throws_async(fetchData(expired_presigned_url), expected_err.message);
2220+
});
2221+
2222+
it('fetch invalid presigned URL - 604800 expiry seconds - URL expired', async () => {
2223+
const now = new Date();
2224+
const expired_presigned_url = cloud_utils.get_signed_url(presigned_url_params, 1) + '&X-Amz-Date=' + now.toISOString() + '&X-Amz-Expires=' + 1;
2225+
// wait for 2 seconds before fetching the url
2226+
await P.delay(2000);
2227+
const expected_err = new S3Error(S3Error.RequestExpired);
2228+
await assert_throws_async(fetchData(expired_presigned_url), expected_err.message);
2229+
});
2230+
2231+
it('fetch invalid presigned URL - expiry expoch - expire in bigger than limit', async () => {
2232+
const invalid_expiry = 604800 + 10;
2233+
const invalid_expiry_presigned_url = cloud_utils.get_signed_url(presigned_url_params, invalid_expiry);
2234+
const expected_err = new S3Error(S3Error.AuthorizationQueryParametersError);
2235+
await assert_throws_async(fetchData(invalid_expiry_presigned_url), expected_err.message);
2236+
});
2237+
2238+
it('fetch invalid presigned URL - expire in bigger than limit', async () => {
2239+
const now = new Date();
2240+
const invalid_expiry = 604800 + 10;
2241+
const invalid_expiry_presigned_url = cloud_utils.get_signed_url(presigned_url_params, invalid_expiry) + '&X-Amz-Date=' + now.toISOString() + '&X-Amz-Expires=' + invalid_expiry;
2242+
const expected_err = new S3Error(S3Error.AuthorizationQueryParametersError);
2243+
await assert_throws_async(fetchData(invalid_expiry_presigned_url), expected_err.message);
2244+
});
2245+
});
2246+
2247+
async function fetchData(presigned_url) {
2248+
const response = await fetch(presigned_url, { agent: new http.Agent({ keepAlive: false }) });
2249+
let data;
2250+
if (!response.ok) {
2251+
data = (await response.text()).trim();
2252+
const err_json = (await http_utils.parse_xml_to_js(data)).Error;
2253+
const err = new Error(err_json.Message);
2254+
err.code = err_json.Code;
2255+
throw err;
2256+
}
2257+
data = await response.text();
2258+
return data.trim();
2259+
}

src/util/cloud_utils.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ async function generate_aws_sts_creds(params, roleSessionName) {
5858
);
5959
}
6060

61-
function get_signed_url(params) {
61+
function get_signed_url(params, expiry = 604800) {
6262
const s3 = new AWS.S3({
6363
endpoint: params.endpoint,
6464
credentials: {
@@ -81,7 +81,7 @@ function get_signed_url(params) {
8181
Bucket: params.bucket.unwrap(),
8282
Key: params.key,
8383
VersionId: params.version_id,
84-
Expires: 604800
84+
Expires: expiry
8585
}
8686
);
8787
}

src/util/signature_utils.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,14 +289,15 @@ function check_request_expiry(req) {
289289
_check_expiry_limit(req.query['X-Amz-Expires']);
290290
_check_expiry_query_v4(req.query['X-Amz-Date'], req.query['X-Amz-Expires']);
291291
} else if (req.query.Expires) {
292-
_check_expiry_limit(req.query.Expires);
292+
const expiry_seconds = req.query.Expires - Math.ceil(Date.now() / 1000);
293+
_check_expiry_limit(expiry_seconds);
293294
_check_expiry_query_s3(req.query.Expires);
294295
}
295296
}
296297

297-
// expiry_seconds limit is 7 days
298+
// expiry_seconds limit is 7 days = 604800 seconds
298299
function _check_expiry_limit(expiry_seconds) {
299-
if (Number(expiry_seconds) > 7 * 24 * 60 * 60 * 1000) {
300+
if (Number(expiry_seconds) > 604800) {
300301
throw new S3Error(S3Error.AuthorizationQueryParametersError);
301302
}
302303
}

0 commit comments

Comments
 (0)