Skip to content

Commit

Permalink
CLDSRV-428: put apis updated for implicit deny
Browse files Browse the repository at this point in the history
In this commit put apis have been updated to check for implicit deny
returned by vault and added as a parameter in the request Object.
Tests have also been added for the metadataUtils validateBucket
function.
MetadataUtils functions have been updated to check for implicit
deny.
The goal is to implement the same authorization
logic as AWS, where an implicit deny from IAM and an Allow from
the Bucket Policy should allow the request for example.
For the delete on the objectPutCopyPart and objectPutPart as we need to
deferentiate between the vault request and the external backend once
a delete is applied to the request directly as it's unique per API call
this value is then added to the request object. here's the link to the
design doc for more details:
https://github.com/scality/citadel/blob/development/1.0/docs/design/bucket-policies.md?plain=1#L263
  • Loading branch information
benzekrimaha committed Nov 27, 2023
1 parent 693ddf8 commit 2596f3f
Show file tree
Hide file tree
Showing 37 changed files with 357 additions and 116 deletions.
2 changes: 2 additions & 0 deletions constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ const constants = {
'objectDeleteTagging',
'objectGetTagging',
'objectPutTagging',
'objectPutLegalHold',
'objectPutRetention',
],
// response header to be sent when there are invalid
// user metadata in the object's metadata
Expand Down
31 changes: 16 additions & 15 deletions lib/api/bucketPutACL.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const aclUtils = require('../utilities/aclUtils');
const { cleanUpBucket } = require('./apiUtils/bucket/bucketCreation');
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const constants = require('../../constants');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const vault = require('../auth/vault');
const { pushMetric } = require('../utapi/utilities');

Expand Down Expand Up @@ -43,7 +43,7 @@ const { pushMetric } = require('../utapi/utilities');
function bucketPutACL(authInfo, request, log, callback) {
log.debug('processing request', { method: 'bucketPutACL' });

const bucketName = request.bucketName;
const { bucketName } = request;
const canonicalID = authInfo.getCanonicalID();
const newCannedACL = request.headers['x-amz-acl'];
const possibleCannedACL = [
Expand All @@ -53,25 +53,14 @@ function bucketPutACL(authInfo, request, log, callback) {
'authenticated-read',
'log-delivery-write',
];
if (newCannedACL && possibleCannedACL.indexOf(newCannedACL) === -1) {
log.trace('invalid canned acl argument', {
acl: newCannedACL,
method: 'bucketPutACL',
});
return callback(errors.InvalidArgument);
}
if (!aclUtils.checkGrantHeaderValidity(request.headers)) {
log.trace('invalid acl header');
return callback(errors.InvalidArgument);
}
const possibleGroups = [constants.allAuthedUsersId,
constants.publicId,
constants.logId,
];
const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutACL',
requestType: request.apiMethods || 'bucketPutACL',
request,
};
const possibleGrants = ['FULL_CONTROL', 'WRITE',
Expand Down Expand Up @@ -102,7 +91,7 @@ function bucketPutACL(authInfo, request, log, callback) {

return async.waterfall([
function waterfall1(next) {
metadataValidateBucket(metadataValParams, log,
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => {
if (err) {
log.trace('request authorization failed', {
Expand All @@ -111,6 +100,18 @@ function bucketPutACL(authInfo, request, log, callback) {
});
return next(err, bucket);
}
// if the API call is allowed, ensure that the parameters are valid
if (newCannedACL && possibleCannedACL.indexOf(newCannedACL) === -1) {
log.trace('invalid canned acl argument', {
acl: newCannedACL,
method: 'bucketPutACL',
});
return next(errors.InvalidArgument);
}
if (!aclUtils.checkGrantHeaderValidity(request.headers)) {
log.trace('invalid acl header');
return next(errors.InvalidArgument);
}
return next(null, bucket);
});
},
Expand Down
5 changes: 3 additions & 2 deletions lib/api/bucketPutCors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const requestType = 'bucketPutCors';
*/
function bucketPutCors(authInfo, request, log, callback) {
log.debug('processing request', { method: 'bucketPutCors' });
const bucketName = request.bucketName;
const { bucketName } = request;
const canonicalID = authInfo.getCanonicalID();

if (!request.post) {
Expand Down Expand Up @@ -66,7 +66,8 @@ function bucketPutCors(authInfo, request, log, callback) {
});
},
function validateBucketAuthorization(bucket, rules, corsHeaders, next) {
if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) {
if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo,
log, request, request.actionImplicitDenies)) {
log.debug('access denied for account on bucket', {
requestType,
});
Expand Down
8 changes: 4 additions & 4 deletions lib/api/bucketPutEncryption.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const async = require('async');
const { parseEncryptionXml } = require('./apiUtils/bucket/bucketEncryption');
const { checkExpectedBucketOwner } = require('./apiUtils/authorization/bucketOwner');
const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const kms = require('../kms/wrapper');
const { pushMetric } = require('../utapi/utilities');
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
Expand All @@ -18,17 +18,17 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
*/

function bucketPutEncryption(authInfo, request, log, callback) {
const bucketName = request.bucketName;
const { bucketName } = request;

const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutEncryption',
requestType: request.apiMethods || 'bucketPutEncryption',
request,
};

return async.waterfall([
next => metadataValidateBucket(metadataValParams, log, next),
next => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, next),
(bucket, next) => checkExpectedBucketOwner(request.headers, bucket, log, err => next(err, bucket)),
(bucket, next) => {
log.trace('parsing encryption config', { method: 'bucketPutEncryption' });
Expand Down
11 changes: 5 additions & 6 deletions lib/api/bucketPutLifecycle.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
const { waterfall } = require('async');
const uuid = require('uuid/v4');
const LifecycleConfiguration =
require('arsenal').models.LifecycleConfiguration;
const { LifecycleConfiguration } = require('arsenal').models;

const parseXML = require('../utilities/parseXML');
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');

/**
Expand All @@ -21,11 +20,11 @@ const { pushMetric } = require('../utapi/utilities');
function bucketPutLifecycle(authInfo, request, log, callback) {
log.debug('processing request', { method: 'bucketPutLifecycle' });

const bucketName = request.bucketName;
const { bucketName } = request;
const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutLifecycle',
requestType: request.apiMethods || 'bucketPutLifecycle',
request,
};
return waterfall([
Expand All @@ -42,7 +41,7 @@ function bucketPutLifecycle(authInfo, request, log, callback) {
return next(null, configObj);
});
},
(lcConfig, next) => metadataValidateBucket(metadataValParams, log,
(lcConfig, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => {
if (err) {
return next(err, bucket);
Expand Down
8 changes: 4 additions & 4 deletions lib/api/bucketPutNotification.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const parseXML = require('../utilities/parseXML');
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const getNotificationConfiguration = require('./apiUtils/bucket/getNotificationConfiguration');
const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');

/**
Expand All @@ -19,11 +19,11 @@ const { pushMetric } = require('../utapi/utilities');
function bucketPutNotification(authInfo, request, log, callback) {
log.debug('processing request', { method: 'bucketPutNotification' });

const bucketName = request.bucketName;
const { bucketName } = request;
const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutNotification',
requestType: request.apiMethods || 'bucketPutNotification',
request,
};

Expand All @@ -34,7 +34,7 @@ function bucketPutNotification(authInfo, request, log, callback) {
const notifConfig = notificationConfig.error ? undefined : notificationConfig;
process.nextTick(() => next(notificationConfig.error, notifConfig));
},
(notifConfig, next) => metadataValidateBucket(metadataValParams, log,
(notifConfig, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => next(err, bucket, notifConfig)),
(bucket, notifConfig, next) => {
bucket.setNotificationConfiguration(notifConfig);
Expand Down
8 changes: 4 additions & 4 deletions lib/api/bucketPutObjectLock.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
const { waterfall } = require('async');
const arsenal = require('arsenal');

const errors = arsenal.errors;
const ObjectLockConfiguration = arsenal.models.ObjectLockConfiguration;
const { errors } = arsenal;
const { models: { ObjectLockConfiguration } } = arsenal;

const parseXML = require('../utilities/parseXML');
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');

/**
Expand Down Expand Up @@ -41,7 +41,7 @@ function bucketPutObjectLock(authInfo, request, log, callback) {
return next(configObj.error || null, configObj);
});
},
(objectLockConfig, next) => metadataValidateBucket(metadataValParams,
(objectLockConfig, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies,
log, (err, bucket) => {
if (err) {
return next(err, bucket);
Expand Down
6 changes: 3 additions & 3 deletions lib/api/bucketPutPolicy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const { errors, models } = require('arsenal');

const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { validatePolicyResource } =
require('./apiUtils/authorization/permissionChecks');
const { BucketPolicy } = models;
Expand Down Expand Up @@ -37,7 +37,7 @@ function bucketPutPolicy(authInfo, request, log, callback) {
const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutPolicy',
requestType: request.apiMethods || 'bucketPutPolicy',
request,
};

Expand Down Expand Up @@ -70,7 +70,7 @@ function bucketPutPolicy(authInfo, request, log, callback) {
return next(null, bucketPolicy);
});
},
(bucketPolicy, next) => metadataValidateBucket(metadataValParams, log,
(bucketPolicy, next) => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => {
if (err) {
return next(err, bucket);
Expand Down
6 changes: 3 additions & 3 deletions lib/api/bucketPutReplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const { waterfall } = require('async');
const { errors } = require('arsenal');

const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');
const { getReplicationConfiguration } =
require('./apiUtils/bucket/getReplicationConfiguration');
Expand All @@ -27,7 +27,7 @@ function bucketPutReplication(authInfo, request, log, callback) {
const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutReplication',
requestType: request.apiMethods || 'bucketPutReplication',
request,
};
return waterfall([
Expand All @@ -36,7 +36,7 @@ function bucketPutReplication(authInfo, request, log, callback) {
// Check bucket user privileges and ensure versioning is 'Enabled'.
(config, next) =>
// TODO: Validate that destination bucket exists and has versioning.
metadataValidateBucket(metadataValParams, log, (err, bucket) => {
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
if (err) {
return next(err);
}
Expand Down
6 changes: 3 additions & 3 deletions lib/api/bucketPutVersioning.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { errors } = require('arsenal');

const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const metadata = require('../metadata/wrapper');
const { metadataValidateBucket } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');
const versioningNotImplBackends =
require('../../constants').versioningNotImplBackends;
Expand Down Expand Up @@ -87,13 +87,13 @@ function bucketPutVersioning(authInfo, request, log, callback) {
const metadataValParams = {
authInfo,
bucketName,
requestType: 'bucketPutVersioning',
requestType: request.apiMethods || 'bucketPutVersioning',
request,
};

return waterfall([
next => _parseXML(request, log, next),
next => metadataValidateBucket(metadataValParams, log,
next => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => next(err, bucket)), // ignore extra null object,
(bucket, next) => parseString(request.post, (err, result) => {
// just for linting; there should not be any parsing error here
Expand Down
5 changes: 3 additions & 2 deletions lib/api/bucketPutWebsite.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const requestType = 'bucketPutWebsite';
*/
function bucketPutWebsite(authInfo, request, log, callback) {
log.debug('processing request', { method: 'bucketPutWebsite' });
const bucketName = request.bucketName;
const { bucketName } = request;
const canonicalID = authInfo.getCanonicalID();

if (!request.post) {
Expand All @@ -46,7 +46,8 @@ function bucketPutWebsite(authInfo, request, log, callback) {
});
},
function validateBucketAuthorization(bucket, config, next) {
if (!isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request)) {
if (!isBucketAuthorized(bucket, request.apiMethods || requestType, canonicalID, authInfo,
log, request, request.actionImplicitDenies)) {
log.debug('access denied for user on bucket', {
requestType,
method: 'bucketPutWebsite',
Expand Down
6 changes: 3 additions & 3 deletions lib/api/objectPut.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { getObjectSSEConfiguration } = require('./apiUtils/bucket/bucketEncryptio
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const createAndStoreObject = require('./apiUtils/object/createAndStoreObject');
const { checkQueryVersionId } = require('./apiUtils/object/versioning');
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');
const { validateHeaders } = require('./apiUtils/object/objectLockHelpers');
const { hasNonPrintables } = require('../utilities/stringChecks');
Expand Down Expand Up @@ -58,7 +58,7 @@ function objectPut(authInfo, request, streamingV4Params, log, callback) {
}
const invalidSSEError = errors.InvalidArgument.customizeDescription(
'The encryption method specified is not supported');
const requestType = 'objectPut';
const requestType = request.apiMethods || 'objectPut';
const valParams = { authInfo, bucketName, objectKey, requestType, request };
const canonicalID = authInfo.getCanonicalID();

Expand All @@ -75,7 +75,7 @@ function objectPut(authInfo, request, streamingV4Params, log, callback) {

log.trace('owner canonicalID to send to data', { canonicalID });

return metadataValidateBucketAndObj(valParams, log,
return standardMetadataValidateBucketAndObj(valParams, request.actionImplicitDenies, log,
(err, bucket, objMD) => {
const responseHeaders = collectCorsHeaders(headers.origin,
method, bucket);
Expand Down
9 changes: 4 additions & 5 deletions lib/api/objectPutACL.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const constants = require('../../constants');
const vault = require('../auth/vault');
const { decodeVersionId, getVersionIdResHeader }
= require('./apiUtils/object/versioning');
const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');

/*
Format of xml request:
Expand Down Expand Up @@ -43,8 +43,7 @@ const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
*/
function objectPutACL(authInfo, request, log, cb) {
log.debug('processing request', { method: 'objectPutACL' });
const bucketName = request.bucketName;
const objectKey = request.objectKey;
const { bucketName, objectKey } = request;
const newCannedACL = request.headers['x-amz-acl'];
const possibleCannedACL = [
'private',
Expand Down Expand Up @@ -82,7 +81,7 @@ function objectPutACL(authInfo, request, log, cb) {
authInfo,
bucketName,
objectKey,
requestType: 'objectPutACL',
requestType: request.apiMethods || 'objectPutACL',
versionId: reqVersionId,
};

Expand All @@ -107,7 +106,7 @@ function objectPutACL(authInfo, request, log, cb) {

return async.waterfall([
function validateBucketAndObj(next) {
return metadataValidateBucketAndObj(metadataValParams, log,
return standardMetadataValidateBucketAndObj(metadataValParams, request.actionImplicitDenies, log,
(err, bucket, objectMD) => {
if (err) {
return next(err);
Expand Down
Loading

0 comments on commit 2596f3f

Please sign in to comment.