Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/w/8.7/improvement/CLDSRV-431-mis…
Browse files Browse the repository at this point in the history
…c-api-implicitDeny' into w/8.8/improvement/CLDSRV-431-misc-api-implicitDeny
  • Loading branch information
benzekrimaha committed Dec 14, 2023
2 parents ffe4ea4 + a855e38 commit 75b293d
Show file tree
Hide file tree
Showing 31 changed files with 162 additions and 38 deletions.
6 changes: 3 additions & 3 deletions lib/api/apiUtils/authorization/permissionChecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,11 @@ function _checkPrincipals(canonicalID, arn, principal) {
return false;
}

function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request) {
function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies) {
let permission = 'defaultDeny';
// if requester is user within bucket owner account, actions should be
// allowed unless explicitly denied (assumes allowed by IAM policy)
if (bucketOwner === canonicalID) {
if (bucketOwner === canonicalID && actionImplicitDenies[requestType] === false) {
permission = 'allow';
}
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
Expand Down Expand Up @@ -324,7 +324,7 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
} else {
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
bucketOwner, log, request);
bucketOwner, log, request, actionImplicitDenies);

if (bucketPolicyPermission === 'explicitDeny') {
processedResult = false;
Expand Down
10 changes: 6 additions & 4 deletions lib/api/apiUtils/bucket/bucketDeletion.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function _deleteMPUbucket(destinationBucketName, log, cb) {
});
}

