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

[Jan Cholasta]

On 6.6.2016 15:32, Fraser Tweedale wrote:
> On Wed, Jun 01, 2016 at 02:49:29PM +1000, Fraser Tweedale wrote:
>> Updated patches attached; comments inline.
>>
>> On Thu, May 05, 2016 at 04:52:29PM +1000, Fraser Tweedale wrote:
>>>> I would rather add a new ACI than have one super-ACI for everything. That
>>>> way you don't have to invent any complicated naming schemes *and* it will be
>>>> more apparent what the ACI does.
>>>>
>>> OK, I'll simplify the scheme and create corresponding ACIs.
>>>
>> I added new ACIs for hosts to manage Dogtag keys; they keys live in
>> a container with RDN cn=dogtag, nested under the main custodia keys
>> container.
>>
>>>>>> However, calling `CAInstance.setup_lightweight_ca_key_retrieval()'
>>>>>> *directly* from `ca.install_step_1' would probably work.  Are you
>>>>>> happy with putting it there, instead of `configure_instance()'?
>>>>
>>>> Works for me.
>>>>
>>> Cool, thanks.
>>>
>> This is implemented in the latest patch.
>>
> Rebased and updated patches attached.  The only substantive change
> is a simplification of the ipa-pki-retrieve-key script (patch 0054)
> following a change in Dogtag's ExternalProcessKeyRetriever (it now
> handles JSON).

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?


Patch 0053:

It seems the `servicename`+`host` and `principal` arguments of set_key() 
are carrying the same information, could you remove one of them?


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

2) I'm curious why you changed the key name from "ca/$NAME" to 
"ca_wrapped/$NAME". Aren't *all* keys in Custodia wrapped?

3) Given that Dogtag ExternalProcessKeyRetriever handles JSON *now*, I 
would expect a minimum required version bump in the spec file.

-- 
Jan Cholasta