Looks good.
> On Dec 5, 2013, at 6:58 PM, Jason Uh <jason...@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 >>
