[SSSD] [PATCH] AD: conflicting gpo policy settings not being resolved correctly

Stephen Gallagher sgallagh at redhat.com
Thu Oct 2 02:50:26 UTC 2014




On Thu, 2014-09-11 at 23:51 -0400, Yassir Elley wrote:
> 
> ----- Original Message -----
> > 
> > 
> > ----- Original Message -----
> > > Hi,
> > > 
> > > The attached patch fixes ticket #2437 (conflicting gpo policy settings not
> > > being resolved correctly)
> > > 
> > > https://fedorahosted.org/sssd/ticket/2437
> > > 
> > > Regards,
> > > Yassir.
> > > 
> > > _______________________________________________
> > > sssd-devel mailing list
> > > sssd-devel at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > 
> > 
> > I discovered some more issues while doing further testing on this ticket
> > relating to "defined but empty" policy settings (see additional comments on
> > ticket #2347). I have also made some minor changes to this patch, b/c they
> > didn't warrant their own ticket. Namely, I have removed some attributes that
> > we weren't using (AD_AT_USER_EXT_NAMES, etc). I have also fixed the way we
> > handle machine_ext_names by allowing for the possibility of a "present but
> > empty" value for AD_AT_MACHINE_EXT_NAMES.
> > 
> > I am attaching a revised path to address these issues.
> > 
> > Regards,
> > Yassir.
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > 
> 
> I have implemented the new design (described in ticket #2347) in which policy settings are written (and potentially over-written) to a "GPO Enforcement" object (which I ended up calling "GPO Result" object) in the sysdb cache. This allows policy settings to be resolved correctly for both the online and offline cases. It is a reasonably large patch, but I think the final implementation is cleaner and simpler. Revised patch is attached.



Sorry it took me so long to finish this review. The code is mostly
right, but I found three issues that needed to be addressed before we
could commit it.

1) The GPO code was returning EACCES instead of ERR_ACCESS_DENIED which
was resulting in the PAM conversation ultimately returning
PAM_SYSTEM_ERR. (Pre-existing, but found while testing this patch) Fixed
in the 0001 patch attached. Can be applied separately.

2) There was an 'attrs' variable being passed in the
sysdb_gpo_get_gpo_result_setting() routine without a NULL terminator,
causing a crash when accessing LDB.

3) The code could not properly handle when a GPO had an explicitly empty
value (such as one might use to expressly disable a setting from a
lower-priority GPO).

Issues 2) and 3) are both addressed by the 0002 patch. For an easy view
of the changes I made, see
https://reviewboard-fedoraserver.rhcloud.com/r/80/diff/2-3/ (I tracked
my review there as I went through, so I wouldn't miss any corrections
and as a sort of proof-of-concept)


I (believe) I have done a very thorough review of this code, so I'd ask
that someone double-check my updates and then we should be good to go
with committing this. It's a pretty important set of fixes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-AD-GPO-Fix-incorrect-return-of-EACCES.patch
Type: text/x-patch
Size: 1602 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141001/69c31b3e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-AD-GPO-resolve-conflicting-policy-settings-correctly.patch
Type: text/x-patch
Size: 64038 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141001/69c31b3e/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141001/69c31b3e/attachment.sig>


More information about the sssd-devel mailing list