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

[Jan Cholasta]

On 9.6.2016 08:31, Fraser Tweedale wrote:
> 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 see, the ACI is OK then.

> 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.

Right.

>
>>>
>>>>
>>>> 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!

Pushed to master: b0d9a4728f0dc78e2bbde344beac17ae50b847a9

-- 
Jan Cholasta