On Tue, 2011-06-07 at 09:03 -0400, Simo Sorce wrote: > On Tue, 2011-06-07 at 08:11 -0400, Stephen Gallagher wrote: > > On Tue, 2011-06-07 at 10:41 +0200, Jakub Hrozek wrote: > > > On 06/07/2011 04:59 AM, Stephen Gallagher wrote: > > > > While attempting to extract the SSSD's HBAC rule evaluation into a > > > > common library, I discovered several shortcomings. Some were related to > > > > extracting the library, others were related to the rule evaluation > > > > itself. > > > > > > > > Related to extracting the library, I needed for the code to be able to > > > > identify and resolve without having direct access to LDAP or the LDB. > > > > This meant I could not rely on the memberOf objects to be valid. > > > > > > > > As for the rule evaluation, I discovered that we were only caching for > > > > offline use those remote hosts that had previously attempted to log in. > > > > This meant that if the IPA server was unreachable, valid users would be > > > > unable to log in from some valid machines. > > > > > > > > After careful consideration (and a few false starts), I decided that a > > > > total rewrite was necessary. So here it is. > > > > > > > > The major changes are these: > > > > > > > > 1) We have moved to always performing a full enumeration of known IPA > > > > hosts and HBAC services. This actually results in a net reduction in > > > > round-trips to the LDAP server, which was a primary bottleneck. > > > > Additionally, this means that we also have a full understanding when > > > > offline as to whether a remote host is permitted access or not. > > > > > > > > 2) I've added a configurable timeout for hbac rule refreshes to the LDAP > > > > server. Previously, if we were online, EVERY pam_account() request would > > > > result in at least one LDAP round-trip. By default, we'll only refresh > > > > the HBAC rules every five seconds now. > > > > > > > > 3) I broke the acquisition and processing of the rules, hosts and > > > > services into separate C files, as ipa_access.c was getting to be much > > > > to large to manage. > > > > > > > > The overall view of the code (when online) is this: > > > > A request comes in, we call to LDAP to get, in order, the set of known > > > > hosts, the set of known PAM services and then finally the set of > > > > *enabled* rules with the current ipa_hostname as the targethost. > > > > > > > > And now, the patches: > > > > > > > > Patch 0001: The new standalone evaluator library. It's very simple at > > > > the moment (but it will become much more complex in the near future when > > > > timerule evaluation is added). Jakub, Rob and I agreed on the interface > > > > and Jakub is creating Python bindings for it. > > > > > > > > > > I'll rebase the bindings on top of these patches and send to the list. > > > > > > > Patch 0002: This is the meat of the new IPA access backend. All of the > > > > helper functions necessary for looking up, storing and converting the > > > > hosts, rules and services are in this patch. > > > > > > > > Patch 0003: This is not really a patch as much as it is a trash can. I > > > > split this from Patch 0004 to make the latter more reviewable. Patch > > > > 0003 contains only the deletions of the old access provider > > > > implementation. > > > > > > > > Patch 0004: Adds the new IPA access provider, taking advantage of the > > > > helper functions from Patch 0002. > > > > > > > > Patch 0005: Adds a refresh timeout to reduce round-trips to IPA while > > > > online. > > > > > > > > > > > > > > Nack, msgs2attrs_array() is not defined: > > > > > > src/providers/ipa/ipa_access.c: In function 'hbac_get_cached_rules': > > > src/providers/ipa/ipa_access.c:589:5: error: implicit declaration of > > > function 'msgs2attrs_array' [-Werror=implicit-function-declaration] > > > cc1: some warnings being treated as errors > > > > > > Whoops, I forgot I reordered some of my patches. I missed two at the > > beginning. > > > > Patch 0001: Retrieve the group name as well as the GID during > > sysdb_initgroups() (used later for getting the user memberships of a > > user for evaluating the rules) > > > > Patch 0002: Add a helper function to convert a list of ldb_messages to > > sysdb_attrs. > > > > The other patches are the same as the previous email, but numbered +2. > > This is a generic NACK due to a concern that seem is not properly > considered here. > > As can be seen by other patches on the list we are going to support > handling MS-PACs. In MS-PACs we do not have group names, only SIDs that > we can transalte back to GIDs and eventually names. But at login names > may not be available unless a cached group entry is found. > > In this case the HBAC evaluator need to be very careful if DENY rules > are present (I hate deny rules, they are evil :-), and we are in offline > mode, not able to resolve incomplete groups. > > If the user is member of any unresolved groups that have fake names, we > must flag it, and during HBAC evaluation if we encounter any DENY rule > that can't resolve the group it refers to we need to virtually match it > and throw access denied at the user. This is because one of the > unresolved groups could be the group in DENY rule, and because we are > not able to resolve it we must consider the worst case (a match). > > If all DENY rules can reference complete cached groups, or the user is > not member of any "fake" group the rule can be evaluated as UNMATCHED if > that's the case as we can be confident the group referenced by the rule > is not a group the user is member of.
Some more data after an IRC discussion with Stephen. The current library have no way to directly consult the user database, so it has no way to know if a group exist and is resolvable, and therefore not one of the unresolved groups of the users. This would force us to DENY access even though we have enough data outside of the evaluation library to rule the DENY rule as unmatching. In order to solve this problem the library should define a callback handler that can be set by the caller so that the library can ask for user/group information. This way on deny rules if the group is not found and the user is marked to have unresolved groups, the callback can be called and it can be verified if this group otherwise exists or if we have to consider it as potentially one of the unresolved groups of the user. If the callback is limited to this use only then callers that can assert fully resolved groups for the user from the get-go, can pass in NULL for the callback. Callers that also do not care about getting a DENY where it may have not matched can also do so as a NULL will be interpreted as group not found result causing a match in the deny operation evaluation. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
