8240871: SSLEngine handshake status immediately after the handshake can be NOT_HANDSHAKING rather than FINISHED with TLSv1.3

Reviewed-by: ascarpino
This commit is contained in:
Xue-Lei Andrew Fan 2020-05-29 13:48:13 -07:00
parent 1d4bd253e4
commit 7514ad9ad0
8 changed files with 165 additions and 69 deletions

@ -482,7 +482,8 @@ final class Finished {
shc.conContext.inputRecord.expectingFinishFlight();
} else {
// Set the session's context based on stateless/cache status
if (shc.handshakeSession.isStatelessable(shc)) {
if (shc.statelessResumption &&
shc.handshakeSession.isStatelessable()) {
shc.handshakeSession.setContext((SSLSessionContextImpl)
shc.sslContext.engineGetServerSessionContext());
} else {
@ -1140,12 +1141,7 @@ final class Finished {
//
// produce
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Sending new session ticket");
}
NewSessionTicket.kickstartProducer.produce(shc);
NewSessionTicket.t13PosthandshakeProducer.produce(shc);
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020, 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
@ -416,6 +416,41 @@ abstract class HandshakeContext implements ConnectionContext {
handshakeType,
fragment
));
// For TLS 1.2 and previous versions, the ChangeCipherSpec
// message is always delivered before the Finished handshake
// message. ChangeCipherSpec is not a handshake message,
// and cannot be wrapped in one TLS record. The processing
// of Finished handshake message is unlikely to be delegated.
//
// However, for TLS 1.3 there is no non-handshake messages
// delivered immediately before Finished message. Then, the
// 'hasDelegated' could be true, and the Finished message is
// handled in a delegated action.
//
// The HandshakeStatus.FINISHED for the final handshake flight
// could be used to determine if the handshake has completed.
// Per the HandshakeStatus.FINISHED specification, it is only
// generated by call to SSLEngine.wrap()/unwrap(). It is
// unlikely to change the spec, so we cannot use delegated
// action and SSLEngine.getHandshakeStatus() to indicate the
// FINISHED handshake status.
//
// To workaround this special user case, the follow-on call to
// SSLEngine.wrap() method will return HandshakeStatus.FINISHED
// status if needed.
//
// As the final handshake flight is always delivered from the
// client side, so we only need to take care of the server
// dispatching processes.
//
// See also the note on
// TransportContext.needHandshakeFinishedStatus.
if (hasDelegated &&
!conContext.sslConfig.isClientMode &&
handshakeType == SSLHandshake.FINISHED.id) {
conContext.hasDelegatedFinished = true;
}
} else {
dispatch(handshakeType, plaintext.fragment);
}

@ -33,6 +33,7 @@ import java.text.MessageFormat;
import java.util.Locale;
import javax.crypto.SecretKey;
import javax.net.ssl.SSLHandshakeException;
import sun.security.ssl.PskKeyExchangeModesExtension.PskKeyExchangeMode;
import sun.security.ssl.PskKeyExchangeModesExtension.PskKeyExchangeModesSpec;
import sun.security.ssl.SessionTicketExtension.SessionTicketSpec;
import sun.security.ssl.SSLHandshake.HandshakeMessage;
@ -49,8 +50,8 @@ final class NewSessionTicket {
new T13NewSessionTicketConsumer();
static final SSLConsumer handshake12Consumer =
new T12NewSessionTicketConsumer();
static final SSLProducer kickstartProducer =
new NewSessionTicketKickstartProducer();
static final SSLProducer t13PosthandshakeProducer =
new T13NewSessionTicketProducer();
static final HandshakeProducer handshake12Producer =
new T12NewSessionTicketProducer();
@ -205,7 +206,7 @@ final class NewSessionTicket {
if (ticket.length == 0) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"No ticket in the NewSessionTicket handshake message");
"No ticket in the NewSessionTicket handshake message");
}
}
@ -307,9 +308,9 @@ final class NewSessionTicket {
}
private static final
class NewSessionTicketKickstartProducer implements SSLProducer {
class T13NewSessionTicketProducer implements SSLProducer {
// Prevent instantiation of this class.
private NewSessionTicketKickstartProducer() {
private T13NewSessionTicketProducer() {
// blank
}
@ -317,10 +318,26 @@ final class NewSessionTicket {
public byte[] produce(ConnectionContext context) throws IOException {
HandshakeContext hc = (HandshakeContext)context;
// See note on TransportContext.needHandshakeFinishedStatus.
//
// Set to need handshake finished status. Reset it later if a
// session ticket get delivered.
if (hc.conContext.hasDelegatedFinished) {
// Reset, as the delegated finished case will be handled later.
hc.conContext.hasDelegatedFinished = false;
hc.conContext.needHandshakeFinishedStatus = true;
}
// The producing happens in server side only.
if (hc instanceof ServerHandshakeContext) {
// Is this session resumable?
if (!hc.handshakeSession.isRejoinable()) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"No session ticket produced: " +
"session is not resumable");
}
return null;
}
@ -332,16 +349,26 @@ final class NewSessionTicket {
PskKeyExchangeModesSpec pkemSpec =
(PskKeyExchangeModesSpec) hc.handshakeExtensions.get(
SSLExtension.PSK_KEY_EXCHANGE_MODES);
if (pkemSpec == null || !pkemSpec.contains(
PskKeyExchangeModesExtension.PskKeyExchangeMode.PSK_DHE_KE)) {
// Client doesn't support PSK with (EC)DHE key establishment.
if (pkemSpec == null ||
!pkemSpec.contains(PskKeyExchangeMode.PSK_DHE_KE)) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"No session ticket produced: " +
"client does not support psk_dhe_ke");
}
return null;
}
} else { // PostHandshakeContext
// Check if we have sent a PSK already, then we know it is using a
// allowable PSK exchange key mode
} else { // PostHandshakeContext
// Check if we have sent a PSK already, then we know it is
// using a allowable PSK exchange key mode.
if (!hc.handshakeSession.isPSKable()) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"No session ticket produced: " +
"No session ticket allowed in this session");
}
return null;
}
}
@ -357,8 +384,10 @@ final class NewSessionTicket {
if (resumptionMasterSecret == null) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Session has no resumption secret. No ticket sent.");
"No session ticket produced: " +
"no resumption secret");
}
return null;
}
@ -373,8 +402,10 @@ final class NewSessionTicket {
if (sessionTimeoutSeconds > MAX_TICKET_LIFETIME) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Session timeout is too long. No ticket sent.");
"No session ticket produced: " +
"session timeout");
}
return null;
}
@ -386,7 +417,8 @@ final class NewSessionTicket {
sessionCopy.setPskIdentity(newId.getId());
// If a stateless ticket is allowed, attempt to make one
if (hc.handshakeSession.isStatelessable(hc)) {
if (hc.statelessResumption &&
hc.handshakeSession.isStatelessable()) {
nstm = new T13NewSessionTicketMessage(hc,
sessionTimeoutSeconds,
hc.sslContext.getSecureRandom(),
@ -398,19 +430,21 @@ final class NewSessionTicket {
} else {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Produced NewSessionTicket stateless " +
"handshake message", nstm);
"Produced NewSessionTicket stateless " +
"post-handshake message", nstm);
}
}
}
// If a session cache ticket is being used, make one
if (!hc.handshakeSession.isStatelessable(hc)) {
if (!hc.statelessResumption ||
!hc.handshakeSession.isStatelessable()) {
nstm = new T13NewSessionTicketMessage(hc, sessionTimeoutSeconds,
hc.sslContext.getSecureRandom(), nonceArr,
newId.getId());
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Produced NewSessionTicket handshake message",
"Produced NewSessionTicket post-handshake message",
nstm);
}
@ -427,11 +461,20 @@ final class NewSessionTicket {
// should never be null
nstm.write(hc.handshakeOutput);
hc.handshakeOutput.flush();
// See note on TransportContext.needHandshakeFinishedStatus.
//
// Reset the needHandshakeFinishedStatus flag. The delivery
// of this post-handshake message will indicate the FINISHED
// handshake status. It is not needed to have a follow-on
// SSLEngine.wrap() any longer.
if (hc.conContext.needHandshakeFinishedStatus) {
hc.conContext.needHandshakeFinishedStatus = false;
}
}
if (hc.negotiatedProtocol.useTLS13PlusSpec()) {
hc.conContext.finishPostHandshake();
}
// clean the post handshake context
hc.conContext.finishPostHandshake();
// The message has been delivered.
return null;
@ -483,7 +526,8 @@ final class NewSessionTicket {
new SessionTicketSpec().encrypt(shc, sessionCopy));
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Produced NewSessionTicket stateless handshake message", nstm);
"Produced NewSessionTicket stateless handshake message",
nstm);
}
// Output the handshake message.
@ -521,7 +565,7 @@ final class NewSessionTicket {
new T13NewSessionTicketMessage(hc, message);
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Consuming NewSessionTicket message", nstm);
"Consuming NewSessionTicket message", nstm);
}
SSLSessionContextImpl sessionCache = (SSLSessionContextImpl)
@ -532,8 +576,8 @@ final class NewSessionTicket {
nstm.ticketLifetime > MAX_TICKET_LIFETIME) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Discarding NewSessionTicket with lifetime "
+ nstm.ticketLifetime, nstm);
"Discarding NewSessionTicket with lifetime " +
nstm.ticketLifetime, nstm);
}
sessionCache.remove(hc.handshakeSession.getSessionId());
return;
@ -542,31 +586,29 @@ final class NewSessionTicket {
if (sessionCache.getSessionTimeout() > MAX_TICKET_LIFETIME) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Session cache lifetime is too long. Discarding ticket.");
"Session cache lifetime is too long. " +
"Discarding ticket.");
}
return;
}
SSLSessionImpl sessionToSave = hc.conContext.conSession;
SecretKey psk = null;
if (hc.negotiatedProtocol.useTLS13PlusSpec()) {
SecretKey resumptionMasterSecret =
sessionToSave.getResumptionMasterSecret();
if (resumptionMasterSecret == null) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Session has no resumption master secret." +
" Ignoring ticket.");
}
return;
SecretKey resumptionMasterSecret =
sessionToSave.getResumptionMasterSecret();
if (resumptionMasterSecret == null) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Session has no resumption master secret. " +
"Ignoring ticket.");
}
// derive the PSK
psk = derivePreSharedKey(
sessionToSave.getSuite().hashAlg,
resumptionMasterSecret, nstm.getTicketNonce());
return;
}
// derive the PSK
SecretKey psk = derivePreSharedKey(
sessionToSave.getSuite().hashAlg,
resumptionMasterSecret, nstm.getTicketNonce());
// create and cache the new session
// The new session must be a child of the existing session so
// they will be invalidated together, etc.
@ -580,10 +622,8 @@ final class NewSessionTicket {
sessionCopy.setPskIdentity(nstm.ticket);
sessionCache.put(sessionCopy);
// clean handshake context
if (hc.negotiatedProtocol.useTLS13PlusSpec()) {
hc.conContext.finishPostHandshake();
}
// clean the post handshake context
hc.conContext.finishPostHandshake();
}
}
@ -615,8 +655,8 @@ final class NewSessionTicket {
nstm.ticketLifetime > MAX_TICKET_LIFETIME) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Discarding NewSessionTicket with lifetime "
+ nstm.ticketLifetime, nstm);
"Discarding NewSessionTicket with lifetime " +
nstm.ticketLifetime, nstm);
}
return;
}
@ -627,7 +667,8 @@ final class NewSessionTicket {
if (sessionCache.getSessionTimeout() > MAX_TICKET_LIFETIME) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine(
"Session cache lifetime is too long. Discarding ticket.");
"Session cache lifetime is too long. " +
"Discarding ticket.");
}
return;
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2020, 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
@ -164,6 +164,13 @@ final class SSLEngineImpl extends SSLEngine implements SSLTransport {
ByteBuffer[] srcs, int srcsOffset, int srcsLength,
ByteBuffer[] dsts, int dstsOffset, int dstsLength) throws IOException {
// See note on TransportContext.needHandshakeFinishedStatus.
if (conContext.needHandshakeFinishedStatus) {
conContext.needHandshakeFinishedStatus = false;
return new SSLEngineResult(
Status.OK, HandshakeStatus.FINISHED, 0, 0);
}
// May need to deliver cached records.
if (isOutboundDone()) {
return new SSLEngineResult(
@ -418,7 +425,7 @@ final class SSLEngineImpl extends SSLEngine implements SSLTransport {
SSLLogger.finest("trigger NST");
}
conContext.conSession.updateNST = false;
NewSessionTicket.kickstartProducer.produce(
NewSessionTicket.t13PosthandshakeProducer.produce(
new PostHandshakeContext(conContext));
return conContext.getHandshakeStatus();
}

@ -27,13 +27,11 @@ package sun.security.ssl;
import sun.security.x509.X509CertImpl;
import java.io.IOException;
import java.lang.reflect.Array;
import java.math.BigInteger;
import java.net.InetAddress;
import java.nio.ByteBuffer;
import java.security.Principal;
import java.security.PrivateKey;
import java.security.cert.CertificateEncodingException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Arrays;
@ -520,11 +518,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
// Some situations we cannot provide a stateless ticket, but after it
// has been negotiated
boolean isStatelessable(HandshakeContext hc) {
if (!hc.statelessResumption) {
return false;
}
boolean isStatelessable() {
// If there is no getMasterSecret with TLS1.2 or under, do not resume.
if (!protocolVersion.useTLS13PlusSpec() &&
getMasterSecret().getEncoded() == null) {
@ -534,6 +528,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
}
return false;
}
if (boundValues != null && boundValues.size() > 0) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.finest("There are boundValues, cannot make" +
@ -541,6 +536,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
}
return false;
}
return true;
}

@ -1537,7 +1537,7 @@ public final class SSLSocketImpl
if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
SSLLogger.finest("trigger new session ticket");
}
NewSessionTicket.kickstartProducer.produce(
NewSessionTicket.t13PosthandshakeProducer.produce(
new PostHandshakeContext(conContext));
}
}

