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.