8212885: TLS 1.3 resumed session does not retain peer certificate chain
Reviewed-by: xuelei, wetmore
This commit is contained in:
parent
65dc116bf6
commit
acd81b508e
@ -260,9 +260,8 @@ final class NewSessionTicket {
|
||||
// create and cache the new session
|
||||
// The new session must be a child of the existing session so
|
||||
// they will be invalidated together, etc.
|
||||
SSLSessionImpl sessionCopy = new SSLSessionImpl(shc,
|
||||
shc.handshakeSession.getSuite(), newId,
|
||||
shc.handshakeSession.getCreationTime());
|
||||
SSLSessionImpl sessionCopy =
|
||||
new SSLSessionImpl(shc.handshakeSession, newId);
|
||||
shc.handshakeSession.addChild(sessionCopy);
|
||||
sessionCopy.setPreSharedKey(psk);
|
||||
sessionCopy.setPskIdentity(newId.getId());
|
||||
@ -375,9 +374,8 @@ final class NewSessionTicket {
|
||||
// they will be invalidated together, etc.
|
||||
SessionId newId =
|
||||
new SessionId(true, hc.sslContext.getSecureRandom());
|
||||
SSLSessionImpl sessionCopy = new SSLSessionImpl(
|
||||
hc, sessionToSave.getSuite(), newId,
|
||||
sessionToSave.getCreationTime());
|
||||
SSLSessionImpl sessionCopy = new SSLSessionImpl(sessionToSave,
|
||||
newId);
|
||||
sessionToSave.addChild(sessionCopy);
|
||||
sessionCopy.setPreSharedKey(psk);
|
||||
sessionCopy.setTicketAgeAdd(nstm.ticketAgeAdd);
|
||||
|
@ -50,9 +50,6 @@ final class PostHandshakeContext extends HandshakeContext {
|
||||
this.localSupportedSignAlgs = new ArrayList<SignatureScheme>(
|
||||
context.conSession.getLocalSupportedSignatureSchemes());
|
||||
|
||||
this.requestedServerNames =
|
||||
context.conSession.getRequestedServerNames();
|
||||
|
||||
handshakeConsumers = new LinkedHashMap<>(consumers);
|
||||
handshakeFinished = true;
|
||||
}
|
||||
|
@ -415,6 +415,16 @@ final class PreSharedKeyExtension {
|
||||
result = false;
|
||||
}
|
||||
|
||||
// Make sure that the server handshake context's localSupportedSignAlgs
|
||||
// field is populated. This is particularly important when
|
||||
// client authentication was used in an initial session and it is
|
||||
// now being resumed.
|
||||
if (shc.localSupportedSignAlgs == null) {
|
||||
shc.localSupportedSignAlgs =
|
||||
SignatureScheme.getSupportedAlgorithms(
|
||||
shc.algorithmConstraints, shc.activeProtocols);
|
||||
}
|
||||
|
||||
// Validate the required client authentication.
|
||||
if (result &&
|
||||
(shc.sslConfig.clientAuthType == CLIENT_AUTH_REQUIRED)) {
|
||||
@ -763,7 +773,7 @@ final class PreSharedKeyExtension {
|
||||
SecretKey earlySecret = hkdf.extract(zeros, psk, "TlsEarlySecret");
|
||||
|
||||
byte[] label = ("tls13 res binder").getBytes();
|
||||
MessageDigest md = MessageDigest.getInstance(hashAlg.toString());;
|
||||
MessageDigest md = MessageDigest.getInstance(hashAlg.name);
|
||||
byte[] hkdfInfo = SSLSecretDerivation.createHkdfInfo(
|
||||
label, md.digest(new byte[0]), hashAlg.hashLength);
|
||||
return hkdf.expand(earlySecret,
|
||||
|
@ -154,6 +154,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
|
||||
this.useExtendedMasterSecret = false;
|
||||
this.creationTime = System.currentTimeMillis();
|
||||
this.identificationProtocol = null;
|
||||
this.boundValues = new ConcurrentHashMap<>();
|
||||
}
|
||||
|
||||
/*
|
||||
@ -204,6 +205,41 @@ final class SSLSessionImpl extends ExtendedSSLSession {
|
||||
}
|
||||
this.creationTime = creationTime;
|
||||
this.identificationProtocol = hc.sslConfig.identificationProtocol;
|
||||
this.boundValues = new ConcurrentHashMap<>();
|
||||
|
||||
if (SSLLogger.isOn && SSLLogger.isOn("session")) {
|
||||
SSLLogger.finest("Session initialized: " + this);
|
||||
}
|
||||
}
|
||||
|
||||
SSLSessionImpl(SSLSessionImpl baseSession, SessionId newId) {
|
||||
this.protocolVersion = baseSession.getProtocolVersion();
|
||||
this.cipherSuite = baseSession.cipherSuite;
|
||||
this.sessionId = newId;
|
||||
this.host = baseSession.getPeerHost();
|
||||
this.port = baseSession.getPeerPort();
|
||||
this.localSupportedSignAlgs =
|
||||
baseSession.localSupportedSignAlgs == null ?
|
||||
Collections.emptySet() :
|
||||
Collections.unmodifiableCollection(
|
||||
baseSession.localSupportedSignAlgs);
|
||||
this.peerSupportedSignAlgs =
|
||||
baseSession.getPeerSupportedSignatureAlgorithms();
|
||||
this.serverNameIndication = baseSession.serverNameIndication;
|
||||
this.requestedServerNames = baseSession.getRequestedServerNames();
|
||||
this.masterSecret = baseSession.getMasterSecret();
|
||||
this.useExtendedMasterSecret = baseSession.useExtendedMasterSecret;
|
||||
this.creationTime = baseSession.getCreationTime();
|
||||
this.lastUsedTime = System.currentTimeMillis();
|
||||
this.identificationProtocol = baseSession.getIdentificationProtocol();
|
||||
this.localCerts = baseSession.localCerts;
|
||||
this.peerCerts = baseSession.peerCerts;
|
||||
this.statusResponses = baseSession.statusResponses;
|
||||
this.resumptionMasterSecret = baseSession.resumptionMasterSecret;
|
||||
this.context = baseSession.context;
|
||||
this.negotiatedMaxFragLen = baseSession.negotiatedMaxFragLen;
|
||||
this.maximumPacketSize = baseSession.maximumPacketSize;
|
||||
this.boundValues = baseSession.boundValues;
|
||||
|
||||
if (SSLLogger.isOn && SSLLogger.isOn("session")) {
|
||||
SSLLogger.finest("Session initialized: " + this);
|
||||
@ -772,8 +808,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
|
||||
* key and the calling security context. This is important since
|
||||
* sessions can be shared across different protection domains.
|
||||
*/
|
||||
private final ConcurrentHashMap<SecureKey, Object> boundValues =
|
||||
new ConcurrentHashMap<>();
|
||||
private final ConcurrentHashMap<SecureKey, Object> boundValues;
|
||||
|
||||
/**
|
||||
* Assigns a session value. Session change events are given if
|
||||
|
@ -23,7 +23,7 @@
|
||||
|
||||
/*
|
||||
* @test
|
||||
* @bug 8206929
|
||||
* @bug 8206929 8212885
|
||||
* @summary ensure that client only resumes a session if certain properties
|
||||
* of the session are compatible with the new connection
|
||||
* @run main/othervm -Djdk.tls.client.protocols=TLSv1.2 ResumeChecksClient BASIC
|
||||
@ -80,7 +80,7 @@ public class ResumeChecksClient {
|
||||
while (!server.started) {
|
||||
Thread.yield();
|
||||
}
|
||||
connect(sslContext, server.port, mode, false);
|
||||
SSLSession firstSession = connect(sslContext, server.port, mode, false);
|
||||
|
||||
server.signal();
|
||||
long secondStartTime = System.currentTimeMillis();
|
||||
@ -93,9 +93,7 @@ public class ResumeChecksClient {
|
||||
switch (mode) {
|
||||
case BASIC:
|
||||
// fail if session is not resumed
|
||||
if (secondSession.getCreationTime() > secondStartTime) {
|
||||
throw new RuntimeException("Session was not reused");
|
||||
}
|
||||
checkResumedSession(firstSession, secondSession);
|
||||
break;
|
||||
case VERSION_2_TO_3:
|
||||
case VERSION_3_TO_2:
|
||||
@ -124,14 +122,17 @@ public class ResumeChecksClient {
|
||||
return !a.toLowerCase().contains(alg.toLowerCase());
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean permits(Set<CryptoPrimitive> primitives, Key key) {
|
||||
return true;
|
||||
}
|
||||
@Override
|
||||
public boolean permits(Set<CryptoPrimitive> primitives,
|
||||
String algorithm, AlgorithmParameters parameters) {
|
||||
|
||||
return test(algorithm);
|
||||
}
|
||||
@Override
|
||||
public boolean permits(Set<CryptoPrimitive> primitives,
|
||||
String algorithm, Key key, AlgorithmParameters parameters) {
|
||||
|
||||
@ -205,6 +206,81 @@ public class ResumeChecksClient {
|
||||
}
|
||||
}
|
||||
|
||||
private static void checkResumedSession(SSLSession initSession,
|
||||
SSLSession resSession) throws Exception {
|
||||
StringBuilder diffLog = new StringBuilder();
|
||||
|
||||
// Initial and resumed SSLSessions should have the same creation
|
||||
// times so they get invalidated together.
|
||||
long initCt = initSession.getCreationTime();
|
||||
long resumeCt = resSession.getCreationTime();
|
||||
if (initCt != resumeCt) {
|
||||
diffLog.append("Session creation time is different. Initial: ").
|
||||
append(initCt).append(", Resumed: ").append(resumeCt).
|
||||
append("\n");
|
||||
}
|
||||
|
||||
// Ensure that peer and local certificate lists are preserved
|
||||
if (!Arrays.equals(initSession.getLocalCertificates(),
|
||||
resSession.getLocalCertificates())) {
|
||||
diffLog.append("Local certificate mismatch between initial " +
|
||||
"and resumed sessions\n");
|
||||
}
|
||||
|
||||
if (!Arrays.equals(initSession.getPeerCertificates(),
|
||||
resSession.getPeerCertificates())) {
|
||||
diffLog.append("Peer certificate mismatch between initial " +
|
||||
"and resumed sessions\n");
|
||||
}
|
||||
|
||||
// Buffer sizes should also be the same
|
||||
if (initSession.getApplicationBufferSize() !=
|
||||
resSession.getApplicationBufferSize()) {
|
||||
diffLog.append(String.format(
|
||||
"App Buffer sizes differ: Init: %d, Res: %d\n",
|
||||
initSession.getApplicationBufferSize(),
|
||||
resSession.getApplicationBufferSize()));
|
||||
}
|
||||
|
||||
if (initSession.getPacketBufferSize() !=
|
||||
resSession.getPacketBufferSize()) {
|
||||
diffLog.append(String.format(
|
||||
"Packet Buffer sizes differ: Init: %d, Res: %d\n",
|
||||
initSession.getPacketBufferSize(),
|
||||
resSession.getPacketBufferSize()));
|
||||
}
|
||||
|
||||
// Cipher suite should match
|
||||
if (!initSession.getCipherSuite().equals(
|
||||
resSession.getCipherSuite())) {
|
||||
diffLog.append(String.format(
|
||||
"CipherSuite does not match - Init: %s, Res: %s\n",
|
||||
initSession.getCipherSuite(), resSession.getCipherSuite()));
|
||||
}
|
||||
|
||||
// Peer host/port should match
|
||||
if (!initSession.getPeerHost().equals(resSession.getPeerHost()) ||
|
||||
initSession.getPeerPort() != resSession.getPeerPort()) {
|
||||
diffLog.append(String.format(
|
||||
"Host/Port mismatch - Init: %s/%d, Res: %s/%d\n",
|
||||
initSession.getPeerHost(), initSession.getPeerPort(),
|
||||
resSession.getPeerHost(), resSession.getPeerPort()));
|
||||
}
|
||||
|
||||
// Check protocol
|
||||
if (!initSession.getProtocol().equals(resSession.getProtocol())) {
|
||||
diffLog.append(String.format(
|
||||
"Protocol mismatch - Init: %s, Res: %s\n",
|
||||
initSession.getProtocol(), resSession.getProtocol()));
|
||||
}
|
||||
|
||||
// If the StringBuilder has any data in it then one of the checks
|
||||
// above failed and we should throw an exception.
|
||||
if (diffLog.length() > 0) {
|
||||
throw new RuntimeException(diffLog.toString());
|
||||
}
|
||||
}
|
||||
|
||||
private static Server startServer() {
|
||||
Server server = new Server();
|
||||
new Thread(server).start();
|
||||
@ -233,6 +309,7 @@ public class ResumeChecksClient {
|
||||
notify();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void run() {
|
||||
try {
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user