From 5deec9631f31efaea105988a1a814f15881f699a Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 29 Sep 2015 09:39:57 +0200 Subject: [PATCH] Add support for server-side renegotiation when using OpenSslEngine. Motivation: JDK SslEngine supports renegotion, so we should at least support it server-side with OpenSslEngine as well. That said OpenSsl does not support sending messages asynchronly while the renegotiation is still in progress, so the application need to ensure there are not writes going on while the renegotiation takes place. See also https://rt.openssl.org/Ticket/Display.html?id=1019 . Modifications: - Add support for renegotiation when OpenSslEngine is used in server mode - Add unit tests. - Upgrade to netty-tcnative 1.1.33.Fork9 Result: Better compatibility with the JDK SSLEngine implementation. --- .../io/netty/handler/ssl/OpenSslEngine.java | 41 +++++- .../handler/ssl/JdkSslRenegotiateTest.java | 24 +++ .../handler/ssl/OpenSslRenegotiateTest.java | 32 ++++ .../io/netty/handler/ssl/RenegotiateTest.java | 137 ++++++++++++++++++ pom.xml | 2 +- 5 files changed, 228 insertions(+), 8 deletions(-) create mode 100644 handler/src/test/java/io/netty/handler/ssl/JdkSslRenegotiateTest.java create mode 100644 handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java create mode 100644 handler/src/test/java/io/netty/handler/ssl/RenegotiateTest.java diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java index 57ae096dc8..98ba61271e 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java @@ -1042,10 +1042,6 @@ public final class OpenSslEngine extends SSLEngine { @Override public synchronized void beginHandshake() throws SSLException { switch (handshakeState) { - case NOT_STARTED: - handshakeState = HandshakeState.STARTED_EXPLICITLY; - handshake(); - break; case STARTED_IMPLICITLY: checkEngineClosed(); @@ -1056,11 +1052,39 @@ public final class OpenSslEngine extends SSLEngine { // for renegotiation. handshakeState = HandshakeState.STARTED_EXPLICITLY; // Next time this method is invoked by the user, - // we should raise an exception. + // we should raise an exception. + break; + case STARTED_EXPLICITLY: + // Nothing to do as the handshake is not done yet. break; case FINISHED: - case STARTED_EXPLICITLY: - throw RENEGOTIATION_UNSUPPORTED; + if (clientMode) { + // Only supported for server mode at the moment. + throw RENEGOTIATION_UNSUPPORTED; + } + // For renegotiate on the server side we need to issue the following command sequence with openssl: + // + // SSL_renegotiate(ssl) + // SSL_do_handshake(ssl) + // ssl->state = SSL_ST_ACCEPT + // SSL_do_handshake(ssl) + // + // Bcause of this we fall-through to call handshake() after setting the state, as this will also take + // care of updating the internal OpenSslSession object. + // + // See also: + // https://github.com/apache/httpd/blob/2.4.16/modules/ssl/ssl_engine_kernel.c#L812 + // http://h71000.www7.hp.com/doc/83final/ba554_90007/ch04s03.html + if (SSL.renegotiate(ssl) != 1 || SSL.doHandshake(ssl) != 1) { + shutdownWithError("renegotiation failed"); + } + + SSL.setState(ssl, SSL.SSL_ST_ACCEPT); + // fall-through + case NOT_STARTED: + handshakeState = HandshakeState.STARTED_EXPLICITLY; + handshake(); + break; default: throw new Error(); } @@ -1078,6 +1102,9 @@ public final class OpenSslEngine extends SSLEngine { } private HandshakeStatus handshake() throws SSLException { + if (handshakeState == HandshakeState.FINISHED) { + return FINISHED; + } checkEngineClosed(); int code = SSL.doHandshake(ssl); if (code <= 0) { diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkSslRenegotiateTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkSslRenegotiateTest.java new file mode 100644 index 0000000000..87b4c078fd --- /dev/null +++ b/handler/src/test/java/io/netty/handler/ssl/JdkSslRenegotiateTest.java @@ -0,0 +1,24 @@ +/* + * Copyright 2015 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.ssl; + +public class JdkSslRenegotiateTest extends RenegotiateTest { + + @Override + protected SslProvider serverSslProvider() { + return SslProvider.JDK; + } +} diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java new file mode 100644 index 0000000000..7b660b4e97 --- /dev/null +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java @@ -0,0 +1,32 @@ +/* + * Copyright 2015 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.ssl; + +import org.junit.Assume; + +public class OpenSslRenegotiateTest extends RenegotiateTest { + + @Override + public void testRenegotiateServer() throws Throwable { + Assume.assumeTrue(OpenSsl.isAvailable()); + super.testRenegotiateServer(); + } + + @Override + protected SslProvider serverSslProvider() { + return SslProvider.OPENSSL; + } +} diff --git a/handler/src/test/java/io/netty/handler/ssl/RenegotiateTest.java b/handler/src/test/java/io/netty/handler/ssl/RenegotiateTest.java new file mode 100644 index 0000000000..2b1be28b43 --- /dev/null +++ b/handler/src/test/java/io/netty/handler/ssl/RenegotiateTest.java @@ -0,0 +1,137 @@ +/* + * Copyright 2015 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.ssl; + +import io.netty.bootstrap.Bootstrap; +import io.netty.bootstrap.ServerBootstrap; +import io.netty.channel.Channel; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.channel.ChannelInitializer; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.local.LocalAddress; +import io.netty.channel.local.LocalChannel; +import io.netty.channel.local.LocalEventLoopGroup; +import io.netty.channel.local.LocalServerChannel; +import io.netty.handler.ssl.util.InsecureTrustManagerFactory; +import io.netty.handler.ssl.util.SelfSignedCertificate; +import io.netty.util.ReferenceCountUtil; +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.FutureListener; +import org.junit.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +public abstract class RenegotiateTest { + + @Test(timeout = 5000) + public void testRenegotiateServer() throws Throwable { + final AtomicReference error = new AtomicReference(); + final CountDownLatch latch = new CountDownLatch(2); + SelfSignedCertificate cert = new SelfSignedCertificate(); + EventLoopGroup group = new LocalEventLoopGroup(); + try { + final SslContext context = SslContextBuilder.forServer(cert.key(), cert.cert()) + .sslProvider(serverSslProvider()).build(); + ServerBootstrap sb = new ServerBootstrap(); + sb.group(group).channel(LocalServerChannel.class) + .childHandler(new ChannelInitializer() { + @Override + protected void initChannel(Channel ch) throws Exception { + ch.pipeline().addLast(context.newHandler(ch.alloc())); + ch.pipeline().addLast(new ChannelInboundHandlerAdapter() { + private boolean renegotiate; + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + ReferenceCountUtil.release(msg); + } + + @Override + public void userEventTriggered( + final ChannelHandlerContext ctx, Object evt) throws Exception { + if (!renegotiate && evt instanceof SslHandshakeCompletionEvent) { + SslHandshakeCompletionEvent event = (SslHandshakeCompletionEvent) evt; + + if (event.isSuccess()) { + final SslHandler handler = ctx.pipeline().get(SslHandler.class); + + renegotiate = true; + handler.renegotiate().addListener(new FutureListener() { + @Override + public void operationComplete(Future future) throws Exception { + if (!future.isSuccess()) { + error.compareAndSet(null, future.cause()); + latch.countDown(); + ctx.close(); + } + } + }); + } else { + error.compareAndSet(null, event.cause()); + latch.countDown(); + + ctx.close(); + } + } + } + }); + } + }); + Channel channel = sb.bind(new LocalAddress("test")).syncUninterruptibly().channel(); + + final SslContext clientContext = SslContextBuilder.forClient() + .trustManager(InsecureTrustManagerFactory.INSTANCE).sslProvider(SslProvider.JDK).build(); + + Bootstrap bootstrap = new Bootstrap(); + bootstrap.group(group).channel(LocalChannel.class) + .handler(new ChannelInitializer() { + @Override + protected void initChannel(Channel ch) throws Exception { + ch.pipeline().addLast(clientContext.newHandler(ch.alloc())); + ch.pipeline().addLast(new ChannelInboundHandlerAdapter() { + @Override + public void userEventTriggered( + ChannelHandlerContext ctx, Object evt) throws Exception { + if (evt instanceof SslHandshakeCompletionEvent) { + SslHandshakeCompletionEvent event = (SslHandshakeCompletionEvent) evt; + if (!event.isSuccess()) { + error.compareAndSet(null, event.cause()); + ctx.close(); + } + latch.countDown(); + } + } + }); + } + }); + + Channel clientChannel = bootstrap.connect(channel.localAddress()).syncUninterruptibly().channel(); + latch.await(); + clientChannel.close().syncUninterruptibly(); + channel.close().syncUninterruptibly(); + Throwable cause = error.get(); + if (cause != null) { + throw cause; + } + } finally { + group.shutdownGracefully(); + } + } + + protected abstract SslProvider serverSslProvider(); +} diff --git a/pom.xml b/pom.xml index 3642dd6fa8..88a45d0ef2 100644 --- a/pom.xml +++ b/pom.xml @@ -701,7 +701,7 @@ ${project.groupId} netty-tcnative - 1.1.33.Fork8 + 1.1.33.Fork9 ${tcnative.classifier} compile true