NSFS | set supplemental groups dynamically to users groups

Signed-off-by: nadav mizrahi <[email protected]>

docs/NooBaaNonContainerized/AccountsAndBuckets.md

@@ -32,6 +32,7 @@ See all available account properties - [NC Account Schema](../../src/server/syst
   - `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` - In addition to the 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.
+  by default supplemental groups based on users groups in the filesystem. in case this value is defined by cli. the value defined will override the users fs groups. in case this value is not assigned in account configuration and failed to fetch users group in the filesystem (eiher because no record exists or because operation failed), supplemental groups will be unset. 
   Note: depending on the file system there may be 'sticky bit' enabled somewhere on the files path. 'sticky bit' is a user ownership access right flag that prevents other users than the file owner and root from deleting or moving files.
   In that case some actions will still get access denied regardless of group permissions enabled. sticky bit is denoted by `t` at the end of the permissions list (example: `drwxrwxrwt`). see https://en.wikipedia.org/wiki/Sticky_bit
 

src/native/util/os_linux.cpp

@@ -8,6 +8,7 @@
 #include <sys/capability.h>
 #include <grp.h>
 #include <limits.h>
+#include <pwd.h>
 
 namespace noobaa
 {
@@ -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: ngroups too small " << ngroups);
+        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 defined in 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
@@ -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));

src/test/system_tests/test_utils.js

@@ -335,7 +335,41 @@ 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 });
+        }
+    }
+}
+
+/**
+ * deletes a file system group. if force is true, delete the group even if its the primary group of a user
+ * @param {string} group_name
+ * @param {boolean} [force]
+ */
+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
@@ -725,6 +759,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;

src/test/unit_tests/test_bucketspace_versioning.js

@@ -53,7 +53,7 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
     const nested_keys_full_path = path.join(tmp_fs_root, nested_keys_bucket_path);
     const versions_path = path.join(full_path, '.versions/');
     const suspended_versions_path = path.join(suspended_full_path, '.versions/');
-    let s3_uid5;
+    let s3_uid1055;
     let s3_uid6;
     let s3_admin;
     const accounts = [];
@@ -173,8 +173,8 @@ 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 });
-        s3_uid5 = generate_s3_client(res.access_key, res.secret_key, CORETEST_ENDPOINT);
+        res = await generate_nsfs_account(rpc_client, EMAIL, new_bucket_path_param, { uid: 1055, gid: 1055 });
+        s3_uid1055 = generate_s3_client(res.access_key, res.secret_key, CORETEST_ENDPOINT);
         accounts.push(res.email);
 
         res = await generate_nsfs_account(rpc_client, EMAIL, new_bucket_path_param);
@@ -206,7 +206,7 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
 
         mocha.it('set bucket versioning - Enabled - should fail - no permissions', async function() {
             try {
-                await s3_uid5.putBucketVersioning({ Bucket: bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Enabled' } });
+                await s3_uid1055.putBucketVersioning({ Bucket: bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Enabled' } });
                 assert.fail(`put bucket versioning succeeded for account without permissions`);
             } catch (err) {
                 assert.equal(err.Code, 'AccessDenied');
@@ -232,7 +232,7 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
 
         mocha.it('set bucket versioning - Suspended - should fail - no permissions', async function() {
             try {
-                await s3_uid5.putBucketVersioning({ Bucket: suspended_bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Suspended' } });
+                await s3_uid1055.putBucketVersioning({ Bucket: suspended_bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Suspended' } });
                 assert.fail(`put bucket versioning succeeded for account without permissions`);
             } catch (err) {
                 assert.equal(err.Code, 'AccessDenied');

src/test/unit_tests/test_nsfs_access.js

@@ -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);
@@ -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, NON_ROOT4_FS_CONFIG.uid, NON_ROOT4_FS_CONFIG.gid); //non root 4
+        await test_utils.create_fs_group_by_platform(test_group_name, NON_ROOT1_FS_CONFIG.gid, 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() {
@@ -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 3 has access to a folder created by user with gid not in supplemental 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 in supplemental groups - ${p} ${non_root_entries}`);
         } catch (err) {
             assert.equal(err.code, 'EACCES');
         }

src/test/unit_tests/test_nsfs_versioning.js

@@ -45,7 +45,7 @@ mocha.describe('namespace_fs - versioning', function() {
 
     const dummy_object_sdk = make_dummy_object_sdk(true);
     const dummy_object_sdk_no_nsfs_config = make_dummy_object_sdk(false);
-    const dummy_object_sdk_no_nsfs_permissions = make_dummy_object_sdk(true, 5, 5);
+    const dummy_object_sdk_no_nsfs_permissions = make_dummy_object_sdk(true, 1055, 1055);
     const ns_tmp = new NamespaceFS({ bucket_path: ns_tmp_bucket_path, bucket_id: '1', namespace_resource_id: undefined });
 
     mocha.it('set bucket versioning - Enabled - should fail - no permissions', async function() {