function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, request, log, cb) {
async.mapLimit(mpus, 1, (mpu, next) => {
const splitterChar = mpu.key.includes(oldSplitter) ?
oldSplitter : splitter;
Expand All @@ -40,7 +40,7 @@ function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
byteLength: partSizeSum,
});
next(err);
});
}, request);
}, cb);
}
/**
Expand All @@ -49,11 +49,13 @@ function _deleteOngoingMPUs(authInfo, bucketName, bucketMD, mpus, log, cb) {
* @param {object} bucketMD - bucket attributes/metadata
* @param {string} bucketName - bucket in which objectMetadata is stored
* @param {string} canonicalID - account canonicalID of requester
* @param {object} request - request object given by router
* including normalized headers
* @param {object} log - Werelogs logger
* @param {function} cb - callback from async.waterfall in bucketDelete
* @return {undefined}
*/
function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, log, cb) {
function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log, cb) {
log.trace('deleting bucket from metadata');
assert.strictEqual(typeof bucketName, 'string');
assert.strictEqual(typeof canonicalID, 'string');
Expand Down Expand Up @@ -100,7 +102,7 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, log, cb) {
}
if (objectsListRes.Contents.length) {
return _deleteOngoingMPUs(authInfo, bucketName,
bucketMD, objectsListRes.Contents, log, err => {
bucketMD, objectsListRes.Contents, request, log, err => {
if (err) {
return next(err);
}
Expand Down
10 changes: 8 additions & 2 deletions lib/api/apiUtils/object/abortMultipartUpload.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const async = require('async');
const constants = require('../../../../constants');
const { data } = require('../../../data/wrapper');
const locationConstraintCheck = require('../object/locationConstraintCheck');
const { metadataValidateBucketAndObj } =
const { standardMetadataValidateBucketAndObj } =
require('../../../metadata/metadataUtils');
const services = require('../../../services');

Expand All @@ -22,10 +22,11 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
// but the requestType is the more general 'objectDelete'
const metadataValParams = Object.assign({}, metadataValMPUparams);
metadataValParams.requestType = 'objectPut';
const authzIdentityResult = request ? request.actionImplicitDenies : false;

async.waterfall([
function checkDestBucketVal(next) {
metadataValidateBucketAndObj(metadataValParams, log,
standardMetadataValidateBucketAndObj(metadataValParams, authzIdentityResult, log,
(err, destinationBucket) => {
if (err) {
return next(err, destinationBucket);
Expand Down Expand Up @@ -56,9 +57,14 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket,
next) {
const location = mpuOverviewObj.controllingLocationConstraint;
const originalIdentityAuthzResults = request.actionImplicitDenies;
// eslint-disable-next-line no-param-reassign
delete request.actionImplicitDenies;
return data.abortMPU(objectKey, uploadId, location, bucketName,
request, destBucket, locationConstraintCheck, log,
(err, skipDataDelete) => {
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = originalIdentityAuthzResults;
if (err) {
return next(err, destBucket);
}
Expand Down
25 changes: 23 additions & 2 deletions lib/api/apiUtils/object/objectLockHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const moment = require('moment');

const { config } = require('../../../Config');
const vault = require('../../../auth/vault');
const { evaluateBucketPolicyWithIAM } = require('../authorization/permissionChecks');

const { scaledMsPerDay } = config.getTimeOptions();
/**
Expand Down Expand Up @@ -304,15 +305,35 @@ function checkUserGovernanceBypass(request, authInfo, bucketMD, objectKey, log,
if (err) {
return cb(err);
}
if (authorizationResults[0].isAllowed !== true) {
const explicitDenyExists = authorizationResults.some(
authzResult => authzResult.isAllowed === false && authzResult.isImplicit === false);
if (explicitDenyExists) {
log.trace('authorization check failed for user',
{
'method': 'checkUserPolicyGovernanceBypass',
's3:BypassGovernanceRetention': false,
});
return cb(errors.AccessDenied);
}
return cb(null);
// Convert authorization results into an easier to handle format
const actionImplicitDenies = authorizationResults.reduce((acc, curr, idx) => {
const apiMethod = authorizationResults[idx].action;
// eslint-disable-next-line no-param-reassign
acc[apiMethod] = curr.isImplicit;
return acc;
}, {});

// Evaluate against the bucket policies
const areAllActionsAllowed = evaluateBucketPolicyWithIAM(
bucketMD,
Object.keys(actionImplicitDenies),
authInfo.getCanonicalID(),
authInfo,
actionImplicitDenies,
log,
request);

return cb(areAllActionsAllowed === true ? null : errors.AccessDenied);
});
}

Expand Down
2 changes: 1 addition & 1 deletion lib/api/bucketDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function bucketDelete(authInfo, request, log, cb) {
log.trace('passed checks',
{ method: 'metadataValidateBucket' });
return deleteBucket(authInfo, bucketMD, bucketName,
authInfo.getCanonicalID(), log, err => {
authInfo.getCanonicalID(), request, log, err => {
if (err) {
monitoring.promMetrics(
'DELETE', bucketName, err.code, 'deleteBucket');
Expand Down
4 changes: 2 additions & 2 deletions lib/api/bucketHead.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');

const { pushMetric } = require('../utapi/utilities');
const monitoring = require('../utilities/monitoringHandler');
Expand All @@ -22,7 +22,7 @@ function bucketHead(authInfo, request, log, callback) {
requestType: 'bucketHead',
request,
};
metadataValidateBucket(metadataValParams, log, (err, bucket) => {
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
if (err) {
Expand Down
9 changes: 7 additions & 2 deletions lib/api/completeMultipartUpload.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const constants = require('../../constants');
const { versioningPreprocessing, checkQueryVersionId, decodeVID, overwritingVersioning }
= require('./apiUtils/object/versioning');
const services = require('../services');
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const locationConstraintCheck
= require('./apiUtils/object/locationConstraintCheck');
const { skipMpuPartProcessing } = storage.data.external.backendUtils;
Expand Down Expand Up @@ -136,7 +136,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
requestType: 'objectPut',
versionId,
};
metadataValidateBucketAndObj(metadataValParams, log, next);
standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log, next);
},
function validateMultipart(destBucket, objMD, next) {
if (objMD) {
Expand Down Expand Up @@ -214,9 +214,14 @@ function completeMultipartUpload(authInfo, request, log, callback) {
const mdInfo = { storedParts, mpuOverviewKey, splitter };
const mpuInfo =
{ objectKey, uploadId, jsonList, bucketName, destBucket };
const originalIdentityImpDenies = request.actionImplicitDenies;
// eslint-disable-next-line no-param-reassign
delete request.actionImplicitDenies;
return data.completeMPU(request, mpuInfo, mdInfo, location,
null, null, null, locationConstraintCheck, log,
(err, completeObjData) => {
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = originalIdentityImpDenies;
if (err) {
return next(err, destBucket);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/api/initiateMultipartUpload.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const { hasNonPrintables } = require('../utilities/stringChecks');
const { cleanUpBucket } = require('./apiUtils/bucket/bucketCreation');
const constants = require('../../constants');
const services = require('../services');
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const locationConstraintCheck
= require('./apiUtils/object/locationConstraintCheck');
const validateWebsiteHeader = require('./apiUtils/object/websiteServing')
Expand Down Expand Up @@ -274,7 +274,7 @@ function initiateMultipartUpload(authInfo, request, log, callback) {
}

async.waterfall([
next => metadataValidateBucketAndObj(metadataValParams, log,
next => standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
(error, destinationBucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, destinationBucket);
if (error) {
Expand Down
4 changes: 2 additions & 2 deletions lib/api/listMultipartUploads.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const convertToXml = s3middleware.convertToXml;
const constants = require('../../constants');
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const services = require('../services');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');
const monitoring = require('../utilities/monitoringHandler');

Expand Down Expand Up @@ -105,7 +105,7 @@ function listMultipartUploads(authInfo, request, log, callback) {
function waterfall1(next) {
// Check final destination bucket for authorization rather
// than multipart upload bucket
metadataValidateBucket(metadataValParams, log,
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => next(err, bucket));
},
function getMPUBucket(bucket, next) {
Expand Down
9 changes: 7 additions & 2 deletions lib/api/listParts.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const locationConstraintCheck =
require('./apiUtils/object/locationConstraintCheck');
const services = require('../services');
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const escapeForXml = s3middleware.escapeForXml;
const { pushMetric } = require('../utapi/utilities');
const monitoring = require('../utilities/monitoringHandler');
Expand Down Expand Up @@ -114,7 +114,7 @@ function listParts(authInfo, request, log, callback) {

async.waterfall([
function checkDestBucketVal(next) {
metadataValidateBucketAndObj(metadataValParams, log,
standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
(err, destinationBucket) => {
if (err) {
return next(err, destinationBucket, null);
Expand Down Expand Up @@ -152,8 +152,13 @@ function listParts(authInfo, request, log, callback) {
mpuOverviewObj,
destBucket,
};
const originalIdentityImpDenies = request.actionImplicitDenies;
// eslint-disable-next-line no-param-reassign
delete request.actionImplicitDenies;
return data.listParts(mpuInfo, request, locationConstraintCheck,
log, (err, backendPartList) => {
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = originalIdentityImpDenies;
if (err) {
return next(err, destBucket);
}
Expand Down
11 changes: 8 additions & 3 deletions lib/api/objectCopy.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const logger = require('../utilities/logger');
const services = require('../services');
const { pushMetric } = require('../utapi/utilities');
const removeAWSChunked = require('./apiUtils/object/removeAWSChunked');
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const validateWebsiteHeader = require('./apiUtils/object/websiteServing')
.validateWebsiteHeader;
const { config } = require('../Config');
Expand Down Expand Up @@ -261,7 +261,7 @@ function objectCopy(authInfo, request, sourceBucket,
}
return async.waterfall([
function checkDestAuth(next) {
return metadataValidateBucketAndObj(valPutParams, log,
return standardMetadataValidateBucketAndObj(valPutParams, request.actionImplicitDenies, log,
(err, destBucketMD, destObjMD) => {
if (err) {
log.debug('error validating put part of request',
Expand All @@ -279,7 +279,7 @@ function objectCopy(authInfo, request, sourceBucket,
});
},
function checkSourceAuthorization(destBucketMD, destObjMD, next) {
return metadataValidateBucketAndObj(valGetParams, log,
return standardMetadataValidateBucketAndObj(valGetParams, request.actionImplicitDenies, log,
(err, sourceBucketMD, sourceObjMD) => {
if (err) {
log.debug('error validating get part of request',
Expand Down Expand Up @@ -450,10 +450,15 @@ function objectCopy(authInfo, request, sourceBucket,
return next(null, storeMetadataParams, dataLocator, destObjMD,
serverSideEncryption, destBucketMD);
}
const originalIdentityImpDenies = request.actionImplicitDenies;
// eslint-disable-next-line no-param-reassign
delete request.actionImplicitDenies;
return data.copyObject(request, sourceLocationConstraintName,
storeMetadataParams, dataLocator, dataStoreContext,
backendInfoDest, sourceBucketMD, destBucketMD, serverSideEncryption, log,
(err, results) => {
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = originalIdentityImpDenies;
if (err) {
return next(err, destBucketMD);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/api/objectHead.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const { getPartNumber, getPartSize, getPartCountFromMd5 } =
const { config } = require('../Config');
const { locationConstraints } = config;

const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { maximumAllowedPartCount } = require('../../constants');
const { setExpirationHeaders } = require('./apiUtils/object/expirationHeaders');

Expand Down Expand Up @@ -52,7 +52,7 @@ function objectHead(authInfo, request, log, callback) {
request,
};

return metadataValidateBucketAndObj(mdValParams, log,
return standardMetadataValidateBucketAndObj(mdValParams, request.actionImplicitDenies, log,
(err, bucket, objMD) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
Expand Down
4 changes: 2 additions & 2 deletions lib/api/websiteHead.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ function websiteHead(request, log, callback) {
{ error: err });
let returnErr = err;
const bucketAuthorized = isBucketAuthorized(bucket,
'bucketGet', constants.publicId, null, log, request);
'bucketGet', constants.publicId, null, log, request, request.actionImplicitDenies);
// if index object does not exist and bucket is private AWS
// returns 403 - AccessDenied error.
if (err.is.NoSuchKey && !bucketAuthorized) {
Expand All @@ -121,7 +121,7 @@ function websiteHead(request, log, callback) {
reqObjectKey, corsHeaders, log, callback);
}
if (!isObjAuthorized(bucket, objMD, 'objectGet',
constants.publicId, null, log, request)) {
constants.publicId, null, log, request, request.actionImplicitDenies)) {
const err = errors.AccessDenied;
log.trace('request not authorized', { error: err });
return _errorActions(err, routingRules, reqObjectKey,
Expand Down
2 changes: 1 addition & 1 deletion lib/metadata/metadataUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ module.exports = {
metadataGetObject,
metadataGetObjects,
metadataValidateBucketAndObj,
standardMetadataValidateBucketAndObj,
metadataValidateBucket,
standardMetadataValidateBucketAndObj,
standardMetadataValidateBucket,
};
Loading

0 comments on commit 75b293d

Please sign in to comment.