andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[#5372] Ensure OpenSslClientContext / OpenSslServerContext can be garbage collected

Browse Source 
Motivation:

OpenSslClientContext / OpenSslServerContext can never be garbage collected as both are part of a reference to a callback that is stored as global reference in jni code.

Modifications:

Ensure the callbacks are static and so not hold the reference.

Result:

No more leak due not collectable OpenSslClientContext / OpenSslServerContext
This commit is contained in:
Norman Maurer 2016-06-08 15:57:47 +02:00
parent ed7c05f7c5
commit c5d70ec7dc
3 changed files with 86 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java
View File

@ -246,24 +246,18 @@ public final class OpenSslClientContext extends OpenSslContext {
}
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)) {
final X509ExtendedTrustManager extendedManager = (X509ExtendedTrustManager) manager;
SSLContext.setCertVerifyCallback(ctx, new AbstractCertificateVerifier() {
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
extendedManager.checkServerTrusted(peerCerts, auth, engine);
}
});
SSLContext.setCertVerifyCallback(ctx,
new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager));
} else {
SSLContext.setCertVerifyCallback(ctx, new AbstractCertificateVerifier() {
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
manager.checkServerTrusted(peerCerts, auth);
}
});
SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager));
}
} catch (Exception e) {
throw new SSLException("unable to setup trustmanager", e);
@ -323,4 +317,34 @@ public final class OpenSslClientContext extends OpenSslContext {
return false;
}
}
private static final class TrustManagerVerifyCallback extends AbstractCertificateVerifier {
private final X509TrustManager manager;
TrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509TrustManager manager) {
super(engineMap);
this.manager = manager;
}
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
manager.checkServerTrusted(peerCerts, auth);
}
}
private static final class ExtendedTrustManagerVerifyCallback extends AbstractCertificateVerifier {
private final X509ExtendedTrustManager manager;
ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) {
super(engineMap);
this.manager = manager;
}
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
manager.checkServerTrusted(peerCerts, auth, engine);
}
}
}

10
handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java
View File

@ -77,6 +77,7 @@ public abstract class OpenSslContext extends SslContext {
/** The OpenSSL SSL_CTX object */
protected volatile long ctx;
final OpenSslEngineMap engineMap = new DefaultOpenSslEngineMap();
long aprPool;
@SuppressWarnings({ "unused", "FieldMayBeFinal" })
private volatile int aprPoolDestroyed;
@ -84,7 +85,6 @@ public abstract class OpenSslContext extends SslContext {
private final List<String> unmodifiableCiphers;
private final long sessionCacheSize;
private final long sessionTimeout;
private final OpenSslEngineMap engineMap = new DefaultOpenSslEngineMap();
private final OpenSslApplicationProtocolNegotiator apn;
private final int mode;
private final Certificate[] keyCertChain;
@ -452,7 +452,13 @@ public abstract class OpenSslContext extends SslContext {
return PlatformDependent.javaVersion() >= 7 && trustManager instanceof X509ExtendedTrustManager;
}
abstract class AbstractCertificateVerifier implements CertificateVerifier {
abstract static class AbstractCertificateVerifier implements CertificateVerifier {
private final OpenSslEngineMap engineMap;
AbstractCertificateVerifier(OpenSslEngineMap engineMap) {
this.engineMap = engineMap;
}
@Override
public final int verify(long ssl, byte[][] chain, String auth) {
X509Certificate[] peerCerts = certificates(chain);

54
handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
View File

@ -416,24 +416,18 @@ public final class OpenSslServerContext extends OpenSslContext {
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)) {
final X509ExtendedTrustManager extendedManager = (X509ExtendedTrustManager) manager;
SSLContext.setCertVerifyCallback(ctx, new AbstractCertificateVerifier() {
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
extendedManager.checkClientTrusted(peerCerts, auth, engine);
}
});
SSLContext.setCertVerifyCallback(ctx,
new ExtendedTrustManagerVerifyCallback(engineMap, (X509ExtendedTrustManager) manager));
} else {
SSLContext.setCertVerifyCallback(ctx, new AbstractCertificateVerifier() {
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
manager.checkClientTrusted(peerCerts, auth);
}
});
SSLContext.setCertVerifyCallback(ctx, new TrustManagerVerifyCallback(engineMap, manager));
}
} catch (Exception e) {
throw new SSLException("unable to setup trustmanager", e);
@ -460,4 +454,34 @@ public final class OpenSslServerContext extends OpenSslContext {
"KeyManagerFactory is currently not supported with OpenSslServerContext");
}
}
private static final class TrustManagerVerifyCallback extends AbstractCertificateVerifier {
private final X509TrustManager manager;
TrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509TrustManager manager) {
super(engineMap);
this.manager = manager;
}
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
manager.checkClientTrusted(peerCerts, auth);
}
}
private static final class ExtendedTrustManagerVerifyCallback extends AbstractCertificateVerifier {
private final X509ExtendedTrustManager manager;
ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) {
super(engineMap);
this.manager = manager;
}
@Override
void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
throws Exception {
manager.checkClientTrusted(peerCerts, auth, engine);
}
}
}