Skip to content

Commit 96557d4

Browse files
leif-scalityleif-scality
committed
CLDSRV-674: handle cross-account bucket policies
1 parent 3d427f2 commit 96557d4

3 files changed

Lines changed: 979 additions & 146 deletions

File tree

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

Lines changed: 194 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,19 @@ const {
1414
const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS
1515
? process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : [];
1616

17+
const checkPrincipalResult = Object.freeze({
18+
KO: 0,
19+
CROSS_ACCOUNT_OK: 1,
20+
OK: 2,
21+
});
22+
23+
const checkBucketPolicyResult = Object.freeze({
24+
DEFAULT_DENY: 0,
25+
EXPLICIT_DENY: 1,
26+
ALLOW: 2,
27+
CROSS_ACCOUNT_ALLOW: 3,
28+
});
29+
1730
/**
1831
* Checks the access control for a given bucket based on the request type and user's canonical ID.
1932
*
@@ -117,7 +130,7 @@ function checkBucketAcls(bucket, requestType, canonicalID, mainApiCall) {
117130
// authorization check should just return true so can move on to check
118131
// rights at the object level.
119132
return (requestTypeParsed === 'objectPutACL' || requestTypeParsed === 'objectGetACL'
120-
|| requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
133+
|| requestTypeParsed === 'objectGet' || requestTypeParsed === 'objectHead');
121134
}
122135

123136
function checkObjectAcls(bucket, objectMD, requestType, canonicalID, requesterIsNotUser,
@@ -265,67 +278,205 @@ function _isAccountId(principal) {
265278
return (principal.length === 12 && /^\d+$/.test(principal));
266279
}
267280

268-
function _checkPrincipal(requester, principal) {
269-
if (principal === '*') {
270-
return true;
281+
/**
282+
* Checks if the ARN represents a root user account
283+
* @param {string} arn - The ARN to check
284+
* @returns {boolean} True if root user, false otherwise
285+
*/
286+
function _isRootUser(arn) {
287+
if (!arn) {
288+
return false;
271289
}
272-
// User in unauthenticated (anonymous request)
273-
if (requester === undefined) {
290+
291+
// Vault returns the following arn when the account makes requests 'arn:aws:iam::295412255825:/master/',
292+
// with an empty resource type ('user/' prefix missing).
293+
const arns = arn.split(':');
294+
if (arns.length < 6) {
274295
return false;
275296
}
276-
if (principal === requester) {
297+
298+
const resource = arns[arns.length - 1];
299+
300+
// If we start with '/' is because we have a empty resource type so we know it is a root account.
301+
if (resource.startsWith('/')) {
277302
return true;
278303
}
279-
if (_isAccountId(principal)) {
280-
return _getAccountId(requester) === principal;
304+
305+
return false;
306+
}
307+
308+
/** _evaluateCrossAccount - checks if it is a cross-account request.
309+
* @param {string} requesterARN - requester ARN
310+
* @param {string} requesterCanonicalID - requester canonical ID
311+
* @param {string} bucketOwnerCanonicalID - bucket owner canonical ID
312+
* @return {checkPrincipalResult} OK if it is not cross-account, CROSS_ACCOUNT_OK otherwise.
313+
*/
314+
function _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID) {
315+
// Vault returns ARNs like 'arn:aws:iam::123456789012:/master/' for root accounts
316+
// with an empty resource type (missing 'user/' prefix)
317+
if (!_isRootUser(requesterARN)) {
318+
return bucketOwnerCanonicalID === requesterCanonicalID ?
319+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
281320
}
282-
if (principal.endsWith('root')) {
283-
return _getAccountId(requester) === _getAccountId(principal);
321+
322+
return checkPrincipalResult.OK;
323+
}
324+
325+
function _checkPrincipalWildcard(requestARN, requesterCanonicalID, bucketOwnerCanonicalID) {
326+
if (requestARN === undefined) { // User in unauthenticated (anonymous request)
327+
return checkPrincipalResult.OK;
284328
}
285-
return false;
329+
330+
return _checkCrossAccount(requestARN, requesterCanonicalID, bucketOwnerCanonicalID);
286331
}
287332

288-
function _checkPrincipals(canonicalID, arn, principal) {
333+
function _checkPrincipalAWS(principal, requesterARN, requesterCanonicalID, bucketOwnerCanonicalID) {
289334
if (principal === '*') {
290-
return true;
335+
return _checkPrincipalWildcard(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
291336
}
292-
if (principal.CanonicalUser) {
293-
if (Array.isArray(principal.CanonicalUser)) {
294-
return principal.CanonicalUser.some(p => _checkPrincipal(canonicalID, p));
337+
338+
if (requesterARN === undefined) { // User in unauthenticated (anonymous request)
339+
return checkPrincipalResult.KO;
340+
}
341+
342+
if (principal === requesterARN) {
343+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
344+
}
345+
346+
if (_isAccountId(principal) && principal === _getAccountId(requesterARN)) {
347+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
348+
}
349+
350+
if (principal.endsWith('root') && _getAccountId(principal) === _getAccountId(requesterARN)) {
351+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
352+
}
353+
354+
return checkPrincipalResult.KO;
355+
}
356+
357+
function _checkPrincipalCanonicalUser(principal, requesterARN, requesterCanonicalID, bucketOwnerCanonicalID) {
358+
if (principal === '*') {
359+
return _checkPrincipalWildcard(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
360+
}
361+
362+
if (requesterARN === undefined) { // User in unauthenticated (anonymous request)
363+
return checkPrincipalResult.KO;
364+
}
365+
366+
if (principal === requesterCanonicalID) {
367+
return _checkCrossAccount(requesterARN, requesterCanonicalID, bucketOwnerCanonicalID);
368+
}
369+
370+
return checkPrincipalResult.KO;
371+
}
372+
373+
function _findBestPrincipalMatch(principalArray, checkFunc) {
374+
let bestMatch = checkPrincipalResult.KO;
375+
if (!principalArray) {
376+
return bestMatch;
377+
}
378+
379+
const principals = Array.isArray(principalArray) ? principalArray : [principalArray];
380+
381+
for (const p of principals) {
382+
const result = checkFunc(p);
383+
if (result === checkPrincipalResult.OK) {
384+
return checkPrincipalResult.OK; // Highest permission, can exit early
385+
}
386+
if (result > bestMatch) {
387+
bestMatch = result;
295388
}
296-
return _checkPrincipal(canonicalID, principal.CanonicalUser);
297389
}
390+
391+
return bestMatch;
392+
}
393+
394+
function _checkPrincipals(canonicalID, arn, principal, bucketOwnerCanonicalID) {
395+
if (principal === '*') {
396+
return _checkPrincipalWildcard(arn, canonicalID, bucketOwnerCanonicalID);
397+
}
398+
399+
if (principal.CanonicalUser) {
400+
return _findBestPrincipalMatch(principal.CanonicalUser,
401+
p => _checkPrincipalCanonicalUser(p, arn, canonicalID, bucketOwnerCanonicalID));
402+
}
403+
298404
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);
405+
return _findBestPrincipalMatch(principal.AWS,
406+
p => _checkPrincipalAWS(p, arn, canonicalID, bucketOwnerCanonicalID));
303407
}
304-
return false;
408+
409+
return checkPrincipalResult.KO;
305410
}
306411

412+
// checkBucketPolicy FSM.
413+
// ┌───────────────────────────┐
414+
// │ ▼
415+
// │ ┌───────┐
416+
// │ ┌────────►│ ALLOW ├──────────────┐
417+
// │ ┌─────┐ │ └──┬────┘ │
418+
// │ │START│ │ │ │
419+
// │ └──┬──┘ │ │ │
420+
// │ │ │ │ │
421+
// │ ▼ │ ▼ ▼
422+
// │┌──────────────┤ ┌────┐ ┌─────┐
423+
// ││ DEFAULT_DENY ├─────────►│DENY├────────────►│ END │
424+
// │└──────┬───────┤ └────┘ └─────┘
425+
// │ │ │ ▲ ▲ ▲
426+
// │ │ │ │ │ │
427+
// │ │ │ │ │ │
428+
// │ │ │ ┌───┴───────────┐ │ │
429+
// │ │ └────────►│ CROSS_ACCOUNT ├──────┘ │
430+
// │ │ └┬──────────────┘ │
431+
// └───────┼──────────────────┘ │
432+
// └──────────────────────────────────────────┘
433+
//
307434
function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies) {
308-
let permission = 'defaultDeny';
435+
let permission = checkBucketPolicyResult.DEFAULT_DENY;
309436
// if requester is user within bucket owner account, actions should be
310437
// allowed unless explicitly denied (assumes allowed by IAM policy)
311438
if (bucketOwner === canonicalID && actionImplicitDenies[requestType] === false) {
312-
permission = 'allow';
439+
permission = checkBucketPolicyResult.ALLOW;
313440
}
314441
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
315442
while (copiedStatement.length > 0) {
316443
const s = copiedStatement[0];
317-
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal);
444+
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal, bucketOwner);
318445
const actionMatch = _checkBucketPolicyActions(requestType, s.Action, log);
319446
const resourceMatch = _checkBucketPolicyResources(request, s.Resource, log);
320447
const conditionsMatch = _checkBucketPolicyConditions(request, s.Condition, log);
321448

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';
449+
const ok = principalMatch === checkPrincipalResult.OK && actionMatch && resourceMatch && conditionsMatch;
450+
const okCross = principalMatch === checkPrincipalResult.CROSS_ACCOUNT_OK
451+
&& actionMatch && resourceMatch && conditionsMatch;
452+
switch (permission) {
453+
case checkBucketPolicyResult.DEFAULT_DENY:
454+
if ((ok || okCross) && s.Effect === 'Deny') {
455+
return checkBucketPolicyResult.EXPLICIT_DENY;
456+
} else if (ok && s.Effect === 'Allow') {
457+
permission = checkBucketPolicyResult.ALLOW;
458+
} else if (okCross && s.Effect === 'Allow') {
459+
permission = checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW;
460+
}
461+
break;
462+
case checkBucketPolicyResult.EXPLICIT_DENY:
463+
return checkBucketPolicyResult.EXPLICIT_DENY;
464+
case checkBucketPolicyResult.ALLOW:
465+
if ((ok || okCross) && s.Effect === 'Deny') {
466+
return checkBucketPolicyResult.EXPLICIT_DENY;
467+
}
468+
break;
469+
case checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW:
470+
if ((ok || okCross) && s.Effect === 'Deny') {
471+
return checkBucketPolicyResult.EXPLICIT_DENY;
472+
} else if (ok && s.Effect === 'Allow') {
473+
permission = checkBucketPolicyResult.ALLOW;
474+
}
475+
break;
476+
default: // Needed for the linter, should be unreachable.
477+
break;
328478
}
479+
329480
copiedStatement = copiedStatement.splice(1);
330481
}
331482
return permission;
@@ -341,9 +492,13 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
341492
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
342493
bucketOwner, log, request, actionImplicitDenies);
343494

344-
if (bucketPolicyPermission === 'explicitDeny') {
495+
if (bucketPolicyPermission === checkBucketPolicyResult.EXPLICIT_DENY) {
345496
processedResult = false;
346-
} else if (bucketPolicyPermission === 'allow') {
497+
} else if (bucketPolicyPermission === checkBucketPolicyResult.ALLOW) {
498+
processedResult = true;
499+
} else if (bucketPolicyPermission === checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW
500+
&& actionImplicitDenies[requestType] === false) {
501+
// If the bucket policy is cross account, only return true if Vault also returned an explicit allow.
347502
processedResult = true;
348503
} else {
349504
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
@@ -405,7 +560,7 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, aut
405560
arn = authInfo.getArn();
406561
}
407562
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
408-
request, true, results, actionImplicitDenies);
563+
request, true, results, actionImplicitDenies);
409564
});
410565
}
411566

@@ -456,8 +611,8 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
456611
// - account is the bucket owner
457612
// - requester is account, not user
458613
if (bucketOwnerActions.includes(parsedMethodName)
459-
&& (bucketOwner === canonicalID)
460-
&& requesterIsNotUser) {
614+
&& (bucketOwner === canonicalID)
615+
&& requesterIsNotUser) {
461616
results[_requestType] = actionImplicitDenies[_requestType] === false;
462617
return results[_requestType];
463618
}
@@ -613,4 +768,6 @@ module.exports = {
613768
validatePolicyConditions,
614769
isLifecycleSession,
615770
evaluateBucketPolicyWithIAM,
771+
checkBucketPolicy,
772+
checkBucketPolicyResult,
616773
};

0 commit comments

Comments
 (0)