From 2a60d23c5067457c5fd1363af07965f8d1c66123 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sun, 1 Nov 2026 10:14:30 +0100 Subject: [PATCH] Do not skip hostname verification when the CLIENT policy has no verifier Since the default verifier fallback was removed, CLIENT or BOTH policy with a null HostnameVerifier performed no hostname verification at all. ConscryptClientTlsStrategy and the deprecated DefaultClientTlsStrategy constructor now select the BUILTIN policy when no verifier is given, and AbstractClientTlsStrategy rejects CLIENT or BOTH without a verifier. --- .../http/ssl/AbstractClientTlsStrategy.java | 4 + .../http/ssl/ConscryptClientTlsStrategy.java | 3 +- .../http/ssl/DefaultClientTlsStrategy.java | 3 +- .../ssl/AbstractClientTlsStrategyTest.java | 30 ++++ .../ssl/TestNullHostnameVerifierPolicy.java | 147 ++++++++++++++++++ httpclient5/src/test/resources/test.keystore | Bin 0 -> 2243 bytes 6 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java create mode 100644 httpclient5/src/test/resources/test.keystore diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java index d403ff18df..0aba6a75ae 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java @@ -99,6 +99,10 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate this.sslBufferManagement = sslBufferManagement != null ? sslBufferManagement : SSLBufferMode.STATIC; this.hostnameVerificationPolicy = hostnameVerificationPolicy != null ? hostnameVerificationPolicy : (hostnameVerifier != null ? HostnameVerificationPolicy.BOTH : HostnameVerificationPolicy.BUILTIN); + Args.check(this.hostnameVerificationPolicy != HostnameVerificationPolicy.CLIENT + && this.hostnameVerificationPolicy != HostnameVerificationPolicy.BOTH + || hostnameVerifier != null, + "Hostname verifier is required for hostname verification policy " + this.hostnameVerificationPolicy); this.hostnameVerifier = hostnameVerifier; } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java index bcf9bfd344..d635866f73 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/ConscryptClientTlsStrategy.java @@ -69,7 +69,8 @@ public ConscryptClientTlsStrategy( final String[] supportedCipherSuites, final SSLBufferMode sslBufferManagement, final HostnameVerifier hostnameVerifier) { - this(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, HostnameVerificationPolicy.CLIENT, hostnameVerifier); + this(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, + hostnameVerifier != null ? HostnameVerificationPolicy.CLIENT : HostnameVerificationPolicy.BUILTIN, hostnameVerifier); } /** diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java index c52997473f..2792df98f0 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java @@ -101,7 +101,8 @@ public DefaultClientTlsStrategy( final SSLBufferMode sslBufferManagement, final HostnameVerifier hostnameVerifier, final Factory tlsDetailsFactory) { - super(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, HostnameVerificationPolicy.CLIENT, hostnameVerifier); + super(sslContext, supportedProtocols, supportedCipherSuites, sslBufferManagement, + hostnameVerifier != null ? HostnameVerificationPolicy.CLIENT : HostnameVerificationPolicy.BUILTIN, hostnameVerifier); this.tlsDetailsFactory = tlsDetailsFactory; } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java index 20a2bbd407..a052ca16e7 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java @@ -27,17 +27,21 @@ package org.apache.hc.client5.http.ssl; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.security.cert.X509Certificate; +import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSession; import javax.security.auth.x500.X500Principal; +import org.apache.hc.core5.function.Factory; import org.apache.hc.core5.reactor.ssl.SSLBufferMode; import org.apache.hc.core5.reactor.ssl.TlsDetails; import org.apache.hc.core5.ssl.SSLContexts; @@ -110,5 +114,31 @@ TlsDetails createTlsDetails(final SSLEngine sslEngine) { } + @Test + void testClientPolicyRequiresHostnameVerifier() { + assertThrows(IllegalArgumentException.class, () -> new DefaultClientTlsStrategy( + SSLContexts.createDefault(), HostnameVerificationPolicy.CLIENT, null)); + assertThrows(IllegalArgumentException.class, () -> new DefaultClientTlsStrategy( + SSLContexts.createDefault(), HostnameVerificationPolicy.BOTH, null)); + } + + @Test + void testBuilderClientPolicyRequiresHostnameVerifier() { + final ClientTlsStrategyBuilder builder = ClientTlsStrategyBuilder.create() + .setSslContext(SSLContexts.createDefault()) + .setHostVerificationPolicy(HostnameVerificationPolicy.CLIENT); + assertThrows(IllegalArgumentException.class, builder::buildClassic); + } + + @Test + @SuppressWarnings("deprecation") + void testNullHostnameVerifierDefaultsToBuiltinPolicy() { + assertDoesNotThrow(() -> new DefaultClientTlsStrategy( + SSLContexts.createDefault(), null, null, SSLBufferMode.STATIC, + (HostnameVerifier) null, (Factory) null)); + assertDoesNotThrow(() -> new DefaultClientTlsStrategy(SSLContexts.createDefault())); + assertDoesNotThrow(() -> new ConscryptClientTlsStrategy(SSLContexts.createDefault())); + } + } \ No newline at end of file diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java new file mode 100644 index 0000000000..02b9ee22b0 --- /dev/null +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestNullHostnameVerifierPolicy.java @@ -0,0 +1,147 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License 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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + + +package org.apache.hc.client5.http.ssl; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.Socket; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; + +import org.apache.hc.core5.function.Factory; +import org.apache.hc.core5.reactor.ssl.SSLBufferMode; +import org.apache.hc.core5.reactor.ssl.TlsDetails; +import org.apache.hc.core5.ssl.SSLContexts; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Handshakes against a server whose certificate is issued for {@code localhost} to make sure + * that a strategy created without a {@link javax.net.ssl.HostnameVerifier} still verifies the host name. + */ +class TestNullHostnameVerifierPolicy { + + private static final char[] PASSWORD = "nopassword".toCharArray(); + + private SSLContext sslContext; + private SSLServerSocket serverSocket; + private Thread serverThread; + + @BeforeEach + void setUp() throws Exception { + sslContext = SSLContexts.custom() + .loadTrustMaterial(getClass().getResource("/test.keystore"), PASSWORD) + .loadKeyMaterial(getClass().getResource("/test.keystore"), PASSWORD, PASSWORD) + .build(); + serverSocket = (SSLServerSocket) sslContext.getServerSocketFactory() + .createServerSocket(0, 1, InetAddress.getLoopbackAddress()); + serverThread = new Thread(() -> { + try (final SSLSocket socket = (SSLSocket) serverSocket.accept()) { + socket.startHandshake(); + } catch (final IOException ignore) { + // the client may abort the handshake + } + }); + serverThread.setDaemon(true); + serverThread.start(); + } + + @AfterEach + void tearDown() throws Exception { + serverSocket.close(); + serverThread.join(5000); + } + + private SSLSocket upgrade(final AbstractClientTlsStrategy strategy, final String target) throws IOException { + final Socket socket = new Socket(InetAddress.getLoopbackAddress(), serverSocket.getLocalPort()); + try { + return strategy.upgrade(socket, target, serverSocket.getLocalPort(), null, null); + } catch (final IOException | RuntimeException ex) { + socket.close(); + throw ex; + } + } + + @SuppressWarnings("deprecation") + private AbstractClientTlsStrategy defaultStrategy() { + return new DefaultClientTlsStrategy( + sslContext, null, null, SSLBufferMode.STATIC, + (HostnameVerifier) null, (Factory) null); + } + + @SuppressWarnings("deprecation") + private AbstractClientTlsStrategy conscryptStrategy() { + return new ConscryptClientTlsStrategy( + sslContext, null, null, SSLBufferMode.STATIC, (HostnameVerifier) null); + } + + @Test + void testDefaultStrategyRejectsMismatchingHost() { + assertThrows(SSLException.class, () -> upgrade(defaultStrategy(), "example.invalid")); + } + + @Test + void testDefaultStrategyAcceptsMatchingHost() throws Exception { + try (final SSLSocket socket = upgrade(defaultStrategy(), "localhost")) { + assertNotNull(socket.getSession().getPeerCertificates()); + } + } + + @Test + void testConscryptStrategyRejectsMismatchingHost() { + assertThrows(SSLException.class, () -> upgrade(conscryptStrategy(), "example.invalid")); + } + + @Test + void testConscryptStrategyAcceptsMatchingHost() throws Exception { + try (final SSLSocket socket = upgrade(conscryptStrategy(), "localhost")) { + assertNotNull(socket.getSession().getPeerCertificates()); + } + } + + @Test + void testConscryptDefaultConstructorRejectsMismatchingHost() { + assertThrows(SSLException.class, () -> upgrade(new ConscryptClientTlsStrategy(sslContext), "example.invalid")); + } + + @Test + void testDefaultConstructorRejectsMismatchingHost() { + assertThrows(SSLException.class, () -> upgrade(new DefaultClientTlsStrategy(sslContext), "example.invalid")); + } + +} diff --git a/httpclient5/src/test/resources/test.keystore b/httpclient5/src/test/resources/test.keystore new file mode 100644 index 0000000000000000000000000000000000000000..f8d5ace1ad25f92675c2b94caeec3c117f857fb8 GIT binary patch literal 2243 zcmcIl`8(8$7oN=?V>ie$6eY`NWDjKtl{J(lWj8XI7=$bh5sjrrl4vlpPLY{wU$SQ_ zk`cnil*qnhxt6c)bARX``2O(z@}Bp3&U4;#p7$Iq9xQ@DAjlEG|AIHr+ZT)RP(b5w zScO23fMAb+qmt*NWs)ff!~_NKutR{0nf(ki7zUPu@PNTgAQ&F@b3!rNw!Lo5RlIWV zGuxtCA&jD%y>C~5HS!SlRBxWVG9JCltLe2=c~M>dM&{0W)i2MwLdfY$U;U~AvqysV zcI1Tu#Msu9ELF{7g_)-9Phw-lXxNW=lP4>tMQ_@^p>J}UbPEOdkXPS44z9{?8+$9B zSNJGlKce|uoXtDKAA0f~!YNratK7t=%AS4mzV7vJkcJncwnC;8v@I8L%3TC^T_Y{l zCtammHQk;j*1Yk?Ny#uo7@2vnaQ0b z9z)6DcaLj)58<6MZvbr~Y($%N?kdY%9Xbup4NC3D>^s&6FR|(AO;)Ht7Ae`i*M*O% zCh``SKLqzM#uYiGCz3Pfex(pUUkL2lfMb!^2$7PD^-f{CwP%|JO6D5}7^2^o<{X-?iX+8;jdDdSHbSq^24>Ug{UYFJTk8`bj<#j;d0_nZeR! zjc#tb-PYch-PTC;o|7LmNctE?YP3w{y3uHI-LZvc68|n*8bg1!vM$=Fu3wwv@BAR& zfM8Ch_O__lMru;d5u}$flGb8rKkyA{6(_vJ)n{L9E!%S*+uYQPE!C3K57%kpUW)p) zGkZ{Y-HU{di18hyI`rqnsIA@>gnr_j@3fo~8NDe}K+{U^^~L7b8%0SAyeZxFia6C- zHQ?9QmeZ#fm6uav2*dx7NyDT0 zr@Jw?$}8uL)kKMNkvWB(sSZ2U>a8q;C@mWKmg;%;8J{#P2@S8#sjJ%$ zN0_mJ>^1rmd70UcgF^Q{2<1y!Q)6E~GTtOD+j#b^Y4a9Mt^0V53D)>1_6U<{_E-tR z<&j_MO9O{rkqRD5{bduwq^(G^^C1qako0w)B%_=It2jY`wct6DX+AoWagj>S$leO|1$OhO^(Z# z9azzxQTtTd{U523I^~|~HW>zxxTW_m3J_R_CQ`d9hs-~4x-Sc{-X_pliJ%!Rju)bd zlaomn>#z)#rBS5g6w)o3N6~k1pskRrJnIxim#6oW3{+_rHmjDk*BmW>jcNd(uzaL5*IR0D2!6)#a$mA%44g7tv{(c^QxIh#y z!2P=pw@@c>u0Z22e`Y7L1>>^tam>!`yajhOp$t(~u8 z|CV_?ZIKmZNPHrxKk(94r<{y#=eKwt#^CenxKF)xX}V=;(I~2@G(&4s-f?!&%yhhA zo5RQh>W_@RO(VDlf+Nmv1R7KmL!>qZ-SkEHHy0Fe$z7M@?!5QQsAj*ihUb^^UU`}l ziLTZvmEfZC1|?UHRfgJ|hCS)%RzTCdQ3jq0Wr*G~$7{j^J5)033?Z-Yo!bs9n=X$h zT5R(i6nwTZ(KpETA1*uXeuG@{U0RqvjU#Y=c+C>c{-8`(t!_B2R;wkGALun9a&*|i zEzWdZD%q)lK*1m|SshRX_>UGR#slL7IPiZgPwY3#1rvnv$3@3pzLK!EKXF*$e8%n* z<%Ibx<4_+s=O}~w_3^nkr7t0 j+=-YtJ;!r<`!BItX5^5E808gChqv2)op0*N=MVn_*7f=z literal 0 HcmV?d00001