Upgrading to Conscrypt 1.0.0.RC9. (netty#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.
dschobel · Aug 3, 2017 · 4448b8f · 4448b8f
1 parent 52f384b
commit 4448b8f
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 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
@@ -19,6 +19,8 @@
 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,13 +33,18 @@
 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;
 
 /**
  * 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 @@ static boolean isEngineSupported(SSLEngine engine) {
         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<String> protocols) {
+    private ConscryptAlpnSslEngine(SSLEngine engine, ByteBufAllocator alloc, List<String> 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 @@ final SSLEngineResult unwrap(ByteBuffer[] srcs, ByteBuffer[] dests) throws SSLEx
     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 @@ private void selectProtocol() throws SSLException {
     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 @@ private static boolean isConscryptEngine(SSLEngine engine, Class<?> enginesClass
     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
@@ -15,6 +15,7 @@
  */
 package io.netty.handler.ssl;
 
+import io.netty.buffer.ByteBufAllocator;
 import javax.net.ssl.SSLEngine;
 
 /**
@@ -108,8 +109,8 @@ public JdkAlpnApplicationProtocolNegotiator(ProtocolSelectorFactory selectorFact
 
     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 SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiato
 
     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
@@ -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,15 +32,16 @@ interface SslEngineWrapperFactory {
          * 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 <ul>
          * <li>{@code true} if the engine is for server side of connections</li>
          * <li>{@code false} if the engine is for client side of connections</li>
          * </ul>
          * @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
@@ -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
@@ -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
@@ -210,15 +210,15 @@ public final long sessionTimeout() {
 
     @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 @@ private SSLEngine configureAndWrapEngine(SSLEngine engine) {
                     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
@@ -73,4 +73,10 @@ public void testMutualAuthValidClientCertChainTooLongFailOptionalClientAuth() th
     @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
@@ -192,7 +192,7 @@
     <tcnative.classifier>${os.detected.classifier}</tcnative.classifier>
     <conscrypt.groupId>org.conscrypt</conscrypt.groupId>
     <conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId>
-    <conscrypt.version>1.0.0.RC7</conscrypt.version>
+    <conscrypt.version>1.0.0.RC9</conscrypt.version>
     <conscrypt.classifier />
     <jni.classifier>${os.detected.name}-${os.detected.arch}</jni.classifier>
     <logging.config>${project.basedir}/../common/src/test/resources/logback-test.xml</logging.config>