On Fri, 23 Jul 2021 17:16:26 GMT, Sean Mullan <mul...@openjdk.org> wrote:
> Please review this fix to change the internal `X509CertImpl.getFingerprint` > method to not return "" as a fingerprint if there is an error generating that > fingerprint. Instead, `null` is now returned, and "" is no longer cached as a > valid fingerprint. Although errors generating fingerprints should be very > rare, this is a cleaner way to handle them. > > Also, debugging messages have been added when there is an exception. And, as > a memory/performance improvement, `X509CertImpl.getFingerprint` now calls > `X509CertImpl.getEncodedInternal` which avoids cloning the encoded bytes if > the `Certificate` is an instance of `X509CertImpl`. src/java.base/share/classes/sun/security/validator/SymantecTLSPolicy.java line 161: > 159: X509Certificate anchor = chain[chain.length-1]; > 160: String fp = fingerprint(anchor); > 161: if (fp != null && FINGERPRINTS.contains(fp)) { I understand the original behavior is also bypassing the check if fingerprint cannot be calculated, but this sounds a little irresponsible. Same as in `UntrustedCertificates`. src/java.base/share/classes/sun/security/x509/X509CertImpl.java line 1924: > 1922: x -> getFingerprintInternal(x, debug)); > 1923: } > 1924: I'm a little confused by these methods. Can you inline `getFingerprintInternal` and rename `getFingerprint` on line 1936 to `getFingerprintInternal`? test/jdk/sun/security/x509/X509CertImpl/GetFingerprintError.java line 53: > 51: > 52: // test cert with bad encoding > 53: X509Certificate fcert = new X509CertificateWithBadEncoding(cert); In fact, `new X509CertImpl()` satisfies your requirement perfectly, which is an unpopulated cert with no encoding. It might be a little weird though. You can continue with your choice. ------------- PR: https://git.openjdk.java.net/jdk/pull/4891