Handle null SSLSessions during invalidation (#34130)

The SSLService invalidates SSLSessions when there is a change to any of
the underlying key or trust material. However, this invalidation code
did not check for a null SSLSession being returned from the context and
assumed that the context would always return a non-null object. The
return of a null object is possible in all versions, but JDK11 seems to
return them more often due to changes for TLS 1.3. There are a number
of reasons that we get a id of a session but the context returns null
when the session with that id is requested. Some of the reasons for
this are:

* Session was evicted by session cache
* Session has timed out
* Session has been invalidated by another caller

To handle this, the SSLService now checks if the value is null before
calling invalidate on the SSLSession.

Closes #32124
This commit is contained in:
Jay Modi 2018-09-28 09:03:35 -06:00 committed by GitHub
parent 95977f4db9
commit 14d841ef21
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 197 additions and 11 deletions

View File

@ -24,6 +24,7 @@ import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSession;
import javax.net.ssl.SSLSessionContext;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
@ -542,17 +543,6 @@ public class SSLService extends AbstractComponent {
return context;
}
/**
* Invalidates the sessions in the provided {@link SSLSessionContext}
*/
private void invalidateSessions(SSLSessionContext sslSessionContext) {
Enumeration<byte[]> sessionIds = sslSessionContext.getIds();
while (sessionIds.hasMoreElements()) {
byte[] sessionId = sessionIds.nextElement();
sslSessionContext.getSession(sessionId).invalidate();
}
}
synchronized void reload() {
invalidateSessions(context.getClientSessionContext());
invalidateSessions(context.getServerSessionContext());
@ -592,6 +582,24 @@ public class SSLService extends AbstractComponent {
}
}
/**
* Invalidates the sessions in the provided {@link SSLSessionContext}
*/
static void invalidateSessions(SSLSessionContext sslSessionContext) {
Enumeration<byte[]> sessionIds = sslSessionContext.getIds();
while (sessionIds.hasMoreElements()) {
byte[] sessionId = sessionIds.nextElement();
SSLSession session = sslSessionContext.getSession(sessionId);
// a SSLSession could be null as there is no lock while iterating, the session cache
// could have evicted a value, the session could be timed out, or the session could
// have already been invalidated, which removes the value from the session cache in the
// sun implementation
if (session != null) {
session.invalidate();
}
}
}
/**
* @return A map of Settings prefix to Settings object
*/

View File

@ -19,6 +19,7 @@ import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.CheckedRunnable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
@ -35,19 +36,29 @@ import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSession;
import javax.net.ssl.SSLSessionContext;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.X509ExtendedTrustManager;
import javax.security.cert.X509Certificate;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.Principal;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.security.cert.Certificate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.contains;
@ -654,6 +665,57 @@ public class SSLServiceTests extends ESTestCase {
assertFalse(iterator.hasNext());
}
public void testSSLSessionInvalidationHandlesNullSessions() {
final int numEntries = randomIntBetween(1, 32);
final AtomicInteger invalidationCounter = new AtomicInteger();
int numNull = 0;
final Map<byte[], SSLSession> sessionMap = new HashMap<>();
for (int i = 0; i < numEntries; i++) {
final byte[] id = randomByteArrayOfLength(2);
final SSLSession sslSession;
if (rarely()) {
sslSession = null;
numNull++;
} else {
sslSession = new MockSSLSession(id, invalidationCounter::incrementAndGet);
}
sessionMap.put(id, sslSession);
}
SSLSessionContext sslSessionContext = new SSLSessionContext() {
@Override
public SSLSession getSession(byte[] sessionId) {
return sessionMap.get(sessionId);
}
@Override
public Enumeration<byte[]> getIds() {
return Collections.enumeration(sessionMap.keySet());
}
@Override
public void setSessionTimeout(int seconds) throws IllegalArgumentException {
}
@Override
public int getSessionTimeout() {
return 0;
}
@Override
public void setSessionCacheSize(int size) throws IllegalArgumentException {
}
@Override
public int getSessionCacheSize() {
return 0;
}
};
SSLService.invalidateSessions(sslSessionContext);
assertEquals(numEntries - numNull, invalidationCounter.get());
}
@Network
public void testThatSSLContextWithoutSettingsWorks() throws Exception {
SSLService sslService = new SSLService(Settings.EMPTY, env);
@ -761,4 +823,120 @@ public class SSLServiceTests extends ESTestCase {
}
}
private static final class MockSSLSession implements SSLSession {
private final byte[] id;
private final Runnable invalidation;
private MockSSLSession(byte[] id, Runnable invalidation) {
this.id = id;
this.invalidation = invalidation;
}
@Override
public byte[] getId() {
return id;
}
@Override
public SSLSessionContext getSessionContext() {
return null;
}
@Override
public long getCreationTime() {
return 0;
}
@Override
public long getLastAccessedTime() {
return 0;
}
@Override
public void invalidate() {
invalidation.run();
}
@Override
public boolean isValid() {
return false;
}
@Override
public void putValue(String name, Object value) {
}
@Override
public Object getValue(String name) {
return null;
}
@Override
public void removeValue(String name) {
}
@Override
public String[] getValueNames() {
return new String[0];
}
@Override
public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException {
return new Certificate[0];
}
@Override
public Certificate[] getLocalCertificates() {
return new Certificate[0];
}
@SuppressForbidden(reason = "need to reference deprecated class to implement JDK interface")
@Override
public X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException {
return new X509Certificate[0];
}
@Override
public Principal getPeerPrincipal() throws SSLPeerUnverifiedException {
return null;
}
@Override
public Principal getLocalPrincipal() {
return null;
}
@Override
public String getCipherSuite() {
return null;
}
@Override
public String getProtocol() {
return null;
}
@Override
public String getPeerHost() {
return null;
}
@Override
public int getPeerPort() {
return 0;
}
@Override
public int getPacketBufferSize() {
return 0;
}
@Override
public int getApplicationBufferSize() {
return 0;
}
}
}