cnabio / duffle

CNAB installer
https://duffle.sh
MIT License
376 stars 54 forks source link

Improve image relocation UX for registries that use self-signed TLS certs #874

Closed st3v closed 4 years ago

st3v commented 4 years ago

Description

At the moment, duffle relocate seems to assume that the certificate chain for the TLS cert presented by the registry can always be verified using the trust store of the host OS. In the case of self-signed certs, the user will have to add the corresponding CA cert to the host's trust store. If they don't, duffle relocate will show the following error...

Error: Get https://my-registry.io/v2/: x509: certificate signed by unknown authority

Although adding the CA cert to the trust store of the host works, I'd argue it's not necessarily the most user-friendly option, in particular, if we consider the typical use-case for self-signed registry certs: people wanting to quickly try out a thick bundle in a sandbox environment.

Reproduce

# get example bundles
git clone git@github.com:deislabs/example-bundles.git
cd example-bundles/helloworld

# create self-signed cert for my-registry
openssl req -newkey rsa:2048 -nodes -keyout registry.key -x509 -days 365 -subj '/CN=my-registry/O=ACME/C=XY' -out registry.crt

# start registry on port 5000 and have it use the self-signed cert
docker run --rm -d -p 5000:5000 --name registry -e REGISTRY_HTTP_HOST=https://my-registry:5000 -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/registry.crt -e REGISTRY_HTTP_TLS_KEY=/certs/registry.key -v `pwd`:/certs registry:2

# add my-regisrty host alias for loopback
sudo sh -c 'echo "127.0.0.1 my-registry" >> /etc/hosts'

# try to relocate to the registry
duffle relocate bundle.json -m map.json -p my-registry:5000 -f

Suggestion

I can think of two potential alternatives to improve the UX for this use-case:

  1. Add --registry-ca-cert-path flag to duffle relocate

    This assumes that the user has access to the CA cert that was used to sign the TLS cert for the registry. They would specify the path to that cert when calling duffle relocate. We would then read the cert for that path and append it to the root CA cert pool used by the underlying HTTP transport.

    If we decide to go down this route, we should probably allow for --registry-ca-cert-path being set multiple times in one command in the case of intermediate certs.

  2. Add --insecure-skip-tls-verify flag to duffle relocate

    If this is set, we would set InsecureSkipVerify to true for the underlying HTTP transport.

    The disadvantage here is that skipping cert validation altogether is obviously insecure as it allows for man-in-the-middle attacks. Because of that, we might not want to encourage people to do this (in particular for production environments).

    The advantage is that the user doing the image relocation doesn't have to have access to the registry's CA cert. It's also worth noting that this would be aligned with what duffle build does via the --docker-tlsverify flag. I haven't looked into what duffle run -d docker does but I wouldn't be surprised if that actually evaluated the DOCKER_TLS_VERIFY env var.

glyn commented 4 years ago

I prefer option 2. I think it's better to keep things simple, if a bit dangerous.

radu-matei commented 4 years ago

I could potentially see the reason to implement both options here.

Any downsides to doing so?

glyn commented 4 years ago

Only complexity and perhaps a higher cost to benefit ratio. I think the usecase that gave rise to this issue would be satisfied by 2. We could also defer the implementation of 1 until it becomes strictly necessary. Shall we leave it up to the implementor as to what gets implemented first? (In other words: if you'd like to take a crack at the full solution, be my guest.)

st3v commented 4 years ago

AFAIK, most tools that deal with TLS cert verification have both options. So I'd agree with @radu-matei's suggestion. If we decided to pick only one, I'd vote for 1, actually.

If y'all are fine with that, I would take a stab at a PR that adds both options. It might take me a few days. But I don't think this is urgent as there is a workaround via the host trust store.

glyn commented 4 years ago

@st3v Great stuff. Take care to base on https://github.com/pivotal/image-relocation/pull/38 which includes a big refactoring. This should be merged soon.

glyn commented 4 years ago

https://github.com/pivotal/image-relocation/pull/38 is now merged.

scottfrederick commented 4 years ago

Just a note on use cases for this issue:

the typical use-case for self-signed registry certs: people wanting to quickly try out a thick bundle in a sandbox environment.

That is definitely one of the common use cases that raises this issue with image relocation. There is another other use case that is just as common but arguably much more important: large enterprises that generate their own certs using an internal root CA authority. These internal root CAs may not get propagated to the trust store on the host OS of every machine or container that needs to access a system secured with such cert. The ability to provide the internal root CA with an option like --ca-cert-path is much better received by the security teams in large enterprises like this than a --skip-tls-verify option.

The PR submitted to resolve this issue includes both options, which IMO is the best way to go.