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

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,7 @@ public byte[] produce(ConnectionContext context,
}

// Produce the extension.
if (chc.localSupportedCertSignAlgs == null) {
chc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
chc.sslConfig,
chc.algorithmConstraints, chc.activeProtocols,
CERTIFICATE_SCOPE);
}
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);

int vectorLen = SignatureScheme.sizeInRecord() *
chc.localSupportedCertSignAlgs.size();
Expand Down Expand Up @@ -245,15 +239,8 @@ public byte[] produce(ConnectionContext context,
}

// Produce the extension.
if (shc.localSupportedCertSignAlgs == null) {
shc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints,
List.of(shc.negotiatedProtocol),
CERTIFICATE_SCOPE);
}

// localSupportedCertSignAlgs has been already updated when we set
// the negotiated protocol.
int vectorLen = SignatureScheme.sizeInRecord()
* shc.localSupportedCertSignAlgs.size();
byte[] extData = new byte[vectorLen + 2];
Expand Down
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
Expand Down Expand Up @@ -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;
}

}

/**
Expand Down Expand Up @@ -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
Expand Up @@ -636,25 +636,11 @@ public byte[] produce(ConnectionContext context,
// The producing happens in server side only.
ServerHandshakeContext shc = (ServerHandshakeContext) context;

if (shc.localSupportedSignAlgs == null) {
shc.localSupportedSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
HANDSHAKE_SCOPE);
}

if (shc.localSupportedCertSignAlgs == null) {
shc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
CERTIFICATE_SCOPE);
}

// According to TLSv1.2 RFC, CertificateRequest message must
// contain signature schemes supported for both:
// handshake signatures and certificate signatures.
// localSupportedSignAlgs and localSupportedCertSignAlgs have been
// already updated when we set the negotiated protocol.
List<SignatureScheme> certReqSignAlgs =
new ArrayList<>(shc.localSupportedSignAlgs);
certReqSignAlgs.retainAll(shc.localSupportedCertSignAlgs);
Expand Down
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
Expand Down Expand Up @@ -825,6 +825,10 @@ private void onClientHello(ServerHandshakeContext context,
"Negotiated protocol version: " + negotiatedProtocol.name);
}

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

// Consume the handshake message for the specific protocol version.
if (negotiatedProtocol.isDTLS) {
if (negotiatedProtocol.useTLS13PlusSpec()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import sun.security.util.HexDumpEncoder;

import static sun.security.ssl.SSLExtension.*;
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;

/**
* Pack of the "pre_shared_key" extension.
Expand Down Expand Up @@ -445,13 +444,7 @@ private static boolean canRejoin(ClientHelloMessage clientHello,
// localSupportedCertSignAlgs field is populated. This is particularly
// important when client authentication was used in an initial session,
// and it is now being resumed.
if (shc.localSupportedCertSignAlgs == null) {
shc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
CERTIFICATE_SCOPE);
}
SignatureScheme.updateHandshakeLocalSupportedAlgs(shc);

// Validate the required client authentication.
if (result &&
Expand Down
43 changes: 8 additions & 35 deletions src/java.base/share/classes/sun/security/ssl/ServerHello.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@

package sun.security.ssl;

import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;
import static sun.security.ssl.SignatureScheme.HANDSHAKE_SCOPE;

import java.io.IOException;
import java.nio.ByteBuffer;
import java.security.AlgorithmConstraints;
Expand Down Expand Up @@ -272,22 +269,6 @@ public byte[] produce(ConnectionContext context,
"Not resumption, and no new session is allowed");
}

if (shc.localSupportedSignAlgs == null) {
shc.localSupportedSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
HANDSHAKE_SCOPE);
}

if (shc.localSupportedCertSignAlgs == null) {
shc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
CERTIFICATE_SCOPE);
}

SSLSessionImpl session =
new SSLSessionImpl(shc, CipherSuite.C_NULL);
session.setMaximumPacketSize(shc.sslConfig.maximumPacketSize);
Expand Down Expand Up @@ -522,22 +503,6 @@ public byte[] produce(ConnectionContext context,
"Not resumption, and no new session is allowed");
}

if (shc.localSupportedSignAlgs == null) {
shc.localSupportedSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
HANDSHAKE_SCOPE);
}

if (shc.localSupportedCertSignAlgs == null) {
shc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints, shc.activeProtocols,
CERTIFICATE_SCOPE);
}

SSLSessionImpl session =
new SSLSessionImpl(shc, CipherSuite.C_NULL);
session.setMaximumPacketSize(shc.sslConfig.maximumPacketSize);
Expand Down Expand Up @@ -959,6 +924,10 @@ private void onHelloRetryRequest(ClientHandshakeContext chc,
"Negotiated protocol version: " + serverVersion.name);
}

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

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

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

if (serverHello.serverRandom.isVersionDowngrade(chc)) {
throw chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
"A potential protocol version downgrade attack");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

import static sun.security.ssl.SSLExtension.CH_SESSION_TICKET;
import static sun.security.ssl.SSLExtension.SH_SESSION_TICKET;
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;

import sun.security.ssl.SSLExtension.ExtensionConsumer;
import sun.security.ssl.SSLExtension.SSLExtensionSpec;
Expand Down Expand Up @@ -353,13 +352,7 @@ public byte[] produce(ConnectionContext context,
return new byte[0];
}

if (chc.localSupportedCertSignAlgs == null) {
chc.localSupportedCertSignAlgs =
SignatureScheme.getSupportedAlgorithms(
chc.sslConfig,
chc.algorithmConstraints, chc.activeProtocols,
CERTIFICATE_SCOPE);
}
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);

return chc.resumingSession.getPskIdentity();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,7 @@ public byte[] produce(ConnectionContext context,
}

// Produce the extension.
if (chc.localSupportedSignAlgs == null) {
chc.localSupportedSignAlgs =
SignatureScheme.getSupportedAlgorithms(
chc.sslConfig,
chc.algorithmConstraints, chc.activeProtocols,
HANDSHAKE_SCOPE);
}
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);

int vectorLen = SignatureScheme.sizeInRecord() *
chc.localSupportedSignAlgs.size();
Expand Down Expand Up @@ -417,18 +411,14 @@ public byte[] produce(ConnectionContext context,
}

// Produce the extension.
List<SignatureScheme> sigAlgs =
SignatureScheme.getSupportedAlgorithms(
shc.sslConfig,
shc.algorithmConstraints,
List.of(shc.negotiatedProtocol),
HANDSHAKE_SCOPE);

int vectorLen = SignatureScheme.sizeInRecord() * sigAlgs.size();
// localSupportedSignAlgs has been already updated when we
// set the negotiated protocol.
int vectorLen = SignatureScheme.sizeInRecord()
* shc.localSupportedSignAlgs.size();
byte[] extData = new byte[vectorLen + 2];
ByteBuffer m = ByteBuffer.wrap(extData);
Record.putInt16(m, vectorLen);
for (SignatureScheme ss : sigAlgs) {
for (SignatureScheme ss : shc.localSupportedSignAlgs) {
Record.putInt16(m, ss.id);
}

Expand Down
Loading