8213202: Possible race condition in TLS 1.3 session resumption

Reviewed-by: jnimeh
This commit is contained in:
Adam Petcher 2018-11-21 15:06:13 -05:00
parent 937fe3be82
commit a5423f142c
2 changed files with 6 additions and 24 deletions

View file

@ -656,7 +656,7 @@ final class PreSharedKeyExtension {
return null; return null;
} }
SecretKey psk = pskOpt.get(); SecretKey psk = pskOpt.get();
Optional<byte[]> pskIdOpt = chc.resumingSession.getPskIdentity(); Optional<byte[]> pskIdOpt = chc.resumingSession.consumePskIdentity();
if (!pskIdOpt.isPresent()) { if (!pskIdOpt.isPresent()) {
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine( SSLLogger.fine(
@ -666,6 +666,11 @@ final class PreSharedKeyExtension {
} }
byte[] pskId = pskIdOpt.get(); byte[] pskId = pskIdOpt.get();
//The session cannot be used again. Remove it from the cache.
SSLSessionContextImpl sessionCache = (SSLSessionContextImpl)
chc.sslContext.engineGetClientSessionContext();
sessionCache.remove(chc.resumingSession.getSessionId());
if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
SSLLogger.fine( SSLLogger.fine(
"Found resumable session. Preparing PSK message."); "Found resumable session. Preparing PSK message.");
@ -828,10 +833,6 @@ final class PreSharedKeyExtension {
"Received pre_shared_key extension: ", shPsk); "Received pre_shared_key extension: ", shPsk);
} }
// The PSK identity should not be reused, even if it is
// not selected.
chc.resumingSession.consumePskIdentity();
if (shPsk.selectedIdentity != 0) { if (shPsk.selectedIdentity != 0) {
chc.conContext.fatal(Alert.ILLEGAL_PARAMETER, chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
"Selected identity index is not in correct range."); "Selected identity index is not in correct range.");
@ -841,11 +842,6 @@ final class PreSharedKeyExtension {
SSLLogger.fine( SSLLogger.fine(
"Resuming session: ", chc.resumingSession); "Resuming session: ", chc.resumingSession);
} }
// remove the session from the cache
SSLSessionContextImpl sessionCache = (SSLSessionContextImpl)
chc.sslContext.engineGetClientSessionContext();
sessionCache.remove(chc.resumingSession.getSessionId());
} }
} }
@ -860,13 +856,6 @@ final class PreSharedKeyExtension {
SSLLogger.fine("Handling pre_shared_key absence."); SSLLogger.fine("Handling pre_shared_key absence.");
} }
if (chc.handshakeExtensions.containsKey(
SSLExtension.CH_PRE_SHARED_KEY)) {
// The PSK identity should not be reused, even if it is
// not selected.
chc.resumingSession.consumePskIdentity();
}
// The server refused to resume, or the client did not // The server refused to resume, or the client did not
// request 1.3 resumption. // request 1.3 resumption.
chc.resumingSession = null; chc.resumingSession = null;

View file

@ -305,13 +305,6 @@ final class SSLSessionImpl extends ExtendedSSLSession {
return this.identificationProtocol; return this.identificationProtocol;
} }
/*
* Get the PSK identity. Take care not to use it in multiple connections.
*/
synchronized Optional<byte[]> getPskIdentity() {
return Optional.ofNullable(pskIdentity);
}
/* PSK identities created from new_session_ticket messages should only /* PSK identities created from new_session_ticket messages should only
* be used once. This method will return the identity and then clear it * be used once. This method will return the identity and then clear it
* so it cannot be used again. * so it cannot be used again.