Bug 38796 - ensure uidNumber and gidNumber do not collide when add/mod user or group
ensure uidNumber and gidNumber do not collide when add/mod user or group
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.0
Other Linux
: P5 normal (vote)
: UCS 4.0-3-errata
Assigned To: Daniel Tröder
Florian Best
Depends on:
Blocks: 39373
  Show dependency treegraph
Reported: 2015-06-30 09:33 CEST by Janis Meybohm
Modified: 2019-11-14 12:37 CET (History)
6 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2015052921000512, 2016111821000476
Bug group (optional):
Max CVSS v3 score:

find colliding gids, change them in LDAP and on disk (7.04 KB, text/x-python)
2015-06-30 23:45 CEST, Daniel Tröder
"v2" (11.16 KB, text/x-python)
2015-07-01 15:33 CEST, Daniel Tröder

Note You need to log in before you can comment on or make changes to this bug.
Description Janis Meybohm univentionstaff 2015-06-30 09:33:12 CEST

We should ensure that a uidNumber is not used as gidNumber and vice versa as that may lead to problems with samba4 idmap ldb.
Comment 1 Daniel Tröder univentionstaff 2015-06-30 23:44:51 CEST
# ./change_colliding-gids.py --help
Usage: change_colliding-gids.py [options]

  -h, --help            show this help message and exit
  -d FILE, --dir=FILE   run chgrp recursivly on directory, expects change
                        information from file with -i
  -i FILE, --infile=FILE
                        read group changes from FILE
  -o FILE, --outfile=FILE
                        write group changes to FILE
  -l, --ldap            Modify GIDs of groups in LDAP, expects change
                        information from file with -i [default=off].
  -v, --verbose         write about current action [default=on].

# ./change_colliding-gids.py -o out.txt
# ./change_colliding-gids.py -i out.txt -l
# ./change_colliding-gids.py -i out.txt -d /home/

Console output omitted. Details logs go to /var/log/changegrp/$DATE.log

PAM caches for about 10min. A reboot is the most secure thing to do after changing the GIDs.
Comment 2 Daniel Tröder univentionstaff 2015-06-30 23:45:53 CEST
Created attachment 6987 [details]
find colliding gids, change them in LDAP and on disk
Comment 3 Daniel Tröder univentionstaff 2015-07-01 09:52:11 CEST
The attachment is for a support ticket to help a customer who manually set UIDs to match those of an existing system. But the problem is a general one and should be addressed in UDM:

If the same uidNumber and gidNumber exist in a domain, problems occur. With uids starting at 2000 and gids starting at 5000 it takes just 3000 users to run into potential problems. UDM should make sure this doesn't happen. My guess is this should be done wherever ualloc.request(lo, position, 'gidNumber') (and uidNumber) goes.
Comment 4 Daniel Tröder univentionstaff 2015-07-01 15:33:18 CEST
Created attachment 6990 [details]

The problem with the first version was, that it did change GIDs of all groups it could find duplicate IDs. But it happens, that a group is used by computer accounts, and those don't show up in users/user and thus were not modified. So suddenly the AppCenter and other functionality ceased to work, because the computer accounts had primaryGroups that didn't exist anymore.

While it would be possible to include computer accounts in the chgrp operation, it is not what this script is intended for, so I decided to exclude groups that are not intended for users.

This version has a blacklists for user and group accounts that should not be touched. The list was compiled on a pristine UCS 4.0 with Samba4. The only not-blacklisted default group is "Domain Users".

The new '-b' switch makes it print the blacklisted users and groups.
Comment 5 Daniel Tröder univentionstaff 2015-07-01 16:05:57 CEST
Der Gruppen Cache kann neu aufgebaut werden durch den Aufruf von:

/usr/lib/univention-pam/ldap-group-to-file.py --check_member
Comment 6 Daniel Tröder univentionstaff 2015-09-09 11:38:22 CEST
The code to reserve UIDs and GIDs before using them has been adapted to make sure no two UIDs and GIDs are the same.

Commits: 63551 - 63554
Merge to 4.1: 63555
YAML (r63557): 2015-09-09-univention-directory-manager-modules.yaml
Comment 7 Arvid Requate univentionstaff 2015-09-24 11:56:02 CEST
See also Bug 26383.
Comment 8 Daniel Tröder univentionstaff 2015-10-15 13:26:15 CEST
It is still possible to create a collision by explicitly modifying a user/group, requesting specific UID/GID, with UDM on the command line. Such a UDM call should fail.
Comment 9 Daniel Tröder univentionstaff 2015-10-15 15:55:59 CEST
Deliberate collisions of uidNumbers and gidNumbers are now forbidden, when using udm. They can be allowed if UCRV directory/manager/uid_gid/uniqueness=no.

Commit: 64513
YAML (64514): 2015-09-09-univention-directory-manager-modules.yaml
Comment 10 Florian Best univentionstaff 2015-10-19 12:17:16 CEST
Please consider users without uidNumber by checking:
if 'posix' in self.options or 'samba' in self.options

Please outsource the same code into a method to remove code duplication.
You can remove the configRegistry.load() as this might cause performance problems if you edit e.g. 1000 users or groups. We never reload UCR variables in a handler. A univention-cli-server restart is better if the variable gets changed which should not occur often.
Use new style raising.
Comment 11 Daniel Tröder univentionstaff 2015-10-19 13:37:56 CEST

Commits 4.0-3: 64575, 64576
Merge 4.1: 64577
YAML: 64579
Comment 12 Daniel Tröder univentionstaff 2015-10-21 19:48:53 CEST
Commit 64709 reduce the code and raises the readability.
YAML: 64711
Comment 13 Florian Best univentionstaff 2015-11-02 13:08:22 CET
OK: creation of users/groups still work
OK: creation of user with existing uidNumber/gidNumber == uidNumber fails
OK: creation of group with existing uidNumber/gidNumber == gidNumber fails
OK: merge to UCS 4.1
OK: code review
Comment 14 Janek Walkenhorst univentionstaff 2015-11-04 17:26:33 CET