RussellSpitzer commented on code in PR #15500:
URL: https://github.com/apache/iceberg/pull/15500#discussion_r3018465649
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -395,6 +406,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);
+
+ 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();
+ 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()) {
+
+ // 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
+ assertThatThrownBy(() -> builtInVerifierClient.head(path, Map.of(),
(unused) -> {}))
+ .rootCause()
+ .isInstanceOf(CertificateException.class)
+ .hasMessage("No subject alternative names matching IP address
127.0.0.1 found");
Review Comment:
Apparently this is wording JDK specific, May come up later if folks are
testing on other architectures. But I think we are mostly testing on OpenJDK,
Oracle JDK and the other OpenJDK based versions.
> OpenJDK / Oracle JDK: "No subject alternative names matching IP address
127.0.0.1 found" — this is the message in sun.security.util.HostnameChecker.
>
> IBM Semeru / J9: IBM's JSSE implementation has its own HostnameChecker
equivalent with different wording. Historically it uses messages like "No name
matching 127.0.0.1 found" or similar variations.
>
> Eclipse Temurin / Azul Zulu: These are typically OpenJDK-based and share
the message, but there's no guarantee across versions — the sun.security
package is internal and its strings can change between releases without notice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
