Skip to content

8350807: Certificates using MD5 algorithm that are disabled by default are incorrectly allowed in TLSv1.3 when re-enabled #24425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
120 changes: 50 additions & 70 deletions src/java.base/share/classes/sun/security/ssl/CertificateMessage.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -697,46 +697,6 @@ private static void checkClientCerts(ServerHandshakeContext shc,
}
}

/**
* When a failure happens during certificate checking from an
* {@link X509TrustManager}, determine what TLS alert description
* to use.
*
* @param cexc The exception thrown by the {@link X509TrustManager}
*
* @return A byte value corresponding to a TLS alert description number.
*/
private static Alert getCertificateAlert(
ClientHandshakeContext chc, CertificateException cexc) {
// The specific reason for the failure will determine how to
// set the alert description value
Alert alert = Alert.CERTIFICATE_UNKNOWN;

Throwable baseCause = cexc.getCause();
if (baseCause instanceof CertPathValidatorException cpve) {
Reason reason = cpve.getReason();
if (reason == BasicReason.REVOKED) {
alert = chc.staplingActive ?
Alert.BAD_CERT_STATUS_RESPONSE :
Alert.CERTIFICATE_REVOKED;
} else if (
reason == BasicReason.UNDETERMINED_REVOCATION_STATUS) {
alert = chc.staplingActive ?
Alert.BAD_CERT_STATUS_RESPONSE :
Alert.CERTIFICATE_UNKNOWN;
} else if (reason == BasicReason.ALGORITHM_CONSTRAINED) {
alert = Alert.UNSUPPORTED_CERTIFICATE;
} else if (reason == BasicReason.EXPIRED) {
alert = Alert.CERTIFICATE_EXPIRED;
} else if (reason == BasicReason.INVALID_SIGNATURE ||
reason == BasicReason.NOT_YET_VALID) {
alert = Alert.BAD_CERTIFICATE;
}
}

return alert;
}

}

/**
@@ -1329,37 +1289,57 @@ private static X509Certificate[] checkServerCerts(
return certs;
}

/**
* When a failure happens during certificate checking from an
* {@link X509TrustManager}, determine what TLS alert description
* to use.
*
* @param cexc The exception thrown by the {@link X509TrustManager}
*
* @return A byte value corresponding to a TLS alert description number.
*/
private static Alert getCertificateAlert(
ClientHandshakeContext chc, CertificateException cexc) {
// The specific reason for the failure will determine how to
// set the alert description value
Alert alert = Alert.CERTIFICATE_UNKNOWN;

Throwable baseCause = cexc.getCause();
if (baseCause instanceof CertPathValidatorException cpve) {
Reason reason = cpve.getReason();
if (reason == BasicReason.REVOKED) {
alert = chc.staplingActive ?
Alert.BAD_CERT_STATUS_RESPONSE :
Alert.CERTIFICATE_REVOKED;
} else if (
reason == BasicReason.UNDETERMINED_REVOCATION_STATUS) {
alert = chc.staplingActive ?
Alert.BAD_CERT_STATUS_RESPONSE :
Alert.CERTIFICATE_UNKNOWN;
}

/**
* When a failure happens during certificate checking from an
* {@link X509TrustManager}, determine what TLS alert description
* to use.
*
* @param cexc The exception thrown by the {@link X509TrustManager}
* @return A byte value corresponding to a TLS alert description number.
*/
private static Alert getCertificateAlert(
ClientHandshakeContext chc, CertificateException cexc) {
// The specific reason for the failure will determine how to
// set the alert description value
Alert alert = Alert.CERTIFICATE_UNKNOWN;

Throwable baseCause = cexc.getCause();
if (baseCause instanceof CertPathValidatorException cpve) {
Reason reason = cpve.getReason();
if (reason == BasicReason.REVOKED) {
alert = chc.staplingActive ?
Alert.BAD_CERT_STATUS_RESPONSE :
Alert.CERTIFICATE_REVOKED;
} else if (reason == BasicReason.UNDETERMINED_REVOCATION_STATUS) {
alert = chc.staplingActive ?
Alert.BAD_CERT_STATUS_RESPONSE :
Alert.CERTIFICATE_UNKNOWN;
} else if (reason == BasicReason.EXPIRED) {
alert = Alert.CERTIFICATE_EXPIRED;
} else if (reason == BasicReason.INVALID_SIGNATURE
|| reason == BasicReason.NOT_YET_VALID) {
alert = Alert.BAD_CERTIFICATE;
} else if (reason == BasicReason.ALGORITHM_CONSTRAINED) {
alert = Alert.UNSUPPORTED_CERTIFICATE;

// Per TLSv1.3 RFC we MUST abort the handshake with a
// "bad_certificate" alert if we reject certificate
// because of the signature using MD5 or SHA1 algorithm.
if (chc.negotiatedProtocol != null
&& chc.negotiatedProtocol.useTLS13PlusSpec()) {
final String exMsg = cexc.getMessage().toUpperCase();

if (exMsg.contains("MD5WITH")
|| exMsg.contains("SHA1WITH")) {
alert = Alert.BAD_CERTIFICATE;
}
}
}

return alert;
}

return alert;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -825,6 +825,10 @@ private void onClientHello(ServerHandshakeContext context,
"Negotiated protocol version: " + negotiatedProtocol.name);
}

// Protocol version is negotiated, reset locally supported
// signature schemes according to the protocol being used.
SignatureScheme.setHandshakeLocalSupportedAlgs(context);

// Consume the handshake message for the specific protocol version.
if (negotiatedProtocol.isDTLS) {
if (negotiatedProtocol.useTLS13PlusSpec()) {
18 changes: 18 additions & 0 deletions src/java.base/share/classes/sun/security/ssl/ServerHello.java
Original file line number Diff line number Diff line change
@@ -359,6 +359,11 @@ public byte[] produce(ConnectionContext context,
shc.handshakeSession = shc.resumingSession;
shc.negotiatedProtocol =
shc.resumingSession.getProtocolVersion();

// Protocol version is negotiated, reset locally supported
// signature schemes according to the protocol being used.
SignatureScheme.setHandshakeLocalSupportedAlgs(shc);

shc.negotiatedCipherSuite = shc.resumingSession.getSuite();
shc.handshakeHash.determine(
shc.negotiatedProtocol, shc.negotiatedCipherSuite);
@@ -570,6 +575,11 @@ public byte[] produce(ConnectionContext context,

shc.negotiatedProtocol =
shc.resumingSession.getProtocolVersion();

// Protocol version is negotiated, reset locally supported
// signature schemes according to the protocol being used.
SignatureScheme.setHandshakeLocalSupportedAlgs(shc);

shc.negotiatedCipherSuite = shc.resumingSession.getSuite();
shc.handshakeHash.determine(
shc.negotiatedProtocol, shc.negotiatedCipherSuite);
@@ -959,6 +969,10 @@ private void onHelloRetryRequest(ClientHandshakeContext chc,
"Negotiated protocol version: " + serverVersion.name);
}

// Protocol version is negotiated, reset locally supported
// signature schemes according to the protocol being used.
SignatureScheme.setHandshakeLocalSupportedAlgs(chc);

// TLS 1.3 key share extension may have produced client
// possessions for TLS 1.3 key exchanges.
//
@@ -1010,6 +1024,10 @@ private void onServerHello(ClientHandshakeContext chc,
"Negotiated protocol version: " + serverVersion.name);
}

// Protocol version is negotiated, reset locally supported
// signature schemes according to the protocol being used.
SignatureScheme.setHandshakeLocalSupportedAlgs(chc);

if (serverHello.serverRandom.isVersionDowngrade(chc)) {
throw chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
"A potential protocol version downgrade attack");
20 changes: 20 additions & 0 deletions src/java.base/share/classes/sun/security/ssl/SignatureScheme.java
Original file line number Diff line number Diff line change
@@ -550,6 +550,26 @@ static Map.Entry<SignatureScheme, Signature> getSignerOfPreferableAlgorithm(
return null;
}

// Convenience method to set all locally supported signature schemes for
// a given HandshakeContext.
static void setHandshakeLocalSupportedAlgs(HandshakeContext hc) {
List<ProtocolVersion> protocols = hc.negotiatedProtocol != null ?
List.of(hc.negotiatedProtocol) :
hc.activeProtocols;

hc.localSupportedSignAlgs = getSupportedAlgorithms(
hc.sslConfig,
hc.algorithmConstraints,
protocols,
HANDSHAKE_SCOPE);

hc.localSupportedCertSignAlgs = getSupportedAlgorithms(
hc.sslConfig,
hc.algorithmConstraints,
protocols,
CERTIFICATE_SCOPE);
}

// Returns true if this signature scheme is supported for the given
// protocol version and SSL scopes.
private boolean isSupportedProtocol(
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -105,6 +105,7 @@ void doServerSide() throws Exception {
(SSLServerSocketFactory) SSLServerSocketFactory.getDefault();
SSLServerSocket sslServerSocket =
(SSLServerSocket) sslssf.createServerSocket(serverPort);
sslServerSocket.setEnabledProtocols(new String[]{"TLSv1.2"});
serverPort = sslServerSocket.getLocalPort();

/*
6 changes: 3 additions & 3 deletions test/jdk/javax/net/ssl/templates/SSLSocketTemplate.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -245,7 +245,7 @@ protected void doClientSide() throws Exception {
//
// The server side takes care of the issue if the server cannot
// get started in 90 seconds. The client side would just ignore
// the test case if the serer is not ready.
// the test case if the server is not ready.
boolean serverIsReady =
serverCondition.await(90L, TimeUnit.SECONDS);
if (!serverIsReady) {
@@ -378,7 +378,7 @@ private void bootup() throws Exception {
* Check various exception conditions.
*/
if ((local != null) && (remote != null)) {
// If both failed, return the curthread's exception.
// If both failed, return the current thread's exception.
local.addSuppressed(remote);
exception = local;
} else if (local != null) {
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -893,7 +893,7 @@ private static SSLContext getSSLContext(String trusedCertStr,
TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
tmf.init(ks);

SSLContext ctx = SSLContext.getInstance("TLS");
SSLContext ctx = SSLContext.getInstance("TLSv1.2");

if (keyCertStr != null) {
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -901,7 +901,7 @@ private static SSLContext getSSLContext(String trusedCertStr,
TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
tmf.init(ks);

SSLContext ctx = SSLContext.getInstance("TLS");
SSLContext ctx = SSLContext.getInstance("TLSv1.2");

if (keyCertStr != null) {
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -900,7 +900,7 @@ private static SSLContext getSSLContext(String trusedCertStr,
TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
tmf.init(ks);

SSLContext ctx = SSLContext.getInstance("TLS");
SSLContext ctx = SSLContext.getInstance("TLSv1.2");

if (keyCertStr != null) {
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -893,7 +893,7 @@ private static SSLContext getSSLContext(String trusedCertStr,
TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
tmf.init(ks);

SSLContext ctx = SSLContext.getInstance("TLS");
SSLContext ctx = SSLContext.getInstance("TLSv1.2");

if (keyCertStr != null) {
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
Loading
Oops, something went wrong.