[Freeipa-devel] [PATCH] 728 default roles

[Martin Kosek]

On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote:
> Rob Crittenden <rcritten at redhat.com> wrote:
> > Jakub Hrozek wrote:
> > > On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:
> > >> Rob Crittenden wrote:
> > >>> Jakub Hrozek wrote:
> > >>>> -----BEGIN PGP SIGNED MESSAGE-----
> > >>>> Hash: SHA1
> > >>>> 
> > >>>> On 02/17/2011 04:35 AM, Rob Crittenden wrote:
> > >>>>> Add default roles and permissions for HBAC, SUDO and pw policy
> > >>>>> 
> > >>>>> Created some default roles as examples. In doing so I realized that
> > >>>>> we were completely missing default rules for HBAC, SUDO and password
> > >>>>> policy so I added those as well.
> > >>>>> 
> > >>>>> I ran into a problem when the updater has a default record and an add
> > >>>>> at the same time, it should handle it better now.
> > >>>>> 
> > >>>>> ticket 585
> > >>>>> 
> > >>>>> rob
> > >>>> 
> > >>>> I'm not sure about the HBAC rules ACIs. They are specified as:
> > >>>> 
> > >>>> 'target = "ldap:///cn=*,cn=hbac,$SUFFIX"'
> > >>>> 
> > >>>> while HBAC rules' DN is:
> > >>>> 
> > >>>> 'ipauniqueid=*,cn=hbac,$SUFFIX'.
> > >>>> 
> > >>>> But HBAC rules do have a cn: attribute, so maybe the ACIs would work?
> > >>> 
> > >>> No, you're right, this is wrong. I'll fix it up and resubmit.
> > >>> 
> > >>>> The patch also needs rebasing on top of recent changes to
> > >>>> install/updates/Makefile.am
> > >>>> 
> > >>>> Other than that, looks OK to me.
> > >>>> 
> > >>>> btw when I was reviewing this patch, I noticed we add a "DNS
> > >>>> Administrators" privilege in dns.ldif. Would it make sense to add DNS
> > >>>> administration to "Security Architect" (replication management) and
> > >>>> "IT Specialist" (hosts management)?
> > >>> 
> > >>> The DNS stuff is added only if DNS is enabled on the server so I can't
> > >>> add them by default.
> > >>> 
> > >>> rob
> > >> 
> > >> Updated patch.
> > >> 
> > >> rob
> > > 
> > > Interdiff looks fine, but I'm not able to apply the patch (not even
> > > 3-way merge), can you rebase?
> > 
> > done
> 
> The patch now applies ok (just one whitespace warning), ack
> 
> Jan
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

I have to NACK this. I have found some issues in the new LDAP records:

1) A wrong groupdn for the following ACI in 40-delegation.update:
add:aci: '(target = "ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX")(version
3.0;acl "permission:Add SUDO rule";allow (add) groupdn = "ldap:///cn=Add
SUDOrule,cn=permissions,cn=pbac,$SUFFIX";)'

It should be dap:///cn=Add SUDO rule,cn=permissions,cn=pbac,$SUFFIX

2) Another wrong target for few ACIs:
ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX
is used instead of
ldap:///ipaUniqueID=*,cn=sudorules,cn=sudo,$SUFFIX


3) Missing Description for the following new privileges:
Write IPA Configuration
Modify Users and Reset passwords
Modify Group membership

Remainder looks good.

Martin