Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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 From
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
Dne 26.9.2014 v 19:54 Jan Cholasta napsal(a): 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... When adding the CA certificates to the temporary database it will report that a failure occurred, but not the exception: +except CalledProcessError, e: +root_logger.info(Failed to add CA to temporary NSS database.) +return CLIENT_INSTALL_ERROR Granted, NSS errors are often obtuse, but should it at least DEBUG log it? It is already logged in ipautil.run. The exception only says that the command failed, there's no point in logging it. 324, 325, 326: ACK 327: So the idea is to just mirror the certs and us the new db to know what was added? Exactly. What if someone has the same nickname, different cert in the two databases? Is that too much of a corner case? IMO it is too much of a corner case, but I plan on adding a better diff/patch algorithm in the future if it turns out to be a problem. 328, 329, 330, 331: ACK As an aside, do you know why the global certs are seen by mod_nss? libnssckbi.so is symlinked into the directory (certutil -L -d /etc/httpd/alias -h all will show all the certs). I'm not sure why it is this way, but the module is linked to the NSS database: $ sudo modutil -list -dbdir /etc/httpd/alias Listing of PKCS #11 Modules --- 1. NSS Internal PKCS #11 Module slots: 2 slots attached status: loaded slot: NSS Internal Cryptographic Services token: NSS Generic Crypto Services slot: NSS User Private Key and Certificate Services token: NSS Certificate DB 2. Root Certs library name: /etc/httpd/alias/libnssckbi.so slots: 2 slots attached status: loaded slot: /etc/pki/ca-trust/source token: System Trust slot: /usr/share/pki/ca-trust-source token: Default Trust --- 332-335 I think the naming and/or comments can be improved. For example, there are now 3 *retrieve_cert commands, all of which do slightly different things. All have external handlers (via the oddly named profile), but some are called internally as well. I have spent quite some time trying to come up with good names for them, but I was not able to do so. Suggestions are welcome. This is rather complex code: a command passes a call onto certmonger which then exuecutes the IPA CA helper... More documentation would definitely help a newcomer figure out how renewal, CA retrieval, etc. works. OK, I'll try to add some. Not to be too pedantic but there is a lot of this in dogtag-ipa-ca-renew-agent-submit: if variable: OR if not variable: Where variable defaults to None. Shouldn't the test be: if variable is [not] None: This doesn't catch empty strings, which may occur in some of these checks. 340: ACK Through some combination of ipa-certupdate and ipa-cacert-manage I seem to have hosed things up: ipa: DEBUG: certmonger request is in state dbus.String(u'CA_UNREACHABLE', variant_level=1) ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG: File /usr/lib/python2.7/site-packages/ipapython/admintool.py, line 171, in execute return_value = self.run() File /usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py, line
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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? When adding the CA certificates to the temporary database it will report that a failure occurred, but not the exception: +except CalledProcessError, e: +root_logger.info(Failed to add CA to temporary NSS database.) +return CLIENT_INSTALL_ERROR Granted, NSS errors are often obtuse, but should it at least DEBUG log it? It is already logged in ipautil.run. The exception only says that the command failed, there's no point in logging it. 324, 325, 326: ACK 327: So the idea is to just mirror the certs and us the new db to know what was added? Exactly. What if someone has the same nickname, different cert in the two databases? Is that too much of a corner case? IMO it is too much of a corner case, but I plan on adding a better diff/patch algorithm in the future if it turns out to be a problem. The result could be a deleted cert, that was my concern. It does seem to be a rather slim case. 328, 329, 330, 331: ACK As an aside, do you know why the global certs are seen by mod_nss? libnssckbi.so is symlinked into the directory (certutil -L -d /etc/httpd/alias -h all will show all the certs). I'm not sure why it is this way, but the module is linked to the NSS database: $ sudo modutil -list -dbdir /etc/httpd/alias Listing of PKCS #11 Modules --- 1. NSS Internal PKCS #11 Module slots: 2 slots attached status: loaded slot: NSS Internal Cryptographic Services token: NSS Generic Crypto Services slot: NSS User Private Key and Certificate Services token: NSS Certificate DB 2. Root Certs library name: /etc/httpd/alias/libnssckbi.so slots: 2 slots attached status: loaded slot: /etc/pki/ca-trust/source token: System Trust slot: /usr/share/pki/ca-trust-source token: Default Trust --- Sorry, that was a rhetorical question. Presumably NSS hunts around for libnssckbi.so but it doesn't seem to look very hard, except that it does seem to find it if it exists in the same directory as the DB, so I symlink it in. I think mod_nss is probably the only thing that does this, but it can make things rather interesting (for good and bad) dealing with global certs. 332-335 I think the naming and/or comments can be improved. For example, there are now 3 *retrieve_cert commands, all of which do slightly different things. All have external handlers (via the oddly named profile), but some are called internally as well. I have spent quite some time trying to come up with good names for them, but I was not able to do so. Suggestions are welcome. This is rather complex code: a command passes a call onto certmonger which then exuecutes the IPA CA helper... More documentation would definitely help a newcomer figure out how renewal, CA retrieval, etc. works. OK, I'll try to add some. Not to be too pedantic but there is a lot of this in dogtag-ipa-ca-renew-agent-submit: if variable: OR if not variable:
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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. When adding the CA certificates to the temporary database it will report that a failure occurred, but not the exception: +except CalledProcessError, e: +root_logger.info(Failed to add CA to temporary NSS database.) +return CLIENT_INSTALL_ERROR Granted, NSS errors are often obtuse, but should it at least DEBUG log it? It is already logged in ipautil.run. The exception only says that the command failed, there's no point in logging it. 324, 325, 326: ACK 327: So the idea is to just mirror the certs and us the new db to know what was added? Exactly. What if someone has the same nickname, different cert in the two databases? Is that too much of a corner case? IMO it is too much of a corner case, but I plan on adding a better diff/patch algorithm in the future if it turns out to be a problem. The result could be a deleted cert, that was my concern. It does seem to be a rather slim case. 328, 329, 330, 331: ACK As an aside, do you know why the global certs are seen by mod_nss? libnssckbi.so is symlinked into the directory (certutil -L -d /etc/httpd/alias -h all will show all the certs). I'm not sure why it is this way, but the module is linked to the NSS database: $ sudo modutil -list -dbdir /etc/httpd/alias Listing of PKCS #11 Modules --- 1. NSS Internal PKCS #11 Module slots: 2 slots attached status: loaded slot: NSS Internal Cryptographic Services token: NSS Generic Crypto Services slot: NSS User Private Key and Certificate Services token: NSS Certificate DB 2. Root Certs library name: /etc/httpd/alias/libnssckbi.so slots: 2 slots attached status: loaded slot: /etc/pki/ca-trust/source token: System Trust slot: /usr/share/pki/ca-trust-source token: Default Trust --- Sorry, that was a rhetorical question. Presumably NSS hunts around for libnssckbi.so but it doesn't seem to look very hard, except that it does seem to find it if it exists in the same directory as the DB, so I symlink it in. I think mod_nss is probably the only thing that does this, but it can make things rather interesting (for good and bad) dealing with global certs. 332-335 I think the naming and/or comments can be improved. For example, there are now 3 *retrieve_cert commands, all of which do slightly different things. All have external handlers (via the oddly named profile), but some are called internally as well. I have spent quite some time trying to come up with good names for them, but I was not able to do so. Suggestions are welcome. This is rather
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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? 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. 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. So ACK for 319, 324-331, 340. 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. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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 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. When adding the CA certificates to the temporary database it will report that a failure occurred, but not the exception: +except CalledProcessError, e: +root_logger.info(Failed to add CA to temporary NSS database.) +return CLIENT_INSTALL_ERROR Granted, NSS errors are often obtuse, but should it at least DEBUG log it? 324, 325, 326: ACK 327: So the idea is to just mirror the certs and us the new db to know what was added? What if someone has the same nickname, different cert in the two databases? Is that too much of a corner case? 328, 329, 330, 331: ACK As an aside, do you know why the global certs are seen by mod_nss? libnssckbi.so is symlinked into the directory (certutil -L -d /etc/httpd/alias -h all will show all the certs). 332-335 I think the naming and/or comments can be improved. For example, there are now 3 *retrieve_cert commands, all of which do slightly different things. All have external handlers (via the oddly named profile), but some are called internally as well. This is rather complex code: a command passes a call onto certmonger which then exuecutes the IPA CA helper... More documentation would definitely help a newcomer figure out how renewal, CA retrieval, etc. works. Not to be too pedantic but there is a lot of this in dogtag-ipa-ca-renew-agent-submit: if variable: OR if not variable: Where variable defaults to None. Shouldn't the test be: if variable is [not] None: 340: ACK Through some combination of ipa-certupdate and ipa-cacert-manage I seem to have hosed things up: ipa: DEBUG: certmonger request is in state dbus.String(u'CA_UNREACHABLE', variant_level=1) ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG: File /usr/lib/python2.7/site-packages/ipapython/admintool.py, line 171, in execute return_value = self.run() File /usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py, line 118, in run rc = self.renew() File /usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py, line 181, in renew return self.renew_self_signed(ca) File /usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py, line 193, in renew_self_signed self.resubmit_request(ca, 'caCACert') File /usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py, line 315, in resubmit_request please check the request manually % self.request_id) ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG: The ipa-cacert-manage command failed, exception: ScriptError: Error resubmitting certmonger request '20140909142830', please check the request manually ipa.ipaserver.install.ipa_cacert_manage.CACertManage: ERROR: Error resubmitting certmonger request '20140909142830', please check the request manually Incidentally, IMHO it should include the command to execute to check the request manually. The CA is actually unreachable. Restarting it fixed things. I'll chalk this up to cosmic rays. Re-running ipa-cacert-manage renew was successful. I confirmed that the CA signing cert was updated in the dogtag database. I then ran ipa-certupdate to distribute this new CA cert around and none of the databases got the updated CA cert, nor did /etc/ipa/ca.crt. Still doing some functional testing. Unrelated to this: 9:freeipa-python-4.0.0GITf6875d4-0.# [100%] Update failed: Operations error: Seems to be related to this: 2014-09-25T19:28:22Z DEBUG - 2014-09-25T19:28:22Z DEBUG Final value after applying
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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... When adding the CA certificates to the temporary database it will report that a failure occurred, but not the exception: +except CalledProcessError, e: +root_logger.info(Failed to add CA to temporary NSS database.) +return CLIENT_INSTALL_ERROR Granted, NSS errors are often obtuse, but should it at least DEBUG log it? It is already logged in ipautil.run. The exception only says that the command failed, there's no point in logging it. 324, 325, 326: ACK 327: So the idea is to just mirror the certs and us the new db to know what was added? Exactly. What if someone has the same nickname, different cert in the two databases? Is that too much of a corner case? IMO it is too much of a corner case, but I plan on adding a better diff/patch algorithm in the future if it turns out to be a problem. 328, 329, 330, 331: ACK As an aside, do you know why the global certs are seen by mod_nss? libnssckbi.so is symlinked into the directory (certutil -L -d /etc/httpd/alias -h all will show all the certs). I'm not sure why it is this way, but the module is linked to the NSS database: $ sudo modutil -list -dbdir /etc/httpd/alias Listing of PKCS #11 Modules --- 1. NSS Internal PKCS #11 Module slots: 2 slots attached status: loaded slot: NSS Internal Cryptographic Services token: NSS Generic Crypto Services slot: NSS User Private Key and Certificate Services token: NSS Certificate DB 2. Root Certs library name: /etc/httpd/alias/libnssckbi.so slots: 2 slots attached status: loaded slot: /etc/pki/ca-trust/source token: System Trust slot: /usr/share/pki/ca-trust-source token: Default Trust --- 332-335 I think the naming and/or comments can be improved. For example, there are now 3 *retrieve_cert commands, all of which do slightly different things. All have external handlers (via the oddly named profile), but some are called internally as well. I have spent quite some time trying to come up with good names for them, but I was not able to do so. Suggestions are welcome. This is rather complex code: a command passes a call onto certmonger which then exuecutes the IPA CA helper... More documentation would definitely help a newcomer figure out how renewal, CA retrieval, etc. works. OK, I'll try to add some. Not to be too pedantic but there is a lot of this in dogtag-ipa-ca-renew-agent-submit: if variable: OR if not variable: Where variable defaults to None. Shouldn't the test be: if variable is [not] None: This doesn't catch empty strings, which may occur in some of these checks. 340: ACK Through some combination of ipa-certupdate and ipa-cacert-manage I seem to have hosed things up: ipa: DEBUG: certmonger request is in state dbus.String(u'CA_UNREACHABLE', variant_level=1) ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG: File /usr/lib/python2.7/site-packages/ipapython/admintool.py, line 171, in execute return_value = self.run() File /usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py, line
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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.
As a side note, it was rather difficult to review parts of this as
different patches modify exactly the same code in different ways.
General:
These patches don't replace /etc/pki/nssdb, they just add a new
location. Do we need to keep using the shared db because of p11-kit?
It's for backward compatibility.
319:
The post-install script tries to get a cert with nickname 'IPA CA'. It
should also look for '$REALM IPA CA'. I'd use
ipapythhon.certdb.py:CA_NICKNAME_FMT. Having a similar option for
External CA is probably not a bad idea.
I included 'IPA CA' and 'External CA cert', because they are the only
nicknames used for the CA certificate in /etc/pki/nssdb before 4.1 (I
checked in git). I did not include '$REALM IPA CA', as it is used only
in unreleased code.
You don't need the password file to import a cert into an NSS database.
Fixed.
It may be too complex in a spec file but would it be better to try to
fetch things out of LDAP? This nearly screams for a separate script to
do the work.
Sorry, you have to run ipa-certupdate manually for now to update the
database. Automatic updates from LDAP will be included in 4.2.
We definitely need a client update script, for other stuff as well.
Are these operations important enough to be logged to
/var/log/ipaupgrade.log? I suspect it's the kind of thing that might
fail and never be noticed.
It couldn't hurt.
At some point we'll upgrade to sql NSS databases. Is this going to
complicate things?
I don't think so. When do we want to do the upgrade?
There are some places you do str(e[2]) that aren't necessary.
Fixed.
Might be worthwhile to clean up the comments regarding XML-RPC while
you're at it and just refer to RPC.
Done.
I'd rename ca_certs_nss to ca_certs_trust for clarity.
OK.
Is the separate helper create_ipa_nssdb() necessary? Should create_db()
be extended to do this extra work?
It is easier to call from the spec file this way.
324:
ACK
325:
The exception from get_cert() is very rough so wouldn't cover the case
of file permissions, missing database, etc, but since the overall
behavior isn't changing, ACK. Might be worth a comment though indicating
a potential source of oddness for the uninitiated.
Comment added.
326:
ACK
327:
@@ -133,6 +116,7 @@ class CertUpdate(admintool.AdminTool):
criteria = {
'cert-database': dogtag_constants.ALIAS_DIR,
'cert-nickname': nickname,
+'ca-name': 'dogtag-ipa-ca-renew-agent',
}
request_id = certmonger.get_request_id(criteria)
if request_id is not None:
Doesn't seem related to this change.
Moved to patch 340, along with similar change in ipa-cacert-manage in
patch 335.
328:
ACK
329:
ACK
330:
I don't understand the reference to ipa.p11-kit (an unowned file, btw).
Added the file to the spec file.
Are you removing any cert that may be left over from some previous install?
Yes. I believe it might what was causing the NSS failures you saw in
http://www.redhat.com/archives/freeipa-devel/2014-July/msg00112.html.
If the file were to exist it could cause update-ca-trust to be executed
twice. Is that needed?
Where?
I think more clarity in the commit message will clear things up.
OK.
331:
ACK
Updated the patch with a simpler fix and a ticket number.
332:
What is the reason for this refactoring?
It makes it easier to make changes such as the one in patch 333. Also it
makes dogtag-ipa-ca-renew-agent more pythonic and easy to read IMHO.
333:
Seems reasonable but why would a CA profile change in the middle of a
request?
334:
Same boat, why?
335:
I kinda get the picture, along with previous patches, but need more info.
Updated commit messages of the above 3 patches with more info.
Some tips on testing would be helpful.
Testing is basically the same as for the original CA management patches
- do some server installs, upgrades and uninstalls and run
ipa-certupdate and ipa-cacert-manage with different options.
rob
Updated patches attached.
--
Jan Cholasta
From df518f7f971c2bcdbbdab99244a8185f721945fc Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 18
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
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.
As a side note, it was rather difficult to review parts of this as
different patches modify exactly the same code in different ways.
General:
These patches don't replace /etc/pki/nssdb, they just add a new
location. Do we need to keep using the shared db because of p11-kit?
319:
The post-install script tries to get a cert with nickname 'IPA CA'. It
should also look for '$REALM IPA CA'. I'd use
ipapythhon.certdb.py:CA_NICKNAME_FMT. Having a similar option for
External CA is probably not a bad idea.
You don't need the password file to import a cert into an NSS database.
It may be too complex in a spec file but would it be better to try to
fetch things out of LDAP? This nearly screams for a separate script to
do the work.
Are these operations important enough to be logged to
/var/log/ipaupgrade.log? I suspect it's the kind of thing that might
fail and never be noticed.
At some point we'll upgrade to sql NSS databases. Is this going to
complicate things?
There are some places you do str(e[2]) that aren't necessary.
Might be worthwhile to clean up the comments regarding XML-RPC while
you're at it and just refer to RPC.
I'd rename ca_certs_nss to ca_certs_trust for clarity.
Is the separate helper create_ipa_nssdb() necessary? Should create_db()
be extended to do this extra work?
324:
ACK
325:
The exception from get_cert() is very rough so wouldn't cover the case
of file permissions, missing database, etc, but since the overall
behavior isn't changing, ACK. Might be worth a comment though indicating
a potential source of oddness for the uninitiated.
326:
ACK
327:
@@ -133,6 +116,7 @@ class CertUpdate(admintool.AdminTool):
criteria = {
'cert-database': dogtag_constants.ALIAS_DIR,
'cert-nickname': nickname,
+'ca-name': 'dogtag-ipa-ca-renew-agent',
}
request_id = certmonger.get_request_id(criteria)
if request_id is not None:
Doesn't seem related to this change.
328:
ACK
329:
ACK
330:
I don't understand the reference to ipa.p11-kit (an unowned file, btw).
Are you removing any cert that may be left over from some previous install?
If the file were to exist it could cause update-ca-trust to be executed
twice. Is that needed?
I think more clarity in the commit message will clear things up.
331:
ACK
332:
What is the reason for this refactoring?
333:
Seems reasonable but why would a CA profile change in the middle of a
request?
334:
Same boat, why?
335:
I kinda get the picture, along with previous patches, but need more info.
Some tips on testing would be helpful.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
