Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement/cldsrv 427 permissions checks #5432

Merged
merged 4 commits into from
Nov 17, 2023
Merged

Improvement/cldsrv 427 permissions checks #5432

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0e323fb
CLDSRV-427: update bucket/object perm checks to account for implicit …
Aug 28, 2023
fa2f877
CLDSRV-427: linting fixups and retrocompatibility changes
benzekrimaha Nov 3, 2023
6caa5cc
CLDSRV-427: Improving functions using helper function
benzekrimaha Nov 10, 2023
693ddf8
Merge branch 'development/7.10' into improvement/CLDSRV-427-permissio…
benzekrimaha Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 127 additions & 71 deletions lib/api/apiUtils/authorization/permissionChecks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
const { evaluators, actionMaps, RequestContext } = require('arsenal').policies;
const constants = require('../../../../constants');

const { allAuthedUsersId, bucketOwnerActions, logId, publicId,
assumedRoleArnResourceType, backbeatLifecycleSessionName, arrayOfAllowed } = constants;
const {
allAuthedUsersId, bucketOwnerActions, logId, publicId,
assumedRoleArnResourceType, backbeatLifecycleSessionName, arrayOfAllowed,
} = constants;

// whitelist buckets to allow public read on objects
const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ?
process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : [];
const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS
? process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : [];

/**
* Checks the access control for a given bucket based on the request type and user's canonical ID.
Expand Down Expand Up @@ -106,8 +108,8 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) {
// objectPutACL, objectGetACL, objectHead or objectGet, the bucket
// authorization check should just return true so can move on to check
// rights at the object level.
return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL' ||
requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL'
|| requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
}

function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIsNotUser,
Expand Down Expand Up @@ -199,9 +201,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;
}
Expand Down Expand Up @@ -298,75 +300,128 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l
return permission;
}

function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log, request) {
// Check to see if user is authorized to perform a
// particular action on bucket based on ACLs.
// TODO: Add IAM checks
let requesterIsNotUser = true;
let arn = null;
if (authInfo) {
requesterIsNotUser = !authInfo.isRequesterAnIAMUser();
arn = authInfo.getArn();
}
// if the bucket owner is an account, users should not have default access
if ((bucket.getOwner() === canonicalID) && requesterIsNotUser) {
return true;
}
const aclPermission = checkBucketAcls(bucket, requestType, canonicalID);
function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner, log,
request, aclPermission, results, actionImplicitDenies) {
const bucketPolicy = bucket.getBucketPolicy();
let processedResult = results[requestType];
if (!bucketPolicy) {
return aclPermission;
}
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType,
canonicalID, arn, bucket.getOwner(), log, request);
if (bucketPolicyPermission === 'explicitDeny') {
return false;
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
} else {
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
bucketOwner, log, request);

if (bucketPolicyPermission === 'explicitDeny') {
processedResult = false;
} else if (bucketPolicyPermission === 'allow') {
processedResult = true;
} else {
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
}
}
return (aclPermission || (bucketPolicyPermission === 'allow'));
return processedResult;
}

function isObjAuthorized(bucket, objectMD, requestType, canonicalID, authInfo, log, request) {
const bucketOwner = bucket.getOwner();
if (!objectMD) {
function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, log, request,
actionImplicitDeniesInput = {}) {
const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput];
const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud here as your default value is {} in request parameters, do we need to check this? Can we use a direct assignment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes as here we also have to check for when the value passed is "false" and assign a {}

const mainApiCall = requestTypes[0];
const results = {};
return requestTypes.every(_requestType => {
// By default, all missing actions are defined as allowed from IAM, to be
// backward compatible
actionImplicitDenies[_requestType] = actionImplicitDenies[_requestType] || false;
// Check to see if user is authorized to perform a
// particular action on bucket based on ACLs.
// TODO: Add IAM checks
let requesterIsNotUser = true;
let arn = null;
if (authInfo) {
requesterIsNotUser = !authInfo.isRequesterAnIAMUser();
arn = authInfo.getArn();
}
// if the bucket owner is an account, users should not have default access
if ((bucket.getOwner() === canonicalID) && requesterIsNotUser) {
results[_requestType] = actionImplicitDenies[_requestType] === false;
return results[_requestType];
}
const aclPermission = checkBucketAcls(bucket, _requestType, canonicalID, mainApiCall);
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
request, aclPermission, results, actionImplicitDenies);
});
}

function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, authInfo, actionImplicitDeniesInput = {},
log, request) {
const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput];
const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput;
const results = {};
return requestTypes.every(_requestType => {
// By default, all missing actions are defined as allowed from IAM, to be
// backward compatible
actionImplicitDenies[_requestType] = actionImplicitDenies[_requestType] || false;
let arn = null;
if (authInfo) {
arn = authInfo.getArn();
}
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
request, true, results, actionImplicitDenies);
});
}

function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authInfo, log, request,
actionImplicitDeniesInput = {}) {
const requestTypes = Array.isArray(requestTypesInput) ? requestTypesInput : [requestTypesInput];
const actionImplicitDenies = !actionImplicitDeniesInput ? {} : actionImplicitDeniesInput;
const results = {};
const mainApiCall = requestTypes[0];
return requestTypes.every(_requestType => {
// By default, all missing actions are defined as allowed from IAM, to be
// backward compatible
actionImplicitDenies[_requestType] = actionImplicitDenies[_requestType] || false;
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
if (requestType === 'objectPut' || requestType === 'objectDelete') {
return true;
if (parsedMethodName === 'objectPut' || parsedMethodName === 'objectDelete') {
results[_requestType] = actionImplicitDenies[_requestType] === false;
return results[_requestType];
}
// check bucket has read access
// 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions
results[_requestType] = isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, log, request,
actionImplicitDenies);
return results[_requestType];
}
// check bucket has read access
// 'bucketGet' covers listObjects and listMultipartUploads, bucket read actions
return isBucketAuthorized(bucket, 'bucketGet', canonicalID, authInfo, log, request);
}
let requesterIsNotUser = true;
let arn = null;
if (authInfo) {
requesterIsNotUser = !authInfo.isRequesterAnIAMUser();
arn = authInfo.getArn();
}
if (objectMD['owner-id'] === canonicalID && requesterIsNotUser) {
return true;
}
// account is authorized if:
// - requesttype is included in bucketOwnerActions and
// - account is the bucket owner
// - requester is account, not user
if (bucketOwnerActions.includes(requestType)
let requesterIsNotUser = true;
let arn = null;
let isUserUnauthenticated = false;
if (authInfo) {
requesterIsNotUser = !authInfo.isRequesterAnIAMUser();
arn = authInfo.getArn();
isUserUnauthenticated = arn === undefined;
}
if (objectMD['owner-id'] === canonicalID && requesterIsNotUser) {
results[_requestType] = actionImplicitDenies[_requestType] === false;
return results[_requestType];
}
// account is authorized if:
// - requesttype is included in bucketOwnerActions and
// - account is the bucket owner
// - requester is account, not user
if (bucketOwnerActions.includes(parsedMethodName)
&& (bucketOwner === canonicalID)
&& requesterIsNotUser) {
return true;
}
const aclPermission = checkObjectAcls(bucket, objectMD, requestType,
canonicalID);
const bucketPolicy = bucket.getBucketPolicy();
if (!bucketPolicy) {
return aclPermission;
}
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType,
canonicalID, arn, bucket.getOwner(), log, request);
if (bucketPolicyPermission === 'explicitDeny') {
return false;
}
return (aclPermission || (bucketPolicyPermission === 'allow'));
results[_requestType] = actionImplicitDenies[_requestType] === false;
return results[_requestType];
}
const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName,
canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall);
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner,
log, request, aclPermission, results, actionImplicitDenies);
});
}

function _checkResource(resource, bucketArn) {
Expand Down Expand Up @@ -413,9 +468,9 @@ function isLifecycleSession(arn) {
const resourceType = resourceNames[0];
const sessionName = resourceNames[resourceNames.length - 1];

return (service === 'sts' &&
resourceType === assumedRoleArnResourceType &&
sessionName === backbeatLifecycleSessionName);
return (service === 'sts'
&& resourceType === assumedRoleArnResourceType
&& sessionName === backbeatLifecycleSessionName);
}

module.exports = {
Expand All @@ -425,4 +480,5 @@ module.exports = {
checkObjectAcls,
validatePolicyResource,
isLifecycleSession,
evaluateBucketPolicyWithIAM,
};
Loading