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

[Sean Mullan]

Just one more comment on RevocationChecker:

- I'd prefer if the getResponderCert methods remained static. You can do 
that by creating a local variable in 
getResponderCerts(X509CertSelector,Set<TrustAnchor>,List<CertStore>)
to hold the certs and then returning that.

--Sean

On 10/22/2013 02:08 PM, Sean Mullan wrote:
> 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.
>>
>