Hi all, There seems to be some agreement around the idea of not exposing HostnameVerificationPolicy. I will update the PR in that direction.
Thanks, Alex On Tue, Mar 31, 2026 at 1:20 PM Steve Loughran <[email protected]> wrote: > > I just had a look in the hadoop code > > https://github.com/apache/hadoop/blob/fb8932a727f757b2e9c1c61a18145878d0eb77bd/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/SSLHostnameVerifier.java > > thiis is declared as "Copied from the not-yet-commons-ssl project at > http://juliusdavies.ca/commons-ssl/";,... "Inspired by Sebastian Hauer's > original StrictSSLProtocolSocketFactory in the > HttpClient "contrib" repository." > > commons-ssl leads to > https://cwiki.apache.org/confluence/display/incubator/CommonsSSLProposal > > Ultimately it allows you configure a hadoop cluster with x.509 and > certificate management needed to have encrypted network traffic, shuffle etc. > > private HostnameVerifier getHostnameVerifier(Configuration conf) > throws GeneralSecurityException, IOException { > return getHostnameVerifier(StringUtils.toUpperCase( > conf.get(SSL_HOSTNAME_VERIFIER_KEY, "DEFAULT").trim())); > } > > public static HostnameVerifier getHostnameVerifier(String verifier) > throws GeneralSecurityException, IOException { > HostnameVerifier hostnameVerifier; > if (verifier.equals("DEFAULT")) { > hostnameVerifier = SSLHostnameVerifier.DEFAULT; > } else if (verifier.equals("DEFAULT_AND_LOCALHOST")) { > hostnameVerifier = SSLHostnameVerifier.DEFAULT_AND_LOCALHOST; > } else if (verifier.equals("STRICT")) { > hostnameVerifier = SSLHostnameVerifier.STRICT; > } else if (verifier.equals("STRICT_IE6")) { > hostnameVerifier = SSLHostnameVerifier.STRICT_IE6; > } else if (verifier.equals("ALLOW_ALL")) { > hostnameVerifier = SSLHostnameVerifier.ALLOW_ALL; > } else { > throw new GeneralSecurityException("Invalid hostname verifier: " + > verifier); > } > return hostnameVerifier; > } > > That code gets minimal maintenance and shows its age (who needs strict IE6 > compatibility?) and I suspect DEFAULT is what everyone uses. Which is good as > httpclient reads the resource mozilla/public-suffix-list.txt to have an up to > date list of country codes and other toplevel domains where you mustn't trust > wildcard certificates (*.co.uk etc). > > What I would say looking at the code is "you need to understand real world > SSL to write a hostname verifier you can trust". if you don't have that > knowledge, your system is probably less secure than you think. > > > > > On Mon, 30 Mar 2026 at 20:17, Steven Wu <[email protected]> wrote: >> >> I agree with Russell's point. Since we don't expose BOTH before, we can keep >> the restricted API for now. Users must know what they are doing from >> security perspective when they choose to override the hostname verifier. >> >> If there is a clear need to expose "HostnameVerificationPolicy.BOTH" in the >> future, we can add the new API then. It is always easier to open up new APIs >> than to remove them. >> >> On Mon, Mar 30, 2026 at 11:32 AM Russell Spitzer <[email protected]> >> wrote: >>> >>> Did a bit of Github Searching with Cursor >>> >>> It gives me 7 Hits of real use cases BOTH mode in non-forks >>> >>> ----- >>> >>> HostnameVerificationPolicy.BOTH — All real-world usages on GitHub >>> (excluding httpcomponents-client itself and forks) >>> >>> Passing library default explicitly (no custom verifier logic) >>> - ch4mpy/spring-addons: Spring Boot REST helpers — TrustAll + default >>> verifier, BOTH is redundant >>> - openziti/ziti-sdk-jvm: Zero-trust networking SDK — copied library >>> internals verbatim >>> - eryanwcp/ec: Chinese enterprise framework — default verifier, just >>> being explicit >>> - dmandalidis/docker-client: Docker API client — passes Docker cert >>> store's full verifier through >>> >>> Misuse (BOTH contradicts intent) >>> - dannguyenmessi1705/Spring (2 files): Personal Spring learning projects >>> — BOTH + NoopHostnameVerifier, trying to disable verification but BOTH >>> keeps JSSE active >>> - kadukm/five-letters: Personal word-game crawler — same bug, BOTH + >>> NoopHostnameVerifier >>> >>> Test code only >>> - dtreskunov/easyssl: TLS testing library — default verifier in >>> integration test helper >>> >>> >>> >>> On Mon, Mar 30, 2026 at 1:13 PM Russell Spitzer <[email protected]> >>> wrote: >>>> >>>> We never exposed BOTH before, so I'm not sure why we would do so now. I'm >>>> not convinced >>>> that there is a theoretical use case we are missing since no one asked for >>>> this before, >>>> and the BOTH option has existed in the underlying library for years. I'm >>>> very conservative >>>> about opening new interface points when we don't have demand. >>>> >>>> In this case we are also specifically trying to avoid a regression, so >>>> this would be the wrong time >>>> to expose a new interface and set of functionality. >>>> >>>> We can always follow up with a new proposal if it's something folks really >>>> want, but I don't >>>> think we should open it up just because it's possible. >>>> >>>> On Mon, Mar 30, 2026 at 12:12 PM Steven Wu <[email protected]> wrote: >>>>> >>>>> If a user wants to customize the hostname verification, is it additive to >>>>> the JSSE verification (BOTH) or does it require a totally independent >>>>> verification algorithm (CLIENT)? This is probably where it boils down. >>>>> >>>>> Russell's suggestion aligns with the latter camp (CLIENT). A user must >>>>> know what they are doing before overriding the hostname verification >>>>> algorithm. >>>>> >>>>> On Mon, Mar 30, 2026 at 8:47 AM Alexandre Dutra <[email protected]> wrote: >>>>>> >>>>>> Hi all, >>>>>> >>>>>> I have some reservations about Russell's suggestion. >>>>>> >>>>>> I believe the HttpClient developers introduced the BOTH value for a >>>>>> compelling reason. This is likely due to the inherent complexity of >>>>>> implementing a robust HostnameVerifier from scratch. The task involves >>>>>> non-trivial aspects like manually parsing SANs, handling complex >>>>>> wildcard matching, managing internationalized domain names, and >>>>>> accounting for legacy CN fallbacks. >>>>>> >>>>>> Consider a custom verifier intended to restrict an HTTP client to >>>>>> specific internal subdomains; it's very easy to write one: >>>>>> >>>>>> HostnameVerifier myVerifier = (hostname, session) -> { >>>>>> return hostname.endsWith(".internal.mycompany.com"); >>>>>> }; >>>>>> >>>>>> Used with BOTH, this verifier does the job. If used with CLIENT, >>>>>> however, it creates a massive security vulnerability because standard >>>>>> JSSE validation is bypassed. >>>>>> >>>>>> IOW, I think there is value in using BOTH, and there is value in >>>>>> exposing the HostnameVerificationPolicy. >>>>>> >>>>>> Thanks, >>>>>> Alex >>>>>> >>>>>> On Thu, Mar 26, 2026 at 2:16 AM Russell Spitzer >>>>>> <[email protected]> wrote: >>>>>> > >>>>>> > I wouldn’t even expose the interface for downstream extensions. We >>>>>> > currently don’t have >>>>>> > any implementations that expect this to be anything other than client >>>>>> > and >>>>>> > I’m not sure we want to encourage folks to use “both.” >>>>>> > >>>>>> >> >>>>>> >> On Wed, Mar 25, 2026 at 10:02 AM Alexandre Dutra <[email protected]> >>>>>> >> wrote: >>>>>> >>> >>>>>> >>> Hi Russell, >>>>>> >>> >>>>>> >>> > I can't think of a situation where it would make sense to use a >>>>>> >>> > custom client verifier and the JSSE verifier. Running two >>>>>> >>> > verifiers just means the stricter one wins and the less strict one >>>>>> >>> > is a noop. >>>>>> >>> >>>>>> >>> I wouldn't say a noop, it could perform additional hostname >>>>>> >>> validation >>>>>> >>> that goes beyond checking the SANs in the certificate. >>>>>> > >>>>>> > >>>>>> > That's the rub here. If the custom implementation goes beyond checking >>>>>> > the SANs then why doesn't it also just check the SANs. If we >>>>>> > didn't, we end up with this design where the custom verifier only does >>>>>> > part of the job and relies >>>>>> > on another component which can change on library updates to do the >>>>>> > rest. The purpose here is to >>>>>> > provide an alternative to the default, so why write one that still >>>>>> > depends on the default running alongside it? >>>>>> > >>>>>> >>> >>>>>> >>> > My solution would be to default hostNameVerifier to "null" instead >>>>>> >>> > of the current non-null response and always pass through CLIENT >>>>>> >>> > when hostNameVerifier is explicitly defined. >>>>>> >>> >>>>>> >>> The issue is that the hostname verifier is *always* defined. >>>>>> >>> TLSConfigurer.hostnameVerifier() by default returns >>>>>> >>> HttpsSupport.getDefaultHostnameVerifier(). While it's technically >>>>>> >>> possible to override the method and return null, I doubt anyone is >>>>>> >>> doing this because the method is not explicitly annotated as >>>>>> >>> @Nullable, and returning null would look like an API misuse. That's >>>>>> >>> why I went with CLIENT, initially. >>>>>> > >>>>>> > >>>>>> > >>>>>> > That's what i'm saying we should change. If we change our default >>>>>> > implementation to return null >>>>>> > i think that's better. We could potentially break folks who are >>>>>> > checking this return value and >>>>>> > assuming it isn't null but I think that's probably better than >>>>>> > continuing with the imho incorrect >>>>>> > default. >>>>>> > >>>>>> > Something like >>>>>> > >>>>>> > public interface TLSConfigurer { >>>>>> > default HostnameVerifier hostnameVerifier() { >>>>>> > return null; >>>>>> > } >>>>>> > } >>>>>> > >>>>>> > >>>>>> > TLSConfigurer tlsConfigurer = loadTlsConfigurer(properties); >>>>>> > if (tlsConfigurer != null) { >>>>>> > HostnameVerifier hostnameVerifier = tlsConfigurer.hostnameVerifier(); >>>>>> > connectionManagerBuilder.setTlsSocketStrategy( >>>>>> > new DefaultClientTlsStrategy( >>>>>> > tlsConfigurer.sslContext(), >>>>>> > tlsConfigurer.supportedProtocols(), >>>>>> > tlsConfigurer.supportedCipherSuites(), >>>>>> > SSLBufferMode.STATIC, >>>>>> > hostnameVerifier != null >>>>>> > ? HostnameVerificationPolicy.CLIENT >>>>>> > : HostnameVerificationPolicy.BUILTIN, >>>>>> > hostnameVerifier)); >>>>>> > } >>>>>> > >>>>>> > Anyone overriding hostnameVerifier get's client, everyone else gets >>>>>> > the deault and "BUILTIN" >>>>>> > >>>>>> >>> >>>>>> >>> >>>>>> >>> >>>>>> >>> That said if we really want to go down that route, I don't mind >>>>>> >>> re-implementing hostnameVerificationPolicy() as per your suggestion: >>>>>> >>> >>>>>> >>> default HostnameVerificationPolicy hostnameVerificationPolicy() { >>>>>> >>> return hostnameVerifier() == null >>>>>> >>> ? HostnameVerificationPolicy.BUILTIN >>>>>> >>> : HostnameVerificationPolicy.CLIENT; >>>>>> >>> } >>>>>> >>> >>>>>> >>> Thanks, >>>>>> >>> Alex >>>>>> >>> >>>>>> >>> On Wed, Mar 25, 2026 at 6:24 PM Russell Spitzer >>>>>> >>> <[email protected]> wrote: >>>>>> >>> > >>>>>> >>> > I don't think BOTH makes sense as a default and I'm not sure it's >>>>>> >>> > actually more secure. >>>>>> >>> > >>>>>> >>> > If I have a custom TLS impl which overrides host name verifier, I >>>>>> >>> > want to be using CLIENT mode. I can't >>>>>> >>> > think of a situation where it would make sense to use a custom >>>>>> >>> > client verifier and the JSSE verifier. Running >>>>>> >>> > two verifiers just means the stricter one wins and the less strict >>>>>> >>> > one is a noop. This means with "both" we can only use >>>>>> >>> > a custom verifier that is stricter than the JSSE one. >>>>>> >>> > >>>>>> >>> > My suggestions would be to change the constructor to use the 6 arg >>>>>> >>> > version for the constructor with an explicit CLIENT >>>>>> >>> > when a verifier is explicitly set. >>>>>> >>> > >>>>>> >>> > If I have a custom TLS impl which doesn't override the host name >>>>>> >>> > verifier, I don't want "BOTH or CLIENT", I want "BUILTIN". Any >>>>>> >>> > client side verification is a waste. >>>>>> >>> > >>>>>> >>> > My solution would be to default hostNameVerifier to "null" instead >>>>>> >>> > of the current non-null response and >>>>>> >>> > always pass through CLIENT when hostNameVerifier is explicitly >>>>>> >>> > defined. >>>>>> >>> > >>>>>> >>> > If we just always use BOTH won't we just end up breaking folks who >>>>>> >>> > tried to put in custom verifiers? The whole >>>>>> >>> > point I would think is that they are trying to use a verifier >>>>>> >>> > which is more permissive than the JSSE one. >>>>>> >>> > >>>>>> >>> > So basically I would only support two paths for custom TLS >>>>>> >>> > >>>>>> >>> > 1) Custom host verifier = CLIENT >>>>>> >>> > 2) No custom host verifier = BUILTIN >>>>>> >>> > >>>>>> >>> > I'm not sure I see a world where a user would want to specify >>>>>> >>> > their own verification policy so I wouldn't really >>>>>> >>> > expose that as an option at all. This would also simplify the >>>>>> >>> > implementation of the PR a bit. >>>>>> >>> > But if someone has such a use case, please let me know. >>>>>> >>> > >>>>>> >>> > On Wed, Mar 25, 2026 at 9:56 AM Eduard Tudenhöfner >>>>>> >>> > <[email protected]> wrote: >>>>>> >>> >> >>>>>> >>> >> I think we should opt for the safer option and go with BOTH. >>>>>> >>> >> >>>>>> >>> >> On Wed, Mar 25, 2026 at 11:22 AM Alexandre Dutra >>>>>> >>> >> <[email protected]> wrote: >>>>>> >>> >>> >>>>>> >>> >>> +1 to using BOTH by default. >>>>>> >>> >>> >>>>>> >>> >>> Le mer. 25 mars 2026 à 00:55, Steven Wu <[email protected]> a >>>>>> >>> >>> écrit : >>>>>> >>> >>>> >>>>>> >>> >>>> Are there any concerns about changing the hostname verification >>>>>> >>> >>>> policy default from CLIENT to BOTH (more secure) in the 1.11 >>>>>> >>> >>>> release? >>>>>> >>> >>>> >>>>>> >>> >>>> This is the last blocker for the 1.11.0 release. Let's decide >>>>>> >>> >>>> to unblock the release. Hopefully we can get 1.11.0 out before >>>>>> >>> >>>> the summit. >>>>>> >>> >>>> >>>>>> >>> >>>> On Fri, Mar 20, 2026 at 12:02 PM Steven Wu >>>>>> >>> >>>> <[email protected]> wrote: >>>>>> >>> >>>>> >>>>>> >>> >>>>> I asked for a dev ML discussion for this. I will share why I >>>>>> >>> >>>>> favor changing the default to HostnameVerificationPolicy.BOTH >>>>>> >>> >>>>> in the next 1.11 release. >>>>>> >>> >>>>> >>>>>> >>> >>>>> * In the production environment, people should use the >>>>>> >>> >>>>> hostname matching the SAN attribute in the certificate. The >>>>>> >>> >>>>> hostname could be a DNS name, an IP address, or both. The >>>>>> >>> >>>>> certificate must be generated with the proper Subject >>>>>> >>> >>>>> Alternative Name (SAN) matching its intended use. While this >>>>>> >>> >>>>> is a slight behavior change for the 1.11 release, the >>>>>> >>> >>>>> practical impact should be very small since production >>>>>> >>> >>>>> deployments probably use a DNS name anyway. >>>>>> >>> >>>>> * For the unit test, Alex's PR #15598 provides the >>>>>> >>> >>>>> customization to allow using the loopback IP address >>>>>> >>> >>>>> (127.0.0.1) with noop hostname verification. >>>>>> >>> >>>>> >>>>>> >>> >>>>> BTW, this is the last blocking PR for version 1.11.0 release. >>>>>> >>> >>>>> It will be great to reach a consensus soon. >>>>>> >>> >>>>> https://github.com/apache/iceberg/milestone/59 >>>>>> >>> >>>>> >>>>>> >>> >>>>> >>>>>> >>> >>>>> On Fri, Mar 20, 2026 at 11:43 AM Alexandre Dutra >>>>>> >>> >>>>> <[email protected]> wrote: >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> Hi all, >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> Last week I opened an issue to report what I believe is a >>>>>> >>> >>>>>> regression >>>>>> >>> >>>>>> in the HTTPClient when using TLS: >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> https://github.com/apache/iceberg/issues/15598 >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> I also opened a PR to fix it: >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> https://github.com/apache/iceberg/pull/15500 >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> The fix is basically to expose the HostnameVerificationPolicy >>>>>> >>> >>>>>> in the >>>>>> >>> >>>>>> TLSConfigurer, and I think there is consensus on that. >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> However I would like to have the community's opinion about >>>>>> >>> >>>>>> the default >>>>>> >>> >>>>>> value we should use for the HostnameVerificationPolicy. >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> We can either go with: >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> - CLIENT, which reproduces the current behavior in 1.10 but >>>>>> >>> >>>>>> is less safe; or >>>>>> >>> >>>>>> - BOTH, which introduces a behavioral change, but is the >>>>>> >>> >>>>>> safest option. >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> What do you think? >>>>>> >>> >>>>>> >>>>>> >>> >>>>>> Thanks, >>>>>> >>> >>>>>> Alex
