On Mon, Jul 04, 2011 at 07:27:17PM -0400, Jakub Hrozek wrote:
On 07/01/2011 04:44 PM, Stephen Gallagher wrote:
> And now with patches attached...
>
> On Fri, 2011-07-01 at 16:42 -0400, Stephen Gallagher wrote:
>> So many changes have been made since the original pass that I have
>> revisited the layout of my patches and have squashed many of them
>> together. With these patches, we are fully compatible with the change to
>> disable DENY rules in FreeIPA.
>>
>> Patch 0001: Add helper function msgs2attrs_array
>> Unchanged from previous versions
>>
>> Patch 0002: Add HBAC evaluator and tests
>> The evaluator library has changed its interface slightly. It now always
>> returns an hbac_info object that contains the name of the rule that
>> matched (if one did).
>>
I only had time to review these two patches as the bindings are built on
top of them -- sorry.
0001 - unchanged, ack
0002 - ack, but please also fix the docstring of hbac_evaluate() before
pushing (error vs. info)
Hi,
0002 also adds an empty line at the end of configure.ac.
0005-Add-new-HBAC-lookup-and-evaluation-routines.patch:
+ base_dn = sysdb_custom_subtree_dn(sysdb, NULL,
+ domain->name,
+ HBAC_RULES_SUBDIR);
+ if (base_dn == NULL) {
+ ipa_access_reply(hbac_ctx, PAM_SYSTEM_ERR);
+ return;
+ }
sysdb_custom_subtree_dn() always fails, because ldb_dn_new_fmt() does
not like a NULL memory context, additionally base_dn is never freed.
0006-Add-ipa_hbac_refresh-option.patch:
I would prefer to save a reference to the ipa_access_ctx in hbac_ctx in
ipa_access_handler() and use this later on. But this is just cosmetics.
0007-Add-ipa_hbac_treat_deny_as-option.patch
+ <para>
+ <emphasis>DENY_ALL</emphasis>: If any HBAC rules
+ are detected, all users will be denied access.
+ </para>
I think you meant 'If any HBAC deny rules ...'
Although it is not the default I think it would make sense to have a
level 5 or above debug message that tells that deny rules are ignored
and not loaded from the server when the rules are evaluated.
I've done some testing and all the logic is working as expected.
bye,
Sumit
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel