Allow user ACE in ACL to match SID in token extra SIDs
Description: After a migration of data from another storage system, some users at a customer were reporting they could no longer access their files. Other users were not affected.
After investigation, it was found that the users who had reported problems existed in a state where they had multiple SIDs in their active directory. This is normal when different AD domains are merged, for example after an acquisition, or integration of formerly separate IT groups within a company.
The users reporting problems had a valid user: ACE on the files and directories they couldn't access, however, the SID associated with that ACE was not their primary SID. The primary SID was typically the one associated with the top level AD domain, i.e. contoso.net. The other SID, when checked, also mapped back to the user in the contoso.net domain, but they had a different domain authority value.
It appears that ZFS, when evaluating user SIDs, only checks the primary SID against the ACL. If the primary SID doesn't show up in a user: ACE, then permission is denied.
What should be happening is all valid SIDs for the user are checked against the ACL to determine access permissions.
Steps to Reproduce: Create a user with multiple SIDs (i.e. via domain migration)
create an SMB share or share subfolder with full_set permissions for the "secondary" SID.
Attempt to access that folder with the user
Expected Results: user is granted access to enter, modify, create files, etc in the folder
Actual Results: user is denied access despite all other factors appearing correct.
Updated by Gordon Ross 4 months ago
The current ZFS ACL evaluation code considers a "User" ACE to apply to the current user only when the identity in that ACE (the UID) matches the "primary" UID in the token, which is crgetuid(token->cred).
The design of that ACL evaluation code did not anticipate the possibility of multiple user SIDs.
In a Windows + AD world, it's possible for a user to have multiple user identities (UIDs, and user SIDs) and, in order to be compatible with Windows, we want these User ACEs to match when the SID in the ACE matches either the primary user ID (and SID) or any ot their "extra" user SIDs.
We don't segregate User SIDs from other SIDs in the credential (because there's no convenient way to know the type of some SID, and for access check purposes there's no need to know).
The easiest way to implement this enhancement is to consider such user ACEs to match when the SID in that ACE matches either the primary ID of the credential, or any of the SIDs in the SID list on the credential.
The result is that User ACEs are handled very similarly as Group ACEs.
Updated by Gordon Ross 4 months ago
Testing is tricky due to the need for an AD user with multiple user SIDs.
Matt Barden found a way to add a sidHistory value to an AD account object. See:
DSInternals' Add-ADDBSidHistory function: https://github.com/MichaelGrafnetter/DSInternals/releases
Verified at the customer. Fix in production since early 2018
Updated by Electric Monk 4 months ago
- Status changed from In Progress to Closed
- % Done changed from 0 to 100
commit 4fbfc69b99ccacf0999510e626df37e53b3d56ef Author: Gordon Ross <email@example.com> Date: 2019-05-30T01:37:59.000Z 10991 Allow user ACE in ACL to match SID in token extra SIDs Reviewed by: Roman Strashkin <firstname.lastname@example.org> Reviewed by: Sanjay Nadkarni <email@example.com> Reviewed by: Evan Layton <firstname.lastname@example.org> Reviewed by: Rick McNeal <email@example.com> Reviewed by: Joyce McIntosh <firstname.lastname@example.org> Reviewed by: Garrett D'Amore <email@example.com> Reviewed by: Gergő Doma <firstname.lastname@example.org> Approved by: Dan McDonald <email@example.com>