[8] Request for Review: 8007967: Infinite loop can happen in sun.security.provider.certpath.SunCertPathBuilder.depthFirstSearchForward()

[Jason Uh]

On 12/05/2013 01:26 PM, Sean Mullan wrote:
> Looks good, just one nit - you can remove the imports of
> PolicyMappingsExtension and X509CertImpl from ForwardBuilder.

Done. Also, updated copyright date in ForwardBuilder.java.
http://cr.openjdk.java.net/~juh/8007967/webrev.02/

> --Sean
>
> On 12/05/2013 04:04 PM, Jason Uh wrote:
>> Thanks, Sean.
>>
>> I've updated the webrev with your suggestions. Also, I've changed the
>> check for loops in ForwardBuilder.java to bail out whenever the same
>> certificate is encountered twice.
>>
>> Please note that none of the policy mapping tests in the NIST PKITS use
>> loops. Also, RFC 4158's section on Certification Path building states
>> that X.509 "requires that certificates are not repeated when building
>> paths," but does not mention policy mapping. It should therefore be okay
>> to make this change.
>>
>> Please take a look:
>> http://cr.openjdk.java.net/~juh/8007967/webrev.01/
>>
>> Thanks,
>> Jason
>>
>> On 12/04/2013 01:18 PM, Sean Mullan wrote:
>>> Just 2 comments on DistributionPointFetcher:
>>>
>>> You can eliminate some duplication of code by changing the existing
>>> getCRLs method to just call the new method with a null prevCert
>>> parameter.
>>>
>>> On lines 659-663, you don't need to add @code tags, that is only for
>>> javadoc comments.
>>>
>>> --Sean
>>>
>>> On 12/03/2013 01:51 PM, Jason Uh wrote:
>>>> Could I please get a review for this change? This change fixes some
>>>> issues in CertPath building and CRL verification. The main
>>>> components of
>>>> this fix are:
>>>>
>>>> 1. Proper setting of TrustAnchors when verifying indirect CRLs obtained
>>>> from CRL Distribution Points. I added an overloaded getCRLs() method to
>>>> DistributionPointFetcher for this.
>>>>
>>>> 2. Terminating the CertPath build immediately when the target cert is
>>>> found to be revoked.
>>>>
>>>> 3. Some clarification in the comments.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~juh/8007967/webrev.00/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8007967
>>>>
>>>> Thanks,
>>>> Jason
>>>
>