CLDSRV-1021: Evaluate the bucket policy against the copy source

lib/api/apiUtils/authorization/permissionChecks.js

@@ -445,7 +445,8 @@ function _checkPrincipals(canonicalID, arn, principal, bucketOwnerCanonicalID) {
 // └───────┼──────────────────┘                       │
 //         └──────────────────────────────────────────┘
 //
-function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies) {
+function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies,
+    policyTarget) {
     let permission = checkBucketPolicyResult.DEFAULT_DENY;
     // if requester is user within bucket owner account, actions should be
     // allowed unless explicitly denied (assumes allowed by IAM policy)
@@ -454,10 +455,20 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l
     }
     let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
 
+    // The policy target, when set, is the resource being authorized when it
+    // differs from the one the request targets (e.g. the source of a copy)
+    let bucketName = request?.bucketName;
+    let objectKey = request?.objectKey;
+    let query = request?.query;
+    if (policyTarget) {
+        bucketName = policyTarget.bucketName;
+        objectKey = policyTarget.objectKey;
+        query = Object.assign({}, query, { versionId: policyTarget.versionId });
+    }
     const ip = request ? requestUtils.getClientIp(request, config) : undefined;
     const isSecure = request ? requestUtils.getHttpProtocolSecurity(request, config) : undefined;
-    const requestContext = request ? new RequestContext(request.headers, request.query,
-        request.bucketName, request.objectKey, ip,
+    const requestContext = request ? new RequestContext(request.headers, query,
+        bucketName, objectKey, ip,
         isSecure, request.resourceType, 's3', null, null,
         null, null, null, null, null, null, null, null, null,
         request.objectLockRetentionDays) : undefined;
@@ -506,7 +517,7 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l
 }
 
 function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner, log,
-    request, aclPermission, results, actionImplicitDenies) {
+    request, aclPermission, results, actionImplicitDenies, policyTarget) {
     const bucketPolicy = bucket.getBucketPolicy();
     let processedResult = results[requestType];
     let aclRequired = false;
@@ -515,7 +526,7 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
         aclRequired = true;
     } else {
         const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
-            bucketOwner, log, request, actionImplicitDenies);
+            bucketOwner, log, request, actionImplicitDenies, policyTarget);
 
         if (bucketPolicyPermission === checkBucketPolicyResult.EXPLICIT_DENY) {
             processedResult = false;
@@ -534,7 +545,7 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
 }
 
 function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, log, request,
-    actionImplicitDeniesInput = {}, isWebsite = false) {
+    actionImplicitDeniesInput = {}, isWebsite = false, policyTarget = undefined) {
     const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput];
     const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput;
     const mainApiCall = requestTypes[0];
@@ -568,7 +579,7 @@ function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, lo
             actionImplicitDenies.objectGet = actionImplicitDenies.objectGet || false;
         }
         const { allowed, aclRequired } = processBucketPolicy(_requestType, bucket, canonicalID, arn,
-            bucket.getOwner(), log, request, aclPermission, results, actionImplicitDenies);
+            bucket.getOwner(), log, request, aclPermission, results, actionImplicitDenies, policyTarget);
         if (aclRequired && request?.serverAccessLog) {
             // eslint-disable-next-line no-param-reassign
             request.serverAccessLog.aclRequired = 'Yes';
@@ -597,7 +608,7 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, aut
 }
 
 function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authInfo, log, request,
-    actionImplicitDeniesInput = {}, isWebsite = false) {
+    actionImplicitDeniesInput = {}, isWebsite = false, policyTarget = undefined) {
     const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput];
     const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput;
     const results = {};
