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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code