Dne 30.9.2014 v 10:09 Martin Kosek napsal(a):
On 09/30/2014 09:59 AM, Jan Cholasta wrote:
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.


Thanks to both for this work! I pushed all patches that Rob reviewer to master
and ipa-4-1.

I also checked Jan's fix up patch 342 and it looks good to me as well.

ACK for that and pushed to master.

Martin


I did a lousy job at rebasing the patches, sorry. The attached patch fixes it.

--
Jan Cholasta
>From 36b3001fb704e9018159e6678d97afd3b50a8db9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 30 Sep 2014 10:13:08 +0200
Subject: [PATCH] Add missing imports to ipapython.certdb

https://fedorahosted.org/freeipa/ticket/4416
---
 ipapython/certdb.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipapython/certdb.py b/ipapython/certdb.py
index 09c87c7..4645b40 100644
--- a/ipapython/certdb.py
+++ b/ipapython/certdb.py
@@ -21,12 +21,14 @@ import os
 import re
 import tempfile
 import shutil
+import base64
 from nss import nss
 from nss.error import NSPRError
 
 from ipaplatform.paths import paths
 from ipapython.ipa_log_manager import root_logger
 from ipapython import ipautil
+from ipalib import x509
 
 CA_NICKNAME_FMT = "%s IPA CA"
 
-- 
1.9.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to