[8] 8012636: OCSP validation fails even when public key is trusted

[Sean Mullan]

I still need to review RevocationChecker but a few more comments ...

* OCSPResponse

- the responderKeyId field is never set. I believe line 299 should be:

     responderKeyId = seq.getData().getOctetString();

- lines 372-385. I think it would be better to not do any matching yet, 
and store all of the certs in a list. The matching should be done later 
in verify if we still need to find a trusted responder cert.

- lines 443-4, it seems expensive to iterate over all the possible 
responders, verifying the signature each time to determine who has 
signed the response if it is signed by the issuing CA. What about the 
following logic instead:

   1. check the responderName or responderId against the list of trusted 
responder certs that are passed in. I think it would be better to store 
the responderCerts in a map instead of a list, using the keyId or name 
as key (you could create a new ResponderKey class that compares either 
the name or the keyId, whichever is set).

   2. If step 1 is successful (finds a matching cert), this cert is 
already trusted so you can proceed to verifying the signature. There is 
no need to look at the certs in the OCSPResponse.

   3. If step 1 fails, then you can look at the certs in the 
OCSPResponse, find the cert that matches the keyId or name, verify the 
signature, then check if it is authorized.

- line 611, it seems the caller could set this if verify returns true. 
Then you could make this method static.

--Sean

On 10/22/2013 12:06 PM, Sean Mullan wrote:
> I am still reviewing, but here are some comments so far:
>
> * X509CertImpl
>
> I would prefer if getSubjectKeyIdentifier returned a KeyIdentifier so
> that it is consistent with the getAuthKeyId method. Also, in
> OCSPResponse, you can then just call KeyIdentifier.equals instead of
> comparing the bytes yourself with Arrays.equals.
>
> * RevocationChecker
>
> RevocationChecker can be re-used for subsequent revocation checks by
> calling the init method. So, you need to clear the contents of the
> responderCerts list each time init is called. You can add this after
> line 323 in the init method
>
>      responderCerts.clear();
>
> --Sean
>
> On 10/21/2013 05:36 PM, Vincent Ryan wrote:
>> Please review this fix to support key-rollover certs
>> (same name, different keys):
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8012636
>> Webrev: http://cr.openjdk.java.net/~vinnie/8012636/webrev.00/
>>
>> This issue arises when an OCSP responder replaces its public key
>> but retains its subject name. The OCSP client must be able to
>> validate responses signed by both keys.
>>
>> Thanks.
>