Hi Vinnie, Here are my comments.
* OCSPRequest [119-20] Should use debug.println instead of System.out.println * OCSPChecker [386] why do you decrement remainingCerts? Is it identifying the wrong certificate for some reason? [473] Is this called by any other class? If not, I suggest making it private. [476] Better to always convert all certs to X509CertImpl, so you always obtain the key id from the certificate (it may not be the same as generated by KeyIdentifier): X509CertImpl certImpl = X509CertImpl.toImpl(cert); * OCSPResponse [168-9] Should use debug.println instead of System.out.println * X509CertImpl [1075-7] Comment should be changed. If the cert doesn't have an AKID, then aki will be null and it won't even get this far. IOException can only be thrown if you pass AKI.get() an attribute it doesn't recognize, but since this code is not doing that, it can never be thrown. [1080] You may want to consider caching this value, since the method can be called more than once for certificates such as trust anchors. [1196-8] Same comment as above. --Sean On 6/18/12 2:41 PM, Vincent Ryan wrote: > Hello, > > Please review the following changeset for JDK 7u6: > http://cr.openjdk.java.net/~vinnie/7168191/ > > The bug report is at: > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7168191 > > This fix addresses a bug in the OCSP client when processing key-rollover > certs. Typically such certs have the same subject name but different > keys. Now the OCSP code examines all the matching candidates (not just > the first one) both when preparing the request and when validating the > response. > > Thanks.