Skip to content

Commit c6e19d6

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: 1cdf8f5497f2b986c13a1c263d806a31d67fe015
1 parent c3f9905 commit c6e19d6

File tree

16 files changed

+354
-186
lines changed

16 files changed

+354
-186
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: 51 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -701,48 +701,6 @@ private static void checkClientCerts(ServerHandshakeContext shc,
701701
}
702702
}
703703

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

748706
/**
@@ -1339,39 +1297,59 @@ private static X509Certificate[] checkServerCerts(
13391297
return certs;
13401298
}
13411299

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

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

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

642-
if (shc.localSupportedSignAlgs == null) {
643-
shc.localSupportedSignAlgs =
644-
SignatureScheme.getSupportedAlgorithms(
645-
shc.sslConfig,
646-
shc.algorithmConstraints, shc.activeProtocols,
647-
HANDSHAKE_SCOPE);
648-
}
649-
650-
if (shc.localSupportedCertSignAlgs == null) {
651-
shc.localSupportedCertSignAlgs =
652-
SignatureScheme.getSupportedAlgorithms(
653-
shc.sslConfig,
654-
shc.algorithmConstraints, shc.activeProtocols,
655-
CERTIFICATE_SCOPE);
656-
}
657-
658642
// According to TLSv1.2 RFC, CertificateRequest message must
659643
// contain signature schemes supported for both:
660644
// handshake signatures and certificate signatures.
645+
// localSupportedSignAlgs and localSupportedCertSignAlgs have been
646+
// already updated when we set the negotiated protocol.
661647
List<SignatureScheme> certReqSignAlgs =
662648
new ArrayList<>(shc.localSupportedSignAlgs);
663649
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
@@ -826,6 +826,10 @@ private void onClientHello(ServerHandshakeContext context,
826826
"Negotiated protocol version: " + negotiatedProtocol.name);
827827
}
828828

829+
// Protocol version is negotiated, update locally supported
830+
// signature schemes according to the protocol being used.
831+
SignatureScheme.updateHandshakeLocalSupportedAlgs(context);
832+
829833
// Consume the handshake message for the specific protocol version.
830834
if (negotiatedProtocol.isDTLS) {
831835
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
@@ -45,7 +45,6 @@
4545
import sun.security.util.HexDumpEncoder;
4646

4747
import static sun.security.ssl.SSLExtension.*;
48-
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;
4948

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

458451
// Validate the required client authentication.
459452
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;
@@ -277,22 +274,6 @@ public byte[] produce(ConnectionContext context,
277274
"Not resumption, and no new session is allowed");
278275
}
279276

280-
if (shc.localSupportedSignAlgs == null) {
281-
shc.localSupportedSignAlgs =
282-
SignatureScheme.getSupportedAlgorithms(
283-
shc.sslConfig,
284-
shc.algorithmConstraints, shc.activeProtocols,
285-
HANDSHAKE_SCOPE);
286-
}
287-
288-
if (shc.localSupportedCertSignAlgs == null) {
289-
shc.localSupportedCertSignAlgs =
290-
SignatureScheme.getSupportedAlgorithms(
291-
shc.sslConfig,
292-
shc.algorithmConstraints, shc.activeProtocols,
293-
CERTIFICATE_SCOPE);
294-
}
295-
296277
SSLSessionImpl session =
297278
new SSLSessionImpl(shc, CipherSuite.C_NULL);
298279
session.setMaximumPacketSize(shc.sslConfig.maximumPacketSize);
@@ -527,22 +508,6 @@ public byte[] produce(ConnectionContext context,
527508
"Not resumption, and no new session is allowed");
528509
}
529510

530-
if (shc.localSupportedSignAlgs == null) {
531-
shc.localSupportedSignAlgs =
532-
SignatureScheme.getSupportedAlgorithms(
533-
shc.sslConfig,
534-
shc.algorithmConstraints, shc.activeProtocols,
535-
HANDSHAKE_SCOPE);
536-
}
537-
538-
if (shc.localSupportedCertSignAlgs == null) {
539-
shc.localSupportedCertSignAlgs =
540-
SignatureScheme.getSupportedAlgorithms(
541-
shc.sslConfig,
542-
shc.algorithmConstraints, shc.activeProtocols,
543-
CERTIFICATE_SCOPE);
544-
}
545-
546511
SSLSessionImpl session =
547512
new SSLSessionImpl(shc, CipherSuite.C_NULL);
548513
session.setMaximumPacketSize(shc.sslConfig.maximumPacketSize);
@@ -964,6 +929,10 @@ private void onHelloRetryRequest(ClientHandshakeContext chc,
964929
"Negotiated protocol version: " + serverVersion.name);
965930
}
966931

932+
// Protocol version is negotiated, update locally supported
933+
// signature schemes according to the protocol being used.
934+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
935+
967936
// TLS 1.3 key share extension may have produced client
968937
// possessions for TLS 1.3 key exchanges.
969938
//
@@ -1015,6 +984,10 @@ private void onServerHello(ClientHandshakeContext chc,
1015984
"Negotiated protocol version: " + serverVersion.name);
1016985
}
1017986

987+
// Protocol version is negotiated, update locally supported
988+
// signature schemes according to the protocol being used.
989+
SignatureScheme.updateHandshakeLocalSupportedAlgs(chc);
990+
1018991
if (serverHello.serverRandom.isVersionDowngrade(chc)) {
1019992
throw chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
1020993
"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
@@ -41,7 +41,6 @@
4141

4242
import static sun.security.ssl.SSLExtension.CH_SESSION_TICKET;
4343
import static sun.security.ssl.SSLExtension.SH_SESSION_TICKET;
44-
import static sun.security.ssl.SignatureScheme.CERTIFICATE_SCOPE;
4544

4645
import java.io.IOException;
4746
import java.nio.ByteBuffer;
@@ -356,13 +355,7 @@ public byte[] produce(ConnectionContext context,
356355
return new byte[0];
357356
}
358357

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

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

‎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)