search

dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.73k stars 1.07k forks source link

Tweak SDK Container insecure registry support to allow subdomains #41771

Open baronfel opened 4 months ago

baronfel commented 4 months ago

Is your feature request related to a problem? Please describe.

Right now users can configure insecure registries in their container engine (docker, podman) or via the SDK_CONTAINER_INSECURE_REGISTRIES environment variable. The current implementation checks any host requested against the set of insecure registries, and if a match is made configures HttpClient to accept any valid certificates that match for the host exactly.

This doesn't handle scenarios where a request for manifest data, configs, or blobs is served by a different sub-domain from the parent domain, for example very simple pulls from mcr.microsoft.com.

Describe the solution you'd like

I propose that the existing insecure registry support be expanded to allow requests under that domain, and potentially allow any url that any request to that domain may specify. For example, requesting a blob may direct the client to download the content from a completely different domain - I think we should support this by dynamically altering the set of domains allowed to be accessed regardless of TLS configuration.

cc @tmds for thoughts

tmds commented 4 months ago

Personally my usage for insecure registries is to a. work with something running on localhost (which is trusted) or, b. something that runs as part of a test environment (where the certificates are just adding complexity without bringing security). In these cases I'm not blocked by the implementation being really strict.

If we make it less strict, it will be usable in more cases where users are probably compromising their "software supply chain".

we should support this by dynamically altering the set of domains allowed to be accessed regardless of TLS configuration.

Tracking the domains will bring some complexity as it needs some interaction with the HttpClient. It will be simpler to ignore all cert errors and ensure the HttpClient is limited to a single Registry instance.

KalleOlaviNiemitalo commented 4 months ago

If the registry is insecure on localhost, but it references blobs on some third-party secure server, then I'd hope that certificate errors on the blob server are not automatically ignored.

baronfel commented 4 months ago

The main reason I raised this was because I was trying to MITM myself with Fiddler to inspect some traffic - I was able to use the insecure registry flag to communicate with mcr.microsoft.com, but the actual blobs backing the manifests, etc are on a different subdomain - <some region>.mcr.microsoft.com. This pattern is also followed by the other major cloud hosts - where an account ID, region name or similar is used as part of the final domain name for the registry and communicated to the client by the HTTP Location header. I agree that the current implementation works great for localhost, self-hosted GitLab container instances, etc - I raised this to capture the broader scenario. We can definitely hold off on doing anything until we get broader feedback from impacted users.

tmds commented 4 months ago

If the registry is insecure on localhost, but it references blobs on some third-party secure server, then I'd hope that certificate errors on the blob server are not automatically ignored.

localhost is special since we implicitly trust it.

If a user explicitly marks a registry as insecure (which could include localhost) I think we can consider to just ignore the certs for all registry operations and consider it all untrusted/insecure.

It would be interesting to try and see what docker/podman do.