@ -260,7 +260,8 @@ final class SessionTicketExtension {
public byte[] encrypt(HandshakeContext hc, SSLSessionImpl session) {
byte[] encrypted;
if (!hc.handshakeSession.isStatelessable(hc)) {
if (!hc.statelessResumption ||
!hc.handshakeSession.isStatelessable()) {
return new byte[0];
}

@ -64,6 +64,23 @@ final class TransportContext implements ConnectionContext {
Exception closeReason = null;
Exception delegatedThrown = null;
// For TLS 1.3 full handshake, the last handshake flight could be wrapped
// and encrypted in one record and delegated task would be used. There is
// no chance to return FINISHED handshake status with SSLEngine.(un)wrap().
// However, per the HandshakeStatus.FINISHED specification, this value is
// only generated by a call to SSLEngine.wrap()/unwrap() and it is never
// generated by SSLEngine.getHandshakeStatus().
//
// In order to workaround this case for TLS 1.3, the FINISHED status is
// present with SSLEngine.wrap() while delivering of the NewSessionTicket
// post-handshake message. If this post-handshake message is not needed,
// a follow-on SSLEngine.wrap() should be called to indicate the FINISHED
// handshake status. Although this special SSLEngine.wrap() should not
// consume or produce any application or network data.
boolean needHandshakeFinishedStatus = false;
boolean hasDelegatedFinished = false;
// negotiated security parameters
SSLSessionImpl conSession;
ProtocolVersion protocolVersion;
@ -589,6 +606,9 @@ final class TransportContext implements ConnectionContext {
// Special case that the inbound was closed, but outbound open.
return HandshakeStatus.NEED_WRAP;
} // Otherwise, both inbound and outbound are closed.
} else if (needHandshakeFinishedStatus) {
// Special case to get FINISHED status for TLS 1.3 full handshake.
return HandshakeStatus.NEED_WRAP;
}
return HandshakeStatus.NOT_HANDSHAKING;