Enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER when using OpenSslContext

normanmaurer · normanmaurer · commit eb1d9da76c39 · 2016-02-03T11:29:07.000+01:00
Motivation: We need to enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER when using OpenSslContext as the memory address of the buffer that is passed to OpenSslEngine.wrap(...) may change during calls and retries. This is the case as if the buffer is a heap-buffer we will need to copy it to a direct buffer to hand it over to the JNI layer. When not enable this mode we may see errors like: 'error:1409F07F:SSL routines:SSL3_WRITE_PENDING: bad write retry'. Related to netty/netty-tcnative#100. Modifications: Explitict set mode to SSL.SSL_MODE_RELEASE_BUFFERS | SSL.SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER . (SSL.SSL_MODE_RELEASE_BUFFERS was used before implicitly). Result: No more 'error:1409F07F:SSL routines:SSL3_WRITE_PENDING: bad write retry' possible when writing heap buffers.
diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java
@@ -196,6 +196,11 @@ public SelectedListenerFailureBehavior selectedListenerFailureBehavior() {
                 SSLContext.setOptions(ctx, SSL.SSL_OP_SINGLE_DH_USE);
                 SSLContext.setOptions(ctx, SSL.SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION);
 
+                // 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);
+
                 /* List the ciphers that are permitted to negotiate. */
                 try {
                     SSLContext.setCipherSuite(ctx, CipherSuiteConverter.toOpenSsl(unmodifiableCiphers));
diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java
@@ -15,13 +15,22 @@
  */
 package io.netty.handler.ssl;
 
+import io.netty.buffer.UnpooledByteBufAllocator;
+import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
+import io.netty.handler.ssl.util.SelfSignedCertificate;
+import io.netty.util.internal.ThreadLocalRandom;
 import org.junit.Test;
 
 import io.netty.handler.ssl.ApplicationProtocolConfig.Protocol;
 import io.netty.handler.ssl.ApplicationProtocolConfig.SelectedListenerFailureBehavior;
 import io.netty.handler.ssl.ApplicationProtocolConfig.SelectorFailureBehavior;
 
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLEngineResult;
+import java.nio.ByteBuffer;
+
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assume.assumeTrue;
 
 public class OpenSslEngineTest extends SSLEngineTest {
@@ -103,6 +112,32 @@ public void testSessionInvalidate() throws Exception {
         super.testSessionInvalidate();
     }
 
+    @Test
+    public void testWrapHeapBuffersNoWritePendingError() throws Exception {
+        assumeTrue(OpenSsl.isAvailable());
+        final SslContext clientContext = SslContextBuilder.forClient()
+                .trustManager(InsecureTrustManagerFactory.INSTANCE)
+                .sslProvider(sslProvider())
+                .build();
+        SelfSignedCertificate ssc = new SelfSignedCertificate();
+        SslContext serverContext = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey())
+                .sslProvider(sslProvider())
+                .build();
+        SSLEngine clientEngine = clientContext.newEngine(UnpooledByteBufAllocator.DEFAULT);
+        SSLEngine serverEngine = serverContext.newEngine(UnpooledByteBufAllocator.DEFAULT);
+        handshake(clientEngine, serverEngine);
+
+        ByteBuffer src = ByteBuffer.allocate(1024 * 10);
+        ThreadLocalRandom.current().nextBytes(src.array());
+        ByteBuffer dst = ByteBuffer.allocate(1);
+        // Try to wrap multiple times so we are more likely to hit the issue.
+        for (int i = 0; i < 100; i++) {
+            src.position(0);
+            dst.position(0);
+            assertSame(SSLEngineResult.Status.BUFFER_OVERFLOW, clientEngine.wrap(src, dst).getStatus());
+        }
+    }
+
     @Override
     protected SslProvider sslProvider() {
         return SslProvider.OPENSSL;
diff --git a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
@@ -366,7 +366,7 @@ protected void testEnablingAnAlreadyDisabledSslProtocol(String[] protocols1, Str
         }
     }
 
-    private static void handshake(SSLEngine clientEngine, SSLEngine serverEngine) throws SSLException {
+    protected static void handshake(SSLEngine clientEngine, SSLEngine serverEngine) throws SSLException {
         int netBufferSize = 17 * 1024;
         ByteBuffer cTOs = ByteBuffer.allocateDirect(netBufferSize);
         ByteBuffer sTOc = ByteBuffer.allocateDirect(netBufferSize);

Original file line number	Diff line number	Diff line change
`@@ -366,7 +366,7 @@ protected void testEnablingAnAlreadyDisabledSslProtocol(String[] protocols1, Str`
`366`	`366`	`}`
`367`	`367`	`}`
`368`	`368`
`369`		`- private static void handshake(SSLEngine clientEngine, SSLEngine serverEngine) throws SSLException {`
	`369`	`+ protected static void handshake(SSLEngine clientEngine, SSLEngine serverEngine) throws SSLException {`
`370`	`370`	`int netBufferSize = 17 * 1024;`
`371`	`371`	`ByteBuffer cTOs = ByteBuffer.allocateDirect(netBufferSize);`
`372`	`372`	`ByteBuffer sTOc = ByteBuffer.allocateDirect(netBufferSize);`