[SSSD] [PATCHES] HBAC access control rewrite

Sumit Bose sbose at redhat.com
Fri Jul 8 12:59:43 UTC 2011


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 at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel



More information about the sssd-devel mailing list