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

Sean Mullan sean.mullan at oracle.com
Fri Dec 6 00:46:57 UTC 2013


Looks good. 

> On Dec 5, 2013, at 6:58 PM, Jason Uh <jason.uh at oracle.com> wrote:
> 
> 
> 
>> 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
>> 



More information about the security-dev mailing list