On 09/30/2014 10:19 AM, Jan Cholasta wrote: > 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.
Ah, thanks, it fixes the issue, lint passes (/me wishes he did lint before pushing). ACK. Pushed to master, ipa-4-1. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel