Merge pull request #9277 from naveenpaul1/iam_bucket_policy_arn_validation

IAM | put_bucket_policy Principal validation updated with ARN

Author: liranmauda

src/api/account_api.js

@@ -620,6 +620,7 @@ module.exports = {
             type: 'object',
             required: ['name', 'email', 'has_s3_access'],
             properties: {
+                _id: { type: 'string'},
                 name: { $ref: 'common_api#/definitions/account_name' },
                 email: { $ref: 'common_api#/definitions/email' },
                 is_support: {
@@ -661,6 +662,9 @@ module.exports = {
                 bucket_claim_owner: {
                     $ref: 'common_api#/definitions/bucket_name',
                 },
+                iam_path: {
+                    type: 'string',
+                },
                 external_connections: {
                     type: 'object',
                     properties: {

src/endpoint/iam/iam_utils.js

@@ -62,6 +62,10 @@ function check_iam_path_was_set(iam_path) {
     return iam_path && iam_path !== iam_constants.IAM_DEFAULT_PATH;
 }
 
+function get_iam_username(requested_account_name) {
+    return requested_account_name.split(iam_constants.IAM_SPLIT_CHARACTERS)[0];
+}
+
 /**
  * parse_max_items converts the input to the needed type
  * assuming that we've got only sting type
@@ -818,6 +822,7 @@ exports.create_arn_for_user = create_arn_for_user;
 exports.create_arn_for_root = create_arn_for_root;
 exports.get_action_message_title = get_action_message_title;
 exports.check_iam_path_was_set = check_iam_path_was_set;
+exports.get_iam_username = get_iam_username;
 exports.parse_max_items = parse_max_items;
 exports.validate_params = validate_params;
 exports.validate_iam_path = validate_iam_path;

src/endpoint/s3/s3_bucket_policy_utils.js

@@ -94,6 +94,7 @@ const OP_NAME_TO_ACTION = Object.freeze({
 
 const qm_regex = /\?/g;
 const ar_regex = /\*/g;
+const IAM_DEFAULT_PATH = '/';
 
 const predicate_map = {
     'StringEquals': (request_value, policy_value) => request_value === policy_value,
@@ -353,7 +354,55 @@ function allows_public_access(policy) {
     return false;
 }
 
+/**
+ * Return IAM ARN for account, if account dont have owner
+ * and User, if the user do have account ower
+ *
+ * @param {Object} account 
+ * @returns {string}
+ */
+function get_bucket_policy_principal_arn(account) {
+    const bucket_policy_arn = account.owner ? create_arn_for_user(account.owner, account.name.unwrap().split(':')[0], account.iam_path) :
+                                        create_arn_for_root(account._id);
+    return bucket_policy_arn;
+}
+
+/**
+ * create_arn_for_root creates the AWS ARN for root account user
+ * see: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html#identifiers-arns
+ * @param {nb.ID} account_id (the root user account id)
+ */
+function create_arn_for_root(account_id) {
+    return `arn:aws:iam::${account_id}:root`;
+}
+
+/**
+ * create_arn_for_user creates the AWS ARN for user
+ * see: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html#identifiers-arns
+ * @param {nb.ID} account_id (the root user account id)
+ * @param {string} username
+ * @param {string} iam_path
+ */
+function create_arn_for_user(account_id, username, iam_path) {
+    const basic_structure = `arn:aws:iam::${account_id}:user`;
+    if (username === undefined) return `${basic_structure}/`;
+    if (check_iam_path_was_set(iam_path)) {
+        return `${basic_structure}${iam_path}${username}`;
+    }
+    return `${basic_structure}/${username}`;
+}
+
+/**
+ * check_iam_path_was_set return true if the iam_path was set
+ * @param {string} iam_path
+ */
+function check_iam_path_was_set(iam_path) {
+    return iam_path && iam_path !== IAM_DEFAULT_PATH;
+}
+
 exports.OP_NAME_TO_ACTION = OP_NAME_TO_ACTION;
 exports.has_bucket_policy_permission = has_bucket_policy_permission;
 exports.validate_s3_policy = validate_s3_policy;
 exports.allows_public_access = allows_public_access;
+exports.get_bucket_policy_principal_arn = get_bucket_policy_principal_arn;
+exports.create_arn_for_root = create_arn_for_root;

src/endpoint/s3/s3_rest.js

@@ -233,7 +233,6 @@ async function authorize_request(req) {
 async function authorize_request_policy(req) {
     if (!req.params.bucket) return;
     if (req.op_name === 'put_bucket') return;
-
     // owner_account is { id: bucket.owner_account, email: bucket.bucket_owner };
     const {
         s3_policy,
@@ -256,6 +255,7 @@ async function authorize_request_policy(req) {
     const account = req.object_sdk.requesting_account;
     const account_identifier_name = req.object_sdk.nsfs_config_root ? account.name.unwrap() : account.email.unwrap();
     const account_identifier_id = req.object_sdk.nsfs_config_root ? account._id : undefined;
+    const account_identifier_arn = s3_bucket_policy_utils.get_bucket_policy_principal_arn(account);
 
     // deny delete_bucket permissions from bucket_claim_owner accounts (accounts that were created by OBC from openshift\k8s)
     // the OBC bucket can still be delete by normal accounts according to the access policy which is checked below
@@ -294,6 +294,7 @@ async function authorize_request_policy(req) {
     // in case we have bucket policy
     let permission_by_id;
     let permission_by_name;
+    let permission_by_arn;
 
     // In NC, we allow principal to be:
     // 1. account name (for backwards compatibility)
@@ -307,7 +308,6 @@ async function authorize_request_policy(req) {
         dbg.log3('authorize_request_policy: permission_by_id', permission_by_id);
     }
     if (permission_by_id === "DENY") throw new S3Error(S3Error.AccessDenied);
-
     if ((!account_identifier_id || permission_by_id !== "DENY") && account.owner === undefined) {
         permission_by_name = await s3_bucket_policy_utils.has_bucket_policy_permission(
             s3_policy, account_identifier_name, method, arn_path, req,
@@ -316,7 +316,17 @@ async function authorize_request_policy(req) {
         dbg.log3('authorize_request_policy: permission_by_name', permission_by_name);
     }
     if (permission_by_name === "DENY") throw new S3Error(S3Error.AccessDenied);
-    if ((permission_by_id === "ALLOW" || permission_by_name === "ALLOW") || is_owner) return;
+    // Containerized deployment always will have account_identifier_id undefined
+    // Policy permission is validated by account arn
+    if (!account_identifier_id) {
+        permission_by_arn = await s3_bucket_policy_utils.has_bucket_policy_permission(
+            s3_policy, account_identifier_arn, method, arn_path, req, public_access_block?.restrict_public_buckets
+        );
+        dbg.log3('authorize_request_policy: permission_by_arn', permission_by_arn);
+    }
+    if (permission_by_arn === "DENY") throw new S3Error(S3Error.AccessDenied);
+
+    if ((permission_by_id === "ALLOW" || permission_by_name === "ALLOW" || permission_by_arn === "ALLOW") || is_owner) return;
 
     throw new S3Error(S3Error.AccessDenied);
 }

src/sdk/accountspace_nb.js

@@ -85,7 +85,7 @@ class AccountSpaceNB {
         const action = IAM_ACTIONS.GET_USER;
         const requesting_account = system_store.get_account_by_email(account_sdk.requesting_account.email);
         const requested_account = validate_and_return_requested_account(params, action, requesting_account);
-        const username = account_util.get_iam_username(params.username || requested_account.name.unwrap());
+        const username = iam_utils.get_iam_username(params.username || requested_account.name.unwrap());
         const iam_arn = iam_utils.create_arn_for_user(requesting_account._id.toString(), username,
                                     requested_account.iam_path || IAM_DEFAULT_PATH);
         const reply = {
@@ -110,7 +110,7 @@ class AccountSpaceNB {
         account_util._check_if_account_exists(action, old_username);
         const requested_account = system_store.get_account_by_email(old_username);
         let iam_path = requested_account.iam_path;
-        let user_name = account_util.get_iam_username(requested_account.name.unwrap());
+        let user_name = iam_utils.get_iam_username(requested_account.name.unwrap());
         account_util._check_username_already_exists(action, { username: params.new_username }, requesting_account);
         account_util._check_if_requested_account_is_root_account_or_IAM_user(action, requesting_account, requested_account);
         account_util._check_if_requested_is_owned_by_root_account(action, requesting_account, requested_account);
@@ -173,7 +173,7 @@ class AccountSpaceNB {
             return account.owner?._id.toString() === requesting_account._id.toString();
         });
         let members = _.map(requesting_account_iam_users, function(iam_user) {
-            const iam_username = account_util.get_iam_username(iam_user.name.unwrap());
+            const iam_username = iam_utils.get_iam_username(iam_user.name.unwrap());
             const iam_path = iam_user.iam_path || IAM_DEFAULT_PATH;
             const member = {
                 user_id: iam_user._id.toString(),
@@ -218,7 +218,7 @@ class AccountSpaceNB {
         }
 
         return {
-            username: account_util.get_iam_username(requested_account.name.unwrap()),
+            username: iam_utils.get_iam_username(requested_account.name.unwrap()),
             access_key: iam_access_key.access_key.unwrap(),
             create_date: iam_access_key.creation_date,
             status: ACCESS_KEY_STATUS_ENUM.ACTIVE,
@@ -238,7 +238,7 @@ class AccountSpaceNB {
             region: dummy_region, // GAP
             last_used_date: Date.now(), // GAP
             service_name: dummy_service_name, // GAP
-            username: username ? account_util.get_iam_username(username) : undefined,
+            username: username ? iam_utils.get_iam_username(username) : undefined,
         };
     }
 

src/server/common_services/auth_server.js

@@ -15,6 +15,7 @@ const addr_utils = require('../../util/addr_utils');
 const kube_utils = require('../../util/kube_utils');
 const jwt_utils = require('../../util/jwt_utils');
 const config = require('../../../config');
+const iam_utils = require('../../endpoint/iam/iam_utils');
 const s3_bucket_policy_utils = require('../../endpoint/s3/s3_bucket_policy_utils');
 
 
@@ -548,10 +549,11 @@ async function has_bucket_action_permission(bucket, account, action, req_query,
     if (!action) {
         throw new Error('has_bucket_action_permission: action is required');
     }
-
+    const arn = account.owner ? iam_utils.create_arn_for_user(account.owner._id.toString(), account.name.unwrap().split(':')[0], account.iam_path) :
+                                    iam_utils.create_arn_for_root(account._id);
     const result = await s3_bucket_policy_utils.has_bucket_policy_permission(
         bucket_policy,
-        account.email.unwrap(),
+        arn,
         action,
         `arn:aws:s3:::${bucket.name.unwrap()}${bucket_path}`,
         req_query

src/server/system_services/account_server.js

@@ -988,7 +988,7 @@ function get_account_info(account, include_connection_cache) {
         'has_login',
         'allowed_ips',
     );
-
+    info._id = account._id.toString();
     if (account.is_support) {
         info.is_support = true;
         info.has_login = true;
@@ -998,7 +998,10 @@ function get_account_info(account, include_connection_cache) {
             secret_key: 'Not Accesible'
         }];
     }
-
+    if (account.owner) {
+        info.owner = account.owner._id.toString();
+    }
+    info.iam_path = account.iam_path;
     if (account.next_password_change) {
         info.next_password_change = account.next_password_change.getTime();
     }

src/server/system_services/bucket_server.js

@@ -11,6 +11,7 @@ const GoogleStorage = require('../../util/google_storage_wrap');
 
 const P = require('../../util/promise');
 const dbg = require('../../util/debug_module')(__filename);
+const SensitiveString = require('../../util/sensitive_string');
 const config = require('../../../config');
 const MDStore = require('../object_services/md_store').MDStore;
 const BucketStatsStore = require('../analytic_services/bucket_stats_store').BucketStatsStore;
@@ -39,6 +40,7 @@ const bucket_semaphore = new KeysSemaphore(1);
 const Quota = require('../system_services/objects/quota');
 const { STORAGE_CLASS_GLACIER_IR } = require('../../endpoint/s3/s3_utils');
 const noobaa_s3_client = require('../../sdk/noobaa_s3_client/noobaa_s3_client');
+const string_utils = require('../../util/string_utils');
 
 const VALID_BUCKET_NAME_REGEXP =
     /^(([a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])\.)*([a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])$/;
@@ -512,12 +514,43 @@ async function get_bucket_policy(req) {
     };
 }
 
+/** 
+    Validate and return account by principal ARN.
+    1. validate basic ARN, like arn prefix `arn:aws:iam::`
+    2. If principal ARN ends with `root` suffix, it's an account and get account with id 
+       eg: arn:aws:iam::${account_id}:root
+    3. if principal ARN contains `user`, it's an IAM user and get account with username and id
+       eg: arn:aws:iam::${account_id}:user/${iam_path}/${user_name}
+         account email = ${iam_user_name}:${account_id}
+
+    @param {SensitiveString | String} principal Bucket policy principal
+*/
+async function get_account_by_principal(principal) {
+    const principal_as_string = principal instanceof SensitiveString ? principal.unwrap() : principal;
+    const root_sufix = 'root';
+    const user_sufix = 'user';
+    const arn_parts = principal_as_string.split(':');
+    if (!string_utils.AWS_IAM_ARN_REGEXP.test(principal_as_string)) {
+        return;
+    }
+    const account_id = arn_parts[4];
+    const arn_sufix = arn_parts[5];
+    if (principal_as_string.endsWith(root_sufix) && !arn_sufix.startsWith(user_sufix)) {
+        return system_store.data.accounts.find(account => account._id.toString() === account_id);
+    } else if (arn_sufix && arn_sufix.startsWith(user_sufix)) {
+        const arn_path_parts = principal_as_string.split('/');
+        const iam_user_name = arn_path_parts[arn_path_parts.length - 1].trim();
+        return system_store.get_account_by_email(new SensitiveString(`${iam_user_name}:${account_id}`));
+    } //else {
+    //  wrong principal ARN should not return anything.
+    //}
+}
 
 async function put_bucket_policy(req) {
     dbg.log0('put_bucket_policy:', req.rpc_params);
     const bucket = find_bucket(req, req.rpc_params.name);
     await bucket_policy_utils.validate_s3_policy(req.rpc_params.policy, bucket.name,
-        principal => system_store.get_account_by_email(principal));
+        principal => get_account_by_principal(principal));
 
     if (
         bucket.public_access_block?.block_public_policy &&