kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.6k stars 8.27k forks source link

More granular control over proxy_ssl_trusted_certificate vs. proxy_ssl_certificate/proxy_ssl_certificate_key directives #9415

Open andremarianiello opened 1 year ago

andremarianiello commented 1 year ago

What do you want to happen?

I would like to be able to have more control over when the proxy_ssl_*certificate* directives are used. Right now, if you specify proxy-ssl-secret and proxy-ssl-verify, the controller's behavior is then completely determined by the contents of the secret. It is contains a ca.crt, then that value is used as the proxy_ssl_trusted_certificate. This is probably the most common goal of using the proxy-ssl-secret and proxy-ssl-verify annotations. However, there is more behavior. If the secret contains tls.crt and tls.key, then those values are used as proxy_ssl_certificate and proxy_ssl_certificate_key, i.e. they are used for mutual TLS. This behavior can not be disabled at the ingress level. The only way to disable it is by providing a secret without the tls.crt/tls.key values. This may seem simple, but is not really possible when integrating with cert-manager, because cert-manager secrets always have all 3 files present.

Enabling mTLS can cause security issues. For example, if someone just wanted nginx to verify the upstream certificate by providing proxy-ssl-secret and proxy-ssl-verify, they could accidentally be enabling mTLS to the upstream as well, which, depending on the upstream, might cause all requests through your ingress to be authenticated! This is very dangerous, and users might not even realize mTLS is happening, and difficult to fix because it cannot be disabled without great difficulty in some circumstances.

Additionally, it does not seem possible to enable upstream cert verification and disable mTLS at the same time, even with configuration snippets. You must use the proxy-ssl-secret annotation to make the ca.crt available to nginx for use in the proxy_ssl_trusted_certificate directive, but you cannot counteract the proxy_ssl_certificate/proxy_ssl_certificate_key directives that are added in the process.

Is there currently another issue associated with this? Not that I could find.

Does it require a particular kubernetes version? No

Proposed solution I propose that two new annotations be added, proxy-ssl-ca-secret and proxy-ssl-client-secret, and that proxy-ssl-secret be deprecated. The secret used in proxy-ssl-ca-secret will only have its ca.crt inspected, and will only set the proxy_ssl_trusted_certificate directive. The secret used in proxy-ssl-client-secret will only have its tls.crt/tls.key inspected, and will only set proxy_ssl_certificate/proxy_ssl_certificate_key directives. Specifying both annotations with the same key will recover the original proxy-ssl-secret behavior. If proxy-ssl-secret is specified along with proxy-ssl-ca-secret, proxy-ssl-ca-secret takes precedence (or an error would be fine too). Same with proxy-ssl-secret vs proxy-ssl-client-secret. Not only would this allow users more direct control over when cert verification and mTLS are enabled, it would also allow them to enabled both with two separate secrets, if desired.

k8s-ci-robot commented 1 year ago

@andremarianiello: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
chancez commented 1 year ago

In addition to a proxy-ssl-ca-secret option, a proxy-ssl-ca-configmap option would also be useful. https://github.com/cert-manager/trust-manager produces a configmap containing a CA bundle.