From 653c3e6abb4d028229e0291f73bedb3c8a9779a3 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Dec 10 2018 09:40:53 +0000 Subject: [PATCH 1/2] FILES: Don't allow duplicate IDs by default Merges: https://pagure.io/libuser/issue/39 Changes the libuser default to disallow duplicate IDs when adding or modifying users and groups. Also adds test cases to test the new behaviour and fixes two existing test which relied on the old behaviour. --- diff --git a/modules/files.c b/modules/files.c index b8bf8a6..e2e92db 100644 --- a/modules/files.c +++ b/modules/files.c @@ -832,6 +832,105 @@ lu_shadow_group_lookup_id(struct lu_module *module, gid_t gid, return ret; } +static gboolean +lu_files_mod_is_id_unique(struct lu_module *module, struct lu_ent *ent, + struct lu_error **error) +{ + id_t id_change = LU_VALUE_INVALID_ID; + struct lu_ent *dup_ent = NULL; + gboolean ret = FALSE; + gboolean found = FALSE; + const char *name_attribute; + const char *id_attribute; + + g_assert(module != NULL); + g_assert(ent != NULL); + g_assert(error != NULL); + + /* Get the array of names for the entity object. */ + if (ent->type == lu_user) { + name_attribute = LU_USERNAME; + id_attribute = LU_UIDNUMBER; + } else if (ent->type == lu_group) { + name_attribute = LU_GROUPNAME; + id_attribute = LU_GIDNUMBER; + } else { + g_assert_not_reached(); + } + + id_change = lu_ent_get_first_id(ent, id_attribute); + if (id_change == LU_VALUE_INVALID_ID) { + /* The GID is not being changed, success */ + return TRUE; + } + + /* If the GID is being changed, check if there is another entry + * with the same GID + */ + dup_ent = lu_ent_new(); + if (dup_ent == NULL) { + return FALSE; + } + + /* Get the array of names for the entity object. */ + if (ent->type == lu_user) { + found = lu_files_user_lookup_id(module, id_change, dup_ent, + error); + } else if (ent->type == lu_group) { + found = lu_files_group_lookup_id(module, id_change, dup_ent, + error); + } else { + g_assert_not_reached(); + } + + if (found == TRUE) { + /* If there is, check if its original name is the same as + * the original name of ent. + */ + const char *dup_name = NULL; + const char *ent_cur_name = NULL; + + dup_name = lu_ent_get_first_string_current(dup_ent, + name_attribute); + if (dup_name == NULL) { + lu_error_new(error, lu_error_generic, + _("duplicate object has no %s attribute"), + name_attribute); + ret = FALSE; + goto done; + } + + ent_cur_name = lu_ent_get_first_string_current(ent, + name_attribute); + if (ent_cur_name == NULL) { + lu_error_new(error, lu_error_generic, + _("original object has no %s attribute"), + name_attribute); + ret = FALSE; + goto done; + } + + /* Another entry already has the same ID we're attempting to + * set, * this can only work if we are also renaming the group + * to the * duplicate name or if we're changing 'self' + */ + if (strcmp(dup_name, ent_cur_name) != 0) { + lu_error_new(error, lu_error_id_used, + _("ID %lu already in use by %s"), + (unsigned long) id_change, dup_name); + ret = FALSE; + goto done; + } + } + + ret = TRUE; +done: + if (dup_ent != NULL) { + lu_ent_free(dup_ent); + } + return ret; +} + /* Format a single field. Return field string for g_free (). */ static char * @@ -1071,6 +1170,13 @@ static gboolean lu_files_user_add(struct lu_module *module, struct lu_ent *ent, struct lu_error **error) { + gboolean ret = FALSE; + + ret = lu_files_mod_is_id_unique(module, ent, error); + if (ret == FALSE) { + return FALSE; + } + return generic_add(module, suffix_passwd, format_passwd, G_N_ELEMENTS(format_passwd), ent, error); } @@ -1112,6 +1218,13 @@ static gboolean lu_files_group_add(struct lu_module *module, struct lu_ent *ent, struct lu_error **error) { + gboolean ret = FALSE; + + ret = lu_files_mod_is_id_unique(module, ent, error); + if (ret == FALSE) { + return FALSE; + } + return generic_add(module, suffix_group, format_group, G_N_ELEMENTS(format_group), ent, error); } @@ -1262,6 +1375,13 @@ static gboolean lu_files_user_mod(struct lu_module *module, struct lu_ent *ent, struct lu_error **error) { + gboolean ret = FALSE; + + ret = lu_files_mod_is_id_unique(module, ent, error); + if (ret == FALSE) { + return FALSE; + } + return generic_mod(module, suffix_passwd, format_passwd, G_N_ELEMENTS(format_passwd), ent, error); } @@ -1271,6 +1391,13 @@ static gboolean lu_files_group_mod(struct lu_module *module, struct lu_ent *ent, struct lu_error **error) { + gboolean ret = FALSE; + + ret = lu_files_mod_is_id_unique(module, ent, error); + if (ret == FALSE) { + return FALSE; + } + return generic_mod(module, suffix_group, format_group, G_N_ELEMENTS(format_group), ent, error); } diff --git a/tests/files_test.py b/tests/files_test.py index 693a3b9..23e19e4 100644 --- a/tests/files_test.py +++ b/tests/files_test.py @@ -278,6 +278,15 @@ class Tests(unittest.TestCase): e[field] = str(e[field][0]) + '\nx' self.assertRaises(RuntimeError, self.a.addUser, e, False, False) + def testUserAdd10(self): + # Adding a duplicate user UID + e = self.a.initUser('user6_10') + self.a.addUser(e, False, False) + + e2 = self.a.initUser('user6_10_2') + e2[libuser.UIDNUMBER] = e[libuser.UIDNUMBER] + self.assertRaises(RuntimeError, self.a.addUser, e2, False, False) + def testUserMod1(self): # A minimal case e = self.a.initUser('user7_1') @@ -301,8 +310,8 @@ class Tests(unittest.TestCase): e[libuser.USERNAME] = 'user7_2username' self.assertNotEqual(e[libuser.USERPASSWORD], ['!!pwuser7_2']) e[libuser.USERPASSWORD] = '!!pwuser7_2' - self.assertNotEqual(e[libuser.UIDNUMBER], [4237]) - e[libuser.UIDNUMBER] = 4237 + self.assertNotEqual(e[libuser.UIDNUMBER], [4238]) + e[libuser.UIDNUMBER] = 4238 self.assertNotEqual(e[libuser.GIDNUMBER], [3742]) e[libuser.GIDNUMBER] = 3742 self.assertNotEqual(e[libuser.GECOS], ['Full Name,Office,1234,4321']) @@ -336,7 +345,7 @@ class Tests(unittest.TestCase): self.assertIsNotNone(e) self.assertEqual(e[libuser.USERNAME], ['user7_2username']) self.assertEqual(e[libuser.USERPASSWORD], ['!!pwuser7_2']) - self.assertEqual(e[libuser.UIDNUMBER], [4237]) + self.assertEqual(e[libuser.UIDNUMBER], [4238]) self.assertEqual(e[libuser.GIDNUMBER], [3742]) self.assertEqual(e[libuser.GECOS], ['Full Name,Office,1234,4321']) self.assertEqual(e[libuser.HOMEDIRECTORY], ['/home/user7_2home']) @@ -457,6 +466,21 @@ class Tests(unittest.TestCase): e[field] = str(e[field][0]) + '\nx' self.assertRaises(RuntimeError, self.a.modifyUser, e, False) + def testUserMod9(self): + # Attempt to modify to create UID duplicates + e = self.a.initUser('user7_9') + self.a.addUser(e, False, False) + e_uid = e[libuser.UIDNUMBER] + del e + + e = self.a.initUser('user7_9_2') + self.a.addUser(e, False, False) + del e + + e = self.a.lookupUserByName('user7_9_2') + e[libuser.UIDNUMBER] = e_uid + self.assertRaises(RuntimeError, self.a.modifyUser, e, False) + def testUserDel(self): e = self.a.initUser('user8_1') self.a.addUser(e, False, False) @@ -946,6 +970,19 @@ class Tests(unittest.TestCase): e[field] = field + '\nx' self.assertRaises(RuntimeError, self.a.addGroup, e) + def testGroupAdd7(self): + # Adding a duplicate group GID + e = self.a.initGroup('group21_7') + e[libuser.MEMBERNAME] = ['group21_7member1', 'group21_7member2'] + self.a.addGroup(e) + e_gid = e[libuser.GIDNUMBER] + del e + + e_dup = self.a.initGroup('group21_7_dup') + self.assertNotEqual(e_dup[libuser.GIDNUMBER], [e_gid]) + e_dup[libuser.GIDNUMBER] = e_gid + self.assertRaises(RuntimeError, self.a.modifyGroup, e_dup) + def testGroupMod1(self): # A minimal case e = self.a.initGroup('group22_1') @@ -971,7 +1008,7 @@ class Tests(unittest.TestCase): self.assertNotEqual(e[libuser.GROUPPASSWORD], ['!!grgroup22_2']) e[libuser.GROUPPASSWORD] = '!!grgroup22_2' self.assertNotEqual(e[libuser.GIDNUMBER], [4237]) - e[libuser.GIDNUMBER] = 4237 + e[libuser.GIDNUMBER] = 4238 v = sorted(e[libuser.MEMBERNAME]) self.assertNotEqual(v, ['group22_2member1', 'group22_2member3']) e[libuser.MEMBERNAME] = ['group22_2member1', 'group22_2member3'] @@ -986,7 +1023,7 @@ class Tests(unittest.TestCase): self.assertIsNotNone(e) self.assertEqual(e[libuser.GROUPNAME], ['group22_2groupname']) self.assertEqual(e[libuser.GROUPPASSWORD], ['!!grgroup22_2']) - self.assertEqual(e[libuser.GIDNUMBER], [4237]) + self.assertEqual(e[libuser.GIDNUMBER], [4238]) v = e[libuser.MEMBERNAME] v.sort() self.assertEqual(v, ['group22_2member1', 'group22_2member3']) @@ -1077,6 +1114,25 @@ class Tests(unittest.TestCase): e[field] = field + '\nx' self.assertRaises(RuntimeError, self.a.modifyGroup, e) + def testGroupMod8(self): + # GID duplicate tests + e = self.a.initGroup('group22_8') + e[libuser.MEMBERNAME] = ['group22_8member1', 'group22_8member2'] + self.a.addGroup(e) + e_gid = e[libuser.GIDNUMBER] + del e + + e_dup = self.a.initGroup('group22_8_2') + e_dup[libuser.MEMBERNAME] = ['group22_8member1', 'group22_8member2'] + self.a.addGroup(e_dup) + del e_dup + + # Changing GID to e's should not work + e_dup = self.a.lookupGroupByName('group22_8_2') + self.assertNotEqual(e_dup[libuser.GIDNUMBER], [e_gid]) + e_dup[libuser.GIDNUMBER] = e_gid + self.assertRaises(RuntimeError, self.a.modifyGroup, e_dup) + def testGroupDel(self): e = self.a.initGroup('group23_1') self.a.addGroup(e) From 2a30eddaf56822d521459053aaeff36470b00934 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Dec 10 2018 09:41:58 +0000 Subject: [PATCH 2/2] WIP: Add a files module option to optionally allow duplicates --- diff --git a/modules/files.c b/modules/files.c index e2e92db..109f4af 100644 --- a/modules/files.c +++ b/modules/files.c @@ -833,6 +833,25 @@ lu_shadow_group_lookup_id(struct lu_module *module, gid_t gid, } static gboolean +lu_files_permits_duplicate_ids(struct lu_module *module) +{ + const char *allow_id_dupes; + + g_assert(module != NULL); + g_assert(module->lu_context != NULL); + + allow_id_dupes = lu_cfg_read_single(module->lu_context, + "files/allow_id_duplicates", + "false"); + if (allow_id_dupes && + strcasecmp(allow_id_dupes, "true") == 0) { + return TRUE; + } + + return FALSE; +} + +static gboolean lu_files_mod_is_id_unique(struct lu_module *module, struct lu_ent *ent, struct lu_error **error) { @@ -847,6 +866,10 @@ lu_files_mod_is_id_unique(struct lu_module *module, struct lu_ent *ent, g_assert(ent != NULL); g_assert(error != NULL); + if (lu_files_permits_duplicate_ids(module)) { + return TRUE; + } + /* Get the array of names for the entity object. */ if (ent->type == lu_user) { name_attribute = LU_USERNAME;