Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

[INFINITY-3520] Make Kafka ciphers configurable and have a secure default. #2483

Merged
merged 5 commits into from
Apr 18, 2018

Conversation

mpereira
Copy link
Contributor

@mpereira mpereira commented Apr 17, 2018

This PR adds plumbing for a new configuration for TLS cipher suites: service.security.transport_encryption.ciphers. That value is plumbed directly to Kafka's server.properties ssl.cipher.suites configuration.

However, Kafka doesn't seem to be accepting the current default configuration value. It is a comma-separated list of cipher suite names prefixed with TLS_. I added the prefix after seeing it on a couple of places. I've tried it without the prefix too, to no avail. If service.security.transport_encryption.ciphers is left blank Kafka accepts connections with the 21 ciphers we saw before. The documentation isn't very specific and doesn't provide example values.

I've tried:
- With TLS_ prefix, separated by dashes. e.g.: TLS_AES128-GCM-SHA256,...
- Without TLS prefix, separated by dashes. e.g.: AES128-GCM-SHA256,...
- With TLS_ prefix, separated by underscores. e.g.: TLS_AES128_GCM_SHA256,...
- Without TLS prefix, separated by underscores. e.g.: AES128_GCM_SHA256,...

I'll look into this tomorrow morning unless someone feels like taking a swing at it. The added test is currently failing due to the above.

Examples of configuring ssl.cipher.suites in the wild:
- IBM/sarama#643 (comment)
- https://phabricator.wikimedia.org/T182993#3855423

@mpereira mpereira requested a review from benclarkwood April 17, 2018 19:08
@mpereira mpereira added the wip label Apr 17, 2018
Copy link
Contributor

@benclarkwood benclarkwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice of Java to have subtly different cipher names.

@mpereira
Copy link
Contributor Author

mpereira commented Apr 18, 2018

Surprisingly enough, Java is the one following the standard in this case!

@mpereira mpereira merged commit f377f02 into master Apr 18, 2018
@mpereira mpereira deleted the infinity-3520-kafka-ciphers branch April 18, 2018 18:15
mpereira added a commit that referenced this pull request Apr 18, 2018
…secure default. (#2483)

* Plumb cipher configuration through.

* Add tests.

* Handle RFC cipher suite names.

* Ciphers are separated by commas in the service configuration.

* Of course I forgot to include this file.
mpereira added a commit that referenced this pull request Apr 23, 2018
…secure default. (#2483) (#2487)

* Plumb cipher configuration through.

* Add tests.

* Handle RFC cipher suite names.

* Ciphers are separated by commas in the service configuration.

* Of course I forgot to include this file.

* Do without sdk_cmd.service_task_exec.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants