diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerSessionContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerSessionContext.java index a5c5efe39c..8c92debfec 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerSessionContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerSessionContext.java @@ -18,6 +18,8 @@ package io.netty.handler.ssl; import io.netty.internal.tcnative.SSL; import io.netty.internal.tcnative.SSLContext; +import java.util.concurrent.locks.Lock; + /** * {@link OpenSslSessionContext} implementation which offers extra methods which are only useful for the server-side. @@ -32,12 +34,24 @@ public final class OpenSslServerSessionContext extends OpenSslSessionContext { if (seconds < 0) { throw new IllegalArgumentException(); } - SSLContext.setSessionCacheTimeout(context.ctx, seconds); + Lock writerLock = context.ctxLock.writeLock(); + writerLock.lock(); + try { + SSLContext.setSessionCacheTimeout(context.ctx, seconds); + } finally { + writerLock.unlock(); + } } @Override public int getSessionTimeout() { - return (int) SSLContext.getSessionCacheTimeout(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return (int) SSLContext.getSessionCacheTimeout(context.ctx); + } finally { + readerLock.unlock(); + } } @Override @@ -45,23 +59,48 @@ public final class OpenSslServerSessionContext extends OpenSslSessionContext { if (size < 0) { throw new IllegalArgumentException(); } - SSLContext.setSessionCacheSize(context.ctx, size); + Lock writerLock = context.ctxLock.writeLock(); + writerLock.lock(); + try { + SSLContext.setSessionCacheSize(context.ctx, size); + } finally { + writerLock.unlock(); + } } @Override public int getSessionCacheSize() { - return (int) SSLContext.getSessionCacheSize(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return (int) SSLContext.getSessionCacheSize(context.ctx); + } finally { + readerLock.unlock(); + } } @Override public void setSessionCacheEnabled(boolean enabled) { long mode = enabled ? SSL.SSL_SESS_CACHE_SERVER : SSL.SSL_SESS_CACHE_OFF; - SSLContext.setSessionCacheMode(context.ctx, mode); + + Lock writerLock = context.ctxLock.writeLock(); + writerLock.lock(); + try { + SSLContext.setSessionCacheMode(context.ctx, mode); + } finally { + writerLock.unlock(); + } } @Override public boolean isSessionCacheEnabled() { - return SSLContext.getSessionCacheMode(context.ctx) == SSL.SSL_SESS_CACHE_SERVER; + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.getSessionCacheMode(context.ctx) == SSL.SSL_SESS_CACHE_SERVER; + } finally { + readerLock.unlock(); + } } /** @@ -74,6 +113,12 @@ public final class OpenSslServerSessionContext extends OpenSslSessionContext { * @return {@code true} if success, {@code false} otherwise. */ public boolean setSessionIdContext(byte[] sidCtx) { - return SSLContext.setSessionIdContext(context.ctx, sidCtx); + Lock writerLock = context.ctxLock.writeLock(); + writerLock.lock(); + try { + return SSLContext.setSessionIdContext(context.ctx, sidCtx); + } finally { + writerLock.unlock(); + } } } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java index 4de787c3a6..846a968735 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java @@ -25,6 +25,7 @@ import javax.net.ssl.SSLSessionContext; import java.util.Arrays; import java.util.Enumeration; import java.util.NoSuchElementException; +import java.util.concurrent.locks.Lock; /** * OpenSSL specific {@link SSLSessionContext} implementation. @@ -76,8 +77,14 @@ public abstract class OpenSslSessionContext implements SSLSessionContext { a += SessionTicketKey.AES_KEY_SIZE; tickets[i] = new SessionTicketKey(name, hmacKey, aesKey); } - SSLContext.clearOptions(context.ctx, SSL.SSL_OP_NO_TICKET); - SSLContext.setSessionTicketKeys(context.ctx, tickets); + Lock writerLock = context.ctxLock.writeLock(); + writerLock.lock(); + try { + SSLContext.clearOptions(context.ctx, SSL.SSL_OP_NO_TICKET); + SSLContext.setSessionTicketKeys(context.ctx, tickets); + } finally { + writerLock.unlock(); + } } /** @@ -85,12 +92,18 @@ public abstract class OpenSslSessionContext implements SSLSessionContext { */ public void setTicketKeys(OpenSslSessionTicketKey... keys) { ObjectUtil.checkNotNull(keys, "keys"); - SSLContext.clearOptions(context.ctx, SSL.SSL_OP_NO_TICKET); SessionTicketKey[] ticketKeys = new SessionTicketKey[keys.length]; for (int i = 0; i < ticketKeys.length; i++) { ticketKeys[i] = keys[i].key; } - SSLContext.setSessionTicketKeys(context.ctx, ticketKeys); + Lock writerLock = context.ctxLock.writeLock(); + writerLock.lock(); + try { + SSLContext.clearOptions(context.ctx, SSL.SSL_OP_NO_TICKET); + SSLContext.setSessionTicketKeys(context.ctx, ticketKeys); + } finally { + writerLock.unlock(); + } } /** diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionStats.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionStats.java index 0c88332a64..f49b95f3ba 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionStats.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionStats.java @@ -18,6 +18,8 @@ package io.netty.handler.ssl; import io.netty.internal.tcnative.SSLContext; +import java.util.concurrent.locks.Lock; + /** * Stats exposed by an OpenSSL session context. * @@ -39,49 +41,91 @@ public final class OpenSslSessionStats { * Returns the current number of sessions in the internal session cache. */ public long number() { - return SSLContext.sessionNumber(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionNumber(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of started SSL/TLS handshakes in client mode. */ public long connect() { - return SSLContext.sessionConnect(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionConnect(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of successfully established SSL/TLS sessions in client mode. */ public long connectGood() { - return SSLContext.sessionConnectGood(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionConnectGood(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of start renegotiations in client mode. */ public long connectRenegotiate() { - return SSLContext.sessionConnectRenegotiate(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionConnectRenegotiate(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of started SSL/TLS handshakes in server mode. */ public long accept() { - return SSLContext.sessionAccept(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionAccept(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of successfully established SSL/TLS sessions in server mode. */ public long acceptGood() { - return SSLContext.sessionAcceptGood(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionAcceptGood(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of start renegotiations in server mode. */ public long acceptRenegotiate() { - return SSLContext.sessionAcceptRenegotiate(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionAcceptRenegotiate(context.ctx); + } finally { + readerLock.unlock(); + } } /** @@ -90,14 +134,26 @@ public final class OpenSslSessionStats { * external cache is counted as a hit. */ public long hits() { - return SSLContext.sessionHits(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionHits(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of successfully retrieved sessions from the external session cache in server mode. */ public long cbHits() { - return SSLContext.sessionCbHits(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionCbHits(context.ctx); + } finally { + readerLock.unlock(); + } } /** @@ -105,7 +161,13 @@ public final class OpenSslSessionStats { * in server mode. */ public long misses() { - return SSLContext.sessionMisses(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionMisses(context.ctx); + } finally { + readerLock.unlock(); + } } /** @@ -114,28 +176,52 @@ public final class OpenSslSessionStats { * count. */ public long timeouts() { - return SSLContext.sessionTimeouts(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionTimeouts(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of sessions that were removed because the maximum session cache size was exceeded. */ public long cacheFull() { - return SSLContext.sessionCacheFull(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionCacheFull(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of times a client presented a ticket that did not match any key in the list. */ public long ticketKeyFail() { - return SSLContext.sessionTicketKeyFail(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionTicketKeyFail(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of times a client did not present a ticket and we issued a new one */ public long ticketKeyNew() { - return SSLContext.sessionTicketKeyNew(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionTicketKeyNew(context.ctx); + } finally { + readerLock.unlock(); + } } /** @@ -143,13 +229,25 @@ public final class OpenSslSessionStats { * and we upgraded to the primary key. */ public long ticketKeyRenew() { - return SSLContext.sessionTicketKeyRenew(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionTicketKeyRenew(context.ctx); + } finally { + readerLock.unlock(); + } } /** * Returns the number of times a client presented a ticket derived from the primary key. */ public long ticketKeyResume() { - return SSLContext.sessionTicketKeyResume(context.ctx); + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + return SSLContext.sessionTicketKeyResume(context.ctx); + } finally { + readerLock.unlock(); + } } } diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java index f41c38b65a..b2135734e9 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java @@ -90,65 +90,63 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted throw new IllegalArgumentException( "Either both keyCertChain and key needs to be null or none of them"); } - synchronized (ReferenceCountedOpenSslContext.class) { - try { - if (!OpenSsl.useKeyManagerFactory()) { - if (keyManagerFactory != null) { - throw new IllegalArgumentException( - "KeyManagerFactory not supported"); - } - if (keyCertChain != null/* && key != null*/) { - setKeyMaterial(ctx, keyCertChain, key, keyPassword); - } - } else { - // javadocs state that keyManagerFactory has precedent over keyCertChain - if (keyManagerFactory == null && keyCertChain != null) { - keyManagerFactory = buildKeyManagerFactory( - keyCertChain, key, keyPassword, keyManagerFactory); - } - - if (keyManagerFactory != null) { - X509KeyManager keyManager = chooseX509KeyManager(keyManagerFactory.getKeyManagers()); - OpenSslKeyMaterialManager materialManager = useExtendedKeyManager(keyManager) ? - new OpenSslExtendedKeyMaterialManager( - (X509ExtendedKeyManager) keyManager, keyPassword) : - new OpenSslKeyMaterialManager(keyManager, keyPassword); - SSLContext.setCertRequestedCallback(ctx, new OpenSslCertificateRequestedCallback( - engineMap, materialManager)); - } + try { + if (!OpenSsl.useKeyManagerFactory()) { + if (keyManagerFactory != null) { + throw new IllegalArgumentException( + "KeyManagerFactory not supported"); + } + if (keyCertChain != null/* && key != null*/) { + setKeyMaterial(ctx, keyCertChain, key, keyPassword); + } + } else { + // javadocs state that keyManagerFactory has precedent over keyCertChain + if (keyManagerFactory == null && keyCertChain != null) { + keyManagerFactory = buildKeyManagerFactory( + keyCertChain, key, keyPassword, keyManagerFactory); + } + + if (keyManagerFactory != null) { + X509KeyManager keyManager = chooseX509KeyManager(keyManagerFactory.getKeyManagers()); + OpenSslKeyMaterialManager materialManager = useExtendedKeyManager(keyManager) ? + new OpenSslExtendedKeyMaterialManager( + (X509ExtendedKeyManager) keyManager, keyPassword) : + new OpenSslKeyMaterialManager(keyManager, keyPassword); + SSLContext.setCertRequestedCallback(ctx, new OpenSslCertificateRequestedCallback( + engineMap, materialManager)); } - } catch (Exception e) { - throw new SSLException("failed to set certificate and key", e); } + } catch (Exception e) { + throw new SSLException("failed to set certificate and key", e); + } - SSLContext.setVerify(ctx, SSL.SSL_CVERIFY_NONE, VERIFY_DEPTH); + SSLContext.setVerify(ctx, SSL.SSL_CVERIFY_NONE, VERIFY_DEPTH); - try { - if (trustCertCollection != null) { - trustManagerFactory = buildTrustManagerFactory(trustCertCollection, trustManagerFactory); - } else if (trustManagerFactory == null) { - trustManagerFactory = TrustManagerFactory.getInstance( - TrustManagerFactory.getDefaultAlgorithm()); - trustManagerFactory.init((KeyStore) null); - } - final X509TrustManager manager = chooseTrustManager(trustManagerFactory.getTrustManagers()); - - // IMPORTANT: The callbacks set for verification must be static to prevent memory leak as - // otherwise the context can never be collected. This is because the JNI code holds - // a global reference to the callbacks. - // - // See https://github.com/netty/netty/issues/5372 - - // Use this to prevent an error when running on java < 7 - if (useExtendedTrustManager(manager)) { - SSLContext.setCertVerifyCallback(ctx, - new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager)); - } else { - SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager)); - } - } catch (Exception e) { - throw new SSLException("unable to setup trustmanager", e); + try { + if (trustCertCollection != null) { + trustManagerFactory = buildTrustManagerFactory(trustCertCollection, trustManagerFactory); + } else if (trustManagerFactory == null) { + trustManagerFactory = TrustManagerFactory.getInstance( + TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init((KeyStore) null); } + final X509TrustManager manager = chooseTrustManager(trustManagerFactory.getTrustManagers()); + + // IMPORTANT: The callbacks set for verification must be static to prevent memory leak as + // otherwise the context can never be collected. This is because the JNI code holds + // a global reference to the callbacks. + // + // See https://github.com/netty/netty/issues/5372 + + // Use this to prevent an error when running on java < 7 + if (useExtendedTrustManager(manager)) { + SSLContext.setCertVerifyCallback(ctx, + new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager)); + } else { + SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager)); + } + } catch (Exception e) { + throw new SSLException("unable to setup trustmanager", e); } return new OpenSslClientSessionContext(thiz); } diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java index d537986937..ee049ab19d 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java @@ -46,6 +46,9 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.net.ssl.KeyManager; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; @@ -106,11 +109,11 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen protected static final int VERIFY_DEPTH = 10; /** - * The OpenSSL SSL_CTX object + * The OpenSSL SSL_CTX object. + * + * {@link #ctxLock} must be hold while using ctx! */ - protected volatile long ctx; - @SuppressWarnings({ "unused", "FieldMayBeFinal" }) - private volatile int aprPoolDestroyed; + protected long ctx; private final List unmodifiableCiphers; private final long sessionCacheSize; private final long sessionTimeout; @@ -144,6 +147,8 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen final String[] protocols; final boolean enableOcsp; final OpenSslEngineMap engineMap = new DefaultOpenSslEngineMap(); + final ReadWriteLock ctxLock = new ReentrantReadWriteLock(); + private volatile boolean rejectRemoteInitiatedRenegotiation; private volatile int bioNonApplicationBufferSize = DEFAULT_BIO_NON_APPLICATION_BUFFER_SIZE; @@ -273,92 +278,90 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen // Create a new SSL_CTX and configure it. boolean success = false; try { - synchronized (ReferenceCountedOpenSslContext.class) { - try { - ctx = SSLContext.make(SSL.SSL_PROTOCOL_ALL, mode); - } catch (Exception e) { - throw new SSLException("failed to create an SSL_CTX", e); - } + try { + ctx = SSLContext.make(SSL.SSL_PROTOCOL_ALL, mode); + } catch (Exception e) { + throw new SSLException("failed to create an SSL_CTX", e); + } - SSLContext.setOptions(ctx, SSLContext.getOptions(ctx) | - SSL.SSL_OP_NO_SSLv2 | - SSL.SSL_OP_NO_SSLv3 | - SSL.SSL_OP_CIPHER_SERVER_PREFERENCE | + SSLContext.setOptions(ctx, SSLContext.getOptions(ctx) | + SSL.SSL_OP_NO_SSLv2 | + SSL.SSL_OP_NO_SSLv3 | + SSL.SSL_OP_CIPHER_SERVER_PREFERENCE | - // We do not support compression at the moment so we should explicitly disable it. - SSL.SSL_OP_NO_COMPRESSION | + // We do not support compression at the moment so we should explicitly disable it. + SSL.SSL_OP_NO_COMPRESSION | - // Disable ticket support by default to be more inline with SSLEngineImpl of the JDK. - // This also let SSLSession.getId() work the same way for the JDK implementation and the OpenSSLEngine. - // If tickets are supported SSLSession.getId() will only return an ID on the server-side if it could - // make use of tickets. - SSL.SSL_OP_NO_TICKET); + // Disable ticket support by default to be more inline with SSLEngineImpl of the JDK. + // This also let SSLSession.getId() work the same way for the JDK implementation and the + // OpenSSLEngine. If tickets are supported SSLSession.getId() will only return an ID on the + // server-side if it could make use of tickets. + SSL.SSL_OP_NO_TICKET); - // We need to enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER as the memory address may change between - // calling OpenSSLEngine.wrap(...). - // See https://github.com/netty/netty-tcnative/issues/100 - SSLContext.setMode(ctx, SSLContext.getMode(ctx) | SSL.SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + // We need to enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER as the memory address may change between + // calling OpenSSLEngine.wrap(...). + // See https://github.com/netty/netty-tcnative/issues/100 + SSLContext.setMode(ctx, SSLContext.getMode(ctx) | SSL.SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); - if (DH_KEY_LENGTH != null) { - SSLContext.setTmpDHLength(ctx, DH_KEY_LENGTH); - } + if (DH_KEY_LENGTH != null) { + SSLContext.setTmpDHLength(ctx, DH_KEY_LENGTH); + } /* List the ciphers that are permitted to negotiate. */ - try { - SSLContext.setCipherSuite(ctx, CipherSuiteConverter.toOpenSsl(unmodifiableCiphers)); - } catch (SSLException e) { - throw e; - } catch (Exception e) { - throw new SSLException("failed to set cipher suite: " + unmodifiableCiphers, e); - } + try { + SSLContext.setCipherSuite(ctx, CipherSuiteConverter.toOpenSsl(unmodifiableCiphers)); + } catch (SSLException e) { + throw e; + } catch (Exception e) { + throw new SSLException("failed to set cipher suite: " + unmodifiableCiphers, e); + } - List nextProtoList = apn.protocols(); + List nextProtoList = apn.protocols(); /* Set next protocols for next protocol negotiation extension, if specified */ - if (!nextProtoList.isEmpty()) { - String[] appProtocols = nextProtoList.toArray(new String[nextProtoList.size()]); - int selectorBehavior = opensslSelectorFailureBehavior(apn.selectorFailureBehavior()); + if (!nextProtoList.isEmpty()) { + String[] appProtocols = nextProtoList.toArray(new String[nextProtoList.size()]); + int selectorBehavior = opensslSelectorFailureBehavior(apn.selectorFailureBehavior()); - switch (apn.protocol()) { - case NPN: - SSLContext.setNpnProtos(ctx, appProtocols, selectorBehavior); - break; - case ALPN: - SSLContext.setAlpnProtos(ctx, appProtocols, selectorBehavior); - break; - case NPN_AND_ALPN: - SSLContext.setNpnProtos(ctx, appProtocols, selectorBehavior); - SSLContext.setAlpnProtos(ctx, appProtocols, selectorBehavior); - break; - default: - throw new Error(); - } + switch (apn.protocol()) { + case NPN: + SSLContext.setNpnProtos(ctx, appProtocols, selectorBehavior); + break; + case ALPN: + SSLContext.setAlpnProtos(ctx, appProtocols, selectorBehavior); + break; + case NPN_AND_ALPN: + SSLContext.setNpnProtos(ctx, appProtocols, selectorBehavior); + SSLContext.setAlpnProtos(ctx, appProtocols, selectorBehavior); + break; + default: + throw new Error(); } + } - /* Set session cache size, if specified */ - if (sessionCacheSize > 0) { - this.sessionCacheSize = sessionCacheSize; - SSLContext.setSessionCacheSize(ctx, sessionCacheSize); - } else { - // Get the default session cache size using SSLContext.setSessionCacheSize() - this.sessionCacheSize = sessionCacheSize = SSLContext.setSessionCacheSize(ctx, 20480); - // Revert the session cache size to the default value. - SSLContext.setSessionCacheSize(ctx, sessionCacheSize); - } + /* Set session cache size, if specified */ + if (sessionCacheSize > 0) { + this.sessionCacheSize = sessionCacheSize; + SSLContext.setSessionCacheSize(ctx, sessionCacheSize); + } else { + // Get the default session cache size using SSLContext.setSessionCacheSize() + this.sessionCacheSize = sessionCacheSize = SSLContext.setSessionCacheSize(ctx, 20480); + // Revert the session cache size to the default value. + SSLContext.setSessionCacheSize(ctx, sessionCacheSize); + } - /* Set session timeout, if specified */ - if (sessionTimeout > 0) { - this.sessionTimeout = sessionTimeout; - SSLContext.setSessionCacheTimeout(ctx, sessionTimeout); - } else { - // Get the default session timeout using SSLContext.setSessionCacheTimeout() - this.sessionTimeout = sessionTimeout = SSLContext.setSessionCacheTimeout(ctx, 300); - // Revert the session timeout to the default value. - SSLContext.setSessionCacheTimeout(ctx, sessionTimeout); - } + /* Set session timeout, if specified */ + if (sessionTimeout > 0) { + this.sessionTimeout = sessionTimeout; + SSLContext.setSessionCacheTimeout(ctx, sessionTimeout); + } else { + // Get the default session timeout using SSLContext.setSessionCacheTimeout() + this.sessionTimeout = sessionTimeout = SSLContext.setSessionCacheTimeout(ctx, 300); + // Revert the session timeout to the default value. + SSLContext.setSessionCacheTimeout(ctx, sessionTimeout); + } - if (enableOcsp) { - SSLContext.enableOcsp(ctx, isClient()); - } + if (enableOcsp) { + SSLContext.enableOcsp(ctx, isClient()); } success = true; } finally { @@ -428,11 +431,17 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen * Be aware that it is freed as soon as the {@link #finalize()} method is called. * At this point {@code 0} will be returned. * - * @deprecated use {@link #sslCtxPointer()} + * @deprecated this method is considered unsafe as the returned pointer may be released later. Dont use it! */ @Deprecated public final long context() { - return ctx; + Lock readerLock = ctxLock.readLock(); + readerLock.lock(); + try { + return ctx; + } finally { + readerLock.unlock(); + } } /** @@ -491,18 +500,29 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen /** * Returns the pointer to the {@code SSL_CTX} object for this {@link ReferenceCountedOpenSslContext}. - * Be aware that it is freed as soon as the {@link #release()} method is called. + * Be aware that it is freed as soon as the {@link #release()} method is called. * At this point {@code 0} will be returned. + * + * @deprecated this method is considered unsafe as the returned pointer may be released later. Dont use it! */ + @Deprecated public final long sslCtxPointer() { - return ctx; + Lock readerLock = ctxLock.readLock(); + readerLock.lock(); + try { + return ctx; + } finally { + readerLock.unlock(); + } } // IMPORTANT: This method must only be called from either the constructor or the finalizer as a user MUST never // get access to an OpenSslSessionContext after this method was called to prevent the user from // producing a segfault. - final void destroy() { - synchronized (ReferenceCountedOpenSslContext.class) { + private void destroy() { + Lock writerLock = ctxLock.writeLock(); + writerLock.lock(); + try { if (ctx != 0) { if (enableOcsp) { SSLContext.disableOcsp(ctx); @@ -511,6 +531,8 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen SSLContext.free(ctx); ctx = 0; } + } finally { + writerLock.unlock(); } } diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java index 8f0ec83534..27460c7149 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -43,6 +43,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; + +import java.util.concurrent.locks.Lock; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLException; @@ -234,7 +236,14 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc localCerts = context.keyCertChain; keyMaterialManager = context.keyMaterialManager(); enableOcsp = context.enableOcsp; - ssl = SSL.newSSL(context.ctx, !context.isClient()); + + Lock readerLock = context.ctxLock.readLock(); + readerLock.lock(); + try { + ssl = SSL.newSSL(context.ctx, !context.isClient()); + } finally { + readerLock.unlock(); + } try { networkBIO = SSL.bioNewByteBuffer(ssl, context.getBioNonApplicationBufferSize()); diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java index aed85d25ca..4c9df3148c 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -106,75 +106,69 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted String keyPassword, KeyManagerFactory keyManagerFactory) throws SSLException { ServerContext result = new ServerContext(); - synchronized (ReferenceCountedOpenSslContext.class) { - try { - SSLContext.setVerify(ctx, SSL.SSL_CVERIFY_NONE, VERIFY_DEPTH); - if (!OpenSsl.useKeyManagerFactory()) { - if (keyManagerFactory != null) { - throw new IllegalArgumentException( - "KeyManagerFactory not supported"); - } - checkNotNull(keyCertChain, "keyCertChain"); - - setKeyMaterial(ctx, keyCertChain, key, keyPassword); - } else { - // javadocs state that keyManagerFactory has precedent over keyCertChain, and we must have a - // keyManagerFactory for the server so build one if it is not specified. - if (keyManagerFactory == null) { - keyManagerFactory = buildKeyManagerFactory( - keyCertChain, key, keyPassword, keyManagerFactory); - } - X509KeyManager keyManager = chooseX509KeyManager(keyManagerFactory.getKeyManagers()); - result.keyMaterialManager = useExtendedKeyManager(keyManager) ? - new OpenSslExtendedKeyMaterialManager( - (X509ExtendedKeyManager) keyManager, keyPassword) : - new OpenSslKeyMaterialManager(keyManager, keyPassword); + try { + SSLContext.setVerify(ctx, SSL.SSL_CVERIFY_NONE, VERIFY_DEPTH); + if (!OpenSsl.useKeyManagerFactory()) { + if (keyManagerFactory != null) { + throw new IllegalArgumentException( + "KeyManagerFactory not supported"); } - } catch (Exception e) { - throw new SSLException("failed to set certificate and key", e); + checkNotNull(keyCertChain, "keyCertChain"); + + setKeyMaterial(ctx, keyCertChain, key, keyPassword); + } else { + // javadocs state that keyManagerFactory has precedent over keyCertChain, and we must have a + // keyManagerFactory for the server so build one if it is not specified. + if (keyManagerFactory == null) { + keyManagerFactory = buildKeyManagerFactory( + keyCertChain, key, keyPassword, keyManagerFactory); + } + X509KeyManager keyManager = chooseX509KeyManager(keyManagerFactory.getKeyManagers()); + result.keyMaterialManager = useExtendedKeyManager(keyManager) ? + new OpenSslExtendedKeyMaterialManager( + (X509ExtendedKeyManager) keyManager, keyPassword) : + new OpenSslKeyMaterialManager(keyManager, keyPassword); + } + } catch (Exception e) { + throw new SSLException("failed to set certificate and key", e); + } + try { + if (trustCertCollection != null) { + trustManagerFactory = buildTrustManagerFactory(trustCertCollection, trustManagerFactory); + } else if (trustManagerFactory == null) { + // Mimic the way SSLContext.getInstance(KeyManager[], null, null) works + trustManagerFactory = TrustManagerFactory.getInstance( + TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init((KeyStore) null); } - try { - if (trustCertCollection != null) { - trustManagerFactory = buildTrustManagerFactory(trustCertCollection, trustManagerFactory); - } else if (trustManagerFactory == null) { - // Mimic the way SSLContext.getInstance(KeyManager[], null, null) works - trustManagerFactory = TrustManagerFactory.getInstance( - TrustManagerFactory.getDefaultAlgorithm()); - trustManagerFactory.init((KeyStore) null); - } - final X509TrustManager manager = chooseTrustManager(trustManagerFactory.getTrustManagers()); + final X509TrustManager manager = chooseTrustManager(trustManagerFactory.getTrustManagers()); - // IMPORTANT: The callbacks set for verification must be static to prevent memory leak as - // otherwise the context can never be collected. This is because the JNI code holds - // a global reference to the callbacks. - // - // See https://github.com/netty/netty/issues/5372 + // IMPORTANT: The callbacks set for verification must be static to prevent memory leak as + // otherwise the context can never be collected. This is because the JNI code holds + // a global reference to the callbacks. + // + // See https://github.com/netty/netty/issues/5372 - // Use this to prevent an error when running on java < 7 - if (useExtendedTrustManager(manager)) { - SSLContext.setCertVerifyCallback(ctx, - new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager)); - } else { - SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager)); - } + // Use this to prevent an error when running on java < 7 + if (useExtendedTrustManager(manager)) { + SSLContext.setCertVerifyCallback(ctx, + new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager)); + } else { + SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager)); + } - X509Certificate[] issuers = manager.getAcceptedIssuers(); - if (issuers != null && issuers.length > 0) { - long bio = 0; - try { - bio = toBIO(issuers); - if (!SSLContext.setCACertificateBio(ctx, bio)) { - throw new SSLException("unable to setup accepted issuers for trustmanager " + manager); - } - } finally { - freeBio(bio); + X509Certificate[] issuers = manager.getAcceptedIssuers(); + if (issuers != null && issuers.length > 0) { + long bio = 0; + try { + bio = toBIO(issuers); + if (!SSLContext.setCACertificateBio(ctx, bio)) { + throw new SSLException("unable to setup accepted issuers for trustmanager " + manager); } + } finally { + freeBio(bio); } - } catch (SSLException e) { - throw e; - } catch (Exception e) { - throw new SSLException("unable to setup trustmanager", e); } if (PlatformDependent.javaVersion() >= 8) { @@ -184,6 +178,10 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted // a global reference to the matcher. SSLContext.setSniHostnameMatcher(ctx, new OpenSslSniHostnameMatcher(engineMap)); } + } catch (SSLException e) { + throw e; + } catch (Exception e) { + throw new SSLException("unable to setup trustmanager", e); } result.sessionContext = new OpenSslServerSessionContext(thiz);