@@ -617,7 +628,7 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
                 permission = 'objectPut';
             }
             results[_requestType] = isBucketAuthorized(bucket, permission, canonicalID, authInfo, log, request,
-                actionImplicitDenies, isWebsite);
+                actionImplicitDenies, isWebsite, policyTarget);
             // User is already authorized on the bucket for FULL_CONTROL or WRITE or
             // bucket has canned ACL public-read-write
             if ((parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete')
@@ -651,7 +662,7 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
         const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName,
             canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall);
         const { allowed, aclRequired } = processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner,
-            log, request, aclPermission, results, actionImplicitDenies);
+            log, request, aclPermission, results, actionImplicitDenies, policyTarget);
         if (aclRequired && request?.serverAccessLog) {
             // eslint-disable-next-line no-param-reassign
             request.serverAccessLog.aclRequired = 'Yes';

lib/api/objectCopy.js

@@ -242,6 +242,9 @@ function objectCopy(authInfo, request, sourceBucket, sourceObject, sourceVersion
          */
         checkQuota: true,
         request,
+        // The request targets the destination, the bucket policy must be
+        // evaluated against the source
+        policyTarget: { bucketName: sourceBucket, objectKey: sourceObject, versionId: sourceVersionId },
     };
     const valPutParams = {
         authInfo,

lib/api/objectPutCopyPart.js

@@ -57,6 +57,9 @@ function objectPutCopyPart(authInfo, request, sourceBucket,
          */
         checkQuota: true,
         request,
+        // The request targets the destination, the bucket policy must be
+        // evaluated against the source
+        policyTarget: { bucketName: sourceBucket, objectKey: sourceObject, versionId: reqVersionId },
     };
 
     const partNumber = Number.parseInt(request.query.partNumber, 10);

lib/metadata/metadataUtils.js

@@ -202,6 +202,8 @@ function metadataGetObjects(bucketName, objectsKeys, log, cb) {
  * @param {string} params.requestType - type of request
  * @param {string} [params.preciseRequestType] - precise type of request
  * @param {object} params.request - http request object
+ * @param {object} [params.policyTarget] - resource to evaluate the bucket
+ *   policy against when it differs from the request target
  *  @param {RequestLogger} log - request logger
  * @param {object} actionImplicitDenies - identity authorization results
  * @return {ArsenalError|null} returns a validation error, or null if validation OK
@@ -212,7 +214,7 @@ function metadataGetObjects(bucketName, objectsKeys, log, cb) {
  * - AccessDenied: bucket is not authorized
  */
 function validateBucket(bucket, params, log, actionImplicitDenies = {}) {
-    const { authInfo, preciseRequestType, request } = params;
+    const { authInfo, preciseRequestType, request, policyTarget } = params;
     let requestType = params.requestType;
     if (bucketShield(bucket, requestType)) {
         log.debug('bucket is shielded from request', {
@@ -249,6 +251,8 @@ function validateBucket(bucket, params, log, actionImplicitDenies = {}) {
             log,
             request,
             actionImplicitDenies,
+            false,
+            policyTarget,
         )
     ) {
         log.debug('access denied for user on bucket', { requestType });
@@ -349,13 +353,19 @@ function checkRateLimitIfNeeded(request, authInfo, bucketMD, log, callback) {
  * @param {object} [params.serverAccessLogOptions] - options for server access logging
  * @param {boolean} [params.serverAccessLogOptions.copySource] - if true, stores source bucket
  *   information in request.sourceServerAccessLog for copy operations
+ * @param {object} [params.policyTarget] - resource to evaluate the bucket policy against
+ *   when it differs from the request target, e.g. the source of a copy
+ * @param {string} params.policyTarget.bucketName - name of bucket
+ * @param {string} params.policyTarget.objectKey - name of object
+ * @param {string} [params.policyTarget.versionId] - version id
  * @param {boolean} actionImplicitDenies - identity authorization results
  * @param {RequestLogger} log - request logger
  * @param {function} callback - callback
  * @return {undefined} - and call callback with params err, bucket md
  */
 function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, callback) {
-    const { authInfo, bucketName, objectKey, versionId, getDeleteMarker, request, withVersionId } = params;
+    const { authInfo, bucketName, objectKey, versionId, getDeleteMarker, request, withVersionId, policyTarget } =
+        params;
     let requestType = params.requestType;
     if (!Array.isArray(requestType)) {
         requestType = [requestType];
@@ -433,6 +443,8 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log,
                         log,
                         request,
                         actionImplicitDenies,
+                        false,
+                        policyTarget,
                     )
                 ) {
                     log.debug('access denied for user on object', { requestType });

tests/functional/aws-node-sdk/test/object/copySourceBucketPolicy.js

@@ -0,0 +1,203 @@
+const assert = require('assert');
+const {
+    S3Client,
+    PutObjectCommand,
+    GetObjectCommand,
+    CopyObjectCommand,
+    CreateMultipartUploadCommand,
+    UploadPartCopyCommand,
+    AbortMultipartUploadCommand,
+    PutBucketPolicyCommand,
+    DeleteBucketPolicyCommand,
+} = require('@aws-sdk/client-s3');
+const {
+    IAMClient,
+    CreateUserCommand,
+    CreateAccessKeyCommand,
+    DeleteAccessKeyCommand,
+    DeleteUserCommand,
+} = require('@aws-sdk/client-iam');
+const { v4: uuid } = require('uuid');
+
+const getConfig = require('../support/config');
+const BucketUtility = require('../../lib/utility/bucket-util');
+const { config } = require('../../../../../lib/Config');
+
+const bucketName = `copy-source-bp-${uuid()}`;
+const userName = `copy-source-bp-user-${uuid()}`;
+const forbiddenKey = 'forbidden/myfile';
+const allowedKey = 'allowed/myfile';
+const mpuKey = 'allowed/mpu-copy';
+const allowedContent = 'allowed content';
+
+// this test needs a real vault to create an IAM user
+const isVaultScality = config.backends.auth !== 'mem';
+const vaultHost = config.vaultd?.host || 'localhost';
+
+const describeWithVault = isVaultScality ? describe : describe.skip;
+
+describeWithVault('Copy source with a bucket policy scoped to key prefixes', () => {
+    const bucketUtilAccount = new BucketUtility('default');
+    const s3ClientAccount = bucketUtilAccount.s3;
+
+    const iamConfig = getConfig('default', { region: 'us-east-1' });
+    iamConfig.endpoint = `http://${vaultHost}:8600`; // define outside of getConfig for Integration
+    const iamClient = new IAMClient(iamConfig);
+
+    let userS3Client;
+    let accessKeyId;
+    let uploadId;
+
+    async function assertAccessDenied(promise) {
+        await assert.rejects(promise, { name: 'AccessDenied' });
+    }
+
+    before(async () => {
+        await bucketUtilAccount.createOne(bucketName);
+        await s3ClientAccount.send(
+            new PutObjectCommand({
+                Bucket: bucketName,
+                Key: forbiddenKey,
+                Body: 'forbidden content',
+            }),
+        );
+        await s3ClientAccount.send(
+            new PutObjectCommand({
+                Bucket: bucketName,
+                Key: allowedKey,
+                Body: allowedContent,
+            }),
+        );
+        await s3ClientAccount.send(
+            new PutBucketPolicyCommand({
+                Bucket: bucketName,
+                Policy: JSON.stringify({
+                    Version: '2012-10-17',
+                    Statement: [
+                        {
+                            Sid: 'AllowReadWriteOnAllowedPrefix',
+                            Effect: 'Allow',
+                            Principal: '*',
+                            Action: ['s3:GetObject', 's3:PutObject'],
+                            Resource: `arn:aws:s3:::${bucketName}/allowed/*`,
+                        },
+                        {
+                            Sid: 'DenyReadOnForbiddenPrefix',
+                            Effect: 'Deny',
+                            Principal: '*',
+                            Action: 's3:GetObject',
+                            Resource: `arn:aws:s3:::${bucketName}/forbidden/*`,
+                        },
+                    ],
+                }),
+            }),
+        );
+
+        // IAM user without any identity policy
+        await iamClient.send(new CreateUserCommand({ UserName: userName }));
+        const accessKeyResponse = await iamClient.send(new CreateAccessKeyCommand({ UserName: userName }));
+        accessKeyId = accessKeyResponse.AccessKey.AccessKeyId;
+        userS3Client = new S3Client(
+            getConfig('default', {
+                credentials: {
+                    accessKeyId,
+                    secretAccessKey: accessKeyResponse.AccessKey.SecretAccessKey,
+                },
+            }),
+        );
+
+        const mpuResponse = await userS3Client.send(
+            new CreateMultipartUploadCommand({
+                Bucket: bucketName,
+                Key: mpuKey,
+            }),
+        );
+        uploadId = mpuResponse.UploadId;
+    });
+
+    after(async () => {
+        if (uploadId) {
+            await s3ClientAccount.send(
+                new AbortMultipartUploadCommand({
+                    Bucket: bucketName,
+                    Key: mpuKey,
+                    UploadId: uploadId,
+                }),
+            );
+        }
+        await s3ClientAccount.send(new DeleteBucketPolicyCommand({ Bucket: bucketName }));
+        await bucketUtilAccount.empty(bucketName);
+        await bucketUtilAccount.deleteOne(bucketName);
+        if (accessKeyId) {
+            await iamClient.send(new DeleteAccessKeyCommand({ UserName: userName, AccessKeyId: accessKeyId }));
+        }
+        await iamClient.send(new DeleteUserCommand({ UserName: userName }));
+    });
+
+    it('should deny GetObject on a source denied by the bucket policy', async () => {
+        await assertAccessDenied(
+            userS3Client.send(
+                new GetObjectCommand({
+                    Bucket: bucketName,
+                    Key: forbiddenKey,
+                }),
+            ),
+        );
+    });
+
+    it('should deny CopyObject from a source denied by the bucket policy', async () => {
+        await assertAccessDenied(
+            userS3Client.send(
+                new CopyObjectCommand({
+                    Bucket: bucketName,
+                    Key: 'allowed/myfile-copy',
+                    CopySource: `${bucketName}/${forbiddenKey}`,
+                }),
+            ),
+        );
+    });
+
+    it('should deny UploadPartCopy from a source denied by the bucket policy', async () => {
+        await assertAccessDenied(
+            userS3Client.send(
+                new UploadPartCopyCommand({
+                    Bucket: bucketName,
+                    Key: mpuKey,
+                    UploadId: uploadId,
+                    PartNumber: 1,
+                    CopySource: `${bucketName}/${forbiddenKey}`,
+                }),
+            ),
+        );
+    });
+
+    it('should allow CopyObject from a source allowed by the bucket policy', async () => {
+        await userS3Client.send(
+            new CopyObjectCommand({
+                Bucket: bucketName,
+                Key: 'allowed/allowed-copy',
+                CopySource: `${bucketName}/${allowedKey}`,
+            }),
+        );
+        const getResponse = await userS3Client.send(
+            new GetObjectCommand({
+                Bucket: bucketName,
+                Key: 'allowed/allowed-copy',
+            }),
+        );
+        assert.strictEqual(await getResponse.Body.transformToString(), allowedContent);
+    });
+
+    it('should allow UploadPartCopy from a source allowed by the bucket policy', async () => {
+        const res = await userS3Client.send(
+            new UploadPartCopyCommand({
+                Bucket: bucketName,
+                Key: mpuKey,
+                UploadId: uploadId,
+                PartNumber: 2,
+                CopySource: `${bucketName}/${allowedKey}`,
+            }),
+        );
+        assert(res.CopyPartResult.ETag, 'Expected a part ETag');
+    });
+});