Merge pull request #8873 from shirady/presignurl-expires-negative

Presigned URL | Add an Error on Expiry with a Negative Number

Author: shirady

docs/NooBaaNonContainerized/S3Ops.md

@@ -99,6 +99,14 @@ Currently we support a couple of options:
 2. Principal by account name: `"Principal": { "AWS": [ "<account-name-1>", "<account-name-2>", ... ,"<account-name-n>"] }`
 3. Principal by account ID: `"Principal": { "AWS": [ "<account-ID-1>", "<account-ID-2>", ... ,"<account-ID-n>"] }`
 
+### Presigned URL Support
+An account can use presigned URLs to grant time-limited access to objects in its bucket without updating the bucket policy. The credentials used by the presigned URL are those of the user who generated the URL (more information in [AWS docs](https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-presigned-url.html)).  
+
+#### Simple Test Using AWS CLI and curl:
+1. Pre-requisites: Create account and a bucket, noobaa service should be running, and upload an object to the bucket. 
+2. Create a presigned URL for a minute (the expiration is in seconds): `aws s3 presign s3://<bucket-name>/<key-name> --expires-in 60 --endpoint-url <endpoint-address>` (if needed you can add `--region us-east-1`).
+3. Then you can use: `curl --insecure <URL>` to test the output (`-v` if you want verbose output).
+
 ### Anonymous Requests Support
 
 Anonymous requests are S3 requests made without an access key or a secret key - 

src/endpoint/s3/s3_errors.js

@@ -546,11 +546,16 @@ S3Error.InvalidEncodingType = Object.freeze({
     message: 'Invalid Encoding Method specified in Request',
     http_code: 400,
 });
-S3Error.AuthorizationQueryParametersError = Object.freeze({
+S3Error.AuthorizationQueryParametersErrorWeek = 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.AuthorizationQueryParametersErrorNonNegative = Object.freeze({
+    code: 'AuthorizationQueryParametersError',
+    message: 'X-Amz-Expires must be non-negative',
+    http_code: 400,
+});
 S3Error.RequestExpired = Object.freeze({
     code: 'AccessDenied',
     message: 'Request has expired',

src/test/unit_tests/test_nsfs_integration.js

@@ -2239,15 +2239,23 @@ mocha.describe('Presigned URL tests', function() {
     it('fetch invalid presigned URL - expiry expoch - expire in bigger than limit', async () => {
         const invalid_expiry = 604800 + 10;
         const invalid_expiry_presigned_url = cloud_utils.get_signed_url(presigned_url_params, invalid_expiry);
-        const expected_err = new S3Error(S3Error.AuthorizationQueryParametersError);
+        const expected_err = new S3Error(S3Error.AuthorizationQueryParametersErrorWeek);
         await assert_throws_async(fetchData(invalid_expiry_presigned_url), expected_err.message);
     });
 
     it('fetch invalid presigned URL - expire in bigger than limit', async () => {
         const now = new Date();
         const invalid_expiry = 604800 + 10;
         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;
-        const expected_err = new S3Error(S3Error.AuthorizationQueryParametersError);
+        const expected_err = new S3Error(S3Error.AuthorizationQueryParametersErrorWeek);
+        await assert_throws_async(fetchData(invalid_expiry_presigned_url), expected_err.message);
+    });
+
+    it('fetch invalid presigned URL - throw an error on exipray with a negative number', async () => {
+        const now = new Date();
+        const invalid_expiry = -7;
+        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;
+        const expected_err = new S3Error(S3Error.AuthorizationQueryParametersErrorNonNegative);
         await assert_throws_async(fetchData(invalid_expiry_presigned_url), expected_err.message);
     });
 

src/util/signature_utils.js

@@ -286,6 +286,7 @@ 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_non_negative(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) {
@@ -298,7 +299,20 @@ function check_request_expiry(req) {
 // expiry_seconds limit is 7 days = 604800 seconds
 function _check_expiry_limit(expiry_seconds) {
     if (Number(expiry_seconds) > 604800) {
-        throw new S3Error(S3Error.AuthorizationQueryParametersError);
+        throw new S3Error(S3Error.AuthorizationQueryParametersErrorWeek);
+    }
+}
+
+/**
+ * _check_expiry_non_negative converts the expiry_seconds that we got
+ * from eq.query['X-Amz-Expires'] to number and checks that it is non negative number
+ * (throws an error otherwise)
+ * Note: we don't run it in the condition of epoch time (req.query.expires)
+ * @param {string} expiry_seconds 
+ */
+function _check_expiry_non_negative(expiry_seconds) {
+    if (Number(expiry_seconds) < 0) {
+        throw new S3Error(S3Error.AuthorizationQueryParametersErrorNonNegative);
     }
 }