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.

> 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 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

Reply via email to