Learn more about these different git repos.
Other Git URLs
The gpo code is not correctly implementing the "last policy wins" conflict resolution mechanism for policy settings. Currently, for each applicable gpo, the code retrieves the gpo's policy files and then enforces the gpo's policy files right then. This is incorrect b/c the list of applicable GPOs is given in priority order (with the highest priority gpo appearing last), and we could be prematurely denying access using a domain GPO's policy settings, while an ou GPO's (higher priority) policy settings would have allowed access (but we didn't even get to processing the ou GPO)
For example, if a domain GPO (lower-priority) includes an !AllowLogonLocally policy setting, and an ou GPO (higher-priority) also includes that same policy setting, the current (buggy) implementation retrieves the first (lower-priority) GPO's policy settings and enforces them right away. If the domain GPO's policy settings indicate that the user should not be allowed access (perhaps b/c the user doesn't appear on the !AllowLogonLocally list of sids), then the current implementation denies access right away, without even retrieving the ou GPO's policy files. Clearly, this is incorrect behavior, b/c the ou GPO's !AllowLogonLocally policy settings (if any) should trump the domain GPO's !AllowLogonLocally policy settings.
Microsoft solves this problem by storing policy settings in the client's registry, with the lower-priority GPO's policy settings being written first, but then potentially being overwritten by a higher-priority GPO's policy settings (but only if they are the exact keys; i.e. !AllowLogonLocally and !DenyLogonLocally are different policy settings). Only after all GPO's have had a chance to write to the registry is the policy actually enforced. This ensures that the "last policy wins" mechanism is implemented correctly.
In the sssd case, since we are not using a registry, we can achieve the same result by doing the following:
Fields changed
cc: sgallagh@redhat.com => sgallagh@redhat.com, yelley patch: 0 => 1
Another part of this bug that needs to be fixed is that we are not correctly handling "defined but empty" policy settings (i.e. we are treating "defined but empty" policy settings the same as "undefined" policy settings). An "undefined" policy setting should always be ignored, but a "defined but empty" policy setting should trump lower-priority policy settings (but only of the same key, of course).
As an example of a "defined but empty" policy setting, suppose an admin creates a !DenyLogonLocally policy setting by clicking on the "Define these policy settings" checkbox. At this point, the admin typically adds a user or group to be denied. However, the admin can also just press "OK" without adding a user or group, leaving the policy setting in a "defined but empty" state. In other words, the !GptTmpl.inf file has a line that looks like: "!SeDenyInteractiveLogonRight ="
This can potentially be useful if an ou GPO wants to make sure that the policy settings of any lower-priority GPOs (e.g. domain GPOs) are overwritten by an empty policy setting.
It turns out that the proposed solution (and therefore the current patch on the list) doesn't work for the offline case, b/c the gpo cache entries stored in the sysdb cache do not capture the priority ordering of the gpos. In other words, for the online case, we are retrieving all policy files, and then iterating through the policy files, keeping track of the final resultant policy settings (keeping in mind the priority ordering). However, in the offline case, we are simply iterating through the policy files in the order that the sysdb cache returns the gpo_guids (which is not necessarily the same as the priority ordering).
One solution is to actually adopt Microsoft's registry implementation strategy, by which I mean we should create a "GPO Enforcement" object in the sysdb cache, which consists of a set of key/value pairs. In the online case, as the policy files are being parsed, we can simply write the key/value pairs in each policy file to the Enforcement object. If a key in a policy file already exists in the Enforcement object, we simply overwrite it (thereby implementing "last policy wins"). Once we are done with online processing, the Enforcement object will represent the final resultant policy settings. If we encounter another online transaction, we clear the Enforcement object and build it up again. However, if we go offline, we can simply reference the Enforcement object for the resultant policy settings, and we can apply them to the user_sid/group_sids of interest.
There is one other caveat. If the service we are dealing with online is "sshd", and we therefore only write the !RemoteInteractive policy settings to the Enforcement object, then we may not be able to handle every potential service in the offline case. For example, once offline, we may encounter the "login" service, but we won't have the Interactive policy settings in the Enforcement object (since we only stored !RemoteInteractive settings). In order to address this issue, we will need to write all policy settings that we know how to process in the online case. In other words, while the online case might have been for the "sshd" service (!RemoteInteractive), we would still write all of the other policy settings that we know how to process to the Enforcement object (Interactive, Network, Batch, Service). In this way, we can correctly handle any supported service in the offline case.
I have implemented this design (with the only difference being that I ended up using the term "GPO Result" object rather than "GPO Enforcement" object). A revised patch has been sent to the sssd-devel list.
milestone: NEEDS_TRIAGE => SSSD 1.12.2
We need to do a release as requested by downstream. Moving tickets that are not fixed already or very close to acking to 1.12.3
milestone: SSSD 1.12.2 => SSSD 1.12.3
These will make 1.12.2 after all.
mark: => 0 milestone: SSSD 1.12.3 => SSSD 1.12.2 owner: somebody => sgallagh
resolution: => fixed status: new => closed
rhbz: => 0
Metadata Update from @yelley: - Issue assigned to sgallagh - Issue set to the milestone: SSSD 1.12.2
SSSD is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in SSSD's github repository.
This issue has been cloned to Github and is available here: - https://github.com/SSSD/sssd/issues/3479
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Login to comment on this ticket.