Skip to content

Commit 92fcb55

Browse files
committed
Fix a bug where SNI was not enabled in Netty NIO Async Client and caused the requests to fail of handshake_failure in some services.
1 parent c68a6de commit 92fcb55

File tree

4 files changed

+77
-3
lines changed

4 files changed

+77
-3
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "Netty NIO HTTP Client",
3+
"type": "bugfix",
4+
"description": "Fix a bug where SNI was not enabled in Netty NIO Async Client for TLS and caused the requests to fail of handshake_failure in some services. See [#1171](https://github.com/aws/aws-sdk-java-v2/issues/1171)"
5+
}

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ protected ChannelPool newPool(URI key) {
193193

194194
AtomicReference<ChannelPool> channelPoolRef = new AtomicReference<>();
195195
ChannelPipelineInitializer handler =
196-
new ChannelPipelineInitializer(protocol, sslContext, maxStreams, channelPoolRef, configuration);
196+
new ChannelPipelineInitializer(protocol, sslContext, maxStreams, channelPoolRef, configuration, key);
197197
channelPoolRef.set(createChannelPool(bootstrap, handler));
198198
return channelPoolRef.get();
199199
}

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,12 @@
3030
import io.netty.handler.logging.LogLevel;
3131
import io.netty.handler.logging.LoggingHandler;
3232
import io.netty.handler.ssl.SslContext;
33+
import io.netty.handler.ssl.SslHandler;
34+
import java.net.URI;
3335
import java.util.concurrent.CompletableFuture;
3436
import java.util.concurrent.atomic.AtomicReference;
37+
import javax.net.ssl.SSLEngine;
38+
import javax.net.ssl.SSLParameters;
3539
import software.amazon.awssdk.annotations.SdkInternalApi;
3640
import software.amazon.awssdk.http.Protocol;
3741
import software.amazon.awssdk.http.nio.netty.internal.http2.Http2SettingsFrameHandler;
@@ -46,25 +50,34 @@ public final class ChannelPipelineInitializer extends AbstractChannelPoolHandler
4650
private final long clientMaxStreams;
4751
private final AtomicReference<ChannelPool> channelPoolRef;
4852
private final NettyConfiguration configuration;
53+
private final URI poolKey;
4954

5055
public ChannelPipelineInitializer(Protocol protocol,
5156
SslContext sslCtx,
5257
long clientMaxStreams,
5358
AtomicReference<ChannelPool> channelPoolRef,
54-
NettyConfiguration configuration) {
59+
NettyConfiguration configuration,
60+
URI poolKey) {
5561
this.protocol = protocol;
5662
this.sslCtx = sslCtx;
5763
this.clientMaxStreams = clientMaxStreams;
5864
this.channelPoolRef = channelPoolRef;
5965
this.configuration = configuration;
66+
this.poolKey = poolKey;
6067
}
6168

6269
@Override
6370
public void channelCreated(Channel ch) {
6471
ch.attr(PROTOCOL_FUTURE).set(new CompletableFuture<>());
6572
ChannelPipeline pipeline = ch.pipeline();
6673
if (sslCtx != null) {
67-
pipeline.addLast(sslCtx.newHandler(ch.alloc()));
74+
75+
// Need to provide host and port to enable SNI
76+
// https://github.com/netty/netty/issues/3801#issuecomment-104274440
77+
SslHandler sslHandler = sslCtx.newHandler(ch.alloc(), poolKey.getHost(), poolKey.getPort());
78+
configureSslEngine(sslHandler.engine());
79+
80+
pipeline.addLast(sslHandler);
6881
pipeline.addLast(SslCloseCompletionEventHandler.getInstance());
6982
}
7083

@@ -87,6 +100,20 @@ public void channelCreated(Channel ch) {
87100
pipeline.addLast(new LoggingHandler(LogLevel.DEBUG));
88101
}
89102

103+
/**
104+
* Enable HostName verification.
105+
*
106+
* See https://netty.io/4.0/api/io/netty/handler/ssl/SslContext.html#newHandler-io.netty.buffer.ByteBufAllocator-java.lang
107+
* .String-int-
108+
*
109+
* @param sslEngine the sslEngine to configure
110+
*/
111+
private void configureSslEngine(SSLEngine sslEngine) {
112+
SSLParameters sslParameters = sslEngine.getSSLParameters();
113+
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
114+
sslEngine.setSSLParameters(sslParameters);
115+
}
116+
90117
private void configureHttp2(Channel ch, ChannelPipeline pipeline) {
91118
ForkedHttp2MultiplexCodecBuilder codecBuilder = ForkedHttp2MultiplexCodecBuilder
92119
.forClient(new NoOpChannelInitializer())
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.pinpoint;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import org.junit.BeforeClass;
21+
import org.junit.Test;
22+
import software.amazon.awssdk.regions.Region;
23+
import software.amazon.awssdk.testutils.service.AwsTestBase;
24+
import software.amazon.awssdk.utils.builder.SdkBuilder;
25+
26+
public class PinpointIntegTest extends AwsTestBase {
27+
28+
protected static PinpointAsyncClient pinpointAsyncClient;
29+
30+
@BeforeClass
31+
public static void setup() {
32+
pinpointAsyncClient = PinpointAsyncClient.builder()
33+
.region(Region.US_WEST_2)
34+
.credentialsProvider(CREDENTIALS_PROVIDER_CHAIN)
35+
.build();
36+
}
37+
38+
@Test
39+
public void getApps() {
40+
assertThat(pinpointAsyncClient.getApps(SdkBuilder::build).join()).isNotNull();
41+
}
42+
}

0 commit comments

Comments
 (0)