Updated webrev:
http://cr.openjdk.java.net/~mullan/webrevs/8207258/webrev.02/
Specific comments below ...
On 12/9/18 9:22 AM, Weijun Wang wrote:
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.
Good catch.
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?
Yes, fixed.
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?
No, it is simply to test the use case where an application passes its
own PKIXBuilderParameters into a TrustManager. However, it is better to
just pass in the cacerts KeyStore in that case to the
PKIXBuilderParameters constructor. So, I have modified the test
accordingly. I also created a new SecurityUtils class in the test
library area and added a utility method for returning the cacerts
KeyStore. We can change other tests over time to use this new utility
method.
191 X509Certificate tlsServerCert = chain[0];
192 chain[0] = new DistrustedTLSServerCert(tlsServerCert,
APRIL_17_2019);
Is it necessary to introduce a local variable?
No, fixed.
Thanks,
Sean
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
