RFR: 8353738: Update TLS unit tests to not use certificates with MD5 signatures [v5]
Daniel Jeliński
djelinski at openjdk.org
Thu Nov 13 14:37:43 UTC 2025
On Fri, 31 Oct 2025 15:02:42 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:
>> This PR updates tests that were using MD5 certificates. For most of the tests, I added test cases for TLSv1.2/MD5withRSA and TLSv1.3/SHA256withRSA.
>
> Matthew Donovan has updated the pull request incrementally with one additional commit since the last revision:
>
> changed line wrapping
Can you convert or remove the `IPAddressDNSIdentities` test as well?
test/jdk/javax/net/ssl/HttpsURLConnection/CriticalSubjectAltName.java line 36:
> 34: * @library /test/lib
> 35: * @modules java.base/sun.security.x509 java.base/sun.security.util
> 36: * @run main/othervm CriticalSubjectAltName TLSv1.2 MD5withRSA
as far as I could tell, this test doesn't verify any functionality that would require a specific key type, it's simply using MD5 because that was the popular choice in 2008. Do we need to keep using MD5, or can we make it use whatever key type is the default?
test/jdk/javax/net/ssl/HttpsURLConnection/CriticalSubjectAltName.java line 39:
> 37:
> 38: /*
> 39: * This test depends on binary keystore, crisubn.jks and trusted.jks. Because
Peease remove the crisubn.jks and trusted.jks files from this directory, they aren't used anywhere else.
test/jdk/javax/net/ssl/HttpsURLConnection/CriticalSubjectAltName.java line 157:
> 155: ks.setCertificateEntry("Trusted Cert", trustedCert);
> 156:
> 157: Certificate[] chain = new Certificate[] {serverCert, trustedCert};
(probably preexisting) you can remove the trusted cert from the chain; real servers usually don't send it.
test/jdk/javax/net/ssl/HttpsURLConnection/CriticalSubjectAltName.java line 190:
> 188: void doClientSide() throws Exception {
> 189:
> 190: if (!serverReady.await(SERVER_WAIT_SECS, TimeUnit.SECONDS)) {
I'd remove the time limit, it only causes trouble when people increase test concurrency and timeout factors.
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/IPIdentities.java line 1:
> 1: /*
This might be preexisting, but this file is identical to `test/jdk/sun/net/www/protocol/https/HttpsURLConnection/IPAddressIPIdentities.java` now. Can we remove one?
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/IdentitiesBase.java line 104:
> 102: CertificateBuilder.KeyUsage.KEY_ENCIPHERMENT)
> 103: .addBasicConstraintsExt(false, false, -1)
> 104: .addExtension(CertificateBuilder.createIPSubjectAltNameExt(true, "127.0.0.1"))
I assume you verified that the DNSIdentities customization overwrites the SAN configured here, but I'd feel more confident if this line were moved to customizeServerCert in IPIdentities
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/IdentitiesBase.java line 119:
> 117: CertificateBuilder.KeyUsage.NONREPUDIATION,
> 118: CertificateBuilder.KeyUsage.KEY_ENCIPHERMENT)
> 119: .addExtension(CertificateBuilder.createIPSubjectAltNameExt(true, "127.0.0.1"))
Same here.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27342#pullrequestreview-3459883168
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523546617
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523539070
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523561169
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523555451
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523677717
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523705701
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2523706426
More information about the net-dev
mailing list