HBAC: Do not rely on originalMemberOf, use the sysdb memberof links instead (sssd-1-13 backprot) #309
Conversation
…nstead The IPA HBAC code used to read the group members from the originalMemberOf attribute value for performance reasons. However, especially on IPA clients trusting an AD domain, the originalMemberOf attribute value is often not synchronized correctly. Instead of going through the work of maintaining both member/memberOf and originalMemberOf, let's just do an ASQ search for the group names of the groups the user is a member of in the cache and read their SYSBD_NAME attribute. To avoid clashing between similarly-named groups in IPA and in AD, we look at the container of the group. Resolves: https://pagure.io/SSSD/sssd/issue/3382 (cherry picked from commit c92e491)
|
This patch looks good. The main difference between this patch and the original one is the use of fqnames by default in the new one. I have an ACK from me, but keep in mind I didn't run any test against this patch. |
|
So the way I tested the patch was to create an external IPA group, add an AD group into it, then add the external IPA group into an IPA POSIX group and reference the IPA POSIX group in an HBAC rule. Then disable allow_all. Make sure that the members of that group are allowed to log in, but nobody else can. btw the group must make scope different than domain-local (so, domain admins or domain users are not good candicate). Either universal or global would do. For added testing, you can also do the same test with IPA group and IPA user and then directly with usernames instead of group names. I don't think it's needed to test other HBAC components like services, because the patch only touches the function that deals with user and group names. |
|
I've talked to @jhrozek that I haven't been able to reproduce the original issue and he mentioned that "nobody ever could reproduce the original issue" and that "the fixes were just based on observation of the affected customer's cache". I've done the steps recommend by @jhrozek in the comment above and the patch didn't break the current functionality. I've already ACK'ed the patch "codewise" and now I'm firing a CI build. The patch will be acked as soon as the I get the results from our CI. |
|
CI passsed, ack! |
|
This is a backport of the same fix we merged earlier.
The IPA HBAC code used to read the group members from the
originalMemberOf attribute value for performance reasons. However,
especially on IPA clients trusting an AD domain, the originalMemberOf
attribute value is often not synchronized correctly.
Instead of going through the work of maintaining both member/memberOf
and originalMemberOf, let's just do an ASQ search for the group names of
the groups the user is a member of in the cache and read their
SYSBD_NAME attribute.
To avoid clashing between similarly-named groups in IPA and in AD, we
look at the container of the group.
Resolves:
https://pagure.io/SSSD/sssd/issue/3382
(cherry picked from commit c92e491)