[Freeipa-devel] [PATCH] 315 Convert external CA chain to PKCS#7 before passing it to pkispawn

[Martin Kosek]

On 08/13/2014 03:12 PM, Petr Viktorin wrote:
> On 08/08/2014 11:50 AM, Jan Cholasta wrote:
>> Dne 8.8.2014 v 11:20 Martin Kosek napsal(a):
>>> On 08/08/2014 10:55 AM, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> the attached patch fixes <https://fedorahosted.org/freeipa/ticket/4397>.
>>>>
>>>> Honza
>>>
>>> Thanks! I did not test, just have couple questions/suggestions:
>>>
>>> 1) Are we testing that the certificate is in proper format, e.g. is
>>> not PKCS7
>>> already? We need to error out properly then
>>
>> Yes, in ipa-server-install.
>>
>>>
>>> 2) Are ipa-server-install --help options as informative as possible?
>>> --external-ca installation is tricky, we need to make sure that is no
>>> doubt
>>> about what the input is.
>>
>> I amended them a little bit.
>>
>>>
>>> 3) We may want to add instructions how to convert PKCS#7 -> PEM to "man
>>> ipa-server-install" too.
>>
>> Added.
>>
>>>
>>> Martin
>>>
>>
>> Updated patch attached.
>>
> 
> Hello,
> This works for me, but I'm not sure if I'm correctly reproducing the specific
> scenario this patch fixes. So as always, can you please add tests for code you
> write?

+1!

> As far as other scenarios, it seems to me that when I do something wrong I get
> a very unhelpful error message late in the installation.
> 
> I tried signing the request using xca but pkispawn choked on the result; I'll
> try to write a reproducer script using command-line tools.
> 
> Attached is a script (based on the external ca integration test) that
> reproduces the same IndexError as mentioned in the ticket. (If necessary,
> adjust the IP addresses, hostnames, etc. to fit your environment.)
> The difference from a working script is that extensions aren't added to the IPA
> cert when it's signed.

This is a very good finding. If Jan's patch fixes the reported problem, let us
push it.

But the missing validation should be fixed too. Can you please extend
https://fedorahosted.org/freeipa/ticket/4480
that is (will be) planned for 4.1 and attach your script as well so that we can
improve the usability by both accepting more certificate types and validation?

Martin