On 4/14/15 11:58 AM, Sean Mullan wrote:
On 04/13/2015 07:40 PM, Jason Uh wrote:
Thanks, Sean.

Here is a revision:
http://cr.openjdk.java.net/~juh/8076117/02/

Looks good. Just one other comment: the type field in Validator should
be private since it isn't referenced by any other code in that package.

No need to post an updated webrev just for this.

Thanks, Sean. I'll push with that change.

--Sean


On 04/13/2015 12:59 PM, Sean Mullan wrote:
On 04/10/2015 07:18 PM, Jason Uh wrote:
Revised webrev: http://cr.openjdk.java.net/~juh/8076117/01/

Hi Jason,

Just a few comments:

* Validator

272: I would name this flag checkUnresolvedCritExts, since we only care
about them if they are critical and unresolved.

273: You should only ignore the extensions if the type is PKIX, so line
273 should be "(type == TYPE_PKIX) ? false : true;" This way if we ever
add another type in the JDK implementation, we won't accidentally ignore
extensions when we shouldn't have.

* EndEntityChecker

127: don't make this a field of the class, as this can cause concurrency
issues if EndEntityChecker is shared by threads and used with different
certificates. Just create and pass in the extensions, ex line 163:

     checkRemainingExtensions(getCriticalExtensions(cert));

and change the other check methods to have a 2nd parameter for the
extensions, ex:

     checkTLSClient(X509Certificate cert, Set<String> exts)

* EndEntityExtensionCheck

Set the validity date using PKIXParameters.setDate to a time within the
certificate's validity so the test won't start failing when the
certificates expire. See other chain validation tests for examples.

--Sean



Reply via email to