[Enhancement] Allow inter-broker TLS to be disabled

[dalelane]

Is your feature request related to a problem? Please describe.
Disabling TLS for Kafka's inter-broker connections would improve the performance. For installs where security isn't a concern, such as those running on private isolated secure clusters, inter-broker TLS may not be needed.

We'd like a way to disable TLS for these cases, so users can decide whether they want to prioritise performance or security.

Describe the solution you'd like
A new boolean attribute in KafkaClusterSpec that enables/disables inter-broker TLS.

e.g.

apiVersion: kafka.strimzi.io/v1beta1
kind: Kafka
metadata:
  name: my-cluster
spec:
  kafka:
    version: 2.3.1
    replicas: 3
    interBrokerTls: false

The new attribute would default to true to maintain current behaviour. Existing CR definitions need not be affected, but there would be an option here to edit an existing cluster by setting this to false. (Rolling updates that modify this would require persistence, as new brokers wouldn't be able to talk to the previous generation brokers so replication wouldn't be possible during the rolling upgrade.)

Describe alternatives you've considered
Alternatively, we could treat this as a new type of listener, that can be configured - calling it something like interbroker or replication:

e.g.

apiVersion: kafka.strimzi.io/v1beta1
kind: Kafka
metadata:
  name: my-cluster
spec:
  kafka:
    version: 2.3.1
    replicas: 3
    listeners:
      plain: {}
      tls: {}
      interbroker:
        tls: false

If we think we'll ever want to make other configuration changes to the inter-broker listeners, this might be a preferable approach. Plus, semantically it feels like a good fit to configure it here rather than as an attribute of the Kafka cluster overall.

Additional context
None.

[scholzj]

I think the spec.kafka.listeners.interbroker looks good in the current API state - it fits well and allows to add more options in the future. However, we had some thoughts (and user requests) about giving users more freedom to define the listeners. E.g. to change to something more "free":

listeners:
 - name: plain
    type: internal
    port: 9092
    tls: false
    authentication:
      type: scram-sha512
 - name: tls
    type: internal
    tls: true
    port: 9093
    authentication:
      type: tls
 - name: external1
    type: external
    tls: true
    port: 9094
    authentication:
      type: tls
 - name: external2
    type: external
    tls: true
    port: 9095
    authentication:
      type: tls

And I'm not sure how well it would fit in here. Maybe having it as spec.kafka.interbroker would be better to cover this?

I personally would wonder if we should call it replication rather than interbroker, but no storng feelings about. Also, would this cover everything - including Zookeeper encryption?

CC @tombentley

[dalelane]

would this cover everything - including Zookeeper encryption?

I wasn't planning on - mainly because I wouldn't expect there to be a significant performance benefit from that, but I've got no objection to adding that as well to keep things symmetrical.

[scholzj]

@dalelane I do not think it needs to be part of the first PR, but might be interesting to have an idea how to deal with it in the API if we ever do it.

One more question ... related to both the PR and the API. Right now we use TLS not just as encryption, but also as authentication (TLS client authentication) / authorization (super users set based on the usernames from certificates). What is the plan to deal with that? I think that we will need to have some authentication not just for security, but also to be able to deal with authorization since Kafka has only one authorizer per cluster and not per listener.

[tomaley]

Thank you @scholzj for raising the point about authz, we will investigate this further.

[talonx]

Inter-broker replication traffic is causing significant load in our cluster - is there any possibility of this making this configurable soon? Or are there any workarounds/hacks?

[scholzj]

I do not think anyone is working on this. But contributions are always welcomed.

[yp201]

@dalelane did you find any work around for this?

[dalelane]

no, sorry

[scholzj]

Triaged on 17.3.2022: Makes sense as a feature. Proposal would be needed to clarify the APIs etc.

[Hronom]

Will be useful for Istio based setup, when all connections secured by Istio mTLS, so no need to add extra security

[Hronom]

Is there any workaround? I realized that this one actually a blocker for Istio deployment.
I don't understand why it's still not in place, especially as service mesh is the popular one. Including Istio.

Is there a plan to move forward with this?

[Neustradamus]

@scholzj: I see a bug in your comment, "scram-sha522", can you replace to "scram-sha512"?

[craiglservin]

This would be a nice feature the CPU savings could be larger than you would first expect. Not only do you lose the TLS overhead, but once the socket is not encrypted the transfer will be offloaded to sendfile() so the data will not have to be copied. I'm sure everyone involved already understands this, I just wanted to comment to show that there is still interest in this feature.

[nbuesing]

what is the status on this issue; in various configurations knowing you are air-gapped or in a properly configured network, adding inter broker communication to be able to replicate with 0-page copy is important (SASL_PLAINTEXT or PLAINTEXT).

If there is the ability to create a plaintext listener, that same functionality would be nice (and extremely beneficial) to also allow for inter broker (and controller) communication to also be plaintext (SASL_PLAINTEXT or PLAINTEXT).