From ca0a6f4262d3a8b91dd89b2e756cc6251630a27b Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 20 Jun 2023 15:48:04 +0200 Subject: [PATCH 01/13] Handle implcit deny results from IAM. The API is started if IAM returns Allow or an implicit Deny. In these cases, we add a new boolean to the request that serves as a context when checking the Bucket/Object ACL or the Bucket policies. Then, we implement the same authorization logic as AWS, where an implicit deny from IAM and an Allow from the Bucket Policy should allow the request. --- lib/metadata/metadataUtils.js | 64 +++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js index 1bdee2169e..6dded924b3 100644 --- a/lib/metadata/metadataUtils.js +++ b/lib/metadata/metadataUtils.js @@ -110,6 +110,25 @@ function metadataGetObject(bucketName, objectKey, versionId, log, cb) { }); } +/** + * Validate that a bucket is accessible and authorized to the user, + * return a specific error code otherwise + * + * @param {BucketInfo} bucket - bucket info + * @param {object} params - function parameters + * @param {AuthInfo} params.authInfo - AuthInfo class instance, requester's info + * @param {string} params.requestType - type of request + * @param {string} [params.preciseRequestType] - precise type of request + * @param {object} params.request - http request object + * @param {object} actionImplicitDenies - authorization results showing if an action is implictly denied + * @param {RequestLogger} log - request logger + * @return {ArsenalError|null} returns a validation error, or null if validation OK + * The following errors may be returned: + * - NoSuchBucket: bucket is shielded + * - MethodNotAllowed: requester is not bucket owner and asking for a + * bucket policy operation + * - AccessDenied: bucket is not authorized + */ function validateBucket(bucket, params, actionImplicitDenies, log) { const { authInfo, preciseRequestType, request } = params; let requestType = params.requestType; @@ -154,25 +173,40 @@ function validateBucket(bucket, params, actionImplicitDenies, log) { * @return {undefined} - and call callback with params err, bucket md */ function metadataValidateBucketAndObj(params, actionImplicitDenies, log, callback) { - const { authInfo, bucketName, objectKey, versionId, request } = params; + const { authInfo, bucketName, objectKey, versionId, getDeleteMarker, request } = params; let requestType = params.requestType; if (!Array.isArray(requestType)) { requestType = [requestType]; } async.waterfall([ - next => metadataGetBucketAndObject(bucketName, - objectKey, versionId, log, (err, bucket, objMD) => { - if (err) { - // if some implicit actionImplicitDenies, return AccessDenied - // before leaking any state information - if (actionImplicitDenies && Object.values(actionImplicitDenies).some(v => v === true)) { - return next(errors.AccessDenied); - } - return next(err); + next => { + // versionId may be 'null', which asks metadata to fetch the null key specifically + const getOptions = { versionId }; + if (getDeleteMarker) { + getOptions.getDeleteMarker = true; + } + return metadata.getBucketAndObjectMD(bucketName, objectKey, getOptions, log, (err, getResult) => { + if (err) { + // if some implicit iamAuthzResults, return AccessDenied + // before leaking any state information + if (actionImplicitDenies && Object.values(actionImplicitDenies).some(v => v === true)) { + return next(errors.AccessDenied); } - return next(null, bucket, objMD); - }), - (bucket, objMD, next) => { + return next(err); + } + return next(null, getResult); + }); + }, + (getResult, next) => { + const bucket = getResult.bucket ? + BucketInfo.deSerialize(getResult.bucket) : undefined; + if (!bucket) { + log.debug('bucketAttrs is undefined', { + bucket: bucketName, + method: 'metadataValidateBucketAndObj', + }); + return next(errors.NoSuchBucket); + } const validationError = validateBucket(bucket, params, actionImplicitDenies, log); if (validationError) { return next(validationError, bucket); @@ -240,8 +274,8 @@ function metadataGetBucket(requestType, bucketName, log, cb) { * @return {undefined} - and call callback with params err, bucket md */ function metadataValidateBucket(params, actionImplicitDenies, log, callback) { - const { bucketName, requestType } = params; - return metadataGetBucket(requestType, bucketName, log, (err, bucket) => { + const { bucketName } = params; + return metadata.getBucket(bucketName, log, (err, bucket) => { if (err) { return callback(err); } From b392385893dc1d4be0932ce5b834ca52786588bd Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 11 Jul 2023 10:11:07 +0200 Subject: [PATCH 02/13] wip --- lib/api/bucketPutACL.js | 2 + lib/api/bucketPutTagging.js | 83 ++++++++++++++++++++++++++++++++++ lib/metadata/metadataUtils.js | 2 +- tests/unit/api/bucketPutACL.js | 2 +- 4 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 lib/api/bucketPutTagging.js diff --git a/lib/api/bucketPutACL.js b/lib/api/bucketPutACL.js index e62eb12db7..bdbc784a71 100644 --- a/lib/api/bucketPutACL.js +++ b/lib/api/bucketPutACL.js @@ -101,10 +101,12 @@ function bucketPutACL(authInfo, request, log, callback) { acl: newCannedACL, method: 'bucketPutACL', }); + monitoring.promMetrics('PUT', bucketName, 400, 'bucketPutACL'); return next(errors.InvalidArgument); } if (!aclUtils.checkGrantHeaderValidity(request.headers)) { log.trace('invalid acl header'); + monitoring.promMetrics('PUT', bucketName, 400, 'bucketPutACL'); return next(errors.InvalidArgument); } return next(null, bucket); diff --git a/lib/api/bucketPutTagging.js b/lib/api/bucketPutTagging.js new file mode 100644 index 0000000000..bd7a026a59 --- /dev/null +++ b/lib/api/bucketPutTagging.js @@ -0,0 +1,83 @@ +const { waterfall } = require('async'); +const { s3middleware } = require('arsenal'); + + +const collectCorsHeaders = require('../utilities/collectCorsHeaders'); +const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const metadata = require('../metadata/wrapper'); +const { pushMetric } = require('../utapi/utilities'); +const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwner'); +const monitoring = require('../utilities/metrics'); +const { parseTagXml } = s3middleware.tagging; + +/** + * Format of xml request: + + + + + string + string + + + + */ + +/** + * Bucket Put Tagging - Create or update bucket Tagging + * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info + * @param {object} request - http request object + * @param {object} log - Werelogs logger + * @param {function} callback - callback to server + * @return {undefined} + */ +function bucketPutTagging(authInfo, request, log, callback) { + log.debug('processing request', { method: 'bucketPutTagging' }); + + const { bucketName, headers } = request; + const metadataValParams = { + authInfo, + bucketName, + requestType: 'bucketPutTagging', + }; + let bucket = null; + return waterfall([ + next => metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, + (err, b) => { + bucket = b; + return next(err); + }), + next => checkExpectedBucketOwner(headers, bucket, log, next), + next => parseTagXml(request.post, log, next), + (tags, next) => { + const tagArray = []; + Object.keys(tags).forEach(key => { + tagArray.push({ Value: tags[key], Key: key }); + }); + bucket.setTags(tagArray); + metadata.updateBucket(bucket.getName(), bucket, log, err => + next(err)); + }, + ], err => { + const corsHeaders = collectCorsHeaders(request.headers.origin, + request.method, bucket); + if (err) { + log.debug('error processing request', { + error: err, + method: 'bucketPutTagging', + }); + monitoring.promMetrics('PUT', bucketName, err.code, + 'putBucketTagging'); + } else { + monitoring.promMetrics( + 'PUT', bucketName, '200', 'putBucketTagging'); + pushMetric('putBucketTagging', log, { + authInfo, + bucket: bucketName, + }); + } + return callback(err, corsHeaders); + }); +} + +module.exports = bucketPutTagging; diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js index 6dded924b3..186beec154 100644 --- a/lib/metadata/metadataUtils.js +++ b/lib/metadata/metadataUtils.js @@ -187,7 +187,7 @@ function metadataValidateBucketAndObj(params, actionImplicitDenies, log, callbac } return metadata.getBucketAndObjectMD(bucketName, objectKey, getOptions, log, (err, getResult) => { if (err) { - // if some implicit iamAuthzResults, return AccessDenied + // if some implicit actionImplicitDenies, return AccessDenied // before leaking any state information if (actionImplicitDenies && Object.values(actionImplicitDenies).some(v => v === true)) { return next(errors.AccessDenied); diff --git a/tests/unit/api/bucketPutACL.js b/tests/unit/api/bucketPutACL.js index 29e629fd17..399300a65f 100644 --- a/tests/unit/api/bucketPutACL.js +++ b/tests/unit/api/bucketPutACL.js @@ -496,7 +496,7 @@ describe('putBucketACL API', () => { + '', url: '/?acl', query: { acl: '' }, - iamAuthzResults: false, + actionImplicitDenies: false, }; bucketPutACL(authInfo, testACLRequest, log, err => { From 93f14f0fcbfa2fe8ed95146ae7008ba43be1b186 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 11 Jul 2023 11:05:45 +0200 Subject: [PATCH 03/13] wip --- lib/api/bucketDeleteTagging.js | 55 ++++++++++++++++ lib/api/bucketGetTagging.js | 112 +++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 lib/api/bucketDeleteTagging.js create mode 100644 lib/api/bucketGetTagging.js diff --git a/lib/api/bucketDeleteTagging.js b/lib/api/bucketDeleteTagging.js new file mode 100644 index 0000000000..3869f22ec3 --- /dev/null +++ b/lib/api/bucketDeleteTagging.js @@ -0,0 +1,55 @@ +const collectCorsHeaders = require('../utilities/collectCorsHeaders'); +const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const { pushMetric } = require('../utapi/utilities'); +const metadata = require('../metadata/wrapper'); +const util = require('node:util'); +const monitoring = require('../utilities/metrics'); + +/** + * Bucket Delete Tagging - Delete a bucket's Tagging + * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info + * @param {object} request - http request object + * @param {object} log - Werelogs logger + * @param {function} callback - callback to server + * @return {undefined} + */ +async function bucketDeleteTagging(authInfo, request, log, callback) { + const bucketName = request.bucketName; + let error = null; + log.debug('processing request', { method: 'bucketDeleteTagging', bucketName }); + + let bucket; + const metadataValidateBucketPromise = util.promisify(metadataValidateBucket); + let updateBucketPromise = util.promisify(metadata.updateBucket); + // necessary to bind metadata as updateBucket calls 'this', causing undefined otherwise + updateBucketPromise = updateBucketPromise.bind(metadata); + const metadataValParams = { + authInfo, + bucketName, + requestType: 'bucketDeleteTagging', + }; + + try { + bucket = await metadataValidateBucketPromise(metadataValParams, request.actionImplicitDenies, log); + bucket.setTags([]); + // eslint-disable-next-line no-unused-expressions + await updateBucketPromise(bucket.getName(), bucket, log); + pushMetric('deleteBucketTagging', log, { + authInfo, + bucket: bucketName, + }); + monitoring.promMetrics( + 'DELETE', bucketName, '200', 'deleteBucketTagging'); + } catch (err) { + error = err; + log.error('error processing request', { error: err, + method: 'deleteBucketTagging', bucketName }); + monitoring.promMetrics('DELETE', bucketName, err.code, + 'deleteBucketTagging'); + } + const corsHeaders = collectCorsHeaders(request.headers.origin, + request.method, bucket); + return callback(error, corsHeaders); +} + +module.exports = bucketDeleteTagging; diff --git a/lib/api/bucketGetTagging.js b/lib/api/bucketGetTagging.js new file mode 100644 index 0000000000..6dfa6fdcfa --- /dev/null +++ b/lib/api/bucketGetTagging.js @@ -0,0 +1,112 @@ +const { metadataValidateBucket } = require('../metadata/metadataUtils'); +const util = require('node:util'); +const collectCorsHeaders = require('../utilities/collectCorsHeaders'); +const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwner'); +const { pushMetric } = require('../utapi/utilities'); +const monitoring = require('../utilities/metrics'); +const { errors, s3middleware } = require('arsenal'); +const escapeForXml = s3middleware.escapeForXml; + +// Sample XML response: +/* + + + + string + string + + + string + string + + + +*/ + +/** + * @typedef Tag + * @type {object} + * @property {string} Value - Value of the tag. + * @property {string} Key - Key of the tag. + */ +/** + * Convert Versioning Configuration object of a bucket into xml format. + * @param {array.} tags - set of bucket tag + * @return {string} - the converted xml string of the versioning configuration + */ +function tagsToXml(tags) { + const xml = []; + + xml.push(''); + + tags.forEach(tag => { + xml.push(''); + xml.push(`${escapeForXml(tag.Key)}`); + xml.push(`${escapeForXml(tag.Value)}`); + xml.push(''); + }); + + xml.push(''); + + return xml.join(''); +} + +/** + * bucketGetVersioning - Return Versioning Configuration for bucket + * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info + * @param {object} request - http request object + * @param {object} log - Werelogs logger + * @param {function} callback - callback to respond to http request + * with either error code or xml response body + * @return {undefined} + */ +async function bucketGetTagging(authInfo, request, log, callback) { + log.debug('processing request', { method: 'bucketGetTagging' }); + + const { bucketName, headers } = request; + const metadataValidateBucketPromise = util.promisify(metadataValidateBucket); + const checkExpectedBucketOwnerPromise = util.promisify(checkExpectedBucketOwner); + + const metadataValParams = { + authInfo, + bucketName, + requestType: 'bucketGetTagging', + request, + }; + + let bucket; + let xml = null; + + try { + bucket = await metadataValidateBucketPromise(metadataValParams, request.actionImplicitDenies, log); + // eslint-disable-next-line no-unused-expressions + await checkExpectedBucketOwnerPromise(headers, bucket, log); + const tags = bucket.getTags(); + if (!tags || !tags.length) { + log.debug('bucket TagSet does not exist', { + method: 'bucketGetTagging', + }); + throw errors.NoSuchTagSet; + } + xml = tagsToXml(tags); + pushMetric('getBucketTagging', log, { + authInfo, + bucket: bucketName, + }); + const corsHeaders = collectCorsHeaders(request.headers.origin, + request.method, bucket); + monitoring.promMetrics( + 'GET', bucketName, '200', 'getBucketTagging'); + return callback(null, xml, corsHeaders); + } catch (err) { + const corsHeaders = collectCorsHeaders(request.headers.origin, + request.method, bucket); + log.debug('error processing request', + { method: 'bucketGetTagging', error: err }); + monitoring.promMetrics('GET', bucketName, err.code, + 'getBucketTagging'); + return callback(err, corsHeaders); + } +} + +module.exports = bucketGetTagging; From e1e51261ff348a5e27cb89254175846f2f40e7b4 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 11 Jul 2023 11:44:35 +0200 Subject: [PATCH 04/13] test bb From 3e8b8e58cc6b01a111a6c865e1ae8d8d47ee9160 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 18 Jul 2023 18:08:03 +0200 Subject: [PATCH 05/13] checkpoint --- .../authorization/permissionChecks.js | 49 ------------------- tests/unit/api/bucketPutACL.js | 24 --------- 2 files changed, 73 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index 73a4d6167e..a4912f629f 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -440,54 +440,6 @@ function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, return Object.keys(results).every(key => results[key] === true); } -function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo, actionImplicitDenies, log, request) { - if (!Array.isArray(requestTypes)) { - // eslint-disable-next-line no-param-reassign - requestTypes = [requestTypes]; - } - if (actionImplicitDenies === false) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies = {}; - } - // By default, all missing actions are defined as allowed from IAM, to be - // backward compatible - requestTypes.forEach(requestType => { - if (actionImplicitDenies[requestType] === undefined) { - // eslint-disable-next-line no-param-reassign - actionImplicitDenies[requestType] = false; - } - }); - - const results = {}; - requestTypes.forEach(_requestType => { - let arn = null; - if (authInfo) { - arn = authInfo.getArn(); - } - const bucketPolicy = bucket.getBucketPolicy(); - if (!bucketPolicy) { - results[_requestType] = actionImplicitDenies[_requestType] === false; - return; - } - const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, - canonicalID, arn, bucket.getOwner(), log, request); - if (bucketPolicyPermission === 'explicitDeny') { - results[_requestType] = false; - return; - } - // If the bucket policy returns an allow, we accept the request, as the - // IAM response here is either Allow or implicit deny. - if (bucketPolicyPermission === 'allow') { - results[_requestType] = true; - return; - } - results[_requestType] = actionImplicitDenies[_requestType] === false; - }); - - // final result is true if all the results are true - return Object.keys(results).every(key => results[key] === true); -} - function _checkResource(resource, bucketArn) { if (resource === bucketArn) { return true; @@ -544,5 +496,4 @@ module.exports = { checkObjectAcls, validatePolicyResource, isLifecycleSession, - evaluateBucketPolicyWithIAM, }; diff --git a/tests/unit/api/bucketPutACL.js b/tests/unit/api/bucketPutACL.js index 399300a65f..6c3a76ad5e 100644 --- a/tests/unit/api/bucketPutACL.js +++ b/tests/unit/api/bucketPutACL.js @@ -437,30 +437,6 @@ describe('putBucketACL API', () => { }); }); - it('should return an error if multiple AccessControlList section', done => { - const testACLRequest = { - bucketName, - namespace, - headers: { host: `${bucketName}.s3.amazonaws.com` }, - post: '' + - '' + - '79a59df900b949e55d96a1e698fbaced' + - 'fd6e09d98eacf8f8d5218e7cd47ef2be' + - 'OwnerDisplayName' + - '' + - '', - url: '/?acl', - query: { acl: '' }, - actionImplicitDenies: false, - }; - - bucketPutACL(authInfo, testACLRequest, log, err => { - assert.deepStrictEqual(err, errors.MalformedACLError); - done(); - }); - }); - it('should return an error if multiple AccessControlList section', done => { const testACLRequest = { bucketName, From 4360c00c99fe6e4b99ca000d70f026fa435cfdbf Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 12:43:38 +0200 Subject: [PATCH 06/13] testing From 731b70cbff56568099c6a32d80ba9a3b4adfb698 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 13:38:23 +0200 Subject: [PATCH 07/13] testing From 64eb593d1f12f837d4cce61fafb191b61b5e236b Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 14:02:32 +0200 Subject: [PATCH 08/13] wip From bbe46d08ff82649742a880afbcf04a6aaa2f4bbd Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 15:10:48 +0200 Subject: [PATCH 09/13] wip --- lib/api/apiUtils/object/abortMultipartUpload.js | 2 +- lib/api/bucketDelete.js | 2 +- lib/api/bucketDeleteCors.js | 2 +- lib/api/bucketDeleteEncryption.js | 2 +- lib/api/bucketDeleteLifecycle.js | 2 +- lib/api/bucketDeletePolicy.js | 2 +- lib/api/bucketDeleteReplication.js | 2 +- lib/api/bucketDeleteTagging.js | 2 +- lib/api/bucketDeleteWebsite.js | 2 +- lib/api/bucketGet.js | 2 +- lib/api/bucketGetACL.js | 2 +- lib/api/bucketGetCors.js | 2 +- lib/api/bucketGetEncryption.js | 2 +- lib/api/bucketGetLifecycle.js | 2 +- lib/api/bucketGetLocation.js | 2 +- lib/api/bucketGetNotification.js | 2 +- lib/api/bucketGetObjectLock.js | 2 +- lib/api/bucketGetPolicy.js | 2 +- lib/api/bucketGetReplication.js | 2 +- lib/api/bucketGetTagging.js | 2 +- lib/api/bucketGetVersioning.js | 2 +- lib/api/bucketGetWebsite.js | 2 +- lib/api/bucketHead.js | 2 +- lib/api/bucketPutTagging.js | 2 +- lib/api/completeMultipartUpload.js | 2 +- lib/api/initiateMultipartUpload.js | 2 +- lib/api/listMultipartUploads.js | 4 ++-- lib/api/listParts.js | 2 +- lib/api/objectDelete.js | 2 +- lib/api/objectDeleteTagging.js | 3 ++- lib/api/objectGet.js | 3 ++- lib/api/objectGetACL.js | 2 +- lib/api/objectGetLegalHold.js | 2 +- lib/api/objectGetRetention.js | 2 +- lib/api/objectGetTagging.js | 2 +- lib/api/objectHead.js | 3 ++- lib/api/objectPutACL.js | 1 + lib/api/objectPutCopyPart.js | 5 +++-- lib/api/objectPutLegalHold.js | 1 + lib/api/objectPutRetention.js | 1 + lib/api/objectPutTagging.js | 1 + lib/api/websiteGet.js | 8 ++++---- lib/api/websiteHead.js | 6 +++--- lib/routes/routeBackbeat.js | 2 +- 44 files changed, 55 insertions(+), 47 deletions(-) diff --git a/lib/api/apiUtils/object/abortMultipartUpload.js b/lib/api/apiUtils/object/abortMultipartUpload.js index 2a9c999c96..13ff44adc0 100644 --- a/lib/api/apiUtils/object/abortMultipartUpload.js +++ b/lib/api/apiUtils/object/abortMultipartUpload.js @@ -14,7 +14,7 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log, bucketName, objectKey, uploadId, - preciseRequestType: 'multipartDelete', + preciseRequestType: request.apiMethods || 'multipartDelete', request, }; // For validating the request at the destinationBucket level diff --git a/lib/api/bucketDelete.js b/lib/api/bucketDelete.js index 636dcff151..378bcd2e74 100644 --- a/lib/api/bucketDelete.js +++ b/lib/api/bucketDelete.js @@ -27,7 +27,7 @@ function bucketDelete(authInfo, request, log, cb) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketDelete', + requestType: request.apiMethods || 'bucketDelete', request, }; diff --git a/lib/api/bucketDeleteCors.js b/lib/api/bucketDeleteCors.js index 007c229a03..439974bb0a 100644 --- a/lib/api/bucketDeleteCors.js +++ b/lib/api/bucketDeleteCors.js @@ -33,7 +33,7 @@ function bucketDeleteCors(authInfo, request, log, callback) { } log.trace('found bucket in metadata'); - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, request.actionImplicitDenies, log, request)) { log.debug('access denied for user on bucket', { requestType, diff --git a/lib/api/bucketDeleteEncryption.js b/lib/api/bucketDeleteEncryption.js index 5ec5442da1..1d208e3863 100644 --- a/lib/api/bucketDeleteEncryption.js +++ b/lib/api/bucketDeleteEncryption.js @@ -21,7 +21,7 @@ function bucketDeleteEncryption(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketDeleteEncryption', + requestType: request.apiMethods || 'bucketDeleteEncryption', request, }; diff --git a/lib/api/bucketDeleteLifecycle.js b/lib/api/bucketDeleteLifecycle.js index c1e7e9fc66..893239f226 100644 --- a/lib/api/bucketDeleteLifecycle.js +++ b/lib/api/bucketDeleteLifecycle.js @@ -17,7 +17,7 @@ function bucketDeleteLifecycle(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketDeleteLifecycle', + requestType: request.apiMethods || 'bucketDeleteLifecycle', request, }; return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketDeletePolicy.js b/lib/api/bucketDeletePolicy.js index 0c509af630..1796cee7c2 100644 --- a/lib/api/bucketDeletePolicy.js +++ b/lib/api/bucketDeletePolicy.js @@ -16,7 +16,7 @@ function bucketDeletePolicy(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketDeletePolicy', + requestType: request.apiMethods || 'bucketDeletePolicy', request, }; return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketDeleteReplication.js b/lib/api/bucketDeleteReplication.js index 5fb58783bd..f7ce36377d 100644 --- a/lib/api/bucketDeleteReplication.js +++ b/lib/api/bucketDeleteReplication.js @@ -17,7 +17,7 @@ function bucketDeleteReplication(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketDeleteReplication', + requestType: request.apiMethods || 'bucketDeleteReplication', request, }; return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketDeleteTagging.js b/lib/api/bucketDeleteTagging.js index 3869f22ec3..f6990b6365 100644 --- a/lib/api/bucketDeleteTagging.js +++ b/lib/api/bucketDeleteTagging.js @@ -26,7 +26,7 @@ async function bucketDeleteTagging(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketDeleteTagging', + requestType: request.apiMethods || 'bucketDeleteTagging', }; try { diff --git a/lib/api/bucketDeleteWebsite.js b/lib/api/bucketDeleteWebsite.js index 74a0c415ca..426b3c2483 100644 --- a/lib/api/bucketDeleteWebsite.js +++ b/lib/api/bucketDeleteWebsite.js @@ -25,7 +25,7 @@ function bucketDeleteWebsite(authInfo, request, log, callback) { } log.trace('found bucket in metadata'); - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, request.actionImplicitDenies, log, request)) { log.debug('access denied for user on bucket', { requestType, diff --git a/lib/api/bucketGet.js b/lib/api/bucketGet.js index 674303ae54..199a63d34b 100644 --- a/lib/api/bucketGet.js +++ b/lib/api/bucketGet.js @@ -322,7 +322,7 @@ function bucketGet(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGet', + requestType: request.apiMethods || 'bucketGet', request, }; const listParams = { diff --git a/lib/api/bucketGetACL.js b/lib/api/bucketGetACL.js index 8faff12551..a4a4ac7564 100644 --- a/lib/api/bucketGetACL.js +++ b/lib/api/bucketGetACL.js @@ -43,7 +43,7 @@ function bucketGetACL(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetACL', + requestType: request.apiMethods || 'bucketGetACL', request, }; const grantInfo = { diff --git a/lib/api/bucketGetCors.js b/lib/api/bucketGetCors.js index 11b1ebf3d8..f01a344086 100644 --- a/lib/api/bucketGetCors.js +++ b/lib/api/bucketGetCors.js @@ -34,7 +34,7 @@ function bucketGetCors(authInfo, request, log, callback) { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, request.actionImplicitDenies, log, request)) { log.debug('access denied for user on bucket', { requestType, diff --git a/lib/api/bucketGetEncryption.js b/lib/api/bucketGetEncryption.js index 2e6f371ba4..97fbd498f6 100644 --- a/lib/api/bucketGetEncryption.js +++ b/lib/api/bucketGetEncryption.js @@ -22,7 +22,7 @@ function bucketGetEncryption(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetEncryption', + requestType: request.apiMethods || 'bucketGetEncryption', request, }; diff --git a/lib/api/bucketGetLifecycle.js b/lib/api/bucketGetLifecycle.js index a8f8cecb29..9d4f820b1e 100644 --- a/lib/api/bucketGetLifecycle.js +++ b/lib/api/bucketGetLifecycle.js @@ -20,7 +20,7 @@ function bucketGetLifecycle(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetLifecycle', + requestType: request.apiMethods || 'bucketGetLifecycle', request, }; return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketGetLocation.js b/lib/api/bucketGetLocation.js index 0b65879ece..2614b36740 100644 --- a/lib/api/bucketGetLocation.js +++ b/lib/api/bucketGetLocation.js @@ -36,7 +36,7 @@ function bucketGetLocation(authInfo, request, log, callback) { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, request.actionImplicitDenies, log, request)) { log.debug('access denied for account on bucket', { requestType, diff --git a/lib/api/bucketGetNotification.js b/lib/api/bucketGetNotification.js index cf701cc361..4f59ad079d 100644 --- a/lib/api/bucketGetNotification.js +++ b/lib/api/bucketGetNotification.js @@ -37,7 +37,7 @@ function bucketGetNotification(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetNotification', + requestType: request.apiMethods || 'bucketGetNotification', request, }; diff --git a/lib/api/bucketGetObjectLock.js b/lib/api/bucketGetObjectLock.js index 9303ccffc2..0fb488e396 100644 --- a/lib/api/bucketGetObjectLock.js +++ b/lib/api/bucketGetObjectLock.js @@ -33,7 +33,7 @@ function bucketGetObjectLock(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetObjectLock', + requestType: request.apiMethods || 'bucketGetObjectLock', request, }; return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketGetPolicy.js b/lib/api/bucketGetPolicy.js index 7cdd0c9a99..56b42fd261 100644 --- a/lib/api/bucketGetPolicy.js +++ b/lib/api/bucketGetPolicy.js @@ -17,7 +17,7 @@ function bucketGetPolicy(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetPolicy', + requestType: request.apiMethods || 'bucketGetPolicy', request, }; diff --git a/lib/api/bucketGetReplication.js b/lib/api/bucketGetReplication.js index 48a9cadf40..56f19bb43f 100644 --- a/lib/api/bucketGetReplication.js +++ b/lib/api/bucketGetReplication.js @@ -20,7 +20,7 @@ function bucketGetReplication(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetReplication', + requestType: request.apiMethods || 'bucketGetReplication', request, }; return metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketGetTagging.js b/lib/api/bucketGetTagging.js index 6dfa6fdcfa..d0e9d6646e 100644 --- a/lib/api/bucketGetTagging.js +++ b/lib/api/bucketGetTagging.js @@ -70,7 +70,7 @@ async function bucketGetTagging(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetTagging', + requestType: request.apiMethods || 'bucketGetTagging', request, }; diff --git a/lib/api/bucketGetVersioning.js b/lib/api/bucketGetVersioning.js index 9ec1c9a1b4..abf4d88b76 100644 --- a/lib/api/bucketGetVersioning.js +++ b/lib/api/bucketGetVersioning.js @@ -53,7 +53,7 @@ function bucketGetVersioning(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketGetVersioning', + requestType: request.apiMethods || 'bucketGetVersioning', request, }; diff --git a/lib/api/bucketGetWebsite.js b/lib/api/bucketGetWebsite.js index e47e98fe48..2a15e58a1e 100644 --- a/lib/api/bucketGetWebsite.js +++ b/lib/api/bucketGetWebsite.js @@ -34,7 +34,7 @@ function bucketGetWebsite(authInfo, request, log, callback) { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); - if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, + if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo, request.actionImplicitDenies, log, request)) { log.debug('access denied for user on bucket', { requestType, diff --git a/lib/api/bucketHead.js b/lib/api/bucketHead.js index df33bee472..f74728b9d7 100644 --- a/lib/api/bucketHead.js +++ b/lib/api/bucketHead.js @@ -18,7 +18,7 @@ function bucketHead(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketHead', + requestType: request.apiMethods || 'bucketHead', request, }; metadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => { diff --git a/lib/api/bucketPutTagging.js b/lib/api/bucketPutTagging.js index bd7a026a59..0d1442b3c1 100644 --- a/lib/api/bucketPutTagging.js +++ b/lib/api/bucketPutTagging.js @@ -38,7 +38,7 @@ function bucketPutTagging(authInfo, request, log, callback) { const metadataValParams = { authInfo, bucketName, - requestType: 'bucketPutTagging', + requestType: request.apiMethods || 'bucketPutTagging', }; let bucket = null; return waterfall([ diff --git a/lib/api/completeMultipartUpload.js b/lib/api/completeMultipartUpload.js index 841441e156..1c01eebbfb 100644 --- a/lib/api/completeMultipartUpload.js +++ b/lib/api/completeMultipartUpload.js @@ -118,7 +118,7 @@ function completeMultipartUpload(authInfo, request, log, callback) { bucketName, // Required permissions for this action // at the destinationBucket level are same as objectPut - requestType: 'objectPut', + requestType: request.apiMethods || 'objectPut', }; metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, next); }, diff --git a/lib/api/initiateMultipartUpload.js b/lib/api/initiateMultipartUpload.js index 50ed82d531..58079c5912 100644 --- a/lib/api/initiateMultipartUpload.js +++ b/lib/api/initiateMultipartUpload.js @@ -92,7 +92,7 @@ function initiateMultipartUpload(authInfo, request, log, callback) { authInfo, bucketName, // Required permissions for this action are same as objectPut - requestType: 'objectPut', + requestType: request.apiMethods || 'objectPut', request, }; const accountCanonicalID = authInfo.getCanonicalID(); diff --git a/lib/api/listMultipartUploads.js b/lib/api/listMultipartUploads.js index 5f50d17650..9bdb184843 100644 --- a/lib/api/listMultipartUploads.js +++ b/lib/api/listMultipartUploads.js @@ -95,8 +95,8 @@ function listMultipartUploads(authInfo, request, log, callback) { // to list the multipart uploads so we have provided here that // the authorization to list multipart uploads is the same // as listing objects in a bucket. - requestType: 'bucketGet', - preciseRequestType: 'listMultipartUploads', + requestType: request.apiMethods || 'bucketGet', + preciseRequestType: request.apiMethods || 'listMultipartUploads', request, }; diff --git a/lib/api/listParts.js b/lib/api/listParts.js index 0a5e227482..8e2e1760e7 100644 --- a/lib/api/listParts.js +++ b/lib/api/listParts.js @@ -95,7 +95,7 @@ function listParts(authInfo, request, log, callback) { bucketName, objectKey, uploadId, - preciseRequestType: 'listParts', + preciseRequestType: request.apiMethods || 'listParts', request, }; // For validating the request at the destinationBucket level diff --git a/lib/api/objectDelete.js b/lib/api/objectDelete.js index ee47a83cd5..cd278581b5 100644 --- a/lib/api/objectDelete.js +++ b/lib/api/objectDelete.js @@ -49,7 +49,7 @@ function objectDelete(authInfo, request, log, cb) { bucketName, objectKey, versionId: reqVersionId, - requestType: 'objectDelete', + requestType: request.apiMethods || 'objectDelete', request, }; diff --git a/lib/api/objectDeleteTagging.js b/lib/api/objectDeleteTagging.js index f9aa5ad809..3ce8233406 100644 --- a/lib/api/objectDeleteTagging.js +++ b/lib/api/objectDeleteTagging.js @@ -40,8 +40,9 @@ function objectDeleteTagging(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectDeleteTagging', versionId: reqVersionId, + getDeleteMarker: true, + requestType: request.apiMethods || 'objectDeleteTagging', request, }; diff --git a/lib/api/objectGet.js b/lib/api/objectGet.js index 75c7bd13b6..e03eef919a 100644 --- a/lib/api/objectGet.js +++ b/lib/api/objectGet.js @@ -44,7 +44,8 @@ function objectGet(authInfo, request, returnTagCount, log, callback) { bucketName, objectKey, versionId, - requestType: 'objectGet', + getDeleteMarker: true, + requestType: request.apiMethods || 'objectGet', request, }; diff --git a/lib/api/objectGetACL.js b/lib/api/objectGetACL.js index d96f0a62d3..274d10ad30 100644 --- a/lib/api/objectGetACL.js +++ b/lib/api/objectGetACL.js @@ -58,7 +58,7 @@ function objectGetACL(authInfo, request, log, callback) { bucketName, objectKey, versionId, - requestType: 'objectGetACL', + requestType: request.apiMethods || 'objectGetACL', request, }; const grantInfo = { diff --git a/lib/api/objectGetLegalHold.js b/lib/api/objectGetLegalHold.js index a450314a2b..8a76fb0c4c 100644 --- a/lib/api/objectGetLegalHold.js +++ b/lib/api/objectGetLegalHold.js @@ -37,8 +37,8 @@ function objectGetLegalHold(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectGetLegalHold', versionId, + requestType: request.apiMethods || 'objectGetLegalHold', request, }; diff --git a/lib/api/objectGetRetention.js b/lib/api/objectGetRetention.js index 825843ac4b..a9111c718f 100644 --- a/lib/api/objectGetRetention.js +++ b/lib/api/objectGetRetention.js @@ -37,8 +37,8 @@ function objectGetRetention(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectGetRetention', versionId: reqVersionId, + requestType: request.apiMethods || 'objectGetRetention', request, }; diff --git a/lib/api/objectGetTagging.js b/lib/api/objectGetTagging.js index 98657156a0..c44c3c7752 100644 --- a/lib/api/objectGetTagging.js +++ b/lib/api/objectGetTagging.js @@ -37,8 +37,8 @@ function objectGetTagging(authInfo, request, log, callback) { authInfo, bucketName, objectKey, - requestType: 'objectGetTagging', versionId: reqVersionId, + requestType: request.apiMethods || 'objectGetTagging', request, }; diff --git a/lib/api/objectHead.js b/lib/api/objectHead.js index 38977bad88..d57d65b630 100644 --- a/lib/api/objectHead.js +++ b/lib/api/objectHead.js @@ -44,7 +44,8 @@ function objectHead(authInfo, request, log, callback) { bucketName, objectKey, versionId, - requestType: 'objectHead', + getDeleteMarker: true, + requestType: request.apiMethods || 'objectHead', request, }; diff --git a/lib/api/objectPutACL.js b/lib/api/objectPutACL.js index d0bb3b3fd8..389b83a4fa 100644 --- a/lib/api/objectPutACL.js +++ b/lib/api/objectPutACL.js @@ -82,6 +82,7 @@ function objectPutACL(authInfo, request, log, cb) { bucketName, objectKey, versionId: reqVersionId, + getDeleteMarker: true, requestType: request.apiMethods || 'objectPutACL', }; diff --git a/lib/api/objectPutCopyPart.js b/lib/api/objectPutCopyPart.js index 5572c4149d..d8c57e3952 100644 --- a/lib/api/objectPutCopyPart.js +++ b/lib/api/objectPutCopyPart.js @@ -43,7 +43,8 @@ function objectPutCopyPart(authInfo, request, sourceBucket, bucketName: sourceBucket, objectKey: sourceObject, versionId: reqVersionId, - requestType: 'objectGet', + getDeleteMarker: true, + requestType: request.apiMethods || 'objectGet', request, }; @@ -63,7 +64,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket, authInfo, bucketName: destBucketName, objectKey: destObjectKey, - requestType: 'objectPut', + requestType: request.apiMethods || 'objectPut', request, }; diff --git a/lib/api/objectPutLegalHold.js b/lib/api/objectPutLegalHold.js index 7f04d4197f..51d5e05ef0 100644 --- a/lib/api/objectPutLegalHold.js +++ b/lib/api/objectPutLegalHold.js @@ -41,6 +41,7 @@ function objectPutLegalHold(authInfo, request, log, callback) { bucketName, objectKey, versionId, + getDeleteMarker: true, requestType: request.apiMethods || 'objectPutLegalHold', request, }; diff --git a/lib/api/objectPutRetention.js b/lib/api/objectPutRetention.js index 8084e26438..41d4072bd3 100644 --- a/lib/api/objectPutRetention.js +++ b/lib/api/objectPutRetention.js @@ -42,6 +42,7 @@ function objectPutRetention(authInfo, request, log, callback) { bucketName, objectKey, versionId: reqVersionId, + getDeleteMarker: true, requestType: request.apiMethods || 'objectPutRetention', request, }; diff --git a/lib/api/objectPutTagging.js b/lib/api/objectPutTagging.js index 8e857926d6..8db01cfda1 100644 --- a/lib/api/objectPutTagging.js +++ b/lib/api/objectPutTagging.js @@ -42,6 +42,7 @@ function objectPutTagging(authInfo, request, log, callback) { bucketName, objectKey, versionId: reqVersionId, + getDeleteMarker: true, requestType: request.apiMethods || 'objectPutTagging', request, }; diff --git a/lib/api/websiteGet.js b/lib/api/websiteGet.js index 0bbfbbedba..9c88e4f03a 100644 --- a/lib/api/websiteGet.js +++ b/lib/api/websiteGet.js @@ -47,7 +47,7 @@ function _errorActions(err, errorDocument, routingRules, } // return the default error message if the object is private // rather than sending a stored error file - if (!isObjAuthorized(bucket, errObjMD, 'objectGet', + if (!isObjAuthorized(bucket, errObjMD, request.apiMethods || 'objectGet', constants.publicId, null, request.actionImplicitDenies, log)) { log.trace('errorObj not authorized', { error: err }); return callback(err, true, null, corsHeaders); @@ -144,8 +144,8 @@ function websiteGet(request, log, callback) { log.trace('error retrieving object metadata', { error: err }); let returnErr = err; - const bucketAuthorized = isBucketAuthorized(bucket, - 'bucketGet', constants.publicId, null, request.actionImplicitDenies, log, request); + const bucketAuthorized = isBucketAuthorized(bucket, request.apiMethods || 'bucketGet', + constants.publicId, null, request.actionImplicitDenies, log, request); // if index object does not exist and bucket is private AWS // returns 403 - AccessDenied error. if (err.is.NoSuchKey && !bucketAuthorized) { @@ -156,7 +156,7 @@ function websiteGet(request, log, callback) { bucket, reqObjectKey, corsHeaders, request, log, callback); } - if (!isObjAuthorized(bucket, objMD, 'objectGet', + if (!isObjAuthorized(bucket, objMD, request.apiMethods || 'objectGet', constants.publicId, null, request.actionImplicitDenies, log, request)) { const err = errors.AccessDenied; log.trace('request not authorized', { error: err }); diff --git a/lib/api/websiteHead.js b/lib/api/websiteHead.js index 493ce8dad6..31684b460a 100644 --- a/lib/api/websiteHead.js +++ b/lib/api/websiteHead.js @@ -103,8 +103,8 @@ function websiteHead(request, log, callback) { log.trace('error retrieving object metadata', { error: err }); let returnErr = err; - const bucketAuthorized = isBucketAuthorized(bucket, - 'bucketGet', constants.publicId, null, request.actionImplicitDenies, log, request); + const bucketAuthorized = isBucketAuthorized(bucket, request.apiMethods || 'bucketGet', + constants.publicId, null, request.actionImplicitDenies, log, request); // if index object does not exist and bucket is private AWS // returns 403 - AccessDenied error. if (err.is.NoSuchKey && !bucketAuthorized) { @@ -113,7 +113,7 @@ function websiteHead(request, log, callback) { return _errorActions(returnErr, routingRules, reqObjectKey, corsHeaders, log, callback); } - if (!isObjAuthorized(bucket, objMD, 'objectGet', + if (!isObjAuthorized(bucket, objMD, request.apiMethods || 'objectGet', constants.publicId, null, request.actionImplicitDenies, log, request)) { const err = errors.AccessDenied; log.trace('request not authorized', { error: err }); diff --git a/lib/routes/routeBackbeat.js b/lib/routes/routeBackbeat.js index 60016ee9a2..d21be91c73 100644 --- a/lib/routes/routeBackbeat.js +++ b/lib/routes/routeBackbeat.js @@ -1272,7 +1272,7 @@ function routeBackbeat(clientIP, request, response, log) { objectKey: request.objectKey, authInfo: userInfo, versionId, - requestType: 'ReplicateObject', + requestType: request.apiMethods || 'ReplicateObject', request, }; return metadataValidateBucketAndObj(mdValParams, request.actionImplicitDenies, log, next); From 083de41f3cae9e167d1d1b4a6de506994f2b67f4 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 18:42:51 +0200 Subject: [PATCH 10/13] checkpoint --- lib/api/apiUtils/authorization/permissionChecks.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index a4912f629f..ddf547315b 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -12,8 +12,8 @@ const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) { // Same logic applies on the Versioned APIs, so let's simplify it. - const requestTypeParsed = requestType.endsWith('Version') - ? requestType.slice(0, -7) : requestType; + const requestTypeParsed = requestType.endsWith('Version') ? + requestType.slice(0, -7) : requestType; if (bucket.getOwner() === canonicalID) { return true; } @@ -196,9 +196,9 @@ function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIs // allow public reads on buckets that are whitelisted for anonymous reads // TODO: remove this after bucket policies are implemented const bucketAcl = bucket.getAcl(); - const allowPublicReads = publicReadBuckets.includes(bucket.getName()) - && bucketAcl.Canned === 'public-read' - && (requestType === 'objectGet' || requestType === 'objectHead'); + const allowPublicReads = publicReadBuckets.includes(bucket.getName()) && + bucketAcl.Canned === 'public-read' && + (requestType === 'objectGet' || requestType === 'objectHead'); if (allowPublicReads) { return true; } From 76ac3a1bdcb3c45cd10a805095170586e4b9e07e Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 19:36:42 +0200 Subject: [PATCH 11/13] checkpoint --- .../apiUtils/authorization/permissionChecks.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index ddf547315b..fe7142d2d5 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -34,6 +34,11 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) { } } + // Backward compatibility + if (requestTypeParsed === 'objectGetTagging') { + return true; + } + const bucketAcl = bucket.getAcl(); if (requestTypeParsed === 'bucketGet' || requestTypeParsed === 'bucketHead') { if (bucketAcl.Canned === 'public-read' @@ -376,12 +381,12 @@ function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, const results = {}; const mainApiCall = requestTypes[0]; requestTypes.forEach(_requestType => { - const parsedMethodName = _requestType.endsWith('Version') - ? _requestType.slice(0, -7) : _requestType; + const parsedMethodName = _requestType.endsWith('Version') ? + _requestType.slice(0, -7) : _requestType; const bucketOwner = bucket.getOwner(); if (!objectMD) { - // User is already authorized on the bucket for FULL_CONTROL or WRITE or - // bucket has canned ACL public-read-write + // 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') { results[_requestType] = actionImplicitDenies[_requestType] === false; return; @@ -409,8 +414,8 @@ function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, // - account is the bucket owner // - requester is account, not user if (bucketOwnerActions.includes(parsedMethodName) - && (bucketOwner === canonicalID) - && requesterIsNotUser) { + && (bucketOwner === canonicalID) + && requesterIsNotUser) { results[_requestType] = actionImplicitDenies[_requestType] === false; return; } From 85d0c8fc48cc4678d36d75ca35d1e32f770b47a7 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 21:50:40 +0200 Subject: [PATCH 12/13] wip --- .../authorization/permissionChecks.js | 56 +++++++++++++++++-- lib/api/multiObjectDelete.js | 28 ++++++++-- lib/api/objectGetTagging.js | 1 - lib/api/objectPutCopyPart.js | 6 +- 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js index fe7142d2d5..53ddf792c2 100644 --- a/lib/api/apiUtils/authorization/permissionChecks.js +++ b/lib/api/apiUtils/authorization/permissionChecks.js @@ -19,12 +19,13 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) { } // Backward compatibility const arrayOfAllowed = [ + 'objectGetTagging', 'objectPutTagging', 'objectPutLegalHold', 'objectPutRetention', ]; if (mainApiCall === 'objectGet') { - if (requestTypeParsed === 'objectGetTagging') { + if (arrayOfAllowed.includes(requestTypeParsed)) { return true; } } @@ -34,11 +35,6 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) { } } - // Backward compatibility - if (requestTypeParsed === 'objectGetTagging') { - return true; - } - const bucketAcl = bucket.getAcl(); if (requestTypeParsed === 'bucketGet' || requestTypeParsed === 'bucketHead') { if (bucketAcl.Canned === 'public-read' @@ -360,6 +356,53 @@ function isBucketAuthorized(bucket, requestTypes, canonicalID, authInfo, actionI return Object.keys(results).every(key => results[key] === true); } +function evaluateBucketPolicyWithIAM(bucket, requestTypes, canonicalID, authInfo, actionImplicitDenies, log, request) { + if (!Array.isArray(requestTypes)) { + // eslint-disable-next-line no-param-reassign + requestTypes = [requestTypes]; + } + if (iamAuthzResults === false) { + // eslint-disable-next-line no-param-reassign + iamAuthzResults = {}; + } + // By default, all missing actions are defined as allowed from IAM, to be + // backward compatible + requestTypes.forEach(requestType => { + if (actionImplicitDenies[requestType] === undefined) { + // eslint-disable-next-line no-param-reassign + actionImplicitDenies[requestType] = false; + } + }); + + const results = {}; + requestTypes.forEach(_requestType => { + let arn = null; + if (authInfo) { + arn = authInfo.getArn(); + } + const bucketPolicy = bucket.getBucketPolicy(); + if (!bucketPolicy) { + results[_requestType] = actionImplicitDenies[_requestType] === false; + return; + } + const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, _requestType, + canonicalID, arn, bucket.getOwner(), log, request); + if (bucketPolicyPermission === 'explicitDeny') { + results[_requestType] = false; + return; + } + // If the bucket policy returns an allow, we accept the request, as the + // IAM response here is either Allow or implicit deny. + if (bucketPolicyPermission === 'allow') { + results[_requestType] = true; + return; + } + results[_requestType] = actionImplicitDenies[_requestType] === false; + }); + + // final result is true if all the results are true + return Object.keys(results).every(key => results[key] === true); +} function isObjAuthorized(bucket, objectMD, requestTypes, canonicalID, authInfo, actionImplicitDenies, log, request) { if (!Array.isArray(requestTypes)) { @@ -496,6 +539,7 @@ function isLifecycleSession(arn) { module.exports = { isBucketAuthorized, + evaluateBucketPolicyWithIAM, isObjAuthorized, checkBucketAcls, checkObjectAcls, diff --git a/lib/api/multiObjectDelete.js b/lib/api/multiObjectDelete.js index 6f540ce40f..2448eaa5c8 100644 --- a/lib/api/multiObjectDelete.js +++ b/lib/api/multiObjectDelete.js @@ -203,6 +203,8 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request, 'null' : versionIdUtils.decode(entry.versionId); } if (decodedVersionId instanceof Error) { + monitoring.promMetrics('DELETE', bucketName, 404, + 'multiObjectDelete'); return callback(errors.NoSuchVersion); } return callback(null, decodedVersionId); @@ -213,6 +215,8 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request, versionId, log, (err, objMD) => { // if general error from metadata return error if (err) { + monitoring.promMetrics('DELETE', bucketName, err.code, + 'multiObjectDelete'); return callback(err); } if (!objMD) { @@ -280,14 +284,16 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request, return callback(null, objMD, versionId); }, - (objMD, versionId, callback) => - preprocessingVersioningDelete(bucketName, bucket, objMD, - versionId, log, (err, options) => callback(err, options, - objMD)), - (options, objMD, callback) => { + (objMD, versionId, callback) => { + const options = preprocessingVersioningDelete( + bucketName, bucket, objMD, versionId, config.nullVersionCompatMode); const deleteInfo = {}; if (options && options.deleteData) { deleteInfo.deleted = true; + if (objMD.uploadId) { + // eslint-disable-next-line + options.replayId = objMD.uploadId; + } return services.deleteObject(bucketName, objMD, entry.key, options, log, err => callback(err, objMD, deleteInfo)); @@ -365,11 +371,15 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request, function multiObjectDelete(authInfo, request, log, callback) { log.debug('processing request', { method: 'multiObjectDelete' }); if (!request.post) { + monitoring.promMetrics('DELETE', request.bucketName, 400, + 'multiObjectDelete'); return callback(errors.MissingRequestBodyError); } const md5 = crypto.createHash('md5') .update(request.post, 'utf8').digest('base64'); if (md5 !== request.headers['content-md5']) { + monitoring.promMetrics('DELETE', request.bucketName, 400, + 'multiObjectDelete'); return callback(errors.BadDigest); } @@ -545,6 +555,8 @@ function multiObjectDelete(authInfo, request, log, callback) { const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket); if (err) { + monitoring.promMetrics('DELETE', bucketName, err.code, + 'multiObjectDelete'); return callback(err, null, corsHeaders); } const xml = _formatXML(quietSetting, errorResults, @@ -563,6 +575,10 @@ function multiObjectDelete(authInfo, request, log, callback) { removedDeleteMarkers, isDelete: true, }); + monitoring.promMetrics('DELETE', bucketName, '200', + 'multiObjectDelete', + Number.parseInt(totalContentLengthDeleted, 10), null, null, + numOfObjectsRemoved); return callback(null, xml, corsHeaders); }); } @@ -570,4 +586,4 @@ function multiObjectDelete(authInfo, request, log, callback) { module.exports = { getObjMetadataAndDelete, multiObjectDelete, -}; +}; \ No newline at end of file diff --git a/lib/api/objectGetTagging.js b/lib/api/objectGetTagging.js index c44c3c7752..22cdb0fb9b 100644 --- a/lib/api/objectGetTagging.js +++ b/lib/api/objectGetTagging.js @@ -41,7 +41,6 @@ function objectGetTagging(authInfo, request, log, callback) { requestType: request.apiMethods || 'objectGetTagging', request, }; - return async.waterfall([ next => metadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, (err, bucket, objectMD) => { diff --git a/lib/api/objectPutCopyPart.js b/lib/api/objectPutCopyPart.js index d8c57e3952..1c8fe9fbc9 100644 --- a/lib/api/objectPutCopyPart.js +++ b/lib/api/objectPutCopyPart.js @@ -44,7 +44,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket, objectKey: sourceObject, versionId: reqVersionId, getDeleteMarker: true, - requestType: request.apiMethods || 'objectGet', + requestType: 'objectGet', request, }; @@ -59,12 +59,12 @@ function objectPutCopyPart(authInfo, request, sourceBucket, // Note that keys in the query object retain their case, so // request.query.uploadId must be called with that exact // capitalization - const { uploadId } = request.query; + const uploadId = request.query.uploadId; const valPutParams = { authInfo, bucketName: destBucketName, objectKey: destObjectKey, - requestType: request.apiMethods || 'objectPut', + requestType: 'objectPut', request, }; From 6a362598334a8989ce23c3e771a4dbf41f88a537 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Wed, 19 Jul 2023 22:31:52 +0200 Subject: [PATCH 13/13] wip