diff --git a/build.gradle b/build.gradle index 765a7fe8203e..92b27c57db8b 100644 --- a/build.gradle +++ b/build.gradle @@ -399,6 +399,13 @@ project(':iceberg-core') { exclude group: 'junit' } testImplementation libs.awaitility + + // Lock BouncyCastle versions to avoid version mismatches + // when these dependencies are added transitively. + // Required for TLS tests with MockServer. + testImplementation libs.bouncycastle.bcpkix + testImplementation libs.bouncycastle.bcutil + testImplementation libs.bouncycastle.bcprov } } diff --git a/core/src/main/java/org/apache/iceberg/rest/HTTPClient.java b/core/src/main/java/org/apache/iceberg/rest/HTTPClient.java index 86eceba21c95..c359404ec6be 100644 --- a/core/src/main/java/org/apache/iceberg/rest/HTTPClient.java +++ b/core/src/main/java/org/apache/iceberg/rest/HTTPClient.java @@ -29,6 +29,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import javax.net.ssl.HostnameVerifier; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.CredentialsProvider; import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; @@ -43,6 +44,7 @@ import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.HostnameVerificationPolicy; import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; @@ -410,13 +412,19 @@ static HttpClientConnectionManager configureConnectionManager(MapIf a non-null verifier is returned, only the custom verifier is executed and the JSSE + * built-in hostname verifier won't be executed. + */ + @Nullable default HostnameVerifier hostnameVerifier() { - return HttpsSupport.getDefaultHostnameVerifier(); + return null; } default String[] supportedProtocols() { diff --git a/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java b/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java index 8cf97bca32ef..701ae699f136 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java @@ -35,22 +35,30 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; import java.net.SocketTimeoutException; +import java.nio.file.Path; +import java.security.KeyStore; +import java.security.cert.CertificateException; import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManagerFactory; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; import org.apache.iceberg.IcebergBuild; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.collect.Sets; import org.apache.iceberg.rest.auth.AuthSession; import org.apache.iceberg.rest.auth.TLSConfigurer; import org.apache.iceberg.rest.responses.ErrorResponse; @@ -58,14 +66,17 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockserver.configuration.Configuration; import org.mockserver.integration.ClientAndServer; +import org.mockserver.logging.MockServerLogger; import org.mockserver.matchers.Times; import org.mockserver.model.HttpRequest; import org.mockserver.model.HttpResponse; +import org.mockserver.socket.tls.KeyStoreFactory; import org.mockserver.verify.VerificationTimes; /** @@ -87,6 +98,7 @@ public class TestHTTPClient { private static RESTClient restClient; public static class DefaultTLSConfigurer implements TLSConfigurer { + public static int count = 0; public DefaultTLSConfigurer() { @@ -95,6 +107,7 @@ public DefaultTLSConfigurer() { } public static class TLSConfigurerMissingNoArgCtor implements TLSConfigurer { + TLSConfigurerMissingNoArgCtor(String str) {} } @@ -395,6 +408,101 @@ public void testLoadTLSConfigurerNotImplementTLSConfigurer() { .hasMessageContaining("does not implement TLSConfigurer"); } + /** A TLSConfigurer that relies on the default (built-in) JSSE verifier. */ + public static class BuiltInHostnameVerifierTLSConfigurer implements TLSConfigurer { + + @Override + public SSLContext sslContext() { + return mockServerSSLContext(); + } + } + + /** A TLSConfigurer that overrides hostnameVerifier() to return a custom verifier. */ + public static class CustomHostnameVerifierTLSConfigurer implements TLSConfigurer { + + @Override + public SSLContext sslContext() { + return mockServerSSLContext(); + } + + @Override + public HostnameVerifier hostnameVerifier() { + return NoopHostnameVerifier.INSTANCE; + } + } + + private static SSLContext mockServerSSLContext() { + try { + KeyStore keyStore = + new KeyStoreFactory(Configuration.configuration(), new MockServerLogger()) + .loadOrCreateKeyStore(); + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(keyStore); + SSLContext sslContext = SSLContext.getInstance("TLSv1.2"); + sslContext.init(null, tmf.getTrustManagers(), null); + return sslContext; + } catch (Exception e) { + throw new RuntimeException("Failed to create SSLContext", e); + } + } + + @Test + public void testTLSConfigurerHostnameVerifier(@TempDir Path temp) throws IOException { + + // Start a dedicated MockServer with a certificate that does NOT include + // 127.0.0.1 or localhost in its SANs. + Configuration tlsConfig = Configuration.configuration(); + tlsConfig.proactivelyInitialiseTLS(true); + tlsConfig.preventCertificateDynamicUpdate(true); + tlsConfig.sslCertificateDomainName("example.com"); + tlsConfig.sslSubjectAlternativeNameIps(Sets.newHashSet("1.2.3.4")); + tlsConfig.sslSubjectAlternativeNameDomains(Sets.newHashSet("example.com")); + tlsConfig.directoryToSaveDynamicSSLCertificate(temp.toFile().getAbsolutePath()); + + int tlsPort = PORT + 1; + try (ClientAndServer server = startClientAndServer(tlsConfig, tlsPort)) { + + String path = "tls/hostname-verifier/path"; + HttpRequest mockRequest = + request() + .withPath("/" + path) + .withMethod(HttpMethod.HEAD.name().toUpperCase(Locale.ROOT)); + HttpResponse mockResponse = response().withStatusCode(200).withBody("TLS response"); + server.when(mockRequest).respond(mockResponse); + + // With no custom hostnameVerifier (null), the BUILTIN policy is used automatically, + // so the JSSE built-in verifier rejects the connection because the SANs don't match + try (HTTPClient builtInVerifierClient = + HTTPClient.builder( + Map.of( + HTTPClient.REST_TLS_CONFIGURER, + BuiltInHostnameVerifierTLSConfigurer.class.getName())) + .uri(String.format("https://127.0.0.1:%d", tlsPort)) + .withAuthSession(AuthSession.EMPTY) + .build()) { + assertThatThrownBy(() -> builtInVerifierClient.head(path, Map.of(), (unused) -> {})) + .rootCause() + .isInstanceOf(CertificateException.class) + .hasMessage("No subject alternative names matching IP address 127.0.0.1 found"); + } + + // With a custom hostnameVerifier (NoopHostnameVerifier), the CLIENT policy is used + // automatically, so hostname verification is bypassed and the request succeeds + try (HTTPClient customVerifierClient = + HTTPClient.builder( + Map.of( + HTTPClient.REST_TLS_CONFIGURER, + CustomHostnameVerifierTLSConfigurer.class.getName())) + .uri(String.format("https://127.0.0.1:%d", tlsPort)) + .withAuthSession(AuthSession.EMPTY) + .build()) { + assertThatCode(() -> customVerifierClient.head(path, Map.of(), (unused) -> {})) + .doesNotThrowAnyException(); + } + } + } + @Test public void testSocketTimeout() throws IOException { long socketTimeoutMs = 2000L; @@ -613,6 +721,7 @@ private static Item doExecuteRequest( } public static class Item implements RESTRequest, RESTResponse { + private Long id; private String data; diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8cd9e566b367..0726b23e9a09 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -36,6 +36,7 @@ awaitility = "4.3.0" awssdk-bom = "2.42.8" azuresdk-bom = "1.3.5" awssdk-s3accessgrants = "2.4.1" +bouncycastle = "1.82" bson-ver = "4.11.5" caffeine = "2.9.3" calcite = "1.41.0" @@ -111,6 +112,9 @@ awssdk-bom = { module = "software.amazon.awssdk:bom", version.ref = "awssdk-bom" awssdk-s3accessgrants = { module = "software.amazon.s3.accessgrants:aws-s3-accessgrants-java-plugin", version.ref = "awssdk-s3accessgrants" } azuresdk-bom = { module = "com.azure:azure-sdk-bom", version.ref = "azuresdk-bom" } bson = { module = "org.mongodb:bson", version.ref = "bson-ver"} +bouncycastle-bcpkix = { module = "org.bouncycastle:bcpkix-jdk18on", version.ref = "bouncycastle" } +bouncycastle-bcprov = { module = "org.bouncycastle:bcprov-jdk18on", version.ref = "bouncycastle" } +bouncycastle-bcutil = { module = "org.bouncycastle:bcutil-jdk18on", version.ref = "bouncycastle" } caffeine = { module = "com.github.ben-manes.caffeine:caffeine", version.ref = "caffeine" } calcite-core = { module = "org.apache.calcite:calcite-core", version.ref = "calcite" } calcite-druid = { module = "org.apache.calcite:calcite-druid", version.ref = "calcite" }