FLUME-3315 fix kafka ssl https verification

[geniusjoe]

issue links

FLUME-3391 (duplicated)

FLUME-3315

steps to reproduce

• using kafka as source
• set transmit protocol like a1.sources.kafka_source.kafka.consumer.security.protocol=SASL_SSL to use ssl security layer
• set bootstrap servers like a1.sources.kafka_source.kafka.bootstrap.servers=172.16.0.1:9092 to use direct ip:port format instead of domain name

First starts flume program, the output shows that ssl.endpoint.identification.algorithm enable https varification feature.

Then flume will fail when handshakes with kafka broker due to FQDN check.

reasons

Kafka changed ssl.endpoint.identification.algorithm default value to https since kafka client 2.0+ , so kafka client always check FQDN.

The default value for ssl.endpoint.identification.algorithm was changed to https, which performs hostname verification (man-in-the-middle attacks are possible otherwise). Set ssl.endpoint.identification.algorithm to an empty string to restore the previous behaviour.

It needs to set ssl.endpoint.identification.algorithm to an empty string to prevent FQDN check. But according to FLUME-3391, one cannot do this because flume has an empty value config validation.

Besides, the default hostname verification shoud not be activated accroding to flume document.

Note: By default the property ssl.endpoint.identification.algorithm is not defined, so hostname verification is not performed. In order to enable hostname verification, set the following properties.

how to fix

Try to check if user set ssl.endpoint.identification.algorithm value. If not, add empty string as its value.

[geniusjoe]

Could you add test(s) that the expected configuration values needed for the workaround show up?

As a small thing that I don't feel strongly on, I think having this PR close out FLUME-3315 with FLUME-3391 marked as duplicate would be marginally better than the current plan to close out FLUME-3391.

@busbey Okay. I had edited this pr aimed for FLUME-3315 instead of FLUME-3391 and added ssl related tests, the tests seem to work just fine. 😀 Please review this pr again. 😁

[geniusjoe]

@rgoers @busbey Hello, any advice would be approciated. 😁

[tmgstevens]

Thinking this through, the current behaviour is that hostname checking is enabled, since we're using the Kafka 2.0 client already. So if we were to approve this change we'd be adding in a security regression. On that basis, I think we should add in a new property, something along the lines of: a1.channels.channel1.disableTLSHostnameVerification and if that has been set to true then we can go and set the producer and consumer ssl.endpoint.identification.algorithm to blank.

Hope this makes sense, just don't want to make things less secure than they are today, even if that means that our docs are actually wrong.

[geniusjoe]

Thinking this through, the current behaviour is that hostname checking is enabled, since we're using the Kafka 2.0 client already. So if we were to approve this change we'd be adding in a security regression. On that basis, I think we should add in a new property, something along the lines of: a1.channels.channel1.disableTLSHostnameVerification and if that has been set to true then we can go and set the producer and consumer ssl.endpoint.identification.algorithm to blank.

Hope this makes sense, just don't want to make things less secure than they are today, even if that means that our docs are actually wrong.

Your advice also sounds reasonable. As a user, what I only need is just an option to disable the hostname checking. It's there a way to edit the document? Do i need to edit related doc content at the same time ?

[geniusjoe]

Thinking this through, the current behaviour is that hostname checking is enabled, since we're using the Kafka 2.0 client already. So if we were to approve this change we'd be adding in a security regression. On that basis, I think we should add in a new property, something along the lines of: a1.channels.channel1.disableTLSHostnameVerification and if that has been set to true then we can go and set the producer and consumer ssl.endpoint.identification.algorithm to blank. Hope this makes sense, just don't want to make things less secure than they are today, even if that means that our docs are actually wrong.

Your advice also sounds reasonable. As a user, what I only need is just an option to disable the hostname checking. It's there a way to edit the document? Do i need to edit related doc content at the same time ?

@tmgstevens Hello, any advice would be greatly appreciated.

[tmgstevens]

The documentation can be updated by adding this file https://github.com/apache/flume/blob/trunk/flume-ng-doc/sphinx/FlumeUserGuide.rst to your pull request.

We'd then just need to add something into the doConfigure method of the KafkaSource, KafkaChannel and KafkaSink that allows this new parameter to be read and converted into a blank value on the consumer properties.

[geniusjoe]

Cause the trunk branch was created too early and cannot be rebase, i will create a up-to-date new pr soon. 😂