RFR: 8270946: X509CertImpl.getFingerprint should not return the empty String

[Weijun Wang]

On Fri, 23 Jul 2021 17:16:26 GMT, Sean Mullan <mullan at 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