ExpediaGroup
Improve TLS connection establishment
Problem statement
NettyConnectionFactory may hand out a TCP connection whose TLS handshake has not yet been completed.
The channel is created and connected here:
private ChannelFuture openConnection(Origin origin, ConnectionSettings connectionSettings) {
bootstrap(connectionSettings);
return bootstrap.connect(origin.host(), origin.port());
}
private synchronized void bootstrap(ConnectionSettings connectionSettings) {
if (bootstrap == null) {
bootstrap = new Bootstrap();
bootstrap.group(executor.eventLoopGroup())
.channel(executor.clientEventLoopClass())
.handler(new Initializer())
.option(TCP_NODELAY, true)
.option(SO_KEEPALIVE, true)
.option(ALLOCATOR, PooledByteBufAllocator.DEFAULT)
.option(CONNECT_TIMEOUT_MILLIS, connectionSettings.connectTimeoutMillis());
for (ChannelOptionSetting setting : httpConfig.channelSettings()) {
bootstrap.option(setting.option(), setting.value());
}
}
}
private class Initializer extends ChannelInitializer<Channel> {
@Override
protected void initChannel(Channel ch) {
}
}
Notice the empty channel initialiser. The channel handlers are not added here where you expect. Instead, they are added later when the connection is established.
Netty connection is emitted via Mono.sink() when the TCP connection establishment completes. This is when the TLS handshake begins. So the handshake continues in the background, and yet the connection is emitted to the consumers.
We should really look at the TLS handshake future, and emit the connection only once it completes.
public Mono<Connection> createConnection(Origin origin, ConnectionSettings connectionSettings, SslContext sslContext) {
return Mono.create(sink -> {
ChannelFuture channelFuture = openConnection(origin, connectionSettings);
channelFuture.addListener(future -> {
if (future.isSuccess()) {
sink.success(new NettyConnection(origin, channelFuture.channel(), httpRequestOperationFactory,
httpConfig, sslContext, sendSni, sniHost));
} else {
sink.error(new OriginUnreachableException(origin, future.cause()));
}
});
});
}
Impact
-
This could result in requests failing unnecessarily if the handshake ends up in failure. The connection pool could have handed out an alternative connection instead.
-
But in a normal production use the TLS handshake would only fail if the origin is improperly configured. Either in Styx or in the remote end (the origin itself).
-
Also, some end-to-end tests may be affected. They are built assuming the connection pool never emits unless the connection is readily usable.
-
Luckily Netty SSL handler buffers all sent data chunks until the handshake completes. So there is no risk of losing data, nor sending anything unencrypted or unauthenticated.
Please clarify and document the Netty behaviour - and close this issue if no work is required.
Please clarify and document the Netty behaviour - and close this issue if no work is required.
Check above. I have edited the description.
Closing all issues over 3 years old. New issues can be created if problems are still occurring.
