Thanks for the comments, Sean; Vinnie, for the patch.
Here's an updated webrev with your suggested changes and Vinnie's patch.
http://cr.openjdk.java.net/~juh/8054037/01/
I've also removed a few unnecessary toString() calls in
AdaptableX509CertSelector.java and DistributionPointFetcher.java.
Jason
On 03/02/2015 09:26 AM, Seán Coffey wrote:
Jason,
thanks for taking this on. Your changes look fine to be and should help
the debugging experience. Some extra comments from me. Here's some
standard output that one sees (early in connection) from a standard TLS
connection attempt with verbose certpath logging :
certpath: PKIXCertPathValidator.engineValidate()...
certpath: AdaptableX509CertSelector.match: subject key IDs don't match
certpath: NO - don't try this trustedCert
Can we print the subject key IDs for reference ? I'm conscious of line
wastage in log files. Bunching the IDs in with the "don't try this
trustedCert line" would work perhaps.
Shortly after that, one reads :
certpath: Executing PKIX certification path validation algorithm.
certpath: Checking cert1 ...
certpath: Set of critical extensions:
certpath: 2.5.29.15
certpath: 2.5.29.19
Could we improve the PKIXMasterCertPathValidator.validate printing in
this case ? Let's print the Subject and/or ID of "cert1"
I'm not sure why we print the critical extension list here. Could we
append them after "Set of critical extensions:" to avoid extra lines ?
Shortly after that, we have this in output :
certpath: ---checking basic constraints...
certpath: i = 1
certpath: maxPathLength = 2
certpath: after processing, maxPathLength = 0
certpath: basic constraints verified.
certpath: ---checking name constraints...
certpath: prevNC = null
certpath: newNC = null
certpath: mergedNC = null
certpath: name constraints verified.
certpath: -checker4 validation succeeded
just a tidy up thought, could some of the lines above be concatenated ?
E.g. : ConstraintsChecker.java
227 debug.println("---checking " + msg + "...");
228 debug.println("i = " + i);
229 debug.println("maxPathLength = " + maxPathLength);
Maybe some room for concatenation here also :
certpath: ---checking timestamp:Mon Mar 02 16:34:47 GMT 2015...
certpath: timestamp verified.
Finally - another reason for why I logged the enhancement request in the
first place..
Take this output :
*** ServerHelloDone
[read] MD5 and SHA1 hashes: len = 4
0000: 0E 00 00 00 ....
*** Certificate chain
***
The final "***" here indicates that the truststore is empty. It's not
very obvious to the novice user!
I believe the output corresponds to :
jdk/src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java#498
We need to at least print "Empty cert chain" where applicable. This
belongs in jsse code but it would be great if this can be improved as
part of this fix.
regards,
Sean.
On 13/02/15 00:05, Jason Uh wrote:
Please review this change, which augments some of the debug statements
for java.security.debug=certpath.
webrev: http://cr.openjdk.java.net/~juh/8054037/00/
bug: https://bugs.openjdk.java.net/browse/JDK-8054037
Thanks,
Jason
