Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow server initiated renegotiate when using OpenSSL / BoringSSL bas… #11601

Merged
merged 2 commits into from
Aug 20, 2021

Conversation

normanmaurer
Copy link
Member

…ed SSLEngine

Motivation:

We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.

Modifications:

  • Upgrade netty-tcnative version to be able to allow renegotiate once
  • Adjust code

Result
Fixes #11529

@normanmaurer
Copy link
Member Author

Let's see if I can somehow write a unit test for this by just using netty.

Here is a "test-case" that needs a remote server that was provided by @slandelle :

import io.netty.bootstrap.Bootstrap;
import io.netty.channel.*;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.nio.NioSocketChannel;
import io.netty.handler.codec.http.*;
import io.netty.handler.logging.LoggingHandler;
import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslHandler;
import io.netty.handler.ssl.SslProvider;
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.FutureListener;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

public class Main {

    private static final Logger LOGGER = LoggerFactory.getLogger(Main.class);

    public static void main(String[] args) throws Exception {

        EventLoopGroup clientGroup = new NioEventLoopGroup();
        final SslContext sslContext = SslContextBuilder.forClient()
                .trustManager(InsecureTrustManagerFactory.INSTANCE)
                .sslProvider(SslProvider.OPENSSL)
                .build();

        final String hostname = "access.stage.api.bbc.com";
        InetAddress address = InetAddress.getByName(hostname);

        LOGGER.info("Connecting to {}", address.getHostAddress());

        final CountDownLatch latch = new CountDownLatch(1);

        try {
            Bootstrap cb = new Bootstrap();
            cb.group(clientGroup)
                    .channel(NioSocketChannel.class)
                    .handler(new ChannelInitializer<NioSocketChannel>() {
                        @Override
                        public void initChannel(NioSocketChannel ch) {
                            ch.pipeline().addLast(
                                    new LoggingHandler(),
                                    new HttpClientCodec(),
                                    new HttpContentDecompressor(),
                                    new HttpObjectAggregator(Integer.MAX_VALUE),
                                    new ChannelInboundHandlerAdapter() {

                                        private boolean responseReceived;

                                        @Override
                                        public void channelInactive(ChannelHandlerContext ctx) {
                                            if (!responseReceived && latch.getCount() > 0) {
                                                LOGGER.error("Premature close: connection was closed before receiving the response!!!");
                                                latch.countDown();
                                            }
                                        }

                                        @Override
                                        public void channelRead(ChannelHandlerContext ctx, Object msg) {
                                            responseReceived = true;
                                            LOGGER.info("Received response {}", msg);
                                            latch.countDown();
                                        }
                                    }
                            );
                        }
                    });

            ChannelFuture whenChannel = cb.connect(new InetSocketAddress(address, 443));

            whenChannel.addListener(new ChannelFutureListener() {
                @Override
                public void operationComplete(ChannelFuture f) throws Exception {
                    if (f.isSuccess()) {
                        final Channel channel = f.channel();
                        SslHandler sslHandler = new SslHandler(sslContext.newEngine(channel.alloc(), hostname, 443));
                        channel.pipeline().addFirst(sslHandler);

                        sslHandler.handshakeFuture().addListener(new FutureListener<Channel>() {
                            @Override
                            public void operationComplete(Future<Channel> f2) throws Exception {
                                if (f2.isSuccess()) {
                                    DefaultHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/v1/user/tokenInfo");
                                    request.headers()
                                            .set(HttpHeaderNames.HOST, hostname)
                                            .set(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.GZIP_DEFLATE);

                                    channel.writeAndFlush(request);
                                } else {
                                    LOGGER.error("Handshake failed", f2.cause());
                                    latch.countDown();
                                }
                            }
                        });
                    } else {
                        LOGGER.error("Connect failed", f.cause());
                        latch.countDown();
                    }
                }
            });

            latch.await(10, TimeUnit.SECONDS);

        } finally {
            clientGroup.shutdownGracefully();
        }
    }
}

@normanmaurer normanmaurer added this to the 4.1.68.Final milestone Aug 19, 2021
Copy link
Contributor

@chrisvest chrisvest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had some questions. Also, I hope you're able to add a test for this.

…ed SSLEngine

Motivation:

We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.

Modifications:

- Upgrade netty-tcnative version to be able to allow renegotiate once
- Adjust code

Result
Fixes #11529
@normanmaurer
Copy link
Member Author

Let me just merge this for now and I will continue trying to come up with a test as a followup

@normanmaurer normanmaurer merged commit 33b63c3 into 4.1 Aug 20, 2021
@normanmaurer normanmaurer deleted the server_init_renegotiate branch August 20, 2021 16:52
normanmaurer added a commit that referenced this pull request Aug 20, 2021
…ed SSLEngine (#11601)


Motivation:

We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.

Modifications:

- Upgrade netty-tcnative version to be able to allow renegotiate once
- Adjust code

Result
Fixes #11529
laosijikaichele pushed a commit to laosijikaichele/netty that referenced this pull request Dec 16, 2021
…ed SSLEngine (netty#11601)


Motivation:

We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.

Modifications:

- Upgrade netty-tcnative version to be able to allow renegotiate once
- Adjust code

Result
Fixes netty#11529
laosijikaichele pushed a commit to laosijikaichele/netty that referenced this pull request Dec 16, 2021
…ed SSLEngine (netty#11601)


Motivation:

We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.

Modifications:

- Upgrade netty-tcnative version to be able to allow renegotiate once
- Adjust code

Result
Fixes netty#11529
raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
…ed SSLEngine (netty#11601)


Motivation:

We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.

Modifications:

- Upgrade netty-tcnative version to be able to allow renegotiate once
- Adjust code

Result
Fixes netty#11529
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BoringSSL + TLSv1.2 client side SSL renegociation fails
3 participants