diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 56a144e128..d67de458c8 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -155,7 +155,7 @@ jobs: if: always() build: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: contents: read packages: write @@ -789,7 +789,7 @@ jobs: if: always() kmip-cluster-ft-tests: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 needs: build env: S3BACKEND: file @@ -879,7 +879,7 @@ jobs: }}${{ matrix.opts.globalEncryptionEnabled && '-global' || '' }} - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 needs: build env: S3BACKEND: file diff --git a/lib/api/apiUtils/authorization/prepareRequestContexts.js b/lib/api/apiUtils/authorization/prepareRequestContexts.js index c62ace060a..9775ab14a5 100644 --- a/lib/api/apiUtils/authorization/prepareRequestContexts.js +++ b/lib/api/apiUtils/authorization/prepareRequestContexts.js @@ -241,9 +241,7 @@ function prepareRequestContexts(apiMethod, request, sourceBucket, generateRequestContext('objectPutTaggingVersion'); requestContexts.push(putObjectVersionRequestContext); } - // AWS only returns an object lock error if a version id - // is specified, else continue to create a delete marker - } else if (sourceVersionId && apiMethodAfterVersionCheck === 'objectDeleteVersion') { + } else if (apiMethodAfterVersionCheck === 'objectDeleteVersion') { const deleteRequestContext = generateRequestContext(apiMethodAfterVersionCheck); requestContexts.push(deleteRequestContext); diff --git a/package.json b/package.json index ab91602749..9b9d7dc36e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "9.2.24-6", + "version": "9.2.24-7", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": { diff --git a/tests/functional/aws-node-sdk/test/object/deleteObjectGovernanceBypass.js b/tests/functional/aws-node-sdk/test/object/deleteObjectGovernanceBypass.js new file mode 100644 index 0000000000..88d88c86a2 --- /dev/null +++ b/tests/functional/aws-node-sdk/test/object/deleteObjectGovernanceBypass.js @@ -0,0 +1,218 @@ +const assert = require('assert'); +const { Credentials, IAM, S3 } = require('aws-sdk'); +const { v4: uuid } = require('uuid'); + +const BucketUtility = require('../../lib/utility/bucket-util'); +const getConfig = require('../support/config'); +const { config } = require('../../../../../lib/Config'); + +// this test needs a real vault to create IAM users +const isVaultScality = config.backends.auth !== 'mem'; +const vaultHost = config.vaultd?.host || 'localhost'; + +const describeWithVault = isVaultScality ? describe : describe.skip; + +describeWithVault('DeleteObject of a GOVERNANCE-locked version by an IAM user', () => { + const bucketName = `delete-governance-bypass-${uuid()}`; + const bucketUtil = new BucketUtility('default'); + const iamConfig = getConfig('default', { region: 'us-east-1' }); + iamConfig.endpoint = `http://${vaultHost}:8600`; // define outside of getConfig for Integration + const iamClient = new IAM(iamConfig); + const users = []; + + const allowDelete = { + Effect: 'Allow', + Action: ['s3:DeleteObject'], + Resource: `arn:aws:s3:::${bucketName}/*`, + }; + const allowDeleteVersion = { + Effect: 'Allow', + Action: ['s3:DeleteObjectVersion'], + Resource: `arn:aws:s3:::${bucketName}/*`, + }; + const allowBypass = { + Effect: 'Allow', + Action: ['s3:BypassGovernanceRetention'], + Resource: `arn:aws:s3:::${bucketName}/*`, + }; + // The bucket policy validation refuses s3:BypassGovernanceRetention on an object resource when it is the only + // statement, so the bucket policies name the action with a wildcard + const allowBypassInBucketPolicy = { ...allowBypass, Principal: '*', Action: 's3:Bypass*' }; + + async function putVersion(key) { + const { VersionId } = await bucketUtil.s3 + .putObject({ + Bucket: bucketName, + Key: key, + Body: 'content', + }) + .promise(); + return VersionId; + } + + async function putLockedVersion(key) { + const versionId = await putVersion(key); + await bucketUtil.s3 + .putObjectRetention({ + Bucket: bucketName, + Key: key, + VersionId: versionId, + Retention: { + Mode: 'GOVERNANCE', + RetainUntilDate: new Date(Date.now() + 24 * 3600 * 1000), + }, + }) + .promise(); + return versionId; + } + + // Creates an IAM user with one managed policy and returns its S3 client + async function createUser(statements) { + const name = `test-user-${uuid()}`; + const { Policy } = await iamClient + .createPolicy({ + PolicyName: name, + PolicyDocument: JSON.stringify({ Version: '2012-10-17', Statement: statements }), + }) + .promise(); + await iamClient.createUser({ UserName: name }).promise(); + await iamClient.attachUserPolicy({ UserName: name, PolicyArn: Policy.Arn }).promise(); + const { AccessKey } = await iamClient.createAccessKey({ UserName: name }).promise(); + users.push({ name, policyArn: Policy.Arn, accessKeyId: AccessKey.AccessKeyId }); + return new S3( + getConfig('default', { + credentials: new Credentials(AccessKey.AccessKeyId, AccessKey.SecretAccessKey), + }), + ); + } + + async function deleteUser({ name, policyArn, accessKeyId }) { + await iamClient.deleteAccessKey({ UserName: name, AccessKeyId: accessKeyId }).promise(); + await iamClient.detachUserPolicy({ UserName: name, PolicyArn: policyArn }).promise(); + await iamClient.deletePolicy({ PolicyArn: policyArn }).promise(); + await iamClient.deleteUser({ UserName: name }).promise(); + } + + function putBucketPolicy(statement) { + return bucketUtil.s3 + .putBucketPolicy({ + Bucket: bucketName, + Policy: JSON.stringify({ Version: '2012-10-17', Statement: [statement] }), + }) + .promise(); + } + + function deleteBucketPolicy() { + return bucketUtil.s3.deleteBucketPolicy({ Bucket: bucketName }).promise(); + } + + function deleteWithBypass(s3Client, key, versionId) { + return s3Client + .deleteObject({ + Bucket: bucketName, + Key: key, + VersionId: versionId, + BypassGovernanceRetention: true, + }) + .promise(); + } + + function headVersion(key, versionId) { + return bucketUtil.s3 + .headObject({ + Bucket: bucketName, + Key: key, + VersionId: versionId, + }) + .promise(); + } + + before(async () => { + await bucketUtil.s3 + .createBucket({ + Bucket: bucketName, + ObjectLockEnabledForBucket: true, + }) + .promise(); + }); + + after(async () => { + await bucketUtil.empty(bucketName, true); + await bucketUtil.deleteOne(bucketName); + await Promise.all(users.map(deleteUser)); + }); + + it('should deny the bypass header to a user without s3:BypassGovernanceRetention', async () => { + const key = 'no-bypass-permission'; + const versionId = await putLockedVersion(key); + const s3Client = await createUser([allowDeleteVersion]); + + await assert.rejects(deleteWithBypass(s3Client, key, versionId), { code: 'AccessDenied' }); + await headVersion(key, versionId); + }); + + it('should deny the bypass header on an unlocked version without s3:BypassGovernanceRetention', async () => { + const key = 'unlocked-no-bypass-permission'; + const versionId = await putVersion(key); + const s3Client = await createUser([allowDeleteVersion]); + + await assert.rejects(deleteWithBypass(s3Client, key, versionId), { code: 'AccessDenied' }); + await headVersion(key, versionId); + }); + + it('should add a delete marker with the bypass header and no version ID', async () => { + const key = 'no-version-id'; + const versionId = await putLockedVersion(key); + const s3Client = await createUser([allowDelete]); + + const { DeleteMarker } = await deleteWithBypass(s3Client, key); + assert.strictEqual(DeleteMarker, true); + await headVersion(key, versionId); + }); + + it('should deny the bypass header when an IAM policy denies s3:BypassGovernanceRetention', async () => { + const key = 'iam-deny-bypass'; + const versionId = await putLockedVersion(key); + const s3Client = await createUser([allowDeleteVersion, { ...allowBypass, Effect: 'Deny' }]); + + await assert.rejects(deleteWithBypass(s3Client, key, versionId), { code: 'AccessDenied' }); + await headVersion(key, versionId); + }); + + it('should deny the bypass header when the bucket policy denies s3:BypassGovernanceRetention', async () => { + const key = 'bucket-policy-deny-bypass'; + const versionId = await putLockedVersion(key); + const s3Client = await createUser([allowDeleteVersion, allowBypass]); + await putBucketPolicy({ ...allowBypassInBucketPolicy, Effect: 'Deny' }); + + try { + await assert.rejects(deleteWithBypass(s3Client, key, versionId), { code: 'AccessDenied' }); + } finally { + await deleteBucketPolicy(); + } + await headVersion(key, versionId); + }); + + it('should delete the version for a user with s3:BypassGovernanceRetention', async () => { + const key = 'bypass-permission'; + const versionId = await putLockedVersion(key); + const s3Client = await createUser([allowDeleteVersion, allowBypass]); + + await deleteWithBypass(s3Client, key, versionId); + await assert.rejects(headVersion(key, versionId), { statusCode: 404 }); + }); + + it('should delete the version for a user allowed s3:BypassGovernanceRetention by the bucket policy', async () => { + const key = 'bucket-policy-allow-bypass'; + const versionId = await putLockedVersion(key); + const s3Client = await createUser([allowDeleteVersion]); + await putBucketPolicy(allowBypassInBucketPolicy); + + try { + await deleteWithBypass(s3Client, key, versionId); + } finally { + await deleteBucketPolicy(); + } + await assert.rejects(headVersion(key, versionId), { statusCode: 404 }); + }); +}); diff --git a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js index 0a08b104f3..474f043c02 100644 --- a/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js +++ b/tests/unit/api/apiUtils/authorization/prepareRequestContexts.js @@ -257,8 +257,7 @@ describe('prepareRequestContexts', () => { it('should return s3:DeleteObject for objectDelete method', () => { const apiMethod = 'objectDelete'; const request = makeRequest(); - const results = prepareRequestContexts(apiMethod, request, sourceBucket, - sourceObject, sourceVersionId); + const results = prepareRequestContexts(apiMethod, request); assert.strictEqual(results.length, 1); assert.strictEqual(results[0].getAction(), 's3:DeleteObject'); @@ -269,8 +268,7 @@ describe('prepareRequestContexts', () => { const request = makeRequest({}, { versionId: 'vid1', }); - const results = prepareRequestContexts(apiMethod, request, sourceBucket, - sourceObject, sourceVersionId); + const results = prepareRequestContexts(apiMethod, request); assert.strictEqual(results.length, 1); assert.strictEqual(results[0].getAction(), 's3:DeleteObjectVersion'); @@ -285,8 +283,7 @@ describe('prepareRequestContexts', () => { }, { versionId: 'vid1', }); - const results = prepareRequestContexts(apiMethod, request, sourceBucket, - sourceObject, sourceVersionId); + const results = prepareRequestContexts(apiMethod, request); assert.strictEqual(results.length, 2); const expectedAction1 = 's3:DeleteObjectVersion'; @@ -303,8 +300,7 @@ describe('prepareRequestContexts', () => { const request = makeRequest({ 'x-amz-bypass-governance-retention': 'true', }); - const results = prepareRequestContexts(apiMethod, request, sourceBucket, - sourceObject, sourceVersionId); + const results = prepareRequestContexts(apiMethod, request); assert.strictEqual(results.length, 1); const expectedAction = 's3:DeleteObject';