[Freeipa-devel] [PATCH 111] ipa-client-install: Publish CA certificate to systemwide store

[Tomas Babej]

On 11/15/2013 03:36 PM, Rob Crittenden wrote:
> Tomas Babej wrote:
>> On 11/15/2013 02:46 PM, Ana Krivokapic wrote:
>>> On 11/13/2013 02:57 PM, Tomas Babej wrote:
>>>> On 09/27/2013 10:14 AM, Martin Kosek wrote:
>>>>> On 09/26/2013 04:46 PM, Jan Cholasta wrote:
>>>>>> On 26.9.2013 12:59, Tomas Babej wrote:
>>>>>>> On 09/26/2013 12:54 PM, Jan Cholasta wrote:
>>>>>>>> On 24.9.2013 18:14, Nalin Dahyabhai wrote:
>>>>>>>>> On Tue, Sep 24, 2013 at 01:30:10PM +0200, Jan Cholasta wrote:
>>>>>>>>>> We discussed this with Tomáš off-line and it turns out that
>>>>>>>>>> ipa-client-install fails if the CA cert is not added to
>>>>>>>>>> /etc/pki/nssdb.
>>>>>>>>>>
>>>>>>>>>> However, according to p11-kit docs it should work:
>>>>>>>>>> <http://p11-glue.freedesktop.org/doc/p11-kit/trust-nss.html>. I
>>>>>>>>>> wonder what needs to be done to make it work in IPA...
>>>>>>>>>
>>>>>>>>> On my system, there's no symlink to libnssckbi.so (or the right
>>>>>>>>> location
>>>>>>>>> in the link farm under /etc/alternatives) in /etc/pki/nssdb, so
>>>>>>>>> that
>>>>>>>>> database isn't going to automatically pull in the list of
>>>>>>>>> trusted CAs
>>>>>>>>> that p11-kit maintains.
>>>>>>>>>
>>>>>>>>> Whether the database under /etc/pki/nssdb should automatically
>>>>>>>>> include
>>>>>>>>> the usual set of trust anchors is probably a different
>>>>>>>>> conversation.
>>>>>>>>
>>>>>>>> Thanks for the info.
>>>>>>>>
>>>>>>>> Tomáš, the patch is fine then. I have one more nitpick though:
>>>>>>>> why did
>>>>>>>> you change "the default NSS database" to "the NSS database"? The
>>>>>>>> database in /etc/pki/nssdb *is* the default NSS database, so 
>>>>>>>> please
>>>>>>>> change it back. Also I think "systemwide CA trust database" is
>>>>>>>> better
>>>>>>>> than "systemwide CA store".
>>>>>>>>
>>>>>>>> Honza
>>>>>>>>
>>>>>>> I fixed the descriptions. Updated patch attached.
>>>>>>>
>>>>>>> Tomas
>>>>>>>
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> There's one more thing: we should probably check if
>>>>>> /usr/bin/update-ca-trust
>>>>>> exists before using it, for the sake of cross-distro compatibility.
>>>>>>
>>>>>
>>>>> Right. I am also thinking if this functionality should not be
>>>>> somehow integrated into the platform files so that it can be
>>>>> overriden in platforms that do not have the systemwide storage.
>>>>>
>>>>> Martin
>>>>
>>>> Updated patch attached, requires my patch 130.
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>
>>> The patch works fine; a couple of nitpicks:
>>>
>>> 1) The import of root_logger in services.py.in is unused.
>>>
>>> 2) In ipa-client-install, you log the return values of functions
>>> insert_ca_cert_into_systemwide_ca_store() and
>>> remove_ca_cert_from_systemwide_ca_store(). But these functions do not
>>> return any values, so you will always be logging `None`.
>>>
>> Thanks for the review,
>>
>> I removed the code (it was meant for debugging purposes only).
>>
>> Updated patch attached.
>
> Adding the CA to the NSS cert database is considered a fatal error. 
> Should adding it to the global trust database be fatal as well?
>
> I don't know the answer, but if we want to do this at some point 
> should these functions return True/False to denote success/failure?
>
> rob

I don't think it should be considered fatal, at least not now.

I updated the patch to return the success/failure status, even though, 
this could be done when it will be required. But doesn't hurt anything 
either, at least other platform files will develop systemwide CA store 
functions with this approach in mind.

Updated patch attached.

-- 
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0111-5-ipa-client-install-Publish-CA-certificate-to-systemw.patch
Type: text/x-patch
Size: 7353 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131118/f51e21b1/attachment.bin>