From d57e3a94b7cf1a8c7f691654d9ba74cf6f23da47 Mon Sep 17 00:00:00 2001 From: Dimitrios Vasilas Date: Tue, 25 Feb 2025 10:59:05 +0200 Subject: [PATCH 1/2] CLDSRV-616: Fix bucket policy check for anonymous req When checking bucket policies and the following conditions are true: - The request is anonymous (`--no-sign-request`) - There is a bucket policy with AWS principal Then `_getAccountId` is called in arn === undefined and causes an exception to be thrown. The reason is that vault return the following authInfo with anonymous requests: { arn: undefined, canonicalID: 'http://acs.amazonaws.com/groups/global/AllUsers', shortid: undefined, email: undefined, accountDisplayName: undefined, IAMdisplayName: undefined } The fix is to check is to check is arn === undefined and fail the check if the policy principal is not '*' --- .../apiUtils/authorization/permissionChecks.js | 4 ++++ tests/unit/api/bucketPolicyAuth.js | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index 170488b44b..cfb685ee7a 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -269,6 +269,10 @@ function _checkPrincipal(requester, principal) { if (principal === '*') { return true; } + // User in unauthenticated (anonymous request) + if (requester === undefined) { + return false; + } if (principal === requester) { return true; } diff --git a/tests/unit/api/bucketPolicyAuth.js b/tests/unit/api/bucketPolicyAuth.js index 687466b701..428e9fb954 100644 --- a/tests/unit/api/bucketPolicyAuth.js +++ b/tests/unit/api/bucketPolicyAuth.js @@ -1,5 +1,6 @@ const assert = require('assert'); const { BucketInfo, BucketPolicy } = require('arsenal').models; +const AuthInfo = require('arsenal').auth.AuthInfo; const constants = require('../../../constants'); const { isBucketAuthorized, isObjAuthorized, validatePolicyResource } = require('../../../lib/api/apiUtils/authorization/permissionChecks'); @@ -35,6 +36,9 @@ const basePolicyObj = { }; const bucketName = 'matchme'; const log = new DummyRequestLogger(); +const publicUserAuthInfo = new AuthInfo({ + canonicalID: constants.publicId, +}); const authTests = [ { @@ -292,11 +296,21 @@ describe('bucket policy authorization', () => { it('should allow access to public user if principal is set to "*"', done => { const allowed = isBucketAuthorized(bucket, bucAction, - constants.publicId, null, log); + constants.publicId, publicUserAuthInfo, log); assert.equal(allowed, true); done(); }); + it('should deny access to public user if principal is not set to "*"', function itFn(done) { + const newPolicy = this.test.basePolicy; + newPolicy.Statement[0].Principal = { AWS: authInfo.getArn() }; + bucket.setBucketPolicy(newPolicy); + const allowed = isBucketAuthorized(bucket, bucAction, + constants.publicId, publicUserAuthInfo, log); + assert.equal(allowed, false); + done(); + }); + authTests.forEach(t => { it(`${t.name}bucket owner`, function itFn(done) { const newPolicy = this.test.basePolicy; @@ -376,7 +390,7 @@ describe('bucket policy authorization', () => { it('should allow access to public user if principal is set to "*"', done => { const allowed = isObjAuthorized(bucket, object, objAction, - constants.publicId, null, log); + constants.publicId, publicUserAuthInfo, log); assert.equal(allowed, true); done(); }); From 965a80fe0d8980675b281903749fd389108d06a0 Mon Sep 17 00:00:00 2001 From: Dimitrios Vasilas Date: Tue, 25 Feb 2025 14:09:22 +0200 Subject: [PATCH 2/2] CLDSRV-616: Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ab5bb28d1a..4eb2151298 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.10.51", + "version": "7.10.52", "description": "S3 connector", "main": "index.js", "engines": {