[Pki-devel] [PATCH] 297, 298 add validity check for external CA

[Endi Sukma Dewata]

On 4/22/2016 2:37 PM, Ade Lee wrote:
> commit 0fe7bf5ff989bbc24875dce30cec8f32e89c0a8f
> Author: Ade Lee <alee at redhat.com>
> Date:   Fri Apr 22 15:31:43 2016 -0400
>
>      Add validity check for the signing certificate in pkispawn
>
>      When either an existing CA or external CA installation is
>      performed, use the pki-server cert validation tool to check
>      the signing certiticate and chain.
>
>      Ticket #2043
>
> commit 9104fdda145c4f2bbbedec7256c73922e8bffcef
> Author: Ade Lee <alee at redhat.com>
> Date:   Wed Apr 20 17:26:23 2016 -0400
>
>      Add CLI to check system certificate status
>
>      We add two different calls:
>      1. pki client-cert-validate - which checks a certificate in the client
>         certdb and calls the System cert verification call performed by JSS
>        in the system self test.  This does some basic extensions and trust
>        tests, and also validates cert validity and cert trust chain.
>
>      2. pki-server subsystem-cert-validate  <subsystem>
>         This calls pki client-cert-validate using the nssdb for the subsystem
>         on all of the system certificates by default (or just one if the
>         nickname is defined).
>
>         This is a great thing to call when healthchecking an instance,
>         and also will be used by pkispawn to verify the signing cert in the
>         externally signed CA case.
>
>      Trac Ticket 2043
>

In general it's ACKed. I have some minor comments/questions:

1. The SubsystemCertificateVerifier probably should be renamed to 
SystemCertificateVerifier since "system certificate" refers to a cert in 
the subsystem/instance's NSS database and "subsystem certificate" could 
be confused with the "subsystemCert cert-pki-tomcat".

2. Instead of storing a shared SubsystemCertificateVerifier object in 
the PKIDeployer object it might be better to create a factory method, so 
the verifier can be used like this:

   verifier = deployer.create_system_cert_verifier()
   verifier.verify_certificate('signing')

That way the life-cycle of the verifier object will be short.

3. The .classpath got changed to point to a local path on your machine.

4. Is the "hardward-<token>" name used consistently in our code?

   passwd = instance.get_password("hardware-%s" % token)

-- 
Endi S. Dewata