[Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes

Tue Sep 30 07:59:19 UTC 2014

Dne 29.9.2014 v 18:09 Rob Crittenden napsal(a):
> Jan Cholasta wrote:
>> Dne 29.9.2014 v 15:05 Rob Crittenden napsal(a):
>>> Jan Cholasta wrote:
>>>> Dne 26.9.2014 v 17:13 Rob Crittenden napsal(a):
>>>>> Jan Cholasta wrote:
>>>>>> Dne 23.9.2014 v 20:39 Rob Crittenden napsal(a):
>>>>>>> Jan Cholasta wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> the attached patches fix various bugs and shortcomings in the CA
>>>>>>>> management and renewal code. Related tickets:
>>>>>>>> <https://fedorahosted.org/freeipa/ticket/4416>,
>>>>>>>> <https://fedorahosted.org/freeipa/ticket/4460>.
>>>>>>>>
>>>>>>>> (Patch 319 was originally posted at
>>>>>>>> <http://www.redhat.com/archives/freeipa-devel/2014-September/msg00132.html>.)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> Only two of the patches includes what ticket(s) they address. Most
>>>>>>> have
>>>>>>> the tersest of commit messages. If got more and more difficult to see
>>>>>>> why the changes were needed at all, as you'll see.
>>>>>>
>>>>>> Sorry, fixed (hopefully).
>>>>>>
>>>>>> Note that most of these patches fix stuff I didn't have time to fix
>>>>>> before I posted the original CA management patches, hence the missing
>>>>>> tickets.
>>>>>
>>>>> Well, the policy is that every commit should have a ticket. So I guess
>>>>> re-open the old tickets or open new ones. This will help someone in the
>>>>> future know the "why" of a patch. I've certainly been guilty
>>>>
>>>> OK, I will reopen the related tickets.
>>>>
>>>>>
>>>>> Here is a new set of reviews as trying to intermingle was making my
>>>>> eyes
>>>>> cross:
>>>>>
>>>>> 319:
>>>>>
>>>>> I guess I still don't understand why you can't pull the certs out of
>>>>> LDAP when creating this database. When this code runs, we know that the
>>>>> client is configured, so we have access to authentication. Why can't
>>>>> create_ipa_nssdb pull the certs directly? It seems more robust to me,
>>>>> and the code is already written in ipa-client-install to do this.
>>>>
>>>> Well, I don't understand why do you want them to be updated so much, as
>>>> nothing will break if they are not. Also try to imagine what would
>>>> happen if, say, 10k clients were updated at the same moment...
>>>
>>> What's the point of a database missing certificates?
>>
>> It won't be missing any certificates if /etc/pki/nssdb was not missing
>> any certificates before the update.
>>
>> As I said, the update will not break anything. It will not fix anything
>> either, but I don't think this kind of fixing should be done during
>> client RPM upgrade. It is not consistent with anything else we do during
>> client RPM upgrade, it does not scale well and it just does not feel
>> right to me in overall.
>
> Ok, I'll concede the point that there is no difference post-update, but
> it doesn't do what ipa-certupdate does. You can potentially end up with
> a completely diffferent set of certificates. So why the difference?

If you end up with a *completely* different set of certificates, it's 
because the admins screwed up, so it's theirs responsibility to fix it, 
not ours IMHO.

>
> Post install of new client bits:
>
> # certutil -L -d /etc/ipa/nssdb/
>
> IPA CA                                                       CT,C,C
>
> After running ipa-certupdate:
>
> # certutil -L -d /etc/ipa/nssdb/
>
> CN=Primary CA,O=example.com,C=US                             ,,
> CN=Secondary CA,O=example.com,C=US                           ,,
> GREYOAK.COM IPA CA                                           CT,C,C
>
> Quite the difference.

Not much of a difference when you realize that "IPA CA" and "GREYOAK.COM 
IPA CA" are the same certificate and the rest are there just for 
completeness.

>
> I'll ACK for now since this doesn't materially change anything and
> shouldn't break any installs but it begs the question of why it is
> acceptable now but not acceptable to make ipa-certupdate do the same.

I have changed the NSS database used by the RPC code from /etc/pki/nssdb 
to /etc/ipa/nssdb. What the RPM update does is a necessary configuration 
update to keep RPC working. ipa-certupdate on the other hand fetches new 
policy from the server, which is quite a different thing IMO.

>
> So ACK for 319, 324-331, 340.

Thanks!

>
>>
>> The LDAP update happens in renew_ca_cert. Are there any relevant errors
>> in /var/log/messages? Is there caRenewalMaster in ipaConfigString of the
>> master entry of the master where you run ipa-cacert-manage renew?
>
> Sep 25 16:06:44 grindle renew_ca_cert: Traceback (most recent call last):
>    File "/usr/lib64/ipa/certmonger/renew_ca_cert", line 214, in <module>
>      main()
>    File "/usr/lib64/ipa/certmonger/renew_ca_cert", line 79, in main
>      ca = cainstance.CAInstance(host_name=api.env.host, ldapi=False)
>    File
> "/usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py", line
> 357, in __init__
>      self.ra_agent_pwd = self.ra_agent_db + "/pwdfile.txt"
> TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'
> Sep 25 16:06:44 grindle certmonger: Certificate named "caSigningCert
> cert-pki-ca" in token "NSS Certificate DB" in database
> "/etc/pki/pki-tomcat/alias" issued by CA and saved.

One of the KRA patches broke this. I assumed you'd be testing on top of 
ipa-4-1, so I didn't fix it, sorry. See the attached patch 342 for a 
fix. Martin, can you please review it?

>
> rob
>

The patches needed a rebase, attached.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-319.4-Introduce-NSS-database-etc-ipa-nssdb.patch
Type: text/x-patch
Size: 16042 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-324.3-Move-NSSDatabase-from-ipaserver.certs-to-ipapython.c.patch
Type: text/x-patch
Size: 37116 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-325.3-Add-NSSDatabase.has_nickname-for-checking-nickname-p.patch
Type: text/x-patch
Size: 1075 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-326.3-Use-NSSDatabase-instead-of-direct-certutil-calls-in-.patch
Type: text/x-patch
Size: 9212 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-327.3-Use-etc-ipa-nssdb-to-get-nicknames-of-IPA-certs-inst.patch
Type: text/x-patch
Size: 8895 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-328.3-Check-if-IPA-client-is-configured-in-ipa-certupdate.patch
Type: text/x-patch
Size: 1081 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-329.3-Get-server-hostname-from-jsonrpc_uri-in-ipa-certupda.patch
Type: text/x-patch
Size: 1209 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-330.3-Remove-ipa-ca.crt-from-systemwide-CA-store-on-client.patch
Type: text/x-patch
Size: 4461 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-331.3-Fix-certmonger.wait_for_request.patch
Type: text/x-patch
Size: 960 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-340.2-Fix-certmonger-search-for-the-CA-cert-in-ipa-certupd.patch
Type: text/x-patch
Size: 1897 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-342-Do-not-crash-in-CAInstance.__init__-when-default-arg.patch
Type: text/x-patch
Size: 1293 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/101b467f/attachment-0010.bin>