Skip to content

Commit ecd78da

Browse files
joviegasdagnir
joviegas
and
dagnir
authored
Add exception handling for TLS half-close in ApacheHTTPClient #5385 (#5398)
* Check if input is shut down before writing (#5257) This commit adds a wrapper socket that ensures the read end of the socket is still open before performing a {@code write()}. In TLS 1.3, it is permitted for the connection to be in a half-closed state, which is dangerous for the Apache client because it can get stuck in a state where it continues to write to the socket and potentially end up a blocked state writing to the socket indefinitely. * Delegate write() methods directly to base * Added test cases to test TLS half close * feat: Add exception handling for TLS half-close in ApacheHTTPClient Implemented a feature to handle TLS half-close scenarios by throwing an exception in the ApacheHTTPClient package. In TLS 1.3, the inbound and outbound close_notify alerts are independent. When the client receives a close_notify alert, it only closes the inbound stream but continues to send data to the server. Previously, the SDK could not detect that the connection was closed on the server side, causing it to get stuck while writing to the socket and eventually timing out. This feature ensures proper detection and handling of closed connections, improving overall reliability by preventing client hangs. * Handled review comemnts * Updated test case to consider Jdk 1.8 builds which donot support TLS1.3 half close --------- Co-authored-by: Dongie Agnir <[email protected]> Co-authored-by: Dongie Agnir <[email protected]>
1 parent f1f28c3 commit ecd78da

File tree

6 files changed

+667
-1
lines changed

6 files changed

+667
-1
lines changed

‎.changes/next-release/bugfix-ApacheHTTPClient-a5c4ad8.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Apache HTTP Client",
4+
"contributor": "",
5+
"description": "Added fix to handle TLS half-close scenarios by throwing an exception. In TLS 1.3, the inbound and outbound close_notify alerts are independent. When the client receives a close_notify alert, it only closes the inbound stream but continues to send data to the server. Previously, the SDK could not detect that the connection was closed on the server side, causing it to get stuck while writing to the socket and eventually timing out. With this bug fix, the SDK will now detect the closed connection and throw an appropriate exception, preventing client hangs and improving overall reliability."
6+
}

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/conn/SdkTlsSocketFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
2727
import org.apache.http.protocol.HttpContext;
2828
import software.amazon.awssdk.annotations.SdkInternalApi;
29+
import software.amazon.awssdk.http.apache.internal.net.InputShutdownCheckingSslSocket;
2930
import software.amazon.awssdk.http.apache.internal.net.SdkSocket;
3031
import software.amazon.awssdk.http.apache.internal.net.SdkSslSocket;
3132
import software.amazon.awssdk.utils.Logger;
@@ -62,7 +63,7 @@ public Socket connectSocket(int connectTimeout,
6263
Socket connectedSocket = super.connectSocket(connectTimeout, socket, host, remoteAddress, localAddress, context);
6364

6465
if (connectedSocket instanceof SSLSocket) {
65-
return new SdkSslSocket((SSLSocket) connectedSocket);
66+
return new InputShutdownCheckingSslSocket(new SdkSslSocket((SSLSocket) connectedSocket));
6667
}
6768

6869
return new SdkSocket(connectedSocket);

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/net/InputShutdownCheckingSslSocket.java

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Copyright 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.http.apache.internal.net;
17+
18+
import java.io.FilterOutputStream;
19+
import java.io.IOException;
20+
import java.io.OutputStream;
21+
import javax.net.ssl.SSLSocket;
22+
import software.amazon.awssdk.annotations.SdkInternalApi;
23+
24+
/**
25+
* Wrapper socket that ensures the read end of the socket is still open before performing a {@code write()}. In TLS 1.3, it is
26+
* permitted for the connection to be in a half-closed state, which is dangerous for the Apache client because it can get stuck in
27+
* a state where it continues to write to the socket and potentially end up a blocked state writing to the socket indefinitely.
28+
*/
29+
@SdkInternalApi
30+
public final class InputShutdownCheckingSslSocket extends DelegateSslSocket {
31+
32+
public InputShutdownCheckingSslSocket(SSLSocket sock) {
33+
super(sock);
34+
}
35+
36+
@Override
37+
public OutputStream getOutputStream() throws IOException {
38+
return new InputShutdownCheckingOutputStream(sock.getOutputStream(), sock);
39+
}
40+
41+
private static class InputShutdownCheckingOutputStream extends FilterOutputStream {
42+
private final SSLSocket sock;
43+
44+
InputShutdownCheckingOutputStream(OutputStream out, SSLSocket sock) {
45+
super(out);
46+
this.sock = sock;
47+
}
48+
49+
@Override
50+
public void write(int b) throws IOException {
51+
checkInputShutdown();
52+
out.write(b);
53+
}
54+
55+
@Override
56+
public void write(byte[] b) throws IOException {
57+
checkInputShutdown();
58+
out.write(b);
59+
}
60+
61+
@Override
62+
public void write(byte[] b, int off, int len) throws IOException {
63+
checkInputShutdown();
64+
out.write(b, off, len);
65+
}
66+
67+
private void checkInputShutdown() throws IOException {
68+
if (sock.isInputShutdown()) {
69+
throw new IOException("Remote end is closed.");
70+
}
71+
72+
try {
73+
sock.getInputStream();
74+
} catch (IOException inputStreamException) {
75+
IOException e = new IOException("Remote end is closed.");
76+
e.addSuppressed(inputStreamException);
77+
throw e;
78+
}
79+
}
80+
}
81+
}

‎http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheClientTlsHalfCloseTest.java

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/*
2+
* Copyright 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.http.apache;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertThrows;
21+
22+
import java.io.ByteArrayInputStream;
23+
import java.io.IOException;
24+
import org.junit.jupiter.api.AfterAll;
25+
import org.junit.jupiter.api.AfterEach;
26+
import org.junit.jupiter.api.BeforeAll;
27+
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.api.condition.EnabledIf;
29+
import software.amazon.awssdk.http.ContentStreamProvider;
30+
import software.amazon.awssdk.http.FileStoreTlsKeyManagersProvider;
31+
import software.amazon.awssdk.http.HttpExecuteRequest;
32+
import software.amazon.awssdk.http.HttpExecuteResponse;
33+
import software.amazon.awssdk.http.SdkHttpClient;
34+
import software.amazon.awssdk.http.SdkHttpFullRequest;
35+
import software.amazon.awssdk.http.SdkHttpMethod;
36+
import software.amazon.awssdk.http.SdkHttpRequest;
37+
import software.amazon.awssdk.http.TlsKeyManagersProvider;
38+
import software.amazon.awssdk.http.server.MockServer;
39+
40+
public class ApacheClientTlsHalfCloseTest extends ClientTlsAuthTestBase {
41+
42+
private static TlsKeyManagersProvider tlsKeyManagersProvider;
43+
private static MockServer mockServer;
44+
private SdkHttpClient httpClient;
45+
46+
private static final int TWO_MB = 2 * 1024 * 1024;
47+
private static final byte[] CONTENT = new byte[TWO_MB];
48+
49+
@Test
50+
@EnabledIf("halfCloseSupported")
51+
public void errorWhenServerHalfClosesSocketWhileStreamIsOpened() {
52+
53+
mockServer = MockServer.createMockServer(MockServer.ServerBehavior.HALF_CLOSE);
54+
mockServer.startServer(tlsKeyManagersProvider);
55+
56+
httpClient = ApacheHttpClient.builder()
57+
.tlsKeyManagersProvider(tlsKeyManagersProvider)
58+
.build();
59+
IOException exception = assertThrows(IOException.class, () -> {
60+
executeHttpRequest(httpClient);
61+
});
62+
assertEquals("Remote end is closed.", exception.getMessage());
63+
}
64+
65+
66+
@Test
67+
public void errorWhenServerFullClosesSocketWhileStreamIsOpened() throws IOException {
68+
mockServer = MockServer.createMockServer(MockServer.ServerBehavior.FULL_CLOSE_IN_BETWEEN);
69+
mockServer.startServer(tlsKeyManagersProvider);
70+
71+
httpClient = ApacheHttpClient.builder()
72+
.tlsKeyManagersProvider(tlsKeyManagersProvider)
73+
.build();
74+
75+
IOException exception = assertThrows(IOException.class, () -> {
76+
executeHttpRequest(httpClient);
77+
});
78+
79+
if(halfCloseSupported()){
80+
assertEquals("Remote end is closed.", exception.getMessage());
81+
82+
}else {
83+
assertEquals("Socket is closed", exception.getMessage());
84+
85+
}
86+
}
87+
88+
@Test
89+
public void successfulRequestForFullCloseSocketAtTheEnd() throws IOException {
90+
mockServer = MockServer.createMockServer(MockServer.ServerBehavior.FULL_CLOSE_AT_THE_END);
91+
mockServer.startServer(tlsKeyManagersProvider);
92+
93+
httpClient = ApacheHttpClient.builder()
94+
.tlsKeyManagersProvider(tlsKeyManagersProvider)
95+
.build();
96+
97+
HttpExecuteResponse response = executeHttpRequest(httpClient);
98+
99+
assertThat(response.httpResponse().isSuccessful()).isTrue();
100+
}
101+
102+
@AfterEach
103+
void tearDown() {
104+
if (mockServer != null) {
105+
mockServer.stopServer();
106+
}
107+
}
108+
109+
@BeforeAll
110+
public static void setUp() throws IOException {
111+
ClientTlsAuthTestBase.setUp();
112+
System.setProperty("javax.net.ssl.trustStore", serverKeyStore.toAbsolutePath().toString());
113+
System.setProperty("javax.net.ssl.trustStorePassword", STORE_PASSWORD);
114+
System.setProperty("javax.net.ssl.trustStoreType", "jks");
115+
tlsKeyManagersProvider = FileStoreTlsKeyManagersProvider.create(clientKeyStore, CLIENT_STORE_TYPE, STORE_PASSWORD);
116+
}
117+
118+
@AfterAll
119+
public static void clear() throws IOException {
120+
System.clearProperty("javax.net.ssl.trustStore");
121+
System.clearProperty("javax.net.ssl.trustStorePassword");
122+
System.clearProperty("javax.net.ssl.trustStoreType");
123+
ClientTlsAuthTestBase.teardown();
124+
125+
}
126+
127+
private static HttpExecuteResponse executeHttpRequest(SdkHttpClient client) throws IOException {
128+
ContentStreamProvider contentStreamProvider = () -> new ByteArrayInputStream(CONTENT);
129+
SdkHttpRequest httpRequest = SdkHttpFullRequest.builder()
130+
.method(SdkHttpMethod.PUT)
131+
.protocol("https")
132+
.host("localhost:" + mockServer.getPort())
133+
.build();
134+
HttpExecuteRequest request = HttpExecuteRequest.builder()
135+
.request(httpRequest)
136+
.contentStreamProvider(contentStreamProvider)
137+
.build();
138+
139+
return client.prepareRequest(request).call();
140+
}
141+
142+
public static boolean halfCloseSupported(){
143+
return MockServer.isTlsHalfCloseSupported();
144+
}
145+
}

‎http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/conn/InputShutdownCheckingSslSocketTest.java

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package software.amazon.awssdk.http.apache.internal.conn;
2+
3+
import org.junit.jupiter.api.Test;
4+
import javax.net.ssl.SSLSocket;
5+
import java.io.IOException;
6+
import java.io.OutputStream;
7+
import software.amazon.awssdk.http.apache.internal.net.InputShutdownCheckingSslSocket;
8+
9+
import static org.junit.jupiter.api.Assertions.assertThrows;
10+
import static org.junit.jupiter.api.Assertions.assertTrue;
11+
import static org.mockito.Mockito.mock;
12+
import static org.mockito.Mockito.verify;
13+
import static org.mockito.Mockito.when;
14+
15+
public class InputShutdownCheckingSslSocketTest {
16+
17+
@Test
18+
public void outputStreamChecksInputShutdown() throws IOException {
19+
SSLSocket mockSocket = mock(SSLSocket.class);
20+
when(mockSocket.isInputShutdown()).thenReturn(true);
21+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
22+
OutputStream os = socket.getOutputStream();
23+
assertThrows(IOException.class, () -> os.write(1));
24+
}
25+
26+
@Test
27+
public void outputStreamWritesNormallyWhenInputNotShutdown() throws IOException {
28+
SSLSocket mockSocket = mock(SSLSocket.class);
29+
OutputStream mockOutputStream = mock(OutputStream.class);
30+
when(mockSocket.isInputShutdown()).thenReturn(false);
31+
when(mockSocket.getOutputStream()).thenReturn(mockOutputStream);
32+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
33+
OutputStream os = socket.getOutputStream();
34+
os.write(1);
35+
verify(mockOutputStream).write(1);
36+
}
37+
38+
@Test
39+
public void writeByteArrayThrowsIOExceptionWhenInputIsShutdown() throws IOException {
40+
SSLSocket mockSocket = mock(SSLSocket.class);
41+
when(mockSocket.isInputShutdown()).thenReturn(true);
42+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
43+
OutputStream os = socket.getOutputStream();
44+
assertThrows(IOException.class, () -> os.write(new byte[10]));
45+
}
46+
47+
@Test
48+
public void writeByteArraySucceedsWhenInputNotShutdown() throws IOException {
49+
SSLSocket mockSocket = mock(SSLSocket.class);
50+
OutputStream mockOutputStream = mock(OutputStream.class);
51+
when(mockSocket.isInputShutdown()).thenReturn(false);
52+
when(mockSocket.getOutputStream()).thenReturn(mockOutputStream);
53+
54+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
55+
OutputStream os = socket.getOutputStream();
56+
57+
byte[] data = new byte[10];
58+
os.write(data);
59+
verify(mockOutputStream).write(data);
60+
}
61+
62+
@Test
63+
public void writeByteArrayWithOffsetThrowsIOExceptionWhenInputIsShutdown() throws IOException {
64+
SSLSocket mockSocket = mock(SSLSocket.class);
65+
when(mockSocket.isInputShutdown()).thenReturn(true);
66+
67+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
68+
OutputStream os = socket.getOutputStream();
69+
70+
assertThrows(IOException.class, () -> os.write(new byte[10], 0, 10));
71+
}
72+
73+
@Test
74+
public void writeByteArrayWithOffsetSucceedsWhenInputNotShutdown() throws IOException {
75+
SSLSocket mockSocket = mock(SSLSocket.class);
76+
OutputStream mockOutputStream = mock(OutputStream.class);
77+
when(mockSocket.isInputShutdown()).thenReturn(false);
78+
when(mockSocket.getOutputStream()).thenReturn(mockOutputStream);
79+
80+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
81+
OutputStream os = socket.getOutputStream();
82+
83+
byte[] data = new byte[10];
84+
os.write(data, 0, 10);
85+
verify(mockOutputStream).write(data, 0, 10);
86+
}
87+
88+
@Test
89+
public void checkInputShutdownThrowsIOExceptionWithSuppressed() throws IOException {
90+
SSLSocket mockSocket = mock(SSLSocket.class);
91+
when(mockSocket.isInputShutdown()).thenReturn(false);
92+
when(mockSocket.getInputStream()).thenThrow(new IOException("InputStream exception"));
93+
94+
InputShutdownCheckingSslSocket socket = new InputShutdownCheckingSslSocket(mockSocket);
95+
OutputStream os = socket.getOutputStream();
96+
97+
IOException thrown = assertThrows(IOException.class, () -> os.write(1));
98+
assertTrue(thrown.getMessage().contains("Remote end is closed."));
99+
assertTrue(thrown.getSuppressed()[0].getMessage().contains("InputStream exception"));
100+
}
101+
}

0 commit comments

Comments
 (0)