NC | Check access - skip write check and mode bits check by default a…

…dd health access check configuration Signed-off-by: Romy <[email protected]>
noobaa · Sep 16, 2024 · a8c3fc7 · a8c3fc7
1 parent 4edc2a4
commit a8c3fc7
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 25 deletions.
diff --git 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
@@ -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 -
 * <u>Key</u>: `NC_DISABLE_ACCESS_CHECK`  
 * <u>Type</u>: Boolean  
 * <u>Default</u>: false  
-* <u>Description</u>: This flag will disable Read/Write accessibility validations in the following flows -  
+* <u>Description</u>: 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. 
+
 * <u>Steps</u>:  
     ```
     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 -
+* <u>Key</u>: `NC_DISABLE_HEALTH_ACCESS_CHECK`  
+* <u>Type</u>: Boolean  
+* <u>Default</u>: false  
+* <u>Description</u>: This flag will disable Read accessibility validations in Health check of buckets and accounts.
+
+* <u>Steps</u>:  
+    ```
+    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 -
+* <u>Key</u>: `NC_DISABLE_POSIX_MODE_ACCESS_CHECK`  
+* <u>Type</u>: Boolean  
+* <u>Default</u>: true  
+* <u>Description</u>: 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.
+
+* <u>Steps</u>:  
+    ```
+    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 -
 * <u>Key</u>: `ENDPOINT_PROCESS_TITLE`  

diff --git 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
@@ -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<void>} 
      */
@@ -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<void>} 
+     */
+    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
@@ -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
@@ -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,44 +1905,46 @@ 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);
 
     it('cli account update - should fail - no read permissions of new_buckets_path', async 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
@@ -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
@@ -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
@@ -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;