Support TLS configuration
- Dominant language
- Go
- Stars
- 14
- Forks
- 48
- PR merge metrics
- No merged PRs in 30d
Description
Support TLS configuration through environment variables controlling `InsecureSkipVerify`, `RootCAs`, 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 https://github.com/kserve/rest-proxy/issues/31#issuecomment-1806030727_
Contributor guide
Research direction
Locate the gRPC transport credentials and tls.Config construction, then trace how environment variables are parsed. Implement configuration for InsecureSkipVerify, RootCAs, and ServerName while preserving insecureSkipVerify: true by default. Done means the rest-proxy can optionally validate the upstream certificate and hostname through environment settings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, grpc
- Domain
- api, backend, security
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100