From 4448b8f42f599e79db1744cd8f5fdfee702c195e Mon Sep 17 00:00:00 2001 From: Nathan Mittler Date: Thu, 3 Aug 2017 14:21:32 -0700 Subject: [PATCH] Upgrading to Conscrypt 1.0.0.RC9. (#7044) Motivation: Starting with 1.0.0.RC9, conscrypt supports a buffer allocator. Modifications: - Updated the creation process for the engine to pass through the ByteBufAllocator. - Wrap a ByteBufAllocator with an adapter for conscrypt. - Added a property to optionally control whether conscrypt uses Netty's buffer allocator. Result: Netty+conscrypt will support using Netty's ByteBufAllocator. --- .../handler/ssl/ConscryptAlpnSslEngine.java | 79 ++++++++++++++++--- .../JdkAlpnApplicationProtocolNegotiator.java | 13 +-- .../ssl/JdkApplicationProtocolNegotiator.java | 6 +- ...kDefaultApplicationProtocolNegotiator.java | 5 +- .../JdkNpnApplicationProtocolNegotiator.java | 5 +- .../io/netty/handler/ssl/JdkSslContext.java | 8 +- .../ssl/ConscryptJdkSslEngineInteropTest.java | 6 ++ pom.xml | 2 +- 8 files changed, 98 insertions(+), 26 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java index 8e7a5447a3..a61a14aabe 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java @@ -19,6 +19,8 @@ import static io.netty.handler.ssl.SslUtils.toSSLHandshakeException; import static io.netty.util.internal.ObjectUtil.checkNotNull; import static java.lang.Math.min; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufAllocator; import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectionListener; import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector; import java.lang.reflect.Method; @@ -31,6 +33,9 @@ import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLException; import io.netty.util.internal.PlatformDependent; +import io.netty.util.internal.SystemPropertyUtil; +import org.conscrypt.AllocatedBuffer; +import org.conscrypt.BufferAllocator; import org.conscrypt.Conscrypt; import org.conscrypt.HandshakeListener; @@ -38,6 +43,8 @@ import org.conscrypt.HandshakeListener; * A {@link JdkSslEngine} that uses the Conscrypt provider or SSL with ALPN. */ abstract class ConscryptAlpnSslEngine extends JdkSslEngine { + private static final boolean USE_BUFFER_ALLOCATOR = SystemPropertyUtil.getBoolean( + "io.netty.handler.ssl.conscrypt.useBufferAllocator", true); private static final Class ENGINES_CLASS = getEnginesClass(); /** @@ -51,19 +58,32 @@ abstract class ConscryptAlpnSslEngine extends JdkSslEngine { return isAvailable() && isConscryptEngine(engine, ENGINES_CLASS); } - static ConscryptAlpnSslEngine newClientEngine(SSLEngine engine, + static ConscryptAlpnSslEngine newClientEngine(SSLEngine engine, ByteBufAllocator alloc, JdkApplicationProtocolNegotiator applicationNegotiator) { - return new ClientEngine(engine, applicationNegotiator); + return new ClientEngine(engine, alloc, applicationNegotiator); } - static ConscryptAlpnSslEngine newServerEngine(SSLEngine engine, + static ConscryptAlpnSslEngine newServerEngine(SSLEngine engine, ByteBufAllocator alloc, JdkApplicationProtocolNegotiator applicationNegotiator) { - return new ServerEngine(engine, applicationNegotiator); + return new ServerEngine(engine, alloc, applicationNegotiator); } - private ConscryptAlpnSslEngine(SSLEngine engine, List protocols) { + private ConscryptAlpnSslEngine(SSLEngine engine, ByteBufAllocator alloc, List protocols) { super(engine); + // Configure the Conscrypt engine to use Netty's buffer allocator. This is a trade-off of memory vs + // performance. + // + // If no allocator is provided, the engine will internally allocate a direct buffer of max packet size in + // order to optimize JNI calls (this happens the first time it is provided a non-direct buffer from the + // application). + // + // Alternatively, if an allocator is provided, no internal buffer will be created and direct buffers will be + // retrieved from the allocator on-demand. + if (USE_BUFFER_ALLOCATOR) { + Conscrypt.Engines.setBufferAllocator(engine, new BufferAllocatorAdapter(alloc)); + } + // Set the list of supported ALPN protocols on the engine. Conscrypt.Engines.setAlpnProtocols(engine, protocols.toArray(new String[protocols.size()])); } @@ -90,9 +110,9 @@ abstract class ConscryptAlpnSslEngine extends JdkSslEngine { private static final class ClientEngine extends ConscryptAlpnSslEngine { private final ProtocolSelectionListener protocolListener; - ClientEngine(SSLEngine engine, + ClientEngine(SSLEngine engine, ByteBufAllocator alloc, JdkApplicationProtocolNegotiator applicationNegotiator) { - super(engine, applicationNegotiator.protocols()); + super(engine, alloc, applicationNegotiator.protocols()); // Register for completion of the handshake. Conscrypt.Engines.setHandshakeListener(engine, new HandshakeListener() { @Override @@ -119,8 +139,9 @@ abstract class ConscryptAlpnSslEngine extends JdkSslEngine { private static final class ServerEngine extends ConscryptAlpnSslEngine { private final ProtocolSelector protocolSelector; - ServerEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator) { - super(engine, applicationNegotiator.protocols()); + ServerEngine(SSLEngine engine, ByteBufAllocator alloc, + JdkApplicationProtocolNegotiator applicationNegotiator) { + super(engine, alloc, applicationNegotiator.protocols()); // Register for completion of the handshake. Conscrypt.Engines.setHandshakeListener(engine, new HandshakeListener() { @@ -173,4 +194,44 @@ abstract class ConscryptAlpnSslEngine extends JdkSslEngine { private static Method getIsConscryptMethod(Class enginesClass) throws NoSuchMethodException { return enginesClass.getMethod("isConscrypt", SSLEngine.class); } + + private static final class BufferAllocatorAdapter extends BufferAllocator { + private final ByteBufAllocator alloc; + + BufferAllocatorAdapter(ByteBufAllocator alloc) { + this.alloc = alloc; + } + + @Override + public AllocatedBuffer allocateDirectBuffer(int capacity) { + return new BufferAdapter(alloc.directBuffer(capacity)); + } + } + + private static final class BufferAdapter extends AllocatedBuffer { + private final ByteBuf nettyBuffer; + private final ByteBuffer buffer; + + BufferAdapter(ByteBuf nettyBuffer) { + this.nettyBuffer = nettyBuffer; + this.buffer = nettyBuffer.nioBuffer(0, nettyBuffer.capacity()); + } + + @Override + public ByteBuffer nioBuffer() { + return buffer; + } + + @Override + public AllocatedBuffer retain() { + nettyBuffer.retain(); + return this; + } + + @Override + public AllocatedBuffer release() { + nettyBuffer.release(); + return this; + } + } } diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java b/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java index f82c7da53b..684d607015 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java @@ -15,6 +15,7 @@ */ package io.netty.handler.ssl; +import io.netty.buffer.ByteBufAllocator; import javax.net.ssl.SSLEngine; /** @@ -108,8 +109,8 @@ public final class JdkAlpnApplicationProtocolNegotiator extends JdkBaseApplicati private static final class FailureWrapper implements SslEngineWrapperFactory { @Override - public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator, - boolean isServer) { + public SSLEngine wrapSslEngine(SSLEngine engine, ByteBufAllocator alloc, + JdkApplicationProtocolNegotiator applicationNegotiator, boolean isServer) { throw new RuntimeException("ALPN unsupported. Is your classpath configured correctly?" + " For Conscrypt, add the appropriate Conscrypt JAR to classpath and set the security provider." + " For Jetty-ALPN, see " @@ -119,11 +120,11 @@ public final class JdkAlpnApplicationProtocolNegotiator extends JdkBaseApplicati private static final class AlpnWrapper implements SslEngineWrapperFactory { @Override - public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator, - boolean isServer) { + public SSLEngine wrapSslEngine(SSLEngine engine, ByteBufAllocator alloc, + JdkApplicationProtocolNegotiator applicationNegotiator, boolean isServer) { if (ConscryptAlpnSslEngine.isEngineSupported(engine)) { - return isServer ? ConscryptAlpnSslEngine.newServerEngine(engine, applicationNegotiator) - : ConscryptAlpnSslEngine.newClientEngine(engine, applicationNegotiator); + return isServer ? ConscryptAlpnSslEngine.newServerEngine(engine, alloc, applicationNegotiator) + : ConscryptAlpnSslEngine.newClientEngine(engine, alloc, applicationNegotiator); } if (JettyAlpnSslEngine.isAvailable()) { return isServer ? JettyAlpnSslEngine.newServerEngine(engine, applicationNegotiator) diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkApplicationProtocolNegotiator.java b/handler/src/main/java/io/netty/handler/ssl/JdkApplicationProtocolNegotiator.java index 1a14331128..c414517025 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkApplicationProtocolNegotiator.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkApplicationProtocolNegotiator.java @@ -15,6 +15,7 @@ */ package io.netty.handler.ssl; +import io.netty.buffer.ByteBufAllocator; import javax.net.ssl.SSLEngine; import java.util.List; import java.util.Set; @@ -31,6 +32,7 @@ public interface JdkApplicationProtocolNegotiator extends ApplicationProtocolNeg * Abstract factory pattern for wrapping an {@link SSLEngine} object. This is useful for NPN/APLN support. * * @param engine The engine to wrap. + * @param alloc the buffer allocator. * @param applicationNegotiator The application level protocol negotiator * @param isServer
    *
  • {@code true} if the engine is for server side of connections
    • @@ -38,8 +40,8 @@ public interface JdkApplicationProtocolNegotiator extends ApplicationProtocolNeg *
* @return The resulting wrapped engine. This may just be {@code engine}. */ - SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator, - boolean isServer); + SSLEngine wrapSslEngine(SSLEngine engine, ByteBufAllocator alloc, + JdkApplicationProtocolNegotiator applicationNegotiator, boolean isServer); } /** diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkDefaultApplicationProtocolNegotiator.java b/handler/src/main/java/io/netty/handler/ssl/JdkDefaultApplicationProtocolNegotiator.java index 4e2a0bfe9d..7c3e4d3c74 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkDefaultApplicationProtocolNegotiator.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkDefaultApplicationProtocolNegotiator.java @@ -15,6 +15,7 @@ */ package io.netty.handler.ssl; +import io.netty.buffer.ByteBufAllocator; import java.util.Collections; import java.util.List; @@ -29,8 +30,8 @@ final class JdkDefaultApplicationProtocolNegotiator implements JdkApplicationPro new JdkDefaultApplicationProtocolNegotiator(); private static final SslEngineWrapperFactory DEFAULT_SSL_ENGINE_WRAPPER_FACTORY = new SslEngineWrapperFactory() { @Override - public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator, - boolean isServer) { + public SSLEngine wrapSslEngine(SSLEngine engine, ByteBufAllocator alloc, + JdkApplicationProtocolNegotiator applicationNegotiator, boolean isServer) { return engine; } }; diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkNpnApplicationProtocolNegotiator.java b/handler/src/main/java/io/netty/handler/ssl/JdkNpnApplicationProtocolNegotiator.java index 06b29b7283..70bd59d917 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkNpnApplicationProtocolNegotiator.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkNpnApplicationProtocolNegotiator.java @@ -15,6 +15,7 @@ */ package io.netty.handler.ssl; +import io.netty.buffer.ByteBufAllocator; import javax.net.ssl.SSLEngine; /** @@ -30,8 +31,8 @@ public final class JdkNpnApplicationProtocolNegotiator extends JdkBaseApplicatio } @Override - public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator, - boolean isServer) { + public SSLEngine wrapSslEngine(SSLEngine engine, ByteBufAllocator alloc, + JdkApplicationProtocolNegotiator applicationNegotiator, boolean isServer) { return new JettyNpnSslEngine(engine, applicationNegotiator, isServer); } }; diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java index 75d4b9da1e..9cf6f26b62 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java @@ -210,15 +210,15 @@ public class JdkSslContext extends SslContext { @Override public final SSLEngine newEngine(ByteBufAllocator alloc) { - return configureAndWrapEngine(context().createSSLEngine()); + return configureAndWrapEngine(context().createSSLEngine(), alloc); } @Override public final SSLEngine newEngine(ByteBufAllocator alloc, String peerHost, int peerPort) { - return configureAndWrapEngine(context().createSSLEngine(peerHost, peerPort)); + return configureAndWrapEngine(context().createSSLEngine(peerHost, peerPort), alloc); } - private SSLEngine configureAndWrapEngine(SSLEngine engine) { + private SSLEngine configureAndWrapEngine(SSLEngine engine, ByteBufAllocator alloc) { engine.setEnabledCipherSuites(cipherSuites); engine.setEnabledProtocols(protocols); engine.setUseClientMode(isClient()); @@ -236,7 +236,7 @@ public class JdkSslContext extends SslContext { throw new Error("Unknown auth " + clientAuth); } } - return apn.wrapperFactory().wrapSslEngine(engine, apn, isServer()); + return apn.wrapperFactory().wrapSslEngine(engine, alloc, apn, isServer()); } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java index e2171365f9..d51487a877 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java @@ -73,4 +73,10 @@ public class ConscryptJdkSslEngineInteropTest extends SSLEngineTest { @Override public void testMutualAuthValidClientCertChainTooLongFailRequireClientAuth() throws Exception { } + + @Override + protected boolean mySetupMutualAuthServerIsValidServerException(Throwable cause) { + // TODO(scott): work around for a JDK issue. The exception should be SSLHandshakeException. + return super.mySetupMutualAuthServerIsValidServerException(cause) || causedBySSLException(cause); + } } diff --git a/pom.xml b/pom.xml index 951bba72f2..3b9007a7b4 100644 --- a/pom.xml +++ b/pom.xml @@ -192,7 +192,7 @@ ${os.detected.classifier} org.conscrypt conscrypt-openjdk-uber - 1.0.0.RC7 + 1.0.0.RC9 ${os.detected.name}-${os.detected.arch} ${project.basedir}/../common/src/test/resources/logback-test.xml