[Freeipa-devel] [PATCH] 0051 Allow CustodiaClient to be used by arbitrary principals

[Fraser Tweedale]

On Mon, Apr 18, 2016 at 03:44:08PM -0400, Simo Sorce wrote:
> On Thu, 2016-04-14 at 16:33 +1000, Fraser Tweedale wrote:
> > On Wed, Apr 13, 2016 at 11:15:50AM +1000, Fraser Tweedale wrote:
> > > On Tue, Apr 12, 2016 at 09:31:30AM -0400, Simo Sorce wrote:
> > > > On Sat, 2016-04-09 at 10:11 +1000, Fraser Tweedale wrote:
> > > > > On Fri, Apr 08, 2016 at 10:47:19AM -0400, Simo Sorce wrote:
> > > > > > On Sat, 2016-04-09 at 00:23 +1000, Fraser Tweedale wrote:
> > > > > > > -        name = gssapi.Name('host@%s' % (self.client,),
> > > > > > > 
> > > > > > > -                           gssapi.NameType.hostbased_service)
> > > > > > 
> > > > > > If you remove this then on a serve that has nfs keys in the keytab you
> > > > > > may end up acquiring the wrong credentials.
> > > > > > You need to pass down what credentials you want to use to initialize the
> > > > > > cred store, we canot rely on ordering in the system keytab case.
> > > > > > 
> > > > > > Simo.
> > > > > > 
> > > > > Thanks Simo; updated patch attached.
> > > > 
> > > > Except the ACI the rest looks good to me.
> > > > For ACI please add a separate patch that follows the naming scheme for
> > > > subCA keys.
> > > > 
> > > The ACI here targets the Custodia server public keys, so the client
> > > can search and read them.  It should just read:
> > > 
> > > add:aci: (target = "ldap:///cn=*,cn=custodia,cn=ipa,cn=etc,$SUFFIX")
> > >     (targetattr = "ipaPublicKey || ipaKeyUsage || memberPrincipal")
> > >     (version 3.0; acl "Anyone can search Custodia public keys";
> > >         allow(read, search, compare) userdn = "ldap:///all";)
> > > 
> > > I don't mind putting the ACI in a separate patch, but it is
> > > necessary to restrict read access on the public keys to only the
> > > dogtag-ipa-custodia service principals.
> > > 
> > Updated patches attached.  ACI was split into new patch and
> > simplified (removed ($dn) macro).
> 
> Ack on the custodia patch.
> However do we really need to allow *anyone* to look up these keys ?
> I know they are "public" keys, but still  ... I think I would prefer a
> stricter ACI.
> 
OK, I rescind the ACI patch (0052) and will include a more specific
ACI in a new version of the patch that adds the principal.

0051 is good to merge now :)

Thanks for the review, Simo.
Fraser