Hi Sean,
CADistrustPolicy.java:
80 String[] policies = (property != null) ? property.split(",") :
null;
81 EnumSet<CADistrustPolicy> set =
EnumSet.noneOf(CADistrustPolicy.class);
82 for (String policy : policies) {
If the property is not defined, the last line above would throw an NPE.
test/Distrust.java:
118 private static final Date APRIL_17_2019 =
119 Date.from(LocalDate.of(2019, 4, 17)
120 .atStartOfDay(ZoneId.systemDefault())
121 .toInstant());
Should UTC be used?
144 testTM(getTMF("PKIX", getParams(VERISIGN_UNIVERSAL_ROOTCA)),
145 loadCertificateChain("verisignuniversalrootca"), !distrust);
What's the purpose of the above test? To make sure that even if one day we
removed these root CAs from cacerts then the mechanism still works as expected?
191 X509Certificate tlsServerCert = chain[0];
192 chain[0] = new DistrustedTLSServerCert(tlsServerCert,
APRIL_17_2019);
Is it necessary to introduce a local variable?
No other comment.
Thanks,
Max
> On Dec 8, 2018, at 12:02 AM, Sean Mullan <sean.mul...@oracle.com> wrote:
>
> Please review this change to Distrust TLS server certificates anchored by
> Symantec Root CAs. Although the restrictions won't kick in until after 12 GA,
> the fix touches code that validates certificate chains, so getting this into
> 12 now will provide more assurance that the chain validation code continues
> to work properly.
>
> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8207258/webrev.01/
> issue: https://bugs.openjdk.java.net/browse/JDK-8207258
>
> Please see the recently posted blog for more information about the
> restrictions that are being imposed:
> https://blogs.oracle.com/java-platform-group/jdk-distrusting-symantec-tls-certificates
>
> Thanks,
> Sean
