CC-9314: Allow disabling hostname verification

[ncliang]

When elastic.https.ssl.endpoint.identification.algorithm is empty, instead
of default hostname verifier, use an all-trusting verifier.

Updated SecurityIT with testcase to verify

[levzem]

LGTM on green build

Seems pretty straight forward to me.

One thing would be nice is if you updated the doc for the config SSL_ENDPOINT_ID_ALGO to mention that leaving it black will disable this feature.

[wicknicks]

one comment, otherwise LGTM. one concern/question is if we are breaking backward compatibility in some way. before this change an empty value for a config will still try to verify hostnames: would that fail in all cases?

[wicknicks]

nit: import org.apache.commons.lang.StringUtils instead?

[ncliang]

I don't think we currently pull in apache commons and I didn't think it is worth it to add the dependency just for this method.

[wicknicks]

that's fair, but I'm not sure if this is intended to be a public API though, for the http library. given the complexity of the code, I'd be more inclined to simply make a copy of the function here.

[ncliang]

I remove the use of this method and replace with simple isEmpty() check. The difference is that null and strings containing spaces will not trigger the feature, which I think is fine since our documentation explicitly says empty string.

[ncliang]

Thanks for review @wicknicks and @levzem . I think you are right that w.r.t. backwards compatibility, there is a risk that someone may have accidentally set this config to empty string and it would have been doing nothing before but now it will disable hostname verification for them. I think this would likely be fine though given that the default for this config is "https" and we have documentation that says setting ssl.endpoint.identification.algorithm to empty string will disable hostname verification 1. Users that have explicitly set this to empty string were likely trying to disable hostname verification and failing. They may have since worked around the issue by adding the hostname to their key/trust store but forgot to remove the setting from their config. In which case, this change would be doing them no harm.

The ConfigDef comes from AK SslConfigs so updating the doc there is none-trivial. I can update our website documentation 2 to add something talking about how empty string will disable hostname verification though.

[wicknicks]

@ncliang that's fine to do as well but maybe not in a patch release, since it sounds like the intended behavior is to verify hosts (and seems like it was doing fine). so maybe change the base of the branch to master, if that's the path we want to take?

[wicknicks]

looking more closely, looks like we are completely ignoring the setting for elastic.https.ssl.endpoint.identification.algorithm. The apache http client explicitly needs us to disable hostname verification as this PR does.

thanks very much for the fixes, @ncliang! LGTM.