Skip to content

Commit 1cdf8f5

Browse files
GoeLinGoeLin
committed
8350807: Certificates using MD5 algorithm that are disabled by default are incorrectly allowed in TLSv1.3 when re-enabled
Reviewed-by: mbaesken Backport-of: abb23828f9dc5f4cdb75d5b924dd6f45925102cd
1 parent 2cea481 commit 1cdf8f5

File tree

16 files changed

+351
-182
lines changed

16 files changed

+351
-182
lines changed

‎src/java.base/share/classes/sun/security/ssl/CertSignAlgsExtension.java‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,7 @@ public byte[] produce(ConnectionContext context,
9999
}
100100

101101
// Produce the extension.
102-
if (chc.localSupportedCertSignAlgs == null) {
103-
chc.localSupportedCertSignAlgs =
104-
SignatureScheme.getSupportedAlgorithms(
105-
chc.sslConfig,
106-
chc.algorithmConstraints, chc.activeProtocols,
107-
CERTIFICATE_SCOPE);
108-
}
102+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
109103

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

247241
// Produce the extension.
248-
if (shc.localSupportedCertSignAlgs == null) {
249-
shc.localSupportedCertSignAlgs =
250-
SignatureScheme.getSupportedAlgorithms(
251-
shc.sslConfig,
252-
shc.algorithmConstraints,
253-
List.of(shc.negotiatedProtocol),
254-
CERTIFICATE_SCOPE);
255-
}
256-
242+
// localSupportedCertSignAlgs has been already updated when we set
243+
// the negotiated protocol.
257244
int vectorLen = SignatureScheme.sizeInRecord()
258245
* shc.localSupportedCertSignAlgs.size();
259246
byte[] extData = new byte[vectorLen + 2];

‎src/java.base/share/classes/sun/security/ssl/CertificateMessage.java‎

Lines changed: 49 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -693,46 +693,6 @@ private static void checkClientCerts(ServerHandshakeContext shc,
693693
}
694694
}
695695

696-
/**
697-
* When a failure happens during certificate checking from an
698-
* {@link X509TrustManager}, determine what TLS alert description
699-
* to use.
700-
*
701-
* @param cexc The exception thrown by the {@link X509TrustManager}
702-
*
703-
* @return A byte value corresponding to a TLS alert description number.
704-
*/
705-
private static Alert getCertificateAlert(
706-
ClientHandshakeContext chc, CertificateException cexc) {
707-
// The specific reason for the failure will determine how to
708-
// set the alert description value
709-
Alert alert = Alert.CERTIFICATE_UNKNOWN;
710-
711-
Throwable baseCause = cexc.getCause();
712-
if (baseCause instanceof CertPathValidatorException cpve) {
713-
Reason reason = cpve.getReason();
714-
if (reason == BasicReason.REVOKED) {
715-
alert = chc.staplingActive ?
716-
Alert.BAD_CERT_STATUS_RESPONSE :
717-
Alert.CERTIFICATE_REVOKED;
718-
} else if (
719-
reason == BasicReason.UNDETERMINED_REVOCATION_STATUS) {
720-
alert = chc.staplingActive ?
721-
Alert.BAD_CERT_STATUS_RESPONSE :
722-
Alert.CERTIFICATE_UNKNOWN;
723-
} else if (reason == BasicReason.ALGORITHM_CONSTRAINED) {
724-
alert = Alert.UNSUPPORTED_CERTIFICATE;
725-
} else if (reason == BasicReason.EXPIRED) {
726-
alert = Alert.CERTIFICATE_EXPIRED;
727-
} else if (reason == BasicReason.INVALID_SIGNATURE ||
728-
reason == BasicReason.NOT_YET_VALID) {
729-
alert = Alert.BAD_CERTIFICATE;
730-
}
731-
}
732-
733-
return alert;
734-
}
735-
736696
}
737697

738698
/**
@@ -1330,37 +1290,57 @@ private static X509Certificate[] checkServerCerts(
13301290
return certs;
13311291
}
13321292

1333-
/**
1334-
* When a failure happens during certificate checking from an
1335-
* {@link X509TrustManager}, determine what TLS alert description
1336-
* to use.
1337-
*
1338-
* @param cexc The exception thrown by the {@link X509TrustManager}
1339-
*
1340-
* @return A byte value corresponding to a TLS alert description number.
1341-
*/
1342-
private static Alert getCertificateAlert(
1343-
ClientHandshakeContext chc, CertificateException cexc) {
1344-
// The specific reason for the failure will determine how to
1345-
// set the alert description value
1346-
Alert alert = Alert.CERTIFICATE_UNKNOWN;
1347-
1348-
Throwable baseCause = cexc.getCause();
1349-
if (baseCause instanceof CertPathValidatorException cpve) {
1350-
Reason reason = cpve.getReason();
1351-
if (reason == BasicReason.REVOKED) {
1352-
alert = chc.staplingActive ?
1353-
Alert.BAD_CERT_STATUS_RESPONSE :
1354-
Alert.CERTIFICATE_REVOKED;
1355-
} else if (
1356-
reason == BasicReason.UNDETERMINED_REVOCATION_STATUS) {
1357-
alert = chc.staplingActive ?
1358-
Alert.BAD_CERT_STATUS_RESPONSE :
1359-
Alert.CERTIFICATE_UNKNOWN;
1293+
}
1294+
1295+
/**
1296+
* When a failure happens during certificate checking from an
1297+
* {@link X509TrustManager}, determine what TLS alert description
1298+
* to use.
1299+
*
1300+
* @param cexc The exception thrown by the {@link X509TrustManager}
1301+
* @return A byte value corresponding to a TLS alert description number.
1302+
*/
1303+
private static Alert getCertificateAlert(
1304+
ClientHandshakeContext chc, CertificateException cexc) {
1305+
// The specific reason for the failure will determine how to
1306+
// set the alert description value
1307+
Alert alert = Alert.CERTIFICATE_UNKNOWN;
1308+
1309+
Throwable baseCause = cexc.getCause();
1310+
if (baseCause instanceof CertPathValidatorException cpve) {
1311+
Reason reason = cpve.getReason();
1312+
if (reason == BasicReason.REVOKED) {
1313+
alert = chc.staplingActive ?
1314+
Alert.BAD_CERT_STATUS_RESPONSE :
1315+
Alert.CERTIFICATE_REVOKED;
1316+
} else if (reason == BasicReason.UNDETERMINED_REVOCATION_STATUS) {
1317+
alert = chc.staplingActive ?
1318+
Alert.BAD_CERT_STATUS_RESPONSE :
1319+
Alert.CERTIFICATE_UNKNOWN;
1320+
} else if (reason == BasicReason.EXPIRED) {
1321+
alert = Alert.CERTIFICATE_EXPIRED;
1322+
} else if (reason == BasicReason.INVALID_SIGNATURE
1323+
|| reason == BasicReason.NOT_YET_VALID) {
1324+
alert = Alert.BAD_CERTIFICATE;
1325+
} else if (reason == BasicReason.ALGORITHM_CONSTRAINED) {
1326+
alert = Alert.UNSUPPORTED_CERTIFICATE;
1327+
1328+
// Per TLSv1.3 RFC we MUST abort the handshake with a
1329+
// "bad_certificate" alert if we reject certificate
1330+
// because of the signature using MD5 or SHA1 algorithm.
1331+
if (chc.negotiatedProtocol != null
1332+
&& chc.negotiatedProtocol.useTLS13PlusSpec()) {
1333+
final String exMsg = cexc.getMessage().toUpperCase();
1334+
1335+
if (exMsg.contains("MD5WITH")
1336+
|| exMsg.contains("SHA1WITH")) {
1337+
alert = Alert.BAD_CERTIFICATE;
1338+
}
13601339
}
13611340
}
1362-
1363-
return alert;
13641341
}
1342+
1343+
return alert;
13651344
}
1345+
13661346
}

‎src/java.base/share/classes/sun/security/ssl/CertificateRequest.java‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -636,25 +636,11 @@ public byte[] produce(ConnectionContext context,
636636
// The producing happens in server side only.
637637
ServerHandshakeContext shc = (ServerHandshakeContext) context;
638638

639-
if (shc.localSupportedSignAlgs == null) {
640-
shc.localSupportedSignAlgs =
641-
SignatureScheme.getSupportedAlgorithms(
642-
shc.sslConfig,
643-
shc.algorithmConstraints, shc.activeProtocols,
644-
HANDSHAKE_SCOPE);
645-
}
646-
647-
if (shc.localSupportedCertSignAlgs == null) {
648-
shc.localSupportedCertSignAlgs =
649-
SignatureScheme.getSupportedAlgorithms(
650-
shc.sslConfig,
651-
shc.algorithmConstraints, shc.activeProtocols,
652-
CERTIFICATE_SCOPE);
653-
}
654-
655639
// According to TLSv1.2 RFC, CertificateRequest message must
656640
// contain signature schemes supported for both:
657641
// handshake signatures and certificate signatures.
642+
// localSupportedSignAlgs and localSupportedCertSignAlgs have been
643+
// already updated when we set the negotiated protocol.
658644
List<SignatureScheme> certReqSignAlgs =
659645
new ArrayList<>(shc.localSupportedSignAlgs);
660646
certReqSignAlgs.retainAll(shc.localSupportedCertSignAlgs);

‎src/java.base/share/classes/sun/security/ssl/ClientHello.java‎

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

828+
// Protocol version is negotiated, update locally supported
829+
// signature schemes according to the protocol being used.
830+
SignatureScheme.updateHandshakeLocalSupportedAlgs(context);
831+
828832
// Consume the handshake message for the specific protocol version.
829833
if (negotiatedProtocol.isDTLS) {
830834
if (negotiatedProtocol.useTLS13PlusSpec()) {

‎src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import sun.security.util.HexDumpEncoder;
4343

4444
import static sun.security.ssl.SSLExtension.*;
45-
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;
4645

4746
/**
4847
* Pack of the "pre_shared_key" extension.
@@ -445,13 +444,7 @@ private static boolean canRejoin(ClientHelloMessage clientHello,
445444
// localSupportedCertSignAlgs field is populated. This is particularly
446445
// important when client authentication was used in an initial session,
447446
// and it is now being resumed.
448-
if (shc.localSupportedCertSignAlgs == null) {
449-
shc.localSupportedCertSignAlgs =
450-
SignatureScheme.getSupportedAlgorithms(
451-
shc.sslConfig,
452-
shc.algorithmConstraints, shc.activeProtocols,
453-
CERTIFICATE_SCOPE);
454-
}
447+
SignatureScheme.updateHandshakeLocalSupportedAlgs(shc);
455448

456449
// Validate the required client authentication.
457450
if (result &&

‎src/java.base/share/classes/sun/security/ssl/ServerHello.java‎

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@
2525

2626
package sun.security.ssl;
2727

28-
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;
29-
import static sun.security.ssl.SignatureScheme.HANDSHAKE_SCOPE;
30-
3128
import java.io.IOException;
3229
import java.nio.ByteBuffer;
3330
import java.security.AlgorithmConstraints;
@@ -272,22 +269,6 @@ public byte[] produce(ConnectionContext context,
272269
"Not resumption, and no new session is allowed");
273270
}
274271

275-
if (shc.localSupportedSignAlgs == null) {
276-
shc.localSupportedSignAlgs =
277-
SignatureScheme.getSupportedAlgorithms(
278-
shc.sslConfig,
279-
shc.algorithmConstraints, shc.activeProtocols,
280-
HANDSHAKE_SCOPE);
281-
}
282-
283-
if (shc.localSupportedCertSignAlgs == null) {
284-
shc.localSupportedCertSignAlgs =
285-
SignatureScheme.getSupportedAlgorithms(
286-
shc.sslConfig,
287-
shc.algorithmConstraints, shc.activeProtocols,
288-
CERTIFICATE_SCOPE);
289-
}
290-
291272
SSLSessionImpl session =
292273
new SSLSessionImpl(shc, CipherSuite.C_NULL);
293274
session.setMaximumPacketSize(shc.sslConfig.maximumPacketSize);
@@ -522,22 +503,6 @@ public byte[] produce(ConnectionContext context,
522503
"Not resumption, and no new session is allowed");
523504
}
524505

525-
if (shc.localSupportedSignAlgs == null) {
526-
shc.localSupportedSignAlgs =
527-
SignatureScheme.getSupportedAlgorithms(
528-
shc.sslConfig,
529-
shc.algorithmConstraints, shc.activeProtocols,
530-
HANDSHAKE_SCOPE);
531-
}
532-
533-
if (shc.localSupportedCertSignAlgs == null) {
534-
shc.localSupportedCertSignAlgs =
535-
SignatureScheme.getSupportedAlgorithms(
536-
shc.sslConfig,
537-
shc.algorithmConstraints, shc.activeProtocols,
538-
CERTIFICATE_SCOPE);
539-
}
540-
541506
SSLSessionImpl session =
542507
new SSLSessionImpl(shc, CipherSuite.C_NULL);
543508
session.setMaximumPacketSize(shc.sslConfig.maximumPacketSize);
@@ -959,6 +924,10 @@ private void onHelloRetryRequest(ClientHandshakeContext chc,
959924
"Negotiated protocol version: " + serverVersion.name);
960925
}
961926

927+
// Protocol version is negotiated, update locally supported
928+
// signature schemes according to the protocol being used.
929+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
930+
962931
// TLS 1.3 key share extension may have produced client
963932
// possessions for TLS 1.3 key exchanges.
964933
//
@@ -1010,6 +979,10 @@ private void onServerHello(ClientHandshakeContext chc,
1010979
"Negotiated protocol version: " + serverVersion.name);
1011980
}
1012981

982+
// Protocol version is negotiated, update locally supported
983+
// signature schemes according to the protocol being used.
984+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
985+
1013986
if (serverHello.serverRandom.isVersionDowngrade(chc)) {
1014987
throw chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
1015988
"A potential protocol version downgrade attack");

‎src/java.base/share/classes/sun/security/ssl/SessionTicketExtension.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040

4141
import static sun.security.ssl.SSLExtension.CH_SESSION_TICKET;
4242
import static sun.security.ssl.SSLExtension.SH_SESSION_TICKET;
43-
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;
4443

4544
import sun.security.action.GetPropertyAction;
4645
import sun.security.ssl.SSLExtension.ExtensionConsumer;
@@ -355,13 +354,7 @@ public byte[] produce(ConnectionContext context,
355354
return new byte[0];
356355
}
357356

358-
if (chc.localSupportedCertSignAlgs == null) {
359-
chc.localSupportedCertSignAlgs =
360-
SignatureScheme.getSupportedAlgorithms(
361-
chc.sslConfig,
362-
chc.algorithmConstraints, chc.activeProtocols,
363-
CERTIFICATE_SCOPE);
364-
}
357+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
365358

366359
return chc.resumingSession.getPskIdentity();
367360
}

‎src/java.base/share/classes/sun/security/ssl/SignatureAlgorithmsExtension.java‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -189,13 +189,7 @@ public byte[] produce(ConnectionContext context,
189189
}
190190

191191
// Produce the extension.
192-
if (chc.localSupportedSignAlgs == null) {
193-
chc.localSupportedSignAlgs =
194-
SignatureScheme.getSupportedAlgorithms(
195-
chc.sslConfig,
196-
chc.algorithmConstraints, chc.activeProtocols,
197-
HANDSHAKE_SCOPE);
198-
}
192+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
199193

200194
int vectorLen = SignatureScheme.sizeInRecord() *
201195
chc.localSupportedSignAlgs.size();
@@ -397,18 +391,14 @@ public byte[] produce(ConnectionContext context,
397391
}
398392

399393
// Produce the extension.
400-
List<SignatureScheme> sigAlgs =
401-
SignatureScheme.getSupportedAlgorithms(
402-
shc.sslConfig,
403-
shc.algorithmConstraints,
404-
List.of(shc.negotiatedProtocol),
405-
HANDSHAKE_SCOPE);
406-
407-
int vectorLen = SignatureScheme.sizeInRecord() * sigAlgs.size();
394+
// localSupportedSignAlgs has been already updated when we
395+
// set the negotiated protocol.
396+
int vectorLen = SignatureScheme.sizeInRecord()
397+
* shc.localSupportedSignAlgs.size();
408398
byte[] extData = new byte[vectorLen + 2];
409399
ByteBuffer m = ByteBuffer.wrap(extData);
410400
Record.putInt16(m, vectorLen);
411-
for (SignatureScheme ss : sigAlgs) {
401+
for (SignatureScheme ss : shc.localSupportedSignAlgs) {
412402
Record.putInt16(m, ss.id);
413403
}
414404

0 commit comments

Comments
 (0)