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 >
