Skip to content

Commit

Permalink
NSFS | set supplemental groups dynamically to users groups
Browse files Browse the repository at this point in the history
Signed-off-by: nadav mizrahi <[email protected]>
  • Loading branch information
nadavMiz committed Jan 14, 2025
1 parent c171502 commit a36991b
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 9 deletions.
52 changes: 46 additions & 6 deletions src/native/util/os_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <sys/capability.h>
#include <grp.h>
#include <limits.h>
#include <pwd.h>

namespace noobaa
{
Expand All @@ -29,6 +30,50 @@ const gid_t ThreadScope::orig_gid = getgid();

const std::vector<gid_t> ThreadScope::orig_groups = get_process_groups();

static int
get_supplemental_groups_by_uid(uid_t uid, std::vector<gid_t>& groups)
{
// getpwuid will only indicate if an error happened by setting errno. set it to 0, so will know if there is a change
errno = 0;
struct passwd* pw = getpwuid(uid);
if (pw == NULL) {
if (errno == 0) {
LOG("get_supplemental_groups_by_uid: no record for uid " << uid);
} else {
LOG("WARNING: get_supplemental_groups_by_uid: getpwuid failed: " << strerror(errno));
}
return -1;
}
int ngroups = NGROUPS_MAX;
groups.resize(ngroups);
if (getgrouplist(pw->pw_name, pw->pw_gid, &groups[0], &ngroups) < 0) {
LOG("get_supplemental_groups_by_uid: getgrouplist failed: " << strerror(errno));
return -1;
}
groups.resize(ngroups);
return 0;
}

/**
* set supplemental groups of the thread according to the following:
* 1. if groups were defined in the account configuration, set the groups list to the one defined
* 2. try to get the list of groups corresponding to the user in the system recods, and set it to it
* 3. if supplemental groups were not defined for the account and getting it from system record failed (either because record doesn't exist ot because of an error)
* set it to be an empty set
*/
static void
set_supplemental_groups(uid_t uid, std::vector<gid_t>& groups) {
//first check if groups were staticly set by the account configuration
if (groups.empty()) {
if (get_supplemental_groups_by_uid(uid, groups) < 0) {
//couldn't get supplemental groups dynamically. set it to be an empty set
MUST_SYS(syscall(SYS_setgroups, 0, NULL));
return;
}
}
MUST_SYS(syscall(SYS_setgroups, groups.size(), &groups[0]));
}

/**
* set the effective uid/gid/supplemental_groups of the current thread using direct syscalls
* we have to bypass the libc wrappers because posix requires it to syncronize
Expand All @@ -38,12 +83,7 @@ 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]));
}
set_supplemental_groups(_uid, _groups);
// must change gid first otherwise will fail on permission
MUST_SYS(syscall(SYS_setresgid, -1, _gid, -1));
MUST_SYS(syscall(SYS_setresuid, -1, _uid, -1));
Expand Down
37 changes: 36 additions & 1 deletion src/test/system_tests/test_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,40 @@ async function delete_fs_user_by_platform(name) {
}
}

/**
/**
* creates a new file system group. if user_name is define add it to the group
* @param {string} group_name
* @param {number} gid
* @param {string} user_name
*/
async function create_fs_group_by_platform(group_name, gid, user_name) {
if (process.platform === 'darwin') {
//TODO not implemented
} else {
const create_group_cmd = `groupadd -g ${gid} ${group_name}`;
await os_utils.exec(create_group_cmd, { return_stdout: true });
if (user_name) {
const add_user_to_group_cmd = `usermod -a -G ${group_name} ${user_name}`;
await os_utils.exec(add_user_to_group_cmd, { return_stdout: true });
}
}
}

/**
* creates a new file system group. if user_name is define add it to the group
* @param {string} group_name
*/
async function delete_fs_group_by_platform(group_name, force = false) {
if (process.platform === 'darwin') {
//TODO not implemented
} else {
const flags = force ? '-f' : '';
const delete_group_cmd = `groupdel ${flags} ${group_name}`;
await os_utils.exec(delete_group_cmd, { return_stdout: true });
}
}

/**
* set_path_permissions_and_owner sets path permissions and owner and group
* @param {string} p
* @param {object} owner_options
Expand Down Expand Up @@ -699,6 +732,8 @@ exports.get_coretest_path = get_coretest_path;
exports.exec_manage_cli = exec_manage_cli;
exports.create_fs_user_by_platform = create_fs_user_by_platform;
exports.delete_fs_user_by_platform = delete_fs_user_by_platform;
exports.create_fs_group_by_platform = create_fs_group_by_platform;
exports.delete_fs_group_by_platform = delete_fs_group_by_platform;
exports.set_path_permissions_and_owner = set_path_permissions_and_owner;
exports.set_nc_config_dir_in_config = set_nc_config_dir_in_config;
exports.generate_anon_s3_client = generate_anon_s3_client;
Expand Down
2 changes: 1 addition & 1 deletion src/test/unit_tests/test_bucketspace_versioning.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
Policy: JSON.stringify(policy)
});

res = await generate_nsfs_account(rpc_client, EMAIL, new_bucket_path_param, { uid: 5, gid: 5 });
res = await generate_nsfs_account(rpc_client, EMAIL, new_bucket_path_param, { uid: 1055, gid: 1055 });
s3_uid5 = generate_s3_client(res.access_key, res.secret_key, CORETEST_ENDPOINT);
accounts.push(res.email);

Expand Down
39 changes: 38 additions & 1 deletion src/test/unit_tests/test_nsfs_access.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ mocha.describe('new tests check', async function() {
const root_dir = 'root_dir';
const non_root_dir = 'non_root_dir';
const non_root_dir2 = 'non_root_dir2';
const test_user_name = 'test_user';
const test_group_name = 'test_group';
const full_path_root = path.join(p, root_dir);
const full_path_non_root = path.join(full_path_root, non_root_dir);
const full_path_non_root1 = path.join(p, non_root_dir);
Expand Down Expand Up @@ -52,16 +54,27 @@ mocha.describe('new tests check', async function() {
supplemental_groups: [1572, 1577], //gid of non-root1 and unrelated gid
warn_threshold_ms: 100,
};

const NON_ROOT4_FS_CONFIG = {
uid: 1575,
gid: 1575,
backend: '',
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, 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 test_utils.create_fs_user_by_platform(test_user_name, test_user_name, 1575, 1575); //non root 4
await test_utils.create_fs_group_by_platform(test_group_name, 1572, test_user_name); //non root 1 group
});

mocha.after(async function() {
await fs_utils.folder_delete(p);
await test_utils.delete_fs_user_by_platform(test_user_name);
await test_utils.delete_fs_group_by_platform(test_group_name);
});

mocha.it('ROOT readdir - sucsses', async function() {
Expand Down Expand Up @@ -127,7 +140,31 @@ mocha.describe('new tests check', async function() {
try {
//non root3 doesn't have non-root2 group as supplemental group, so it should fail
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}`);
assert.fail(`non root 4 has access to a folder created by user with gid not insupplemental groups - ${p} ${non_root_entries}`);
} catch (err) {
assert.equal(err.code, 'EACCES');
}
});

mocha.it('NON ROOT 4 with dynamicly allocated suplemental groups - success', async function() {
//non root4 has non-root1 group as supplemental group, so it should succeed
if (process.platform === 'darwin') {
const self = this; // eslint-disable-line no-invalid-this
self.skip();
}
const non_root_entries = await nb_native().fs.readdir(NON_ROOT4_FS_CONFIG, full_path_non_root1);
assert.equal(non_root_entries && non_root_entries.length, 0);
});

mocha.it('NON ROOT 4 with dynamicly allocated suplemental groups that dont contains the files gid - failure', async function() {
if (process.platform === 'darwin') {
const self = this; // eslint-disable-line no-invalid-this
self.skip();
}
try {
//non root4 doesn't have non-root2 group as supplemental group, so it should fail
const non_root_entries = await nb_native().fs.readdir(NON_ROOT4_FS_CONFIG, full_path_non_root2);
assert.fail(`non root 4 has access to a folder created by user with gid not insupplemental groups - ${p} ${non_root_entries}`);
} catch (err) {
assert.equal(err.code, 'EACCES');
}
Expand Down

0 comments on commit a36991b

Please sign in to comment.