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

Support TLS configuration #32

Open
rafvasq opened this issue Nov 10, 2023 · 0 comments
Open

Support TLS configuration #32

rafvasq opened this issue Nov 10, 2023 · 0 comments

Comments

@rafvasq
Copy link
Member

rafvasq commented Nov 10, 2023

Support TLS configuration through environment variables controlling InsecureSkipVerifyRootCAs, and ServerName, leaving insecureSkipVerify: true as the default.

Adding some context about the use of InsecureSkipVerify: true in ModelMesh Serving:
The transportCreds being modified are used to create a gRPC channel to the upstream gRPC server; this does not effect the security of the rest proxy's server listening for external requests. The intended of the rest-proxy is to run as a container within the ServingRuntime pods, so this rest-proxy -> model mesh connection would be on localhost within the pod. For this use-case the exposure from insecureSkipVerify is limited (a MITM attack would need to launch a server inside one of the pod's containers).

By removing InsecureSkipVerify: true, the rest-proxy would have to be configured to validate the server cert, which has two parts that will need additional configuration: (1) using the CA/cert chain to trust the cert and (2) knowing the hostname of the server for hostname validation.
For (1), the cert used by ModelMesh will typically generated from a custom CA or be self-signed, so the rest-proxy will need to be configured to trust the CA that signed the MM certificate by passing that to the transportCreds with the RootCAs field. I think that the certs used by the rest proxy will be the same as the certs used by MM when ran with modelmesh-serving, so the reference to the certificate would just need to be processed and added to the cert pool in the RootCAs field of the tls.Config.
For (2), the hostname validation, the typical intra-pod communication between rest-proxy and model mesh will use localhost. The certificate used by MM will not typically have localhost as one of its Subject Alternative Names (SANs) (I think that has some security issues as well), so the proxy will need to be configured with the hostname that MM is serving on (eg. the Kube Service name) by passing that in to the ServerName field of the tls.Config.

So, I don't think removing insecureSkipVerify will "just work". I think the best way forward would be to make the tls.Config configurable with env vars that control InsecureSkipVerify, RootCAs, and ServerName with insecureSkipVerify: true as the default. Then the rest proxy could be configured with certificate validation on the gRPC connection if desired, even if modelmesh-serving would need more changes to actually use it.

Originally posted by @tjohnson31415 in #31 (comment)

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

No branches or pull requests

1 participant