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

Add support to add user given CAs to trusted store #4168

Closed
JorTurFer opened this issue Jan 30, 2023 · 6 comments · Fixed by #4191
Closed

Add support to add user given CAs to trusted store #4168

JorTurFer opened this issue Jan 30, 2023 · 6 comments · Fixed by #4191
Assignees
Labels
governance security All issues related to security

Comments

@JorTurFer
Copy link
Member

JorTurFer commented Jan 30, 2023

Proposal

There are some cases where a user could want to use TLS with custom CAs in their own services, and KEDA should support an option to add those CAs to trusted store.
As the container is distroless, I'd add it using golang directly

Is this a feature you are interested in implementing yourself?

Yes

@JorTurFer JorTurFer added needs-discussion feature-request All issues for new features that have not been committed to labels Jan 30, 2023
@JorTurFer JorTurFer self-assigned this Jan 30, 2023
@JorTurFer JorTurFer added security All issues related to security governance and removed needs-discussion feature-request All issues for new features that have not been committed to labels Jan 30, 2023
@JorTurFer
Copy link
Member Author

JorTurFer commented Jan 31, 2023

I have been thinking about this, and I'm not sure which is the best approach 🤔
I could read all the CAs for a given folder and stores them globally, but I'm not sure if this could be better as part of TriggerAuthentication (adding a new field), applying them only in SO/SJ where they are attached. I can see scenarios for both cases. From a security pov, I guess that trusting a CA is an administrator action more than a developer action (because it can be risky), but I'm not sure what is better (maybe both in different PRs).

In the other hand, the approach of reading all the certs from a single path could be complicated to test using kustomize...
Another option I can imagine (but maybe it's overkill) is introducing a new CRD for managing this, something like (Cluster)TrustedCertificateAuthority, with the option to pass one or multiple TrustedCertificateAuthority to trust in them for a specific SO/SJ.
This cloud simplify both cases but still doesn't cover some of them, for example, if all the certs in the clsuter are generated using the same CA, just registering it is easier than using CRDs

Maybe as starting point, we can add the support to read all the certs in a given folder and that's all (all the trusted CAs would be mounted and read from a single directory and trust in them for any HTTP request)

WDYT @kedacore/keda-core-contributors ?

@zroubalik
Copy link
Member

Yeah, I agree with the approach. Start with the global store but also support it on TriggerAuth ClusterTriggerAuth level.

@QualoZe0t
Copy link

@JorTurFer Please keep in mind that as a User I can configure the AWS MSK cluster using TLS without ACM private certificate . So there should be option for using tls without specifying custom certificate

@JorTurFer
Copy link
Member Author

Hi @QualoZe0t ,
What do you mean? AWS CAs are not trusted, so there are 2 options:

  • Registering their CAs in the trusted store (covered by this issue)
  • Adding an option to unsafeSsl to the kafka scaler (not covered by this issue)

@QualoZe0t
Copy link

@JorTurFer so once ,,default,, AWS CA certificates will be registered in the trusted store then doesn't matter if I wll use or not use ACM option will be able to establish connectivity between KEDA and MSK kafka ?

@JorTurFer
Copy link
Member Author

JorTurFer commented Feb 7, 2023

I can't guarantee that this is the only problem, but definitively the certificate should be valid or ignorable, otherwise sarama client will raise an error.
Just to clarify, we won't add AWS certificates to the trusted store, we are just adding support to add them as end-users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
governance security All issues related to security
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants