diff --git a/.changes/next-release/bugfix-AWSCRTHTTPClient-1ad80f1.json b/.changes/next-release/bugfix-AWSCRTHTTPClient-1ad80f1.json new file mode 100644 index 000000000000..655786a81b68 --- /dev/null +++ b/.changes/next-release/bugfix-AWSCRTHTTPClient-1ad80f1.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS CRT HTTP Client", + "contributor": "", + "description": "Preserve the underlying CRT HttpException (including the CRT error code) as the cause of the SSLHandshakeException and ConnectException surfaced for TLS negotiation failures and socket timeouts, so callers can differentiate transient failures from persistent ones by inspecting the exception cause chain." +} diff --git a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml index 4e81373b0be3..ff9c1056bec2 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml @@ -72,4 +72,9 @@ + + + diff --git a/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheSslHandshakeBehaviorTest.java b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheSslHandshakeBehaviorTest.java new file mode 100644 index 000000000000..4e0b979db02f --- /dev/null +++ b/http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheSslHandshakeBehaviorTest.java @@ -0,0 +1,26 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.apache; + +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpClientSslHandshakeBehaviorTestSuite; + +public class ApacheSslHandshakeBehaviorTest extends SdkHttpClientSslHandshakeBehaviorTestSuite { + @Override + protected SdkHttpClient createSdkHttpClient() { + return ApacheHttpClient.builder().build(); + } +} diff --git a/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5SslHandshakeBehaviorTest.java b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5SslHandshakeBehaviorTest.java new file mode 100644 index 000000000000..cfa80a7491b9 --- /dev/null +++ b/http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5SslHandshakeBehaviorTest.java @@ -0,0 +1,26 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.apache5; + +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpClientSslHandshakeBehaviorTestSuite; + +public class Apache5SslHandshakeBehaviorTest extends SdkHttpClientSslHandshakeBehaviorTestSuite { + @Override + protected SdkHttpClient createSdkHttpClient() { + return Apache5HttpClient.builder().build(); + } +} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtUtils.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtUtils.java index 8e1650c56057..a3c16f3f1387 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtUtils.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtUtils.java @@ -94,11 +94,15 @@ public static Throwable wrapCrtException(Throwable throwable) { int httpErrorCode = httpException.getErrorCode(); if (httpErrorCode == CRT_TLS_NEGOTIATION_ERROR_CODE) { - return new SSLHandshakeException(httpException.getMessage()); + SSLHandshakeException sslHandshakeException = new SSLHandshakeException(httpException.getMessage()); + sslHandshakeException.initCause(httpException); + return sslHandshakeException; } if (httpErrorCode == CRT_SOCKET_TIMEOUT) { - return new ConnectException(httpException.getMessage()); + ConnectException connectException = new ConnectException(httpException.getMessage()); + connectException.initCause(httpException); + return connectException; } return wrapWithIoExceptionIfRetryable((HttpException) throwable); diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/CrtAsyncSslHandshakeBehaviorTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/CrtAsyncSslHandshakeBehaviorTest.java new file mode 100644 index 000000000000..2666ea96d406 --- /dev/null +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/CrtAsyncSslHandshakeBehaviorTest.java @@ -0,0 +1,31 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.crt; + +import java.time.Duration; +import software.amazon.awssdk.http.SdkAsyncHttpClientSslHandshakeBehaviorTestSuite; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; + +public class CrtAsyncSslHandshakeBehaviorTest extends SdkAsyncHttpClientSslHandshakeBehaviorTestSuite { + @Override + protected SdkAsyncHttpClient createSdkAsyncHttpClient() { + // CRT negotiates TLS during connection acquisition; the default 10s acquisition timeout can + // expire first and surface an acquisition-timeout exception instead of SSLHandshakeException. + return AwsCrtAsyncHttpClient.builder() + .connectionAcquisitionTimeout(Duration.ofSeconds(30)) + .build(); + } +} diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/CrtSslHandshakeBehaviorTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/CrtSslHandshakeBehaviorTest.java new file mode 100644 index 000000000000..c797c227dcab --- /dev/null +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/CrtSslHandshakeBehaviorTest.java @@ -0,0 +1,31 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.crt; + +import java.time.Duration; +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpClientSslHandshakeBehaviorTestSuite; + +public class CrtSslHandshakeBehaviorTest extends SdkHttpClientSslHandshakeBehaviorTestSuite { + @Override + protected SdkHttpClient createSdkHttpClient() { + // CRT negotiates TLS during connection acquisition; the default 10s acquisition timeout can + // expire first and surface an acquisition-timeout exception instead of SSLHandshakeException. + return AwsCrtHttpClient.builder() + .connectionAcquisitionTimeout(Duration.ofSeconds(30)) + .build(); + } +} diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutorTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutorTest.java index 89d5e2fb2f65..0c3c99267fdd 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutorTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutorTest.java @@ -184,7 +184,9 @@ public void execute_httpException_mapsToCorrectException(Entry executeFuture = requestExecutor.execute(context); - assertThatThrownBy(executeFuture::join).hasCauseInstanceOf(expectedExceptionClass); + assertThatThrownBy(executeFuture::join).hasCauseInstanceOf(expectedExceptionClass) + .hasCauseInstanceOf(IOException.class) + .hasRootCause(exception); } @Test diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java index 456000ac1150..b1374d050304 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java @@ -134,7 +134,9 @@ public void execute_httpException_mapsToCorrectException(Entry executeFuture = requestExecutor.execute(context); - assertThatThrownBy(executeFuture::join).hasCauseInstanceOf(expectedExceptionClass); + assertThatThrownBy(executeFuture::join).hasCauseInstanceOf(expectedExceptionClass) + .hasCauseInstanceOf(IOException.class) + .hasRootCause(exception); } @Test diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtUtilsTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtUtilsTest.java index 5cee0bb4770a..c9c43f238941 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtUtilsTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtUtilsTest.java @@ -18,6 +18,9 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; +import java.net.ConnectException; +import javax.net.ssl.SSLHandshakeException; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import software.amazon.awssdk.crt.http.HttpException; @@ -70,4 +73,37 @@ public void wrapWithIoExceptionIfRetryable_nonRetryableCode_returnsHttpException assertThat(result).isSameAs(httpException); } + + /** + * TLS negotiation failures must chain the original {@link HttpException} (with its CRT error code) + * as cause, so callers can differentiate transient from persistent failures. + */ + @Test + public void wrapCrtException_tlsNegotiationError_wrapsInSslHandshakeExceptionWithCause() { + HttpException httpException = new HttpException(CrtUtils.CRT_TLS_NEGOTIATION_ERROR_CODE); + + Throwable result = CrtUtils.wrapCrtException(httpException); + + assertThat(result).isInstanceOf(SSLHandshakeException.class) + .hasMessage(httpException.getMessage()) + .hasCause(httpException); + assertThat(((HttpException) result.getCause()).getErrorCode()) + .isEqualTo(CrtUtils.CRT_TLS_NEGOTIATION_ERROR_CODE); + } + + /** + * Socket timeouts must chain the original {@link HttpException} as cause. + */ + @Test + public void wrapCrtException_socketTimeout_wrapsInConnectExceptionWithCause() { + HttpException httpException = new HttpException(CrtUtils.CRT_SOCKET_TIMEOUT); + + Throwable result = CrtUtils.wrapCrtException(httpException); + + assertThat(result).isInstanceOf(ConnectException.class) + .hasMessage(httpException.getMessage()) + .hasCause(httpException); + assertThat(((HttpException) result.getCause()).getErrorCode()) + .isEqualTo(CrtUtils.CRT_SOCKET_TIMEOUT); + } } diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettySslHandshakeBehaviorTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettySslHandshakeBehaviorTest.java new file mode 100644 index 000000000000..9140a6edf92f --- /dev/null +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettySslHandshakeBehaviorTest.java @@ -0,0 +1,26 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.nio.netty; + +import software.amazon.awssdk.http.SdkAsyncHttpClientSslHandshakeBehaviorTestSuite; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; + +public class NettySslHandshakeBehaviorTest extends SdkAsyncHttpClientSslHandshakeBehaviorTestSuite { + @Override + protected SdkAsyncHttpClient createSdkAsyncHttpClient() { + return NettyNioAsyncHttpClient.builder().build(); + } +} diff --git a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionSslHandshakeBehaviorTest.java b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionSslHandshakeBehaviorTest.java new file mode 100644 index 000000000000..13ee6c728f97 --- /dev/null +++ b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionSslHandshakeBehaviorTest.java @@ -0,0 +1,26 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.urlconnection; + +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpClientSslHandshakeBehaviorTestSuite; + +public class UrlConnectionSslHandshakeBehaviorTest extends SdkHttpClientSslHandshakeBehaviorTestSuite { + @Override + protected SdkHttpClient createSdkHttpClient() { + return UrlConnectionHttpClient.builder().build(); + } +} diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientSslHandshakeBehaviorTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientSslHandshakeBehaviorTestSuite.java new file mode 100644 index 000000000000..923ed5152069 --- /dev/null +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientSslHandshakeBehaviorTestSuite.java @@ -0,0 +1,78 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +import com.github.tomakehurst.wiremock.WireMockServer; +import java.util.concurrent.CompletionException; +import javax.net.ssl.SSLHandshakeException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; + +/** + * Validates that a TLS handshake failure against an untrusted (self-signed) certificate surfaces as + * {@link SSLHandshakeException} with a cause chain, so callers can differentiate transient + * interruptions from persistent certificate failures. + */ +public abstract class SdkAsyncHttpClientSslHandshakeBehaviorTestSuite { + private WireMockServer selfSignedServer; + + protected abstract SdkAsyncHttpClient createSdkAsyncHttpClient(); + + @BeforeEach + public void setup() { + selfSignedServer = HttpTestUtils.createSelfSignedServer(); + selfSignedServer.start(); + } + + @AfterEach + public void teardown() { + if (selfSignedServer != null) { + selfSignedServer.stop(); + selfSignedServer = null; + } + } + + @Test + public void sslHandshakeFailure_surfacesAsSslHandshakeException() { + Throwable thrown = executeRequestAgainstUntrustedServer(); + + assertThat(thrown).isInstanceOf(SSLHandshakeException.class); + } + + @Test + public void sslHandshakeFailure_exceptionCarriesDiagnosableCause() { + Throwable thrown = executeRequestAgainstUntrustedServer(); + + assertThat(thrown).isInstanceOf(SSLHandshakeException.class); + assertThat(thrown.getCause()) + .withFailMessage("SSLHandshakeException must chain the underlying failure as its cause " + + "so callers can differentiate transient from persistent TLS failures. Was: %s", thrown) + .isNotNull(); + } + + private Throwable executeRequestAgainstUntrustedServer() { + try (SdkAsyncHttpClient client = createSdkAsyncHttpClient()) { + Throwable thrown = catchThrowable( + () -> HttpTestUtils.sendGetRequest(selfSignedServer.httpsPort(), client, true).join()); + return thrown instanceof CompletionException ? thrown.getCause() : thrown; + } + } +} diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientSslHandshakeBehaviorTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientSslHandshakeBehaviorTestSuite.java new file mode 100644 index 000000000000..23ab7d1c33c7 --- /dev/null +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientSslHandshakeBehaviorTestSuite.java @@ -0,0 +1,98 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +import com.github.tomakehurst.wiremock.WireMockServer; +import java.io.ByteArrayInputStream; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import javax.net.ssl.SSLHandshakeException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Validates that a TLS handshake failure against an untrusted (self-signed) certificate surfaces as + * {@link SSLHandshakeException} with a cause chain, so callers can differentiate transient + * interruptions from persistent certificate failures. + */ +public abstract class SdkHttpClientSslHandshakeBehaviorTestSuite { + private WireMockServer selfSignedServer; + + /** + * Implemented by a child class to create an HTTP client to validate + */ + protected abstract SdkHttpClient createSdkHttpClient(); + + @BeforeEach + public void setup() { + selfSignedServer = HttpTestUtils.createSelfSignedServer(); + selfSignedServer.start(); + } + + @AfterEach + public void teardown() { + if (selfSignedServer != null) { + selfSignedServer.stop(); + selfSignedServer = null; + } + } + + @Test + public void sslHandshakeFailure_surfacesAsSslHandshakeException() { + Throwable thrown = executeRequestAgainstUntrustedServer(); + + assertThat(thrown).isInstanceOf(SSLHandshakeException.class); + } + + @Test + public void sslHandshakeFailure_exceptionCarriesDiagnosableCause() { + Throwable thrown = executeRequestAgainstUntrustedServer(); + + assertThat(thrown).isInstanceOf(SSLHandshakeException.class); + assertThat(thrown.getCause()) + .withFailMessage("SSLHandshakeException must chain the underlying failure as its cause " + + "so callers can differentiate transient from persistent TLS failures. Was: %s", thrown) + .isNotNull(); + } + + private Throwable executeRequestAgainstUntrustedServer() { + try (SdkHttpClient client = createSdkHttpClient()) { + SdkHttpFullRequest request = httpsRequest(selfSignedServer.httpsPort()); + return catchThrowable(client.prepareRequest(HttpExecuteRequest.builder() + .request(request) + .contentStreamProvider( + request.contentStreamProvider().orElse(null)) + .build()) + ::call); + } + } + + private static SdkHttpFullRequest httpsRequest(int httpsPort) { + URI uri = URI.create("https://localhost:" + httpsPort); + byte[] content = "Body".getBytes(StandardCharsets.UTF_8); + return SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.POST) + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(content.length)) + .contentStreamProvider(() -> new ByteArrayInputStream(content)) + .build(); + } +}