From b07ff705c1f9a1d950a776eadc57d0d93e945eed Mon Sep 17 00:00:00 2001 From: nadav mizrahi Date: Thu, 21 Nov 2024 18:17:51 +0200 Subject: [PATCH] NSFS | NC | add option to set account supplemental groups Signed-off-by: nadav mizrahi --- .../AccountsAndBuckets.md | 2 + docs/NooBaaNonContainerized/NooBaaCLI.md | 10 ++ docs/NooBaaNonContainerized/S3Ops.md | 2 + src/api/account_api.js | 5 +- src/api/common_api.js | 11 +- src/cmd/manage_nsfs.js | 7 +- src/manage_nsfs/manage_nsfs_cli_errors.js | 5 + src/manage_nsfs/manage_nsfs_cli_utils.js | 18 ++- src/manage_nsfs/manage_nsfs_constants.js | 8 +- src/manage_nsfs/manage_nsfs_help_utils.js | 6 +- src/manage_nsfs/manage_nsfs_validations.js | 37 ++++- src/native/fs/fs_napi.cpp | 47 +++++- src/native/util/os.h | 8 +- src/native/util/os_darwin.cpp | 18 +++ src/native/util/os_linux.cpp | 23 +-- src/sdk/accountspace_fs.js | 1 + src/sdk/nb.d.ts | 1 + .../schemas/nsfs_account_schema.js | 3 + .../test_nc_nsfs_account_cli.test.js | 136 ++++++++++++++++++ src/test/unit_tests/test_nsfs_access.js | 45 ++++-- src/util/native_fs_utils.js | 3 +- 21 files changed, 357 insertions(+), 39 deletions(-) diff --git a/docs/NooBaaNonContainerized/AccountsAndBuckets.md b/docs/NooBaaNonContainerized/AccountsAndBuckets.md index 08b6462e02..4526f258ae 100644 --- a/docs/NooBaaNonContainerized/AccountsAndBuckets.md +++ b/docs/NooBaaNonContainerized/AccountsAndBuckets.md @@ -30,6 +30,8 @@ See all available account properties - [NC Account Schema](../../src/server/syst - `encrypted_secret_key` - Account's secrets will be kept encrypted in the account's configuration file. - `uid/gid/user` - An account's access key is mapped to a file system uid/gid (or user). Before performing any file system operation, NooBaa switches to the account's UID/GID, ensuring that accounts access to buckets and objects is enforced by the file system. + + - `supplemental_groups` - Besides account main GID, an account can have supplementary group IDs that are used to determine permissions for accessing files. these GIDs are validated against a files group (GID) permissions. - `new_buckets_path` - When an account creates a bucket using the S3 protocol, NooBaa will create the underlying file system directory. This directory will be created under new_buckets_path. Note that the account must have read and write access to its `new_buckets_path`. Must be an absolute path. diff --git a/docs/NooBaaNonContainerized/NooBaaCLI.md b/docs/NooBaaNonContainerized/NooBaaCLI.md index 40557c3353..a9878c8333 100644 --- a/docs/NooBaaNonContainerized/NooBaaCLI.md +++ b/docs/NooBaaNonContainerized/NooBaaCLI.md @@ -84,6 +84,11 @@ noobaa-cli account add --name --uid --gid [--user] - Type: String - Description: Specifies the File system user representing the account. (user can be replaced by --uid and --gid option) +- `supplemental_groups` + - Type: Number[] + - Description: specifies additional groups(GID) this user can be a part of. allows access to reasorces assigned to + one of these groups + - `new_buckets_path` - Type: String - Description: Specifies a file system directory to be used for creating underlying directories that represent buckets created by an account using the S3 API. @@ -152,6 +157,11 @@ noobaa-cli account update --name [--new_name][--uid][--gid][--use - Type: Number - Description: Specifies the File system user representing the account. (user can be replaced by --uid and --gid option) +- `supplemental_groups` + - Type: Number[] + - Description: specifies additional groups(GID) this user can be a part of. allows access to reasorces assigned to + one of these groups + - `new_buckets_path` - Type: String - Description: Specifies a file system directory to be used for creating underlying directories that represent buckets created by an account using the S3 API. diff --git a/docs/NooBaaNonContainerized/S3Ops.md b/docs/NooBaaNonContainerized/S3Ops.md index 8b03c3bc24..1b84e49ebf 100644 --- a/docs/NooBaaNonContainerized/S3Ops.md +++ b/docs/NooBaaNonContainerized/S3Ops.md @@ -53,6 +53,8 @@ The following lists describe the bucket and object operations available in NooBa - Bucket policies use JSON-based policy language (for more information see [basic elements in bucket policy](https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-policy-language-overview.html) in AWS documentation) - Bucket policy can be added to a bucket using the S3 API or the noobaa-cli. - Bucket policy is an additional layer of permissions to the FS permissions (UID and GID), which means that if two accounts do not have the same permissions (UID, GID) just setting bucket policy on the bucket is not enough. +- Besides account main GID, an account can have supplementary group IDs that are used to determine permissions for +accessing files. these GIDs are validated against a files group (GID) permissions. supplementary groups can be added using the noobaa-cli #### Bucket Policy in NooBaa CLI 1. Adding a bucket policy: diff --git a/src/api/account_api.js b/src/api/account_api.js index 6e21f0a6e6..cbf152e5f6 100644 --- a/src/api/account_api.js +++ b/src/api/account_api.js @@ -312,7 +312,10 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { + $ref: 'common_api#/definitions/supplemental_groups' + }, } }, }, diff --git a/src/api/common_api.js b/src/api/common_api.js index 671f520cca..1c35d7d31b 100644 --- a/src/api/common_api.js +++ b/src/api/common_api.js @@ -1346,6 +1346,12 @@ module.exports = { } } }, + supplemental_groups: { + type: 'array', + items: { + type: 'number' + } + }, nsfs_account_config: { oneOf: [{ type: 'object', @@ -1354,7 +1360,10 @@ module.exports = { uid: { type: 'number' }, gid: { type: 'number' }, new_buckets_path: { type: 'string' }, - nsfs_only: { type: 'boolean' } + nsfs_only: { type: 'boolean' }, + supplemental_groups: { + $ref: '#/definitions/supplemental_groups' + }, } }, { type: 'object', diff --git a/src/cmd/manage_nsfs.js b/src/cmd/manage_nsfs.js index 2f5766679c..ccc2f11c93 100644 --- a/src/cmd/manage_nsfs.js +++ b/src/cmd/manage_nsfs.js @@ -24,7 +24,7 @@ const { print_usage } = require('../manage_nsfs/manage_nsfs_help_utils'); const { TYPES, ACTIONS, LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants'); const { throw_cli_error, get_bucket_owner_account_by_name, write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level, - is_name_update, is_access_key_update } = require('../manage_nsfs/manage_nsfs_cli_utils'); + is_name_update, is_access_key_update, parse_comma_delimited_string } = require('../manage_nsfs/manage_nsfs_cli_utils'); const manage_nsfs_validations = require('../manage_nsfs/manage_nsfs_validations'); const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance(); const notifications_util = require('../util/notifications_util'); @@ -358,6 +358,11 @@ async function fetch_account_data(action, user_input) { data.access_keys[0].secret_key = data.access_keys[0].secret_key === undefined ? undefined : new SensitiveString(String(data.access_keys[0].secret_key)); } + //since supplemental_groups is an array, new list will merge with the old one instead of replacing it in fetch_existing_account_data + //so we need to replace this value after merging the data + data.nsfs_account_config.supplemental_groups = user_input.supplemental_groups === undefined ? + data.nsfs_account_config.supplemental_groups : parse_comma_delimited_string(user_input.supplemental_groups); + if (data.new_access_key) data.new_access_key = new SensitiveString(data.new_access_key); // fs_backend deletion specified with empty string '' (but it is not part of the schema) data.nsfs_account_config.fs_backend = data.nsfs_account_config.fs_backend || undefined; diff --git a/src/manage_nsfs/manage_nsfs_cli_errors.js b/src/manage_nsfs/manage_nsfs_cli_errors.js index 19ad998768..c5bca29457 100644 --- a/src/manage_nsfs/manage_nsfs_cli_errors.js +++ b/src/manage_nsfs/manage_nsfs_cli_errors.js @@ -346,6 +346,11 @@ ManageCLIError.InvalidGlacierOperation = Object.freeze({ message: 'only "migrate", "restore" and "expiry" subcommands are supported', http_code: 400, }); +ManageCLIError.InvalidSupplementalGroupsList = Object.freeze({ + code: 'InvalidSupplementalGroupsList', + message: 'supplemental groups must be a list of group ids (positive integers)', + http_code: 400, +}); //////////////////////// diff --git a/src/manage_nsfs/manage_nsfs_cli_utils.js b/src/manage_nsfs/manage_nsfs_cli_utils.js index 94ce986a90..c210037258 100644 --- a/src/manage_nsfs/manage_nsfs_cli_utils.js +++ b/src/manage_nsfs/manage_nsfs_cli_utils.js @@ -90,8 +90,21 @@ function get_boolean_or_string_value(value) { } } -/** - * get_options_from_file will read a JSON file that include key-value of the options +function parse_comma_delimited_string(value) { + if (typeof value === 'object') { + return value; + } + if (typeof value === 'number') { + return [value]; + } + if (typeof value === 'string') { + return value.split(',').map(val => Number(val)); + } + return value; +} + +/**_ + * get_options_fromfile will read a JSON file that include key-value of the options * (instead of flags) and return its content * @param {string} file_path */ @@ -156,6 +169,7 @@ function is_access_key_update(data) { exports.throw_cli_error = throw_cli_error; exports.write_stdout_response = write_stdout_response; exports.get_boolean_or_string_value = get_boolean_or_string_value; +exports.parse_comma_delimited_string = parse_comma_delimited_string; exports.get_bucket_owner_account_by_name = get_bucket_owner_account_by_name; exports.get_bucket_owner_account_by_id = get_bucket_owner_account_by_id; exports.get_options_from_file = get_options_from_file; diff --git a/src/manage_nsfs/manage_nsfs_constants.js b/src/manage_nsfs/manage_nsfs_constants.js index 4e4ae5605a..5839deaba3 100644 --- a/src/manage_nsfs/manage_nsfs_constants.js +++ b/src/manage_nsfs/manage_nsfs_constants.js @@ -44,16 +44,16 @@ const FROM_FILE = 'from_file'; const ANONYMOUS = 'anonymous'; const VALID_OPTIONS_ACCOUNT = { - 'add': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', FROM_FILE, ...CLI_MUTUAL_OPTIONS]), - 'update': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', 'new_name', 'regenerate', ...CLI_MUTUAL_OPTIONS]), + 'add': new Set(['name', 'uid', 'gid', 'supplemental_groups', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', FROM_FILE, ...CLI_MUTUAL_OPTIONS]), + 'update': new Set(['name', 'uid', 'gid', 'supplemental_groups', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'iam_operate_on_root_account', 'new_name', 'regenerate', ...CLI_MUTUAL_OPTIONS]), 'delete': new Set(['name', ...CLI_MUTUAL_OPTIONS]), 'list': new Set(['wide', 'show_secrets', 'gid', 'uid', 'user', 'name', 'access_key', ...CLI_MUTUAL_OPTIONS]), 'status': new Set(['name', 'access_key', 'show_secrets', ...CLI_MUTUAL_OPTIONS]), }; const VALID_OPTIONS_ANONYMOUS_ACCOUNT = { - 'add': new Set(['uid', 'gid', 'user', 'anonymous', ...CLI_MUTUAL_OPTIONS]), - 'update': new Set(['uid', 'gid', 'user', 'anonymous', ...CLI_MUTUAL_OPTIONS]), + 'add': new Set(['uid', 'gid', 'user', 'supplemental_groups', 'anonymous', ...CLI_MUTUAL_OPTIONS]), + 'update': new Set(['uid', 'gid', 'user', 'supplemental_groups', 'anonymous', ...CLI_MUTUAL_OPTIONS]), 'delete': new Set(['anonymous', ...CLI_MUTUAL_OPTIONS]), 'status': new Set(['anonymous', ...CLI_MUTUAL_OPTIONS]), }; diff --git a/src/manage_nsfs/manage_nsfs_help_utils.js b/src/manage_nsfs/manage_nsfs_help_utils.js index 2338bc8251..c204ef63ac 100644 --- a/src/manage_nsfs/manage_nsfs_help_utils.js +++ b/src/manage_nsfs/manage_nsfs_help_utils.js @@ -78,6 +78,7 @@ Flags: --name Set the name for the account --uid Set the User Identifier (UID) (UID and GID can be replaced by --user option) --gid Set the Group Identifier (GID) (UID and GID can be replaced by --user option) +--supplemental_groups (optional) Set the supplemntal group list (List of GID) seperated by comma (,) example: '212,211,202' --new_buckets_path (optional) Set the filesystem's root path where each subdirectory is a bucket --user (optional) Set the OS user name (instead of UID and GID) --access_key (optional) Set the access key for the account (default is generated) @@ -98,6 +99,7 @@ Flags: --new_name (optional) Update the account name --uid (optional) Update the User Identifier (UID) --gid (optional) Update the Group Identifier (GID) +--supplemental_groups (optional) Update the list of supplemental groups (List of GID) seperated by comma(,) (example: 211,202,23) (will override existing list) --new_buckets_path (optional) Update the filesystem's root path where each subdirectory is a bucket --user (optional) Update the OS user name (instead of uid and gid) --regenerate (optional) Update automatically generated access key and secret key @@ -303,13 +305,13 @@ But updates of the config directory will be blocked during the upgrade of the co 'upgrade start' should be executed on one node, the config directory changes will be available for all the nodes of the cluster. Usage: - + noobaa-cli upgrade start [flags] Flags: --expected_version The expected target version of the upgrade ---expected_hosts The expected hosts running NooBaa NC, a string of hosts separated by , +--expected_hosts The expected hosts running NooBaa NC, a string of hosts separated by , --skip_verification (optional) skip verification of the hosts package version WARNING: can cause corrupted config dir files created by hosts running old code --custom_upgrade_scripts_dir (optional) custom upgrade scripts dir, use for running custom config dir upgrade scripts diff --git a/src/manage_nsfs/manage_nsfs_validations.js b/src/manage_nsfs/manage_nsfs_validations.js index af837a44de..2ee0a3d275 100644 --- a/src/manage_nsfs/manage_nsfs_validations.js +++ b/src/manage_nsfs/manage_nsfs_validations.js @@ -20,7 +20,7 @@ const { check_root_account_owns_user } = require('../nc/nc_utils'); //// GENERAL VALIDATIONS //// ///////////////////////////// -/** +/** * validate_input_types checks if input option are valid. * if the the user uses from_file then the validation is on the file (in different iteration) * @param {string} type @@ -175,7 +175,11 @@ function validate_options_type_by_value(input_options_with_data) { if ((option === 'bucket_policy' || option === 'notifications') && type_of_value === 'object') { continue; } - const details = `type of flag ${option} should be ${type_of_option}`; + //special case for supplemental groups + if (option === 'supplemental_groups' && validate_supplemental_groups(value)) { + continue; + } + const details = `type of flag ${option} should be ${type_of_option} ${type_of_value} ${value}`; throw_cli_error(ManageCLIError.InvalidArgumentType, details); } } @@ -197,6 +201,35 @@ function validate_boolean_string_value(value) { return false; } +/** + * validates supplemental groups array. verifies that string is comma seperated positive numbers. string should not + * begin or end with a comma. if only one number it should be positive + * @param {number|string} value + */ +function validate_supplemental_groups(value) { + if (value && typeof value === 'string') { + const regex = /^[0-9]+(,[0-9]+)+$/; + if (!regex.test(value)) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } + return true; + } else if (typeof value === 'number') { + if (value < 0) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } else { + return true; + } + } else if (value && typeof value === 'object') { + for (const entry of Object.values(value)) { + if (isNaN(Number(entry)) || Number(entry) < 0) { + throw_cli_error(ManageCLIError.InvalidSupplementalGroupsList); + } + } + return true; + } + return false; +} + /** * validate_min_flags_for_update validates that we have at least one flag of a property to update * @param {string} type diff --git a/src/native/fs/fs_napi.cpp b/src/native/fs/fs_napi.cpp index af7bfcf137..2bd5868c3e 100644 --- a/src/native/fs/fs_napi.cpp +++ b/src/native/fs/fs_napi.cpp @@ -537,6 +537,40 @@ load_xattr_get_keys(Napi::Object& options, std::vector& _xattr_get_ } } +/** +* converts Napi::Array of numbers to std::vector +* typename T - type of the vector to convert to (e.g int, uint, gid_t) +*/ +template +static std::vector convert_nappi_array_to_number_vector(const Napi::Array& arr) { + std::vector new_vector; + const std::size_t arr_length = arr.Length(); + for (std::size_t i = 0; i < arr_length; ++i) { + new_vector.push_back(static_cast(arr[i]).ToNumber()); + } + return new_vector; +} + +/** + * converts std::vector to comma seperated string so it can be printed to logs + */ +template +static std::string stringfy_vector(std::vector& vec) { + std::stringstream ss; + std::size_t size = vec.size(); + for(std::size_t i = 0; i < size; ++i) { + if (i > 0) ss << ','; + ss << vec[i]; + } + return ss.str(); +} + + +static std::string get_groups_as_string() { + std::vector groups = ThreadScope::get_process_groups(); + return stringfy_vector(groups); +} + /** * FSWorker is a general async worker for our fs operations */ @@ -560,6 +594,7 @@ struct FSWorker : public Napi::AsyncWorker double _took_time; Napi::FunctionReference _report_fs_stats; bool _should_add_thread_capabilities; + std::vector _supplemental_groups; // executes the ctime check in the stat and read file fuctions // NOTE: If _do_ctime_check = false, then some functions will fallback to using mtime check @@ -576,6 +611,7 @@ struct FSWorker : public Napi::AsyncWorker , _warn_threshold_ms(0) , _took_time(0) , _should_add_thread_capabilities(false) + , _supplemental_groups() , _do_ctime_check(false) { for (int i = 0; i < (int)info.Length(); ++i) _args_ref.Set(i, info[i]); @@ -586,6 +622,9 @@ struct FSWorker : public Napi::AsyncWorker if (fs_context.Get("backend").ToBoolean()) { _backend = fs_context.Get("backend").ToString(); } + if(fs_context.Has("supplemental_groups")) { + _supplemental_groups = convert_nappi_array_to_number_vector(fs_context.Get("supplemental_groups").As()); + } if (fs_context.Get("warn_threshold_ms").ToBoolean()) { _warn_threshold_ms = fs_context.Get("warn_threshold_ms").ToNumber(); } @@ -604,10 +643,12 @@ struct FSWorker : public Napi::AsyncWorker virtual void Work() = 0; void Execute() override { - DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(_backend)); + const std::string supplemental_groups = stringfy_vector(_supplemental_groups); + DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(_backend) << DVAL(supplemental_groups)); ThreadScope tx; - tx.set_user(_uid, _gid); - DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(geteuid()) << DVAL(getegid()) << DVAL(getuid()) << DVAL(getgid())); + tx.set_user(_uid, _gid, _supplemental_groups); + std::string new_supplemental_groups = get_groups_as_string(); + DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(geteuid()) << DVAL(getegid()) << DVAL(getuid()) << DVAL(getgid()) << DVAL(new_supplemental_groups)); if(_should_add_thread_capabilities) { tx.add_thread_capabilities(); diff --git a/src/native/util/os.h b/src/native/util/os.h index c8064e8982..3178ecad8a 100644 --- a/src/native/util/os.h +++ b/src/native/util/os.h @@ -2,6 +2,8 @@ #pragma once #include +#include +#include namespace noobaa { @@ -31,10 +33,11 @@ class ThreadScope restore_user(); } - void set_user(uid_t uid, gid_t gid) + void set_user(uid_t uid, gid_t gid, std::vector& groups) { _uid = uid; _gid = gid; + _groups = groups; change_user(); } @@ -44,11 +47,14 @@ class ThreadScope const static gid_t orig_gid; const static std::vector orig_groups; + static std::vector get_process_groups(); + private: void change_user(); void restore_user(); uid_t _uid; gid_t _gid; + std::vector _groups; }; } // namespace noobaa diff --git a/src/native/util/os_darwin.cpp b/src/native/util/os_darwin.cpp index bb14d9c920..96c35a4725 100644 --- a/src/native/util/os_darwin.cpp +++ b/src/native/util/os_darwin.cpp @@ -3,6 +3,8 @@ #include "common.h" #include // for KAUTH_UID_NONE +#include +#include namespace noobaa { @@ -37,12 +39,20 @@ get_current_uid() const uid_t ThreadScope::orig_uid = getuid(); const gid_t ThreadScope::orig_gid = getgid(); +const std::vector ThreadScope::orig_groups = get_process_groups(); + void ThreadScope::change_user() { if (_uid != orig_uid || _gid != orig_gid) { MUST_SYS(_mac_thread_setugid(_uid, _gid)); + if (_groups.empty()) { + MUST_SYS(syscall(setgroups, 0, NULL)); + } + else { + MUST_SYS(syscall(setgroups, _groups.size(), &_groups[0])); + } } } @@ -51,6 +61,7 @@ ThreadScope::restore_user() { if (_uid != orig_uid || _gid != orig_gid) { MUST_SYS(_mac_thread_setugid(KAUTH_UID_NONE, KAUTH_UID_NONE)); + MUST_SYS(syscall(SYS_setgroups, orig_groups.size(), &orig_groups[0])); } } @@ -62,6 +73,13 @@ ThreadScope::add_thread_capabilities() return -1; } +static std::vector get_process_groups() { + std::vector groups(NGROUPS_MAX); + int r = getgroups(NGROUPS_MAX, &groups[0]); + groups.resize(r); + return groups;s +} + } // namespace noobaa #endif diff --git a/src/native/util/os_linux.cpp b/src/native/util/os_linux.cpp index 5519db80fa..cd5ecfc44f 100644 --- a/src/native/util/os_linux.cpp +++ b/src/native/util/os_linux.cpp @@ -27,12 +27,7 @@ get_current_uid() const uid_t ThreadScope::orig_uid = getuid(); const gid_t ThreadScope::orig_gid = getgid(); -const std::vector ThreadScope::orig_groups = [] { - std::vector groups(NGROUPS_MAX); - int r = getgroups(NGROUPS_MAX, &groups[0]); - groups.resize(r); - return groups; -}(); +const std::vector ThreadScope::orig_groups = get_process_groups(); /** * set the effective uid/gid of the current thread using direct syscalls @@ -44,8 +39,13 @@ void ThreadScope::change_user() { if (_uid != orig_uid || _gid != orig_gid) { + if (_groups.empty()) { + MUST_SYS(syscall(SYS_setgroups, 0, NULL)); + } + else { + MUST_SYS(syscall(SYS_setgroups, _groups.size(), &_groups[0])); + } // must change gid first otherwise will fail on permission - MUST_SYS(syscall(SYS_setgroups, 0, NULL)); MUST_SYS(syscall(SYS_setresgid, -1, _gid, -1)); MUST_SYS(syscall(SYS_setresuid, -1, _uid, -1)); } @@ -61,7 +61,7 @@ ThreadScope::restore_user() // must restore uid first otherwise will fail on permission MUST_SYS(syscall(SYS_setresuid, -1, orig_uid, -1)); MUST_SYS(syscall(SYS_setresgid, -1, orig_gid, -1)); - MUST_SYS(syscall(SYS_setgroups, ThreadScope::orig_groups.size(), &ThreadScope::orig_groups[0])); + MUST_SYS(syscall(SYS_setgroups, orig_groups.size(), &orig_groups[0])); } } @@ -100,6 +100,13 @@ ThreadScope::add_thread_capabilities() { return 0; } +std::vector ThreadScope::get_process_groups() { + std::vector groups(NGROUPS_MAX); + int r = getgroups(NGROUPS_MAX, &groups[0]); + groups.resize(r); + return groups; +} + } // namespace noobaa #endif diff --git a/src/sdk/accountspace_fs.js b/src/sdk/accountspace_fs.js index 87d3b7b723..8d5b92c913 100644 --- a/src/sdk/accountspace_fs.js +++ b/src/sdk/accountspace_fs.js @@ -503,6 +503,7 @@ class AccountSpaceFS { distinguished_name: distinguished_name, uid: distinguished_name ? undefined : requesting_account.nsfs_account_config.uid, gid: distinguished_name ? undefined : requesting_account.nsfs_account_config.gid, + supplemental_groups: requesting_account.nsfs_account_config.supplemental_groups, new_buckets_path: requesting_account.nsfs_account_config.new_buckets_path, fs_backend: requesting_account.nsfs_account_config.fs_backend, } diff --git a/src/sdk/nb.d.ts b/src/sdk/nb.d.ts index 29abd76fb4..4c25e49663 100644 --- a/src/sdk/nb.d.ts +++ b/src/sdk/nb.d.ts @@ -1031,6 +1031,7 @@ interface NativeFSContext { uid?: number; gid?: number; backend?: string; + supplemental_groups?: number[]; warn_threshold_ms?: number; report_fs_stats?: Function; do_ctime_check?: boolean; diff --git a/src/server/system_services/schemas/nsfs_account_schema.js b/src/server/system_services/schemas/nsfs_account_schema.js index 9224c3eb95..ecc3dcc44b 100644 --- a/src/server/system_services/schemas/nsfs_account_schema.js +++ b/src/server/system_services/schemas/nsfs_account_schema.js @@ -81,6 +81,9 @@ module.exports = { properties: { uid: { type: 'number' }, gid: { type: 'number' }, + supplemental_groups: { + $ref: 'common_api#/definitions/supplemental_groups' + }, new_buckets_path: { type: 'string' }, fs_backend: { $ref: 'common_api#/definitions/fs_backend' 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 5142207789..1960015e13 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 @@ -504,6 +504,72 @@ describe('manage nsfs cli account flow', () => { const res = await exec_manage_cli(type, action, account_options); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountName.code); }); + + it('cli account add - cli create account with supplemental groups)', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = '303,211'; + const expected_groups = [303, 211]; + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli account add - cli create account with supplemental groups - single group (string))', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303"; + const expected_groups = [303]; + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli account add - cli create account with supplemental groups - single group (number))', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = 303; + const expected_groups = [303]; + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + await exec_manage_cli(type, action, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli account add - cli create account with supplemental groups - invalid list', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303l,201"; //group cannot contain charecters + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + const res = await exec_manage_cli(type, action, account_options); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); + + it('cli account add - cli create account with supplemental groups - invalid list end with comma', async function() { + const { type, new_buckets_path, uid, gid, name } = defaults; + const supplemental_groups = "303,"; //group cannot end with comma + const account_options = { config_root, name, new_buckets_path, uid, gid, supplemental_groups}; + const action = ACTIONS.ADD; + await fs_utils.create_fresh_path(new_buckets_path); + await fs_utils.file_must_exist(new_buckets_path); + await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700); + const res = await exec_manage_cli(type, action, account_options); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); }); describe('cli update account', () => { @@ -1004,6 +1070,35 @@ describe('manage nsfs cli account flow', () => { const res = await exec_manage_cli(type, ACTIONS.UPDATE, account_options); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountName.code); }); + + it('cli account add - cli update account with supplemental groups)', async function() { + const { name } = defaults; + const supplemental_groups = '303,211'; + const expected_groups = [303, 211]; + const account_options = { config_root, name, supplemental_groups}; + await exec_manage_cli(type, ACTIONS.UPDATE, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli account add - cli update account with supplemental groups - single group (string))', async function() { + const { name } = defaults; + const supplemental_groups = "303"; + const expected_groups = [303]; + const account_options = { config_root, name, supplemental_groups}; + await exec_manage_cli(type, ACTIONS.UPDATE, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(expected_groups); + }); + + it('cli account add - cli update account with supplemental groups - single group (number))', async function() { + const { name } = defaults; + const supplemental_groups = 303; + const account_options = { config_root, name, supplemental_groups}; + await exec_manage_cli(type, ACTIONS.UPDATE, account_options); + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual([supplemental_groups]); + }); }); describe('cli update account (has distinguished name)', () => { @@ -1765,6 +1860,47 @@ describe('manage nsfs cli account flow', () => { const res = await exec_manage_cli(type, action, command_flags); expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidAccountName.code); }); + + it('cli create account using from_file with supplemental_groups', async () => { + const action = ACTIONS.ADD; + const { name, new_buckets_path, uid, gid } = defaults; + const supplemental_groups = [212, 112]; + const account_options = { name, new_buckets_path, uid, gid, supplemental_groups }; + // write the json_file_options + const path_to_option_json_file_name = await create_json_account_options(path_to_json_account_options_dir, account_options); + const command_flags = {config_root, from_file: path_to_option_json_file_name}; + // create the account and check the details + await exec_manage_cli(type, action, command_flags); + // compare the details + const account = await config_fs.get_account_by_name(name, config_fs_account_options); + expect(account.nsfs_account_config.supplemental_groups).toStrictEqual(supplemental_groups); + }); + + it('should fail - cli create account using from_file with invalid supplemental_groups (negative number)', async () => { + const action = ACTIONS.ADD; + const { name, new_buckets_path, uid, gid } = defaults; + const supplemental_groups = [212, -112]; // gid cannot be negative number + const account_options = { name, new_buckets_path, uid, gid, supplemental_groups }; + // write the json_file_options + const path_to_option_json_file_name = await create_json_account_options(path_to_json_account_options_dir, account_options); + const command_flags = {config_root, from_file: path_to_option_json_file_name}; + // create the account and check the details + const res = await exec_manage_cli(type, action, command_flags); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); + + it('should fail - cli create account using from_file with invalid supplemental_groups (string)', async () => { + const action = ACTIONS.ADD; + const { name, new_buckets_path, uid, gid } = defaults; + const supplemental_groups = ["212d", 112]; // gid cannot a non number type + const account_options = { name, new_buckets_path, uid, gid, supplemental_groups }; + // write the json_file_options + const path_to_option_json_file_name = await create_json_account_options(path_to_json_account_options_dir, account_options); + const command_flags = {config_root, from_file: path_to_option_json_file_name}; + // create the account and check the details + const res = await exec_manage_cli(type, action, command_flags); + expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidSupplementalGroupsList.code); + }); }); }); diff --git a/src/test/unit_tests/test_nsfs_access.js b/src/test/unit_tests/test_nsfs_access.js index 5353012ae6..32d3e88962 100644 --- a/src/test/unit_tests/test_nsfs_access.js +++ b/src/test/unit_tests/test_nsfs_access.js @@ -18,8 +18,10 @@ mocha.describe('new tests check', async function() { const p = '/tmp/dir/'; const root_dir = 'root_dir'; const non_root_dir = 'non_root_dir'; + const non_root_dir2 = 'non_root_dir2'; const full_path_root = path.join(p, root_dir); const full_path_non_root = path.join(p, non_root_dir); + const full_path_non_root2 = path.join(p, non_root_dir2); const ROOT_FS_CONFIG = { uid: process.getuid(), @@ -40,12 +42,22 @@ mocha.describe('new tests check', async function() { backend: '', warn_threshold_ms: 100, }; + + const NON_ROOT3_FS_CONFIG = { + uid: 1574, + gid: 1574, + backend: '', + supplemental_groups: [1572, 1577], + warn_threshold_ms: 100, + }; mocha.before(async function() { if (test_utils.invalid_nsfs_root_permissions()) this.skip(); // eslint-disable-line no-invalid-this - await fs_utils.create_fresh_path(p, 0o770); + await fs_utils.create_fresh_path(p, 0o777); await fs_utils.file_must_exist(p); await fs_utils.create_fresh_path(full_path_root, 0o770); await fs_utils.file_must_exist(full_path_root); + await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, full_path_non_root, 0o770); + await nb_native().fs.mkdir(NON_ROOT2_FS_CONFIG, full_path_non_root2, 0o770); }); @@ -66,35 +78,42 @@ mocha.describe('new tests check', async function() { } }); mocha.it('NON ROOT 1 mkdir - failure', async function() { + const new_folder = "folder1"; + const new_path = path.join(full_path_root, new_folder); try { - const mkdir_res = await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, full_path_non_root, 0o770); + const mkdir_res = await nb_native().fs.mkdir(NON_ROOT1_FS_CONFIG, new_path, 0o770); assert.fail(`non root has access to mkdir under root dir ${mkdir_res}`); } catch (err) { assert.equal(err.code, 'EACCES'); } }); mocha.it('ROOT readdir - dir created by non dir - success', async function() { - try { - const root_entries = await nb_native().fs.readdir(ROOT_FS_CONFIG, full_path_non_root); - assert.fail(`root has access to a folder that should not exist - ${root_entries}`); - } catch (err) { - assert.equal(err.code, 'ENOENT'); - } + await nb_native().fs.readdir(ROOT_FS_CONFIG, full_path_non_root); }); mocha.it('NON ROOT 1 readdir - success', async function() { + const non_root_entries = await nb_native().fs.readdir(NON_ROOT1_FS_CONFIG, full_path_non_root); + assert.equal(non_root_entries && non_root_entries.length, 0); + }); + + mocha.it('NON ROOT 2 readdir - failure', async function() { try { - const non_root_entries = await nb_native().fs.readdir(NON_ROOT1_FS_CONFIG, full_path_non_root); - assert.fail(`non root 1 has access to a folder created by root with 770 perm - ${p} - ${non_root_entries}`); + const non_root_entries = await nb_native().fs.readdir(NON_ROOT2_FS_CONFIG, full_path_non_root); + assert.fail(`non root 2 has access to a folder created by root with 770 perm - ${p} ${non_root_entries}`); } catch (err) { assert.equal(err.code, 'EACCES'); } }); - mocha.it('NON ROOT 2 readdir - failure', async function() { + mocha.it('NON ROOT with suplemental group - success', async function() { + const non_root_entries = await nb_native().fs.readdir(NON_ROOT3_FS_CONFIG, full_path_non_root); + assert.equal(non_root_entries && non_root_entries.length, 0); + }); + + mocha.it('NON ROOT with different suplemental group - failure', async function() { try { - const non_root_entries = await nb_native().fs.readdir(NON_ROOT2_FS_CONFIG, full_path_non_root); - assert.fail(`non root 2 has access to a folder created by root with 770 perm - ${p} ${non_root_entries}`); + const non_root_entries = await nb_native().fs.readdir(NON_ROOT3_FS_CONFIG, full_path_non_root2); + assert.fail(`non root 3 has access to a folder created by user with gid not insupplemental groups - ${p} ${non_root_entries}`); } catch (err) { assert.equal(err.code, 'EACCES'); } diff --git a/src/util/native_fs_utils.js b/src/util/native_fs_utils.js index 2608f0fe4c..19875da816 100644 --- a/src/util/native_fs_utils.js +++ b/src/util/native_fs_utils.js @@ -520,7 +520,8 @@ async function get_fs_context(nsfs_account_config, fs_backend) { uid: (account_ids_by_dn && account_ids_by_dn.uid) ?? nsfs_account_config.uid, gid: (account_ids_by_dn && account_ids_by_dn.gid) ?? nsfs_account_config.gid, warn_threshold_ms: config.NSFS_WARN_THRESHOLD_MS, - backend: fs_backend + backend: fs_backend, + ...(nsfs_account_config.supplemental_groups && {supplemental_groups: nsfs_account_config.supplemental_groups}) }; }