Skip to content

Commit d4a82f0

Browse files
leif-scalityleif-scality
committed
CLDSRV-674: handle cross-account bucket policies
1 parent 089bb87 commit d4a82f0

File tree

3 files changed

+969
-147
lines changed

3 files changed

+969
-147
lines changed

‎lib/api/apiUtils/authorization/permissionChecks.js‎

Lines changed: 184 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) {
117117
// authorization check should just return true so can move on to check
118118
// rights at the object level.
119119
return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL'
120-
|| requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
120+
|| requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
121121
}
122122

123123
function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIsNotUser,
@@ -265,67 +265,207 @@ function _isAccountId(principal) {
265265
return (principal.length === 12 && /^\d+$/.test(principal));
266266
}
267267

268-
function _checkPrincipal(requester, principal) {
269-
if (principal === '*') {
270-
return true;
271-
}
272-
// User in unauthenticated (anonymous request)
273-
if (requester === undefined) {
268+
function _isRootUser(arn) {
269+
if (!arn) {
274270
return false;
275271
}
276-
if (principal === requester) {
272+
273+
// Vault returns the following arn when the account makes requests 'arn:aws:iam::295412255825:/master/',
274+
// with an empty resource type ('user/' prefix missing).
275+
const arns = arn.split(':');
276+
const resource = arns.at(-1);
277+
278+
// If we start with '/' is because we have a empty resource type so we know it is a root account.
279+
if (resource.startsWith('/')) {
277280
return true;
278281
}
279-
if (_isAccountId(principal)) {
280-
return _getAccountId(requester) === principal;
282+
283+
return false;
284+
}
285+
286+
const checkPrincipalResult = Object.freeze({
287+
KO: 0,
288+
CROSS_ACCOUNT_OK: 1,
289+
OK: 2,
290+
});
291+
292+
/** _evaluateCrossAccount - checks if it is a cross-account request.
293+
* @param {string} requesterARN - requester ARN
294+
* @param {string} requesterCanonicalID - requester canonical ID
295+
* @param {string} bucketOwnerCanonicalID - bucket owner canonical ID
296+
* @return {checkPrincipalResult} OK if it is not cross-account, CROSS_ACCOUNT_OK otherwise.
297+
*/
298+
function _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID) {
299+
if (!_isRootUser(requesterARN)) {
300+
return bucketOwnerCanonicalID === requesterCanonicalID ?
301+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
281302
}
282-
if (principal.endsWith('root')) {
283-
return _getAccountId(requester) === _getAccountId(principal);
303+
304+
return checkPrincipalResult.OK;
305+
}
306+
307+
function _checkPrincipalWildcard(requestARN, requesterCanonicalID, bucketOwnerCanonicalID) {
308+
if (requestARN === undefined) { // User in unauthenticated (anonymous request)
309+
return checkPrincipalResult.OK;
284310
}
285-
return false;
311+
312+
return _checkCrossAccount(requestARN, requesterCanonicalID, bucketOwnerCanonicalID);
286313
}
287314

288-
function _checkPrincipals(canonicalID, arn, principal) {
315+
function _checkPrincipalAWS(principal, requesterARN, requesterCanonicalID, bucketOwnerCanonicalID) {
289316
if (principal === '*') {
290-
return true;
317+
return _checkPrincipalWildcard(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
291318
}
292-
if (principal.CanonicalUser) {
293-
if (Array.isArray(principal.CanonicalUser)) {
294-
return principal.CanonicalUser.some(p => _checkPrincipal(canonicalID, p));
319+
320+
if (requesterARN === undefined) { // User in unauthenticated (anonymous request)
321+
return checkPrincipalResult.KO;
322+
}
323+
324+
if (principal === requesterARN) {
325+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
326+
}
327+
328+
if (_isAccountId(principal) && principal === _getAccountId(requesterARN)) {
329+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
330+
}
331+
332+
if (principal.endsWith('root') && _getAccountId(principal) === _getAccountId(requesterARN)) {
333+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
334+
}
335+
336+
return checkPrincipalResult.KO;
337+
}
338+
339+
function _checkPrincipalCanonicalUser(principal, requesterARN, requesterCanonicalID, bucketOwnerCanonicalID) {
340+
if (principal === '*') {
341+
return _checkPrincipalWildcard(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
342+
}
343+
344+
if (requesterARN === undefined) { // User in unauthenticated (anonymous request)
345+
return checkPrincipalResult.KO;
346+
}
347+
348+
if (principal === requesterCanonicalID) {
349+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
350+
}
351+
352+
return checkPrincipalResult.KO;
353+
}
354+
355+
function _findBestPrincipalMatch(principalArray, checkFunc) {
356+
let bestMatch = checkPrincipalResult.KO;
357+
if (!principalArray) {
358+
return bestMatch;
359+
}
360+
361+
const principals = Array.isArray(principalArray) ? principalArray : [principalArray];
362+
363+
for (const p of principals) {
364+
const result = checkFunc(p);
365+
if (result === checkPrincipalResult.OK) {
366+
return checkPrincipalResult.OK; // Highest permission, can exit early
367+
}
368+
if (result > bestMatch) {
369+
bestMatch = result;
295370
}
296-
return _checkPrincipal(canonicalID, principal.CanonicalUser);
297371
}
372+
373+
return bestMatch;
374+
}
375+
376+
function _checkPrincipals(canonicalID, arn, principal, bucketOwnerCanonicalID) {
377+
if (principal === '*') {
378+
return _checkPrincipalWildcard(arn, canonicalID, bucketOwnerCanonicalID);
379+
}
380+
381+
if (principal.CanonicalUser) {
382+
return _findBestPrincipalMatch(principal.CanonicalUser,
383+
p => _checkPrincipalCanonicalUser(p, arn, canonicalID, bucketOwnerCanonicalID));
384+
}
385+
298386
if (principal.AWS) {
299-
if (Array.isArray(principal.AWS)) {
300-
return principal.AWS.some(p => _checkPrincipal(arn, p));
301-
}
302-
return _checkPrincipal(arn, principal.AWS);
387+
return _findBestPrincipalMatch(principal.AWS,
388+
p => _checkPrincipalAWS(p, arn, canonicalID, bucketOwnerCanonicalID));
303389
}
304-
return false;
390+
391+
return checkPrincipalResult.KO;
305392
}
306393

394+
const checkBucketPolicyResult = Object.freeze({
395+
DEFAULT_DENY: 0,
396+
EXPLICIT_DENY: 1,
397+
ALLOW: 2,
398+
CROSS_ACCOUNT_ALLOW: 3,
399+
});
400+
401+
// checkBucketPolicy FSM.
402+
// ┌───────────────────────────┐
403+
// │ ▼
404+
// │ ┌───────┐
405+
// │ ┌────────►│ ALLOW ├──────────────┐
406+
// │ ┌─────┐ │ └──┬────┘ │
407+
// │ │START│ │ │ │
408+
// │ └──┬──┘ │ │ │
409+
// │ │ │ │ │
410+
// │ ▼ │ ▼ ▼
411+
// │┌──────────────┤ ┌────┐ ┌─────┐
412+
// ││ DEFAULT_DENY ├─────────►│DENY├────────────►│ END │
413+
// │└──────┬───────┤ └────┘ └─────┘
414+
// │ │ │ ▲ ▲ ▲
415+
// │ │ │ │ │ │
416+
// │ │ │ │ │ │
417+
// │ │ │ ┌───┴───────────┐ │ │
418+
// │ │ └────────►│ CROSS_ACCOUNT ├──────┘ │
419+
// │ │ └┬──────────────┘ │
420+
// └───────┼──────────────────┘ │
421+
// └──────────────────────────────────────────┘
422+
//
307423
function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies) {
308-
let permission = 'defaultDeny';
424+
let permission = checkBucketPolicyResult.DEFAULT_DENY;
309425
// if requester is user within bucket owner account, actions should be
310426
// allowed unless explicitly denied (assumes allowed by IAM policy)
311427
if (bucketOwner === canonicalID && actionImplicitDenies[requestType] === false) {
312-
permission = 'allow';
428+
permission = checkBucketPolicyResult.ALLOW;
313429
}
314430
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
315431
while (copiedStatement.length > 0) {
316432
const s = copiedStatement[0];
317-
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal);
433+
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal, bucketOwner);
318434
const actionMatch = _checkBucketPolicyActions(requestType, s.Action, log);
319435
const resourceMatch = _checkBucketPolicyResources(request, s.Resource, log);
320436
const conditionsMatch = _checkBucketPolicyConditions(request, s.Condition, log);
321437

322-
if (principalMatch && actionMatch && resourceMatch && conditionsMatch && s.Effect === 'Deny') {
323-
// explicit deny trumps any allows, so return immediately
324-
return 'explicitDeny';
325-
}
326-
if (principalMatch && actionMatch && resourceMatch && conditionsMatch && s.Effect === 'Allow') {
327-
permission = 'allow';
438+
const ok = principalMatch === checkPrincipalResult.OK && actionMatch && resourceMatch && conditionsMatch;
439+
const okCross = principalMatch === checkPrincipalResult.CROSS_ACCOUNT_OK
440+
&& actionMatch && resourceMatch && conditionsMatch;
441+
switch (permission) {
442+
case checkBucketPolicyResult.DEFAULT_DENY:
443+
if ((ok || okCross) && s.Effect === 'Deny') {
444+
return checkBucketPolicyResult.EXPLICIT_DENY;
445+
} else if (ok && s.Effect === 'Allow') {
446+
permission = checkBucketPolicyResult.ALLOW;
447+
} else if (okCross && s.Effect === 'Allow') {
448+
permission = checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW;
449+
}
450+
break;
451+
case checkBucketPolicyResult.EXPLICIT_DENY:
452+
return checkBucketPolicyResult.EXPLICIT_DENY;
453+
case checkBucketPolicyResult.ALLOW:
454+
if ((ok || okCross) && s.Effect === 'Deny') {
455+
return checkBucketPolicyResult.EXPLICIT_DENY;
456+
}
457+
break;
458+
case checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW:
459+
if ((ok || okCross) && s.Effect === 'Deny') {
460+
return checkBucketPolicyResult.EXPLICIT_DENY;
461+
} else if (ok && s.Effect === 'Allow') {
462+
permission = checkBucketPolicyResult.ALLOW;
463+
}
464+
break;
465+
default: // Needed for the linter, should be unreachable.
466+
break;
328467
}
468+
329469
copiedStatement = copiedStatement.splice(1);
330470
}
331471
return permission;
@@ -341,9 +481,13 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
341481
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
342482
bucketOwner, log, request, actionImplicitDenies);
343483

344-
if (bucketPolicyPermission === 'explicitDeny') {
484+
if (bucketPolicyPermission === checkBucketPolicyResult.EXPLICIT_DENY) {
345485
processedResult = false;
346-
} else if (bucketPolicyPermission === 'allow') {
486+
} else if (bucketPolicyPermission === checkBucketPolicyResult.ALLOW) {
487+
processedResult = true;
488+
} else if (bucketPolicyPermission === checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW
489+
&& actionImplicitDenies[requestType] === false) {
490+
// If the bucket policy is cross account, only return true if Vault also returned an explicit allow.
347491
processedResult = true;
348492
} else {
349493
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
@@ -405,7 +549,7 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, aut
405549
arn = authInfo.getArn();
406550
}
407551
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
408-
request, true, results, actionImplicitDenies);
552+
request, true, results, actionImplicitDenies);
409553
});
410554
}
411555

@@ -456,8 +600,8 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
456600
// - account is the bucket owner
457601
// - requester is account, not user
458602
if (bucketOwnerActions.includes(parsedMethodName)
459-
&& (bucketOwner === canonicalID)
460-
&& requesterIsNotUser) {
603+
&& (bucketOwner === canonicalID)
604+
&& requesterIsNotUser) {
461605
results[_requestType] = actionImplicitDenies[_requestType] === false;
462606
return results[_requestType];
463607
}
@@ -613,4 +757,6 @@ module.exports = {
613757
validatePolicyConditions,
614758
isLifecycleSession,
615759
evaluateBucketPolicyWithIAM,
760+
checkBucketPolicy,
761+
checkBucketPolicyResult,
616762
};

0 commit comments

Comments
 (0)