From a8c3fc7eebd552db31fcfbfd071f2a0b37495c72 Mon Sep 17 00:00:00 2001 From: Romy <35330373+romayalon@users.noreply.github.com> Date: Thu, 12 Sep 2024 19:37:36 +0300 Subject: [PATCH] NC | Check access - skip write check and mode bits check by default add health access check configuration Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com> --- config.js | 2 + .../ConfigFileCustomizations.md | 40 +++++++++++++++++-- src/manage_nsfs/health.js | 28 +++++++++---- src/sdk/config_fs.js | 9 ++++- .../schemas/nsfs_config_schema.js | 10 ++++- .../test_nc_nsfs_account_cli.test.js | 24 ++++++----- .../test_nc_nsfs_bucket_cli.test.js | 9 ++++- ...c_nsfs_new_buckets_path_validation.test.js | 4 +- src/util/native_fs_utils.js | 3 ++ 9 files changed, 104 insertions(+), 25 deletions(-) diff --git a/config.js b/config.js index facbd2e31e..2a66a589ec 100644 --- a/config.js +++ b/config.js @@ -890,6 +890,8 @@ config.NC_MASTER_KEYS_MANAGER_REFRESH_THRESHOLD = -1; // currently we want to di config.MASTER_KEYS_EXEC_MAX_RETRIES = 3; config.NC_DISABLE_ACCESS_CHECK = false; +config.NC_DISABLE_HEALTH_ACCESS_CHECK = false; +config.NC_DISABLE_POSIX_MODE_ACCESS_CHECK = true; config.NC_DISABLE_SCHEMA_CHECK = false; ////////// GPFS ////////// diff --git a/docs/NooBaaNonContainerized/ConfigFileCustomizations.md b/docs/NooBaaNonContainerized/ConfigFileCustomizations.md index 0e363c9319..93773a5ab2 100644 --- a/docs/NooBaaNonContainerized/ConfigFileCustomizations.md +++ b/docs/NooBaaNonContainerized/ConfigFileCustomizations.md @@ -356,14 +356,16 @@ Warning: After setting this configuration, NooBaa will skip schema validations a ``` -### 25. Disable Read/Write accessibility check - +### 25. Disable Read accessibility check - * Key: `NC_DISABLE_ACCESS_CHECK` * Type: Boolean * Default: false -* Description: This flag will disable Read/Write accessibility validations in the following flows - +* Description: Setting this flag to true will disable Read accessibility validations in the following flows - 1. Bucket creation/update - NooBaa will not validate that the bucket owner has read/write permissions to the bucket's path. 2. Account creation/update - NooBaa will not validate that the account owner has read/write permissions to the account's new_buckets_path. - Warning - setting this configuration to true might result with unexpected behavior. + 3. Health buckets and accounts accessibility check. + Warning - setting this configuration to true might result with unexpected behavior. + * Steps: ``` 1. Open /path/to/config_dir/config.json file. @@ -372,6 +374,38 @@ Warning: After setting this configuration, NooBaa will skip schema validations a "NC_DISABLE_ACCESS_CHECK": true ``` +### 26. Disable Read accessibility check on the Health CLI - +* Key: `NC_DISABLE_HEALTH_ACCESS_CHECK` +* Type: Boolean +* Default: false +* Description: This flag will disable Read accessibility validations in Health check of buckets and accounts. + +* Steps: + ``` + 1. Open /path/to/config_dir/config.json file. + 2. Set the config key - + Example: + "NC_DISABLE_HEALTH_ACCESS_CHECK": true + ``` + +### 27. Disable Read/Write POSIX mode bits check - +* Key: `NC_DISABLE_POSIX_MODE_ACCESS_CHECK` +* Type: Boolean +* Default: true +* Description: Setting this flag to false will enable Read/Write mode bits accessibility validations by in the following flows - + 1. Bucket creation/update - NooBaa will validate that the bucket owner has read/write permissions to the bucket's path. + 2. Account creation/update - NooBaa will validate that the account owner has read/write permissions to the account's new_buckets_path. + 3. Health buckets and accounts accessibility check. + Warning - setting this configuration to false won't support a check of the ACLs and be based only on mode bits check. + +* Steps: + ``` + 1. Open /path/to/config_dir/config.json file. + 2. Set the config key - + Example: + "NC_DISABLE_POSIX_MODE_ACCESS_CHECK": false + ``` + ### 26. Set Endpoint process title - * Key: `ENDPOINT_PROCESS_TITLE` diff --git a/src/manage_nsfs/health.js b/src/manage_nsfs/health.js index 9cf742ec4a..0dadcef33c 100644 --- a/src/manage_nsfs/health.js +++ b/src/manage_nsfs/health.js @@ -361,7 +361,7 @@ class NSFSHealth { config_data.nsfs_account_config.new_buckets_path; if (type === TYPES.ACCOUNT) { - const config_file_path = this.config_fs.get_account_path_by_name(config_file.name); + const config_file_path = this.config_fs.get_account_path_by_name(config_file); res = await is_new_buckets_path_valid(config_file_path, config_data, storage_path); } else if (type === TYPES.BUCKET) { res = await is_bucket_storage_path_exists(this.config_fs.fs_context, config_data, storage_path); @@ -474,12 +474,14 @@ async function is_new_buckets_path_valid(config_file_path, config_data, new_buck } try { - await nb_native().fs.stat(account_fs_context, new_buckets_path); - const accessible = await native_fs_utils.is_dir_rw_accessible(account_fs_context, new_buckets_path); - if (!accessible) { - const new_err = new Error('ACCESS DENIED'); - new_err.code = 'EACCES'; - throw new_err; + if (!_should_skip_health_access_check()) { + await nb_native().fs.stat(account_fs_context, new_buckets_path); + const accessible = await native_fs_utils.is_dir_rw_accessible(account_fs_context, new_buckets_path); + if (!accessible) { + const new_err = new Error('ACCESS DENIED'); + new_err.code = 'EACCES'; + throw new_err; + } } res_obj = get_valid_object(config_data.name, undefined, new_buckets_path); } catch (err) { @@ -505,7 +507,9 @@ async function is_new_buckets_path_valid(config_file_path, config_data, new_buck async function is_bucket_storage_path_exists(fs_context, config_data, storage_path) { let res_obj; try { - await nb_native().fs.stat(fs_context, storage_path); + if (!_should_skip_health_access_check()) { + await nb_native().fs.stat(fs_context, storage_path); + } res_obj = get_valid_object(config_data.name, undefined, storage_path); } catch (err) { let err_code; @@ -556,5 +560,13 @@ function get_invalid_object(name, config_path, storage_path, err_code) { }; } +/** + * _should_skip_access_check returns true if the health CLI should skip access check + * @returns {Boolean} + */ +function _should_skip_health_access_check() { + return config.NC_DISABLE_HEALTH_ACCESS_CHECK || config.NC_DISABLE_ACCESS_CHECK; +} + exports.get_health_status = get_health_status; exports.NSFSHealth = NSFSHealth; diff --git a/src/sdk/config_fs.js b/src/sdk/config_fs.js index ee3901be80..adb3dff848 100644 --- a/src/sdk/config_fs.js +++ b/src/sdk/config_fs.js @@ -161,7 +161,7 @@ class ConfigFS { } /** - * create_config_json_file created the config.json file with the configuration data + * create_config_json_file creates the config.json file with the configuration data * @param {object} data * @returns {Promise} */ @@ -178,6 +178,13 @@ class ConfigFS { await native_fs_utils.update_config_file(this.fs_context, this.config_root, this.config_json_path, data); } + /** + * delete_config_json_file deletes the config.json file + * @returns {Promise} + */ + async delete_config_json_file() { + await native_fs_utils.delete_config_file(this.fs_context, this.config_root, this.config_json_path); + } /** * get_config_data reads a config file and returns its content diff --git a/src/server/system_services/schemas/nsfs_config_schema.js b/src/server/system_services/schemas/nsfs_config_schema.js index aaaa9ab7fa..5d00f77616 100644 --- a/src/server/system_services/schemas/nsfs_config_schema.js +++ b/src/server/system_services/schemas/nsfs_config_schema.js @@ -124,7 +124,15 @@ const nsfs_node_config_schema = { }, NC_DISABLE_ACCESS_CHECK: { type: 'boolean', - doc: 'indicate whether read/write access will be validated on bucket/account creation/update.' + doc: 'indicate whether read access will be validated on bucket/account creation/update.' + }, + NC_DISABLE_HEALTH_ACCESS_CHECK: { + type: 'boolean', + doc: 'indicate whether read access will be validated on bucket/account health check.' + }, + NC_DISABLE_POSIX_MODE_ACCESS_CHECK: { + type: 'boolean', + doc: 'indicate whether posix mode read/write access will be validated on bucket/account creation/update or health check.' }, ENDPOINT_PROCESS_TITLE: { type: 'string', diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js index 179d5ea8b8..9337d9b1da 100644 --- a/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js +++ b/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js @@ -1773,7 +1773,9 @@ describe('manage nsfs cli account flow', () => { describe('cli account flow distinguished_name - permissions', function() { const type = TYPES.ACCOUNT; const config_root = path.join(tmp_fs_path, 'config_root_manage_dn'); + const config_fs = new ConfigFS(config_root); const new_buckets_path = path.join(tmp_fs_path, 'new_buckets_path_user_dn_test/'); + const accounts = { root: { cli_options: { @@ -1903,32 +1905,34 @@ describe('cli account flow distinguished_name - permissions', function() { }, timeout); it('cli account update - should fail - no permissions to new_buckets_path', async function() { - const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_perm_to_owner/`; + const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_perm_to_owner1/`; await fs_utils.create_fresh_path(no_permissions_new_buckets_path); await fs_utils.file_must_exist(no_permissions_new_buckets_path); - await set_path_permissions_and_owner(new_buckets_path, accounts.accessible_user.fs_options, 0o077); + await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o000); const action = ACTIONS.UPDATE; const update_options = { config_root, - name: accounts.root.cli_options.name, + ...accounts.accessible_user.cli_options, new_buckets_path: no_permissions_new_buckets_path, }; const res = await exec_manage_cli(type, action, update_options); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleAccountNewBucketsPath.code); }, timeout); - it('cli account update - should fail - no write permissions of new_buckets_path', async function() { + it('cli account update - should fail - posix mode write permissions of new_buckets_path', async function() { const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_r_perm_to_owner/`; await fs_utils.create_fresh_path(no_permissions_new_buckets_path); await fs_utils.file_must_exist(no_permissions_new_buckets_path); - await set_path_permissions_and_owner(new_buckets_path, accounts.accessible_user.fs_options, 0o477); + await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o477); const action = ACTIONS.UPDATE; const update_options = { config_root, - name: accounts.root.cli_options.name, + ...accounts.accessible_user.cli_options, new_buckets_path: no_permissions_new_buckets_path, }; + await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_POSIX_MODE_ACCESS_CHECK: false })); const res = await exec_manage_cli(type, action, update_options); + await config_fs.delete_config_json_file(); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleAccountNewBucketsPath.code); }, timeout); @@ -1936,11 +1940,11 @@ describe('cli account flow distinguished_name - permissions', function() { const no_permissions_new_buckets_path = `${tmp_fs_path}/new_buckets_path_no_w_perm_to_owner/`; await fs_utils.create_fresh_path(no_permissions_new_buckets_path); await fs_utils.file_must_exist(no_permissions_new_buckets_path); - await set_path_permissions_and_owner(new_buckets_path, accounts.accessible_user.fs_options, 0o277); + await set_path_permissions_and_owner(no_permissions_new_buckets_path, accounts.accessible_user.fs_options, 0o227); const action = ACTIONS.UPDATE; const update_options = { config_root, - name: accounts.root.cli_options.name, + ...accounts.accessible_user.cli_options, new_buckets_path: no_permissions_new_buckets_path, }; const res = await exec_manage_cli(type, action, update_options); @@ -1951,8 +1955,10 @@ describe('cli account flow distinguished_name - permissions', function() { let action = ACTIONS.ADD; config.NC_DISABLE_ACCESS_CHECK = true; set_nc_config_dir_in_config(config_root); - await fs.promises.writeFile(path.join(config_root, 'config.json'), JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true })); + await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true })); const res = await exec_manage_cli(type, action, accounts.inaccessible_user.cli_options); + await config_fs.delete_config_json_file(); + console.log('RES: ', res); expect(JSON.parse(res).response.code).toEqual(ManageCLIResponse.AccountCreated.code); assert_account(JSON.parse(res).response.reply, { ...accounts.inaccessible_user.cli_options }, false); action = ACTIONS.DELETE; diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_cli.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_cli.test.js index 024cb1b719..d19fad9d85 100644 --- a/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_cli.test.js +++ b/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_cli.test.js @@ -113,13 +113,15 @@ describe('manage nsfs cli bucket flow', () => { expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code); }); - it('should fail - cli create bucket - owner does not have write permission to path', async () => { + it('should fail - cli create bucket - owner does not have posix write permission to path', async () => { const action = ACTIONS.ADD; const bucket_options = { config_root, ...bucket_defaults}; await fs_utils.create_fresh_path(bucket_options.path); await fs_utils.file_must_exist(bucket_options.path); await set_path_permissions_and_owner(bucket_options.path, account_defaults, 0o477); + await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_POSIX_MODE_ACCESS_CHECK: false })); const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options); + await config_fs.delete_config_json_file(); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code); }); @@ -150,9 +152,10 @@ describe('manage nsfs cli bucket flow', () => { await fs_utils.create_fresh_path(bucket_options.path); await fs_utils.file_must_exist(bucket_options.path); set_nc_config_dir_in_config(config_root); - await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true })); await set_path_permissions_and_owner(bucket_options.path, account_defaults, 0o000); + await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true })); const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options); + await config_fs.delete_config_json_file(); expect(JSON.parse(res).response.code).toEqual(ManageCLIResponse.BucketCreated.code); }); @@ -532,7 +535,9 @@ describe('manage nsfs cli bucket flow', () => { await fs_utils.create_fresh_path(bucket_defaults.path); await fs_utils.file_must_exist(bucket_defaults.path); await set_path_permissions_and_owner(bucket_defaults.path, account_defaults2, 0o477); + await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_POSIX_MODE_ACCESS_CHECK: false })); const res = await exec_manage_cli(TYPES.BUCKET, action, bucket_options); + await config_fs.delete_config_json_file(); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InaccessibleStoragePath.code); }); diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_new_buckets_path_validation.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_new_buckets_path_validation.test.js index 50f5caecfd..a60576ae0c 100644 --- a/src/test/unit_tests/jest_tests/test_nc_nsfs_new_buckets_path_validation.test.js +++ b/src/test/unit_tests/jest_tests/test_nc_nsfs_new_buckets_path_validation.test.js @@ -6,6 +6,7 @@ process.env.DISABLE_INIT_RANDOM_SEED = "true"; const fs = require('fs'); const path = require('path'); +const config = require('../../../../config'); const fs_utils = require('../../../util/fs_utils'); const test_utils = require('../../system_tests/test_utils'); const { get_fs_context, is_dir_rw_accessible } = require('../../../util/native_fs_utils'); @@ -17,8 +18,9 @@ if (process.platform === MAC_PLATFORM) { } const timeout = 50000; -describe('new_buckets_path access validation account', () => { +describe('new_buckets_path posix mode access validation account', () => { const new_buckets_path = path.join(tmp_fs_path, 'new_buckets_path'); + config.NC_DISABLE_POSIX_MODE_ACCESS_CHECK = false; const owner_user = 'owner_user'; const group_user = 'group_user'; const other_user = 'other_user'; diff --git a/src/util/native_fs_utils.js b/src/util/native_fs_utils.js index ec9695678e..1760c4bd21 100644 --- a/src/util/native_fs_utils.js +++ b/src/util/native_fs_utils.js @@ -531,6 +531,9 @@ async function is_dir_rw_accessible(fs_context, dir_path) { } catch (err) { return false; } + + if (config.NC_DISABLE_POSIX_MODE_ACCESS_CHECK) return true; + const is_owner = fs_context.uid === stat.uid; const is_group = fs_context.gid === stat.gid;