Skip to content

Commit 45fe1e7

Browse files
authored
Merge pull request #9281 from shirady/iam-user-policy-rest-s3
IAM | Authorize Requests for IAM Users According to IAM User Inline Policy
2 parents 477d3ae + 55850a2 commit 45fe1e7

File tree

5 files changed

+86
-13
lines changed

5 files changed

+86
-13
lines changed

src/api/account_api.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,15 @@ module.exports = {
749749
},
750750
nsfs_account_config: {
751751
$ref: 'common_api#/definitions/nsfs_account_config'
752+
},
753+
iam_user_policies: {
754+
type: 'array',
755+
items: {
756+
$ref: 'common_api#/definitions/iam_user_policy',
757+
}
758+
},
759+
owner: {
760+
type: 'string'
752761
}
753762
},
754763
},

src/endpoint/s3/s3_bucket_policy_utils.js

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,28 @@ async function _is_object_version_fit(req, predicate, value) {
149149
return res;
150150
}
151151

152-
async function has_bucket_policy_permission(policy, account, method, arn_path, req, disallow_public_access = false) {
152+
async function has_bucket_policy_permission(policy, account, method, arn_path, req,
153+
{ disallow_public_access = false, should_pass_principal = true } = {}) {
153154
const [allow_statements, deny_statements] = _.partition(policy.Statement, statement => statement.Effect === 'Allow');
154155

155156
// the case where the permission is an array started in op get_object_attributes
156157
const method_arr = Array.isArray(method) ? method : [method];
157158

158159
// look for explicit denies
159160
const res_arr_deny = await is_statement_fit_of_method_array(
160-
deny_statements, account, method_arr, arn_path, req); // No need to disallow in "DENY"
161+
deny_statements, account, method_arr, arn_path, req, {
162+
disallow_public_access: false, // No need to disallow in "DENY"
163+
should_pass_principal
164+
}
165+
);
161166
if (res_arr_deny.every(item => item)) return 'DENY';
162167

163168
// look for explicit allows
164169
const res_arr_allow = await is_statement_fit_of_method_array(
165-
allow_statements, account, method_arr, arn_path, req, disallow_public_access);
170+
allow_statements, account, method_arr, arn_path, req, {
171+
disallow_public_access,
172+
should_pass_principal
173+
});
166174
if (res_arr_allow.every(item => item)) return 'ALLOW';
167175

168176
// implicit deny
@@ -217,15 +225,19 @@ function _is_resource_fit(arn_path, statement) {
217225
return statement.Resource ? resource_fit : !resource_fit;
218226
}
219227

220-
async function is_statement_fit_of_method_array(statements, account, method_arr, arn_path, req, disallow_public_access = false) {
228+
async function is_statement_fit_of_method_array(statements, account, method_arr, arn_path, req,
229+
{ disallow_public_access = false, should_pass_principal = true } = {}) {
221230
return Promise.all(method_arr.map(method_permission =>
222-
_is_statements_fit(statements, account, method_permission, arn_path, req, disallow_public_access)));
231+
_is_statements_fit(statements, account, method_permission, arn_path, req, { disallow_public_access, should_pass_principal })));
223232
}
224233

225-
async function _is_statements_fit(statements, account, method, arn_path, req, disallow_public_access = false) {
234+
async function _is_statements_fit(statements, account, method, arn_path, req,
235+
{ disallow_public_access = false, should_pass_principal = true} = {}) {
226236
for (const statement of statements) {
227237
const action_fit = _is_action_fit(method, statement);
228-
const principal_fit = _is_principal_fit(account, statement, disallow_public_access);
238+
// When evaluating IAM user inline policies, should_pass_principal is false since these policies
239+
// don't have a Principal field (the principal is implicitly the user)
240+
const principal_fit = should_pass_principal ? _is_principal_fit(account, statement, disallow_public_access) : true;
229241
const resource_fit = _is_resource_fit(arn_path, statement);
230242
const condition_fit = await _is_condition_fit(statement, req, method);
231243

src/endpoint/s3/s3_rest.js

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,9 @@ async function authorize_request(req) {
224224
req.object_sdk.authorize_request_account(req),
225225
// authorize_request_policy(req) is supposed to
226226
// allow owners access unless there is an explicit DENY policy
227-
authorize_request_policy(req)
227+
authorize_request_policy(req),
228+
// authorize_request_iam_policy(req) is for users only
229+
authorize_request_iam_policy(req),
228230
]);
229231
}
230232

@@ -299,14 +301,17 @@ async function authorize_request_policy(req) {
299301
// we start the permission check on account identifier intentionally
300302
if (account_identifier_id) {
301303
permission_by_id = await s3_bucket_policy_utils.has_bucket_policy_permission(
302-
s3_policy, account_identifier_id, method, arn_path, req, public_access_block?.restrict_public_buckets);
304+
s3_policy, account_identifier_id, method, arn_path, req,
305+
{ disallow_public_access: public_access_block?.restrict_public_buckets }
306+
);
303307
dbg.log3('authorize_request_policy: permission_by_id', permission_by_id);
304308
}
305309
if (permission_by_id === "DENY") throw new S3Error(S3Error.AccessDenied);
306310

307311
if ((!account_identifier_id || permission_by_id !== "DENY") && account.owner === undefined) {
308312
permission_by_name = await s3_bucket_policy_utils.has_bucket_policy_permission(
309-
s3_policy, account_identifier_name, method, arn_path, req, public_access_block?.restrict_public_buckets
313+
s3_policy, account_identifier_name, method, arn_path, req,
314+
{ disallow_public_access: public_access_block?.restrict_public_buckets }
310315
);
311316
dbg.log3('authorize_request_policy: permission_by_name', permission_by_name);
312317
}
@@ -316,11 +321,50 @@ async function authorize_request_policy(req) {
316321
throw new S3Error(S3Error.AccessDenied);
317322
}
318323

324+
async function authorize_request_iam_policy(req) {
325+
const auth_token = req.object_sdk.get_auth_token();
326+
const is_anonymous = !(auth_token && auth_token.access_key);
327+
if (is_anonymous) return;
328+
329+
const account = req.object_sdk.requesting_account;
330+
const is_iam_user = account.owner !== undefined;
331+
if (!is_iam_user) return; // IAM policy is only on IAM users (account root user is authorized here)
332+
333+
const resource_arn = _get_arn_from_req_path(req);
334+
const method = _get_method_from_req(req);
335+
const iam_policies = account.iam_user_policies || [];
336+
if (iam_policies.length === 0) return;
337+
338+
// parallel policy check
339+
const promises = [];
340+
for (const iam_policy of iam_policies) {
341+
// We are reusing the bucket policy util function as it checks the policy document
342+
const promise = s3_bucket_policy_utils.has_bucket_policy_permission(
343+
iam_policy.policy_document, undefined, method, resource_arn, req,
344+
{ should_pass_principal: false }
345+
);
346+
promises.push(promise);
347+
}
348+
const permission_result = await Promise.all(promises);
349+
let has_allow_permission = false;
350+
for (const permission of permission_result) {
351+
if (permission === "DENY") throw new S3Error(S3Error.AccessDenied);
352+
if (permission === "ALLOW") {
353+
has_allow_permission = true;
354+
}
355+
}
356+
if (has_allow_permission) return;
357+
dbg.log1('authorize_request_iam_policy: user have inline policies but none of them matched the method');
358+
throw new S3Error(S3Error.AccessDenied);
359+
}
360+
319361
async function authorize_anonymous_access(s3_policy, method, arn_path, req, public_access_block) {
320362
if (!s3_policy) throw new S3Error(S3Error.AccessDenied);
321363

322364
const permission = await s3_bucket_policy_utils.has_bucket_policy_permission(
323-
s3_policy, undefined, method, arn_path, req, public_access_block?.restrict_public_buckets);
365+
s3_policy, undefined, method, arn_path, req,
366+
{ disallow_public_access: public_access_block?.restrict_public_buckets }
367+
);
324368
if (permission === "ALLOW") return;
325369

326370
throw new S3Error(S3Error.AccessDenied);

src/sdk/bucketspace_fs.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
951951
action,
952952
`arn:aws:s3:::${bucket.name.unwrap()}${bucket_path}`,
953953
undefined,
954-
bucket.public_access_block?.restrict_public_buckets,
954+
{ disallow_public_access: bucket.public_access_block?.restrict_public_buckets }
955955
);
956956
if (permission_by_id === "DENY") return false;
957957
// we (currently) allow account identified to be both id and name,
@@ -963,7 +963,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
963963
action,
964964
`arn:aws:s3:::${bucket.name.unwrap()}${bucket_path}`,
965965
undefined,
966-
bucket.public_access_block?.restrict_public_buckets,
966+
{ disallow_public_access: bucket.public_access_block?.restrict_public_buckets }
967967
);
968968
}
969969
if (permission_by_name === 'DENY') return false;

src/server/system_services/account_server.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,6 +1049,14 @@ function get_account_info(account, include_connection_cache) {
10491049
};
10501050
info.role_config = account.role_config;
10511051
info.force_md5_etag = account.force_md5_etag;
1052+
1053+
if (account.iam_user_policies) {
1054+
info.iam_user_policies = account.iam_user_policies;
1055+
}
1056+
if (account.owner) {
1057+
info.owner = account.owner._id.toString();
1058+
}
1059+
10521060
return info;
10531061
}
10541062

0 commit comments

Comments
 (0)