fix: Remove InsecureSkipVerify

[rafvasq]

Removes InsecureSkipVerify: true, which controls whether a client verifies the server's certificate chain/hostname. From the docs, if InsecureSkipVerify is true, any certificate presented by the server and any host name in that certificate is accepted.

• Fixes https://github.com/kserve/rest-proxy/security/code-scanning/1

[ckadner]

@rafvasq -- did you do a full integration test, i.e. deploy this rest-proxy change in a full modelmesh-serving deployment and setup TLS?

[tjohnson31415]

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.

[rafvasq]

Thanks @tjohnson31415. Based on your recommendation, I'll create an issue from your comment to work from and just close this PR instead of repurposing it.