[Freeipa-devel] [PATCH] 0052..0054 Configure lightweight CA key replication

[Fraser Tweedale]

On Thu, Jun 09, 2016 at 07:49:51AM +0200, Jan Cholasta wrote:
> On 9.6.2016 06:07, Fraser Tweedale wrote:
> > Updated patches 0053-6 and 0054-6 attached.  Comments inline.
> > 
> > Thanks,
> > Fraser
> > 
> > On Wed, Jun 08, 2016 at 10:31:07AM +0200, Jan Cholasta wrote:
> > > Patch 0052:
> > > 
> > > The target of the "Dogtag service principals can search Custodia keys" ACI
> > > matches keys in the top-level Custodia container, but not in the Dogtag
> > > container. Is this intentional?
> > > 
> > ACI applies to all entries under the 'cn=custodia' tree, not only
> > the entries directly under it.
> 
> You are right:
> 
> <https://access.redhat.com/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Creating_ACIs_Manually.html#Defining_Targets-Targeting_a_Directory_Entry>
> 
> Mea culpa, I thought the target matching behavior was simpler than this.
> 
> Anyway, could the ACI be tightened to allow access only to the Dogtag
> subtree?
> 
Dogtag does need access to the host Custodia keys.  I'm not sure how
to tighten the ACI to prevent access to "deeper" levels.  In any
case they're all public keys and it's read only access, so I don't
think it's an issue.

> > 
> > > 
> > > Patch 0053:
> > > 
> > > It seems the `servicename`+`host` and `principal` arguments of set_key() are
> > > carrying the same information, could you remove one of them?
> > > 
> > OK, just `principal` now.
> > 
> > > 
> > > Patch 0054:
> > > 
> > > 1) Please use ipalib.config to read IPA configuration in
> > > ipa-pki-retrieve-key:
> > > 
> > >     from ipalib.config import Env
> > > 
> > >     env = Env(in_server=True)
> > >     hostname = env.host
> > >     realm = env.realm
> > > 
> > I have implemented this; it was necessary to also call
> > `env._finalize()`.
> 
> Right. I wrote the above snippet off the top of my head, hence the omission.
> 
> > 
> > > 2) I'm curious why you changed the key name from "ca/$NAME" to
> > > "ca_wrapped/$NAME". Aren't *all* keys in Custodia wrapped?
> > > 
> > The payload of the `ca_wrapped` Custodia store is a
> > PKIArchiveOptions object wrapped by the main CA's private key.
> > (This payload is then encrypted by Custodia, as all Custodia
> > payloads are).
> > 
> > See my patch 0056 [1] for implementation details.
> > 
> > [1] https://www.redhat.com/archives/freeipa-devel/2016-May/msg00079.html
> 
> Wow, I totally missed patch 0055 and 0056!
> 
No worries; easily done because I sent more emails with fewer,
largely independent patches.

> > 
> > > 3) Given that Dogtag ExternalProcessKeyRetriever handles JSON *now*, I would
> > > expect a minimum required version bump in the spec file.
> > > 
> > Indeed; I added this in latest patches.
> 
> Thanks. ACK if the ACI in patch 52 does not need tightening.
> 
Thanks for reviewing!