Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen the restrictions for TLS options in type: custom listener authentication to enable new features #11052

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

scholzj
Copy link
Member

@scholzj scholzj commented Jan 16, 2025

Type of change

  • Enhancement / new feature

Description

There are some feature requests that pop-up from time to time related to type: tls authentication. For example:

  • Trusting custom CA without providing the full custom Clients CA because the user wants to sign its own certificates (currently, this is possible by passing a dummy value as the private key in custom Clients CA)
  • Trusting multiple different clients CAs at the same time
  • Being able to override the ssl.principal.mapping.rules when using a custom CA with more complex subjects

Right now, when using the type: custom authentication, we do not allow to configure any options starting with ssl.. This makes it impossible to use type custom authentication for the features described above.

This PR loosens the restriction and forbids only the keystore configuration options - i.e. starting with ssl.keystore.. But let's the users configure other TLS options. That means that the TLS keystore will be still configured by Strimzi based on the listener configuration. But users could for example freely:

  • Enable TLS authentication
  • Pass custom truststore
  • Configure detailed TLS options such as ssl.principal.mapping.rules for given listener

For example, the 3 feature requests listed above can be done with following configuration:

spec:
  kafka:
    listeners:
      - name: tls
        port: 9093
        tls: true
        type: internal
        authentication:
          type: custom
          sasl: false
          listenerConfig:
            ssl.client.auth: required
            ssl.principal.mapping.rules: RULE:^CN=(.*?),(.*)$/CN=$1/
            ssl.truststore.location: /opt/kafka/custom-authn-secrets/custom-listener-tls-9093/custom-truststore/ca.crt
            ssl.truststore.type: PEM
          secrets:
            - key: ca.crt
              secretName: custom-truststore

This:

  • Configures custom principal mapping rules to use only the certificate CN for the username
  • Uses custom PEM file as truststore with one or more trusted CAs

This should resolve #2900 and resolve #6566

Checklist

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md

@scholzj scholzj added this to the 0.46.0 milestone Jan 16, 2025
@scholzj
Copy link
Member Author

scholzj commented Jan 17, 2025

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@da-off13 da-off13 mentioned this pull request Jan 17, 2025
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

Thanks Jakub.
Would it make sense to mention this with the config exceptions in KafkaClusterSpec schema reference, as with allowed ssl config for cipher suites?

Co-authored-by: PaulRMellor <[email protected]>
Signed-off-by: Jakub Scholz <[email protected]>
@scholzj
Copy link
Member Author

scholzj commented Jan 21, 2025

Would it make sense to mention this with the config exceptions in KafkaClusterSpec schema reference, as with allowed ssl config for cipher suites?

Those are different than this. This does not apply to the .spec.kafka.config section but only tot he type: custom authentication. So I would probably avoid mising those together as I think it might confuse people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants