Skip to content

Commit ccf75c1

Browse files
leif-scalityleif-scality
committed
CLDSRV-674: handle cross-account bucket policies
1 parent 14a7779 commit ccf75c1

File tree

3 files changed

+871
-135
lines changed

3 files changed

+871
-135
lines changed

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

Lines changed: 173 additions & 31 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,203 @@ function _isAccountId(principal) {
265265
return (principal.length === 12 && /^\d+$/.test(principal));
266266
}
267267

268-
function _checkPrincipal(requester, principal) {
269-
if (principal === '*') {
268+
function _isRootUser(arn) {
269+
if (!arn) {
270+
return false;
271+
}
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('/')) {
270280
return true;
271281
}
272-
// User in unauthenticated (anonymous request)
273-
if (requester === undefined) {
274-
return false;
282+
283+
return false;
284+
}
285+
286+
const checkPrincipalResult = {
287+
KO: 0,
288+
OK: 1,
289+
CROSS_ACCOUNT_OK: 2,
290+
};
291+
292+
function _checkPrincipal(requester, principal, buckerOwnerCanonicalID, requesterCanonicalID) {
293+
if (principal === '*') {
294+
if (!_isRootUser(requester)) {
295+
return buckerOwnerCanonicalID === requesterCanonicalID ?
296+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
297+
}
298+
299+
return checkPrincipalResult.OK;
300+
}
301+
if (requester === undefined) { // User in unauthenticated (anonymous request)
302+
return checkPrincipalResult.KO;
275303
}
276304
if (principal === requester) {
277-
return true;
305+
if (!_isRootUser(requester)) {
306+
return buckerOwnerCanonicalID === requesterCanonicalID ?
307+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
308+
}
309+
310+
return checkPrincipalResult.OK;
278311
}
279312
if (_isAccountId(principal)) {
280-
return _getAccountId(requester) === principal;
313+
if (_getAccountId(requester) !== principal) {
314+
return checkPrincipalResult.KO;
315+
}
316+
317+
if (!_isRootUser(requester)) {
318+
return buckerOwnerCanonicalID === requesterCanonicalID ?
319+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
320+
}
321+
322+
return checkPrincipalResult.OK;
281323
}
282324
if (principal.endsWith('root')) {
283-
return _getAccountId(requester) === _getAccountId(principal);
325+
if (_getAccountId(requester) !== _getAccountId(principal)) {
326+
return checkPrincipalResult.KO;
327+
}
328+
329+
if (!_isRootUser(requester)) {
330+
return buckerOwnerCanonicalID === requesterCanonicalID ?
331+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
332+
}
333+
334+
return checkPrincipalResult.OK;
284335
}
285-
return false;
336+
return checkPrincipalResult.KO;
286337
}
287338

288-
function _checkPrincipals(canonicalID, arn, principal) {
339+
function _checkPrincipals(canonicalID, arn, principal, buckerOwnerCanonicalID) {
289340
if (principal === '*') {
290-
return true;
341+
if (arn === undefined) { // User in unauthenticated (anonymous request)
342+
return checkPrincipalResult.OK;
343+
}
344+
345+
if (!_isRootUser(arn)) {
346+
return buckerOwnerCanonicalID === canonicalID ?
347+
checkPrincipalResult.OK : checkPrincipalResult.CROSS_ACCOUNT_OK;
348+
}
349+
350+
return checkPrincipalResult.OK;
291351
}
292352
if (principal.CanonicalUser) {
293353
if (Array.isArray(principal.CanonicalUser)) {
294-
return principal.CanonicalUser.some(p => _checkPrincipal(canonicalID, p));
354+
let out = checkPrincipalResult.KO;
355+
for (const p of principal.CanonicalUser) {
356+
if (out === checkPrincipalResult.OK) {
357+
break;
358+
}
359+
360+
const res = _checkPrincipal(canonicalID, p, buckerOwnerCanonicalID, canonicalID);
361+
if (res !== checkPrincipalResult.KO) {
362+
out = res;
363+
}
364+
}
365+
return out;
295366
}
296-
return _checkPrincipal(canonicalID, principal.CanonicalUser);
367+
return _checkPrincipal(canonicalID, principal.CanonicalUser, buckerOwnerCanonicalID, canonicalID);
297368
}
298369
if (principal.AWS) {
299370
if (Array.isArray(principal.AWS)) {
300-
return principal.AWS.some(p => _checkPrincipal(arn, p));
371+
let out = checkPrincipalResult.KO;
372+
for (const p of principal.AWS) {
373+
if (out === checkPrincipalResult.OK) {
374+
break;
375+
}
376+
377+
const res = _checkPrincipal(arn, p, buckerOwnerCanonicalID, canonicalID);
378+
if (res !== checkPrincipalResult.KO) {
379+
out = res;
380+
}
381+
}
382+
383+
return out;
301384
}
302-
return _checkPrincipal(arn, principal.AWS);
385+
return _checkPrincipal(arn, principal.AWS, buckerOwnerCanonicalID, canonicalID);
303386
}
304-
return false;
387+
return checkPrincipalResult.KO;
305388
}
306389

390+
const checkBucketPolicyResult = {
391+
DEFAULT_DENY: 0,
392+
EXPLICIT_DENY: 1,
393+
ALLOW: 2,
394+
CROSS_ACCOUNT_ALLOW: 3,
395+
};
396+
397+
// checkBucketPolicy FSM.
398+
// ┌───────────────────────────┐
399+
// │ ▼
400+
// │ ┌───────┐
401+
// │ ┌────────►│ ALLOW ├──────────────┐
402+
// │ ┌─────┐ │ └──┬────┘ │
403+
// │ │START│ │ │ │
404+
// │ └──┬──┘ │ │ │
405+
// │ │ │ │ │
406+
// │ ▼ │ ▼ ▼
407+
// │┌──────────────┤ ┌────┐ ┌─────┐
408+
// ││ DEFAULT_DENY ├─────────►│DENY├────────────►│ END │
409+
// │└──────┬───────┤ └────┘ └─────┘
410+
// │ │ │ ▲ ▲ ▲
411+
// │ │ │ │ │ │
412+
// │ │ │ │ │ │
413+
// │ │ │ ┌───┴───────────┐ │ │
414+
// │ │ └────────►│ CROSS_ACCOUNT ├──────┘ │
415+
// │ │ └┬──────────────┘ │
416+
// └───────┼──────────────────┘ │
417+
// └──────────────────────────────────────────┘
418+
//
307419
function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, log, request, actionImplicitDenies) {
308-
let permission = 'defaultDeny';
420+
let permission = checkBucketPolicyResult.DEFAULT_DENY;
309421
// if requester is user within bucket owner account, actions should be
310422
// allowed unless explicitly denied (assumes allowed by IAM policy)
311423
if (bucketOwner === canonicalID && actionImplicitDenies[requestType] === false) {
312-
permission = 'allow';
424+
permission = checkBucketPolicyResult.ALLOW;
313425
}
314426
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
315427
while (copiedStatement.length > 0) {
316428
const s = copiedStatement[0];
317-
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal);
429+
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal, bucketOwner);
318430
const actionMatch = _checkBucketPolicyActions(requestType, s.Action, log);
319431
const resourceMatch = _checkBucketPolicyResources(request, s.Resource, log);
320432
const conditionsMatch = _checkBucketPolicyConditions(request, s.Condition, log);
321433

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';
434+
const ok = principalMatch === checkPrincipalResult.OK && actionMatch && resourceMatch && conditionsMatch;
435+
const okCross = principalMatch === checkPrincipalResult.CROSS_ACCOUNT_OK
436+
&& actionMatch && resourceMatch && conditionsMatch;
437+
switch (permission) {
438+
case checkBucketPolicyResult.DEFAULT_DENY:
439+
if ((ok || okCross) && s.Effect === 'Deny') {
440+
return checkBucketPolicyResult.EXPLICIT_DENY;
441+
} else if (ok && s.Effect === 'Allow') {
442+
permission = checkBucketPolicyResult.ALLOW;
443+
} else if (okCross && s.Effect === 'Allow') {
444+
permission = checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW;
445+
}
446+
break;
447+
case checkBucketPolicyResult.EXPLICIT_DENY:
448+
return checkBucketPolicyResult.EXPLICIT_DENY;
449+
case checkBucketPolicyResult.ALLOW:
450+
if ((ok || okCross) && s.Effect === 'Deny') {
451+
return checkBucketPolicyResult.EXPLICIT_DENY;
452+
}
453+
break;
454+
case checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW:
455+
if ((ok || okCross) && s.Effect === 'Deny') {
456+
return checkBucketPolicyResult.EXPLICIT_DENY;
457+
} else if (ok && s.Effect === 'Allow') {
458+
permission = checkBucketPolicyResult.ALLOW;
459+
}
460+
break;
461+
default: // Needed for the linter, should be unreachable.
462+
break;
328463
}
464+
329465
copiedStatement = copiedStatement.splice(1);
330466
}
331467
return permission;
@@ -341,9 +477,13 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
341477
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
342478
bucketOwner, log, request, actionImplicitDenies);
343479

344-
if (bucketPolicyPermission === 'explicitDeny') {
480+
if (bucketPolicyPermission === checkBucketPolicyResult.EXPLICIT_DENY) {
345481
processedResult = false;
346-
} else if (bucketPolicyPermission === 'allow') {
482+
} else if (bucketPolicyPermission === checkBucketPolicyResult.ALLOW) {
483+
processedResult = true;
484+
} else if (bucketPolicyPermission === checkBucketPolicyResult.CROSS_ACCOUNT_ALLOW
485+
&& actionImplicitDenies[requestType] === false) {
486+
// If the bucket policy is cross account, only return true if Vault also returned an explicit allow.
347487
processedResult = true;
348488
} else {
349489
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
@@ -405,7 +545,7 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, aut
405545
arn = authInfo.getArn();
406546
}
407547
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
408-
request, true, results, actionImplicitDenies);
548+
request, true, results, actionImplicitDenies);
409549
});
410550
}
411551

@@ -456,8 +596,8 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
456596
// - account is the bucket owner
457597
// - requester is account, not user
458598
if (bucketOwnerActions.includes(parsedMethodName)
459-
&& (bucketOwner === canonicalID)
460-
&& requesterIsNotUser) {
599+
&& (bucketOwner === canonicalID)
600+
&& requesterIsNotUser) {
461601
results[_requestType] = actionImplicitDenies[_requestType] === false;
462602
return results[_requestType];
463603
}
@@ -613,4 +753,6 @@ module.exports = {
613753
validatePolicyConditions,
614754
isLifecycleSession,
615755
evaluateBucketPolicyWithIAM,
756+
checkBucketPolicy,
757+
checkBucketPolicyResult,
616758
};

0 commit comments

Comments
 (0)