Skip to content

Commit

Permalink
8231780: Better TLS messaging support
Browse files Browse the repository at this point in the history
Reviewed-by: ascarpino, rhalade, mschoene
  • Loading branch information
Jamil Nimeh committed Oct 29, 2019
1 parent a0f8feb commit c5f884c
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/java.base/share/classes/sun/security/ssl/Alert.java
Expand Up @@ -271,8 +271,14 @@ public void consume(ConnectionContext context,
ClientAuthType.CLIENT_AUTH_REQUESTED)) { ClientAuthType.CLIENT_AUTH_REQUESTED)) {
throw tc.fatal(Alert.HANDSHAKE_FAILURE, throw tc.fatal(Alert.HANDSHAKE_FAILURE,
"received handshake warning: " + alert.description); "received handshake warning: " + alert.description);
} // Otherwise, ignore the warning } else {
} // Otherwise, ignore the warning. // Otherwise ignore the warning but remove the
// CertificateVerify handshake consumer so the state
// machine doesn't expect it.
tc.handshakeContext.handshakeConsumers.remove(
SSLHandshake.CERTIFICATE_VERIFY.id);
}
} // Otherwise, ignore the warning
} else { // fatal or unknown } else { // fatal or unknown
String diagnostic; String diagnostic;
if (alert == null) { if (alert == null) {
Expand Down
Expand Up @@ -371,6 +371,10 @@ private void onCertificate(ServerHandshakeContext shc,
T12CertificateMessage certificateMessage )throws IOException { T12CertificateMessage certificateMessage )throws IOException {
List<byte[]> encodedCerts = certificateMessage.encodedCertChain; List<byte[]> encodedCerts = certificateMessage.encodedCertChain;
if (encodedCerts == null || encodedCerts.isEmpty()) { if (encodedCerts == null || encodedCerts.isEmpty()) {
// For empty Certificate messages, we should not expect
// a CertificateVerify message to follow
shc.handshakeConsumers.remove(
SSLHandshake.CERTIFICATE_VERIFY.id);
if (shc.sslConfig.clientAuthType != if (shc.sslConfig.clientAuthType !=
ClientAuthType.CLIENT_AUTH_REQUESTED) { ClientAuthType.CLIENT_AUTH_REQUESTED) {
// unexpected or require client authentication // unexpected or require client authentication
Expand Down Expand Up @@ -1165,6 +1169,10 @@ private void onConsumeCertificate(ServerHandshakeContext shc,
T13CertificateMessage certificateMessage )throws IOException { T13CertificateMessage certificateMessage )throws IOException {
if (certificateMessage.certEntries == null || if (certificateMessage.certEntries == null ||
certificateMessage.certEntries.isEmpty()) { certificateMessage.certEntries.isEmpty()) {
// For empty Certificate messages, we should not expect
// a CertificateVerify message to follow
shc.handshakeConsumers.remove(
SSLHandshake.CERTIFICATE_VERIFY.id);
if (shc.sslConfig.clientAuthType == CLIENT_AUTH_REQUIRED) { if (shc.sslConfig.clientAuthType == CLIENT_AUTH_REQUIRED) {
throw shc.conContext.fatal(Alert.BAD_CERTIFICATE, throw shc.conContext.fatal(Alert.BAD_CERTIFICATE,
"Empty client certificate chain"); "Empty client certificate chain");
Expand Down
Expand Up @@ -287,6 +287,17 @@ public void consume(ConnectionContext context,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// The consuming happens in server side only. // The consuming happens in server side only.
ServerHandshakeContext shc = (ServerHandshakeContext)context; ServerHandshakeContext shc = (ServerHandshakeContext)context;

// Clean up this consumer
shc.handshakeConsumers.remove(SSLHandshake.CERTIFICATE_VERIFY.id);

// Ensure that the CV message follows the CKE
if (shc.handshakeConsumers.containsKey(
SSLHandshake.CLIENT_KEY_EXCHANGE.id)) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE,
"Unexpected CertificateVerify handshake message");
}

S30CertificateVerifyMessage cvm = S30CertificateVerifyMessage cvm =
new S30CertificateVerifyMessage(shc, message); new S30CertificateVerifyMessage(shc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
Expand Down Expand Up @@ -529,6 +540,17 @@ public void consume(ConnectionContext context,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// The consuming happens in server side only. // The consuming happens in server side only.
ServerHandshakeContext shc = (ServerHandshakeContext)context; ServerHandshakeContext shc = (ServerHandshakeContext)context;

// Clean up this consumer
shc.handshakeConsumers.remove(SSLHandshake.CERTIFICATE_VERIFY.id);

// Ensure that the CV message follows the CKE
if (shc.handshakeConsumers.containsKey(
SSLHandshake.CLIENT_KEY_EXCHANGE.id)) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE,
"Unexpected CertificateVerify handshake message");
}

T10CertificateVerifyMessage cvm = T10CertificateVerifyMessage cvm =
new T10CertificateVerifyMessage(shc, message); new T10CertificateVerifyMessage(shc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
Expand Down Expand Up @@ -767,6 +789,17 @@ public void consume(ConnectionContext context,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// The consuming happens in server side only. // The consuming happens in server side only.
ServerHandshakeContext shc = (ServerHandshakeContext)context; ServerHandshakeContext shc = (ServerHandshakeContext)context;

// Clean up this consumer
shc.handshakeConsumers.remove(SSLHandshake.CERTIFICATE_VERIFY.id);

// Ensure that the CV message follows the CKE
if (shc.handshakeConsumers.containsKey(
SSLHandshake.CLIENT_KEY_EXCHANGE.id)) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE,
"Unexpected CertificateVerify handshake message");
}

T12CertificateVerifyMessage cvm = T12CertificateVerifyMessage cvm =
new T12CertificateVerifyMessage(shc, message); new T12CertificateVerifyMessage(shc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
Expand Down Expand Up @@ -1120,6 +1153,10 @@ public void consume(ConnectionContext context,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// The producing happens in handshake context only. // The producing happens in handshake context only.
HandshakeContext hc = (HandshakeContext)context; HandshakeContext hc = (HandshakeContext)context;

// Clean up this consumer
hc.handshakeConsumers.remove(SSLHandshake.CERTIFICATE_VERIFY.id);

T13CertificateVerifyMessage cvm = T13CertificateVerifyMessage cvm =
new T13CertificateVerifyMessage(hc, message); new T13CertificateVerifyMessage(hc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
Expand Down
30 changes: 30 additions & 0 deletions src/java.base/share/classes/sun/security/ssl/Finished.java
Expand Up @@ -589,6 +589,16 @@ private void onConsumeFinished(ClientHandshakeContext chc,


private void onConsumeFinished(ServerHandshakeContext shc, private void onConsumeFinished(ServerHandshakeContext shc,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// Make sure that any expected CertificateVerify message
// has been received and processed.
if (!shc.isResumption) {
if (shc.handshakeConsumers.containsKey(
SSLHandshake.CERTIFICATE_VERIFY.id)) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE,
"Unexpected Finished handshake message");
}
}

FinishedMessage fm = new FinishedMessage(shc, message); FinishedMessage fm = new FinishedMessage(shc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine( SSLLogger.fine(
Expand Down Expand Up @@ -883,6 +893,16 @@ public void consume(ConnectionContext context,


private void onConsumeFinished(ClientHandshakeContext chc, private void onConsumeFinished(ClientHandshakeContext chc,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// Make sure that any expected CertificateVerify message
// has been received and processed.
if (!chc.isResumption) {
if (chc.handshakeConsumers.containsKey(
SSLHandshake.CERTIFICATE_VERIFY.id)) {
throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE,
"Unexpected Finished handshake message");
}
}

FinishedMessage fm = new FinishedMessage(chc, message); FinishedMessage fm = new FinishedMessage(chc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine( SSLLogger.fine(
Expand Down Expand Up @@ -1005,6 +1025,16 @@ private void onConsumeFinished(ClientHandshakeContext chc,


private void onConsumeFinished(ServerHandshakeContext shc, private void onConsumeFinished(ServerHandshakeContext shc,
ByteBuffer message) throws IOException { ByteBuffer message) throws IOException {
// Make sure that any expected CertificateVerify message
// has been received and processed.
if (!shc.isResumption) {
if (shc.handshakeConsumers.containsKey(
SSLHandshake.CERTIFICATE_VERIFY.id)) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE,
"Unexpected Finished handshake message");
}
}

FinishedMessage fm = new FinishedMessage(shc, message); FinishedMessage fm = new FinishedMessage(shc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine( SSLLogger.fine(
Expand Down

0 comments on commit c5f884c

Please sign in to comment.