[Freeipa-devel] [PATCHES] 241-253 CA certificate renewal

[Rob Crittenden]

Jan Cholasta wrote:
> Hi,
>
> the attached patches implement automatic CA certificate renewal as well
> as the initial version of the CA certificate management tool.
>
> Requires my patches 172-196.
>
> In order to test, you must install current git version of certmonger
> (see <https://fedorahosted.org/certmonger/ticket/26>) and set SELinux to
> permissive (see <https://bugzilla.redhat.com/show_bug.cgi?id=1078783>).
> Make sure you install certmonger before running
> ipa-server-install/ipa-replica-install. On F20 you can use RPMs located
> at <http://jcholast.fedorapeople.org/certmonger-git/>.
>
> To test automatic renewal, move system time forward (see
> <https://fedorahosted.org/freeipa/ticket/2803#comment:17> for more info
> about certificate renewal testing, nickname of the CA certificate is
> "caSigningCert cert-pki-ca"). In CA-full installs the renewal should be
> fully automatic, in CA-less installs you should be alerted via syslog to
> renew the certificate using ipa-cacert-manage.
>
> To test manual renewal, run "ipa-cacert-manage renew". You can run it on
> any CA master. To make the renewed certificate available on other CA
> masters, you must run "getcert resubmit -d /etc/pki/pki-tomcat/alias -n
> 'caSigningCert cert-pki-ca'" on each of them. Note that currently you
> can't change the chaining of the CA certificate.

241

Not to be too anal, but would it be too outlandish to compare the 
Authority Key Identifier (if there is one) with the Subject Key ID to 
see if the cert is self-signed? Same subject then yeah, it is probably 
self-signed. The keys match? Definitely.

242

I wonder if it would be clearer to use variables instead of a raw list 
in the return value for these handlers: (result, message) = handler(...) 
rather than examining result[0], etc. That may be beyond the scope of 
this patch though.

x509.normalize_certificate() can raise an exception, there should be a 
try/except around it.

For an invalid cookie, please include the values seen in the environment 
in the error message.

243

You are going to end up with a lot of acis with the same comment value. 
Perhaps add the host in there as well.

These are not removed when a master is deleted.

244

There are now several different places where the caCertificate value is 
updated. I wonder if it's time for a function. I found it it in 
dsinstance.py, upload_cacert and now renew_ca_cert.

246

caRenewalMaster should be checked when a replica is deleted and moved to 
another master. This is a good idea. I wonder if a ticket should be 
opened to do something similar for CRL generation.

247

We've been burned by hardcoded timeouts in the past. Should this be 
configurable? This module doesn't currently do any logging but it might 
be worth spitting out a "waiting" message, at least for debugging.

249

nss_init() is always scary to me since we can only have one. I didn't 
see anything blow up, just wondering if this should be wrapped with an 
is_initialized()->shutdown() at the top.

The find_cert_from_nickname() should be in a try/except in case the cert 
is not found for some reason.

251

The tool should provide some feedback while it's running. For the 
impatient (me) it takes a really long time and it's hard to know what is 
going on, something in between nothing and full debug output.

The man page needs some more work too. I think some more explanation is 
needed and an example would probably be really helpful as well. I think 
particularly an example for external certs and a description of what you 
mean by Self-signed CA (I assume you mean IPA-provided). I don't think 
it really matters how many steps there are unless you are going to 
provide progress output.

Got a backtrace when running as non-root:

$ ipa-cacert-manage -v renew
ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG:   File 
"/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 168, in 
execute
     self.validate_options()
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_cacert_manage.py", 
line 62, in validate_options
     super(CACertManage, self).validate_options(needs_root=True)
   File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 
189, in validate_options
     raise ScriptError('Must be root to run %s' % self.command_name, 1)

ipa.ipaserver.install.ipa_cacert_manage.CACertManage: DEBUG: The 
ipa-cacert-manage command failed, exception: ScriptError: Must be root 
to run ipa-cacert-manage
ipa.ipaserver.install.ipa_cacert_manage.CACertManage: ERROR: Must be 
root to run ipa-cacert-manage

252

In what context would this be executing? My guess is that certmonger is 
trying to do the renewal but a new cert hasn't been issued yet, so this 
gets sysloged?

Still doing functional testing.

rob