Add in registry mutual tls #361
Conversation
|
I had a conversation with @joaopapereira on this and I have tested this on my local with nginx in front of a docker registry and am able to push images with mutual tls set to true and client and key provided. If I set it to false I get a 403 error which is what I receive if the client cert and key are not set. Let me know how else I can test this. |
|
Expected Failure with mutual-tls false ./imgpkg copy --tar /Users/brieger/Documents/nginx.tar --to-repo 127.0.0.1:5043/nginx --registry-mutual-tls=false --registry-client-cert-path /Users/brieger/.docker/certs.d/127.0.0.1\:5043/client.cert --registry-client-key-path /Users/brieger/.docker/certs.d/127.0.0.1\:5043/client.key --registry-ca-cert-path /Users/brieger/.docker/certs.d/127.0.0.1\:5043/ca.crt --registry-username testuser --registry-password testpassword
copy | importing 1 images...
copy |
copy | done uploading images
imgpkg: Error: Error while preparing a transport to talk with the registry:
Unable to create round tripper:
GET https://127.0.0.1:5043/v2/: unexpected status code 400 Bad Request: <html>
<head><title>400 No required SSL certificate was sent</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<center>No required SSL certificate was sent</center>
<hr><center>nginx/1.21.6</center>
</body>
</html>Expected Success with mutual tls true: brieger-a01:carvel-imgpkg brieger$ ./imgpkg copy --tar /Users/brieger/Documents/nginx.tar --to-repo 127.0.0.1:5043/nginx --registry-mutual-tls=true --registry-client-cert-path /Users/brieger/.docker/certs.d/127.0.0.1\:5043/client.cert --registry-client-key-path /Users/brieger/.docker/certs.d/127.0.0.1\:5043/client.key --registry-ca-cert-path /Users/brieger/.docker/certs.d/127.0.0.1\:5043/ca.crt --registry-username testuser --registry-password testpassword
copy | importing 1 images...
426.03 MiB / 426.03 MiB [----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------] 100.00% 1.53 GiB p/s
copy |
copy | done uploading images
426.03 MiB / 426.03 MiB [----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------] 100.00% 1.53 GiB p/s
Succeeded |
| @@ -33,6 +37,10 @@ func (r *RegistryFlags) Set(cmd *cobra.Command) { | |||
| cmd.Flags().BoolVar(&r.VerifyCerts, "registry-verify-certs", true, "Set whether to verify server's certificate chain and host name") | |||
| cmd.Flags().BoolVar(&r.Insecure, "registry-insecure", false, "Allow the use of http when interacting with registries") | |||
|
|
|||
| cmd.Flags().BoolVar(&r.MutualTLS, "registry-mutual-tls", false, "Set whether or not to use mutual TLS. If true set registry-client-cert-path and registry-client-key-path") | |||
There was a problem hiding this comment.
by using flags we are making it only possible to use this for a single registry -- this may work well if fetching a single image, but is not great for anything beyond that (bundles for example may be referencing images in different registries).
can you mention what use case you are going after? what are you fetching (single image?)?
There was a problem hiding this comment.
ah I suppose yeah if we are pulling them from different registries. I suppose I could use a slice like we do for ca certs but then have to somehow keep the pairs together. I am fetching a bundle and pushing it to a single registry(that has mutual tls on it). Airgapped deployment of tanzu kubernetes grid in amazon-c2s
There was a problem hiding this comment.
We might need to allow something similar to https://carvel.dev/imgpkg/docs/v0.27.0/auth/#via-environment-variables as well in order to get this working for multiple registries.
This will have consequences as well on the Registry and when we are creating the transports because we would need to attach these certificates not on the base transport but on the a particular transport that is used for a particular registry
There was a problem hiding this comment.
@joaopapereira been searching around the codebase where is the _0, _1, _n for env support added for registry, password, and username. I only see it loading IMGPKG_USERNAME or IMGPKG_PASSWORD for example.
Also, are you saying updating the newHTTPTransport function for each registry will not fix this as that is the base transport?
There was a problem hiding this comment.
Yes you are right, _0 and _1 are used to populate the keychain that is used when talking to the registry.
But in order to support MutalTLS for more than 1 registry we will need to extend the registry/transport.go structs to when they create a new round-tripper to select the appropriate keys based on the MutualTLS configuration.
Doesn't look very straightforward.
- The keychain would have to signal that it uses MutualTLS to the
CreateRoundTripperfunction - The baseRoundTripper attribute would have to be converted into a
http.Transportor we can try to cast it on the fly so that the MutualTLS could be appropriately set on the transport provided totransport.NewWithContext - Keychain would have to know how to read the _0 for the MutualTLS
Let me know if this makes sense, I am also free to try to talk this through a little more
|
My team is happy to use your patch @btrieger, so thank you for your work. I've submitted a merge request to make this work with v0.35.0. |
Some Docker Registries have mutual TLS on them meaning that you need to pass a client certificate and key in addition to the registry ca. I have added this functionality with 3 cli flags. --registry-mutual-tls(bool) default false , --registry-client-cert-path, and --registry-client-cert-key