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

Sean Mullan sean.mullan at oracle.com
Thu Dec 5 13:26:02 PST 2013


Looks good, just one nit - you can remove the imports of 
PolicyMappingsExtension and X509CertImpl from ForwardBuilder.

--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