Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3 driver should support Insecure TLS #2204

Closed
pdevine opened this issue Mar 2, 2017 · 7 comments
Closed

S3 driver should support Insecure TLS #2204

pdevine opened this issue Mar 2, 2017 · 7 comments

Comments

@pdevine
Copy link

pdevine commented Mar 2, 2017

The S3 driver currently has the "secure" option which will turn off TLS, however there is currently no option for using TLS but with an untrusted cert.

This prevents alternative S3 storage such as Minio, Scality and StorageGRID from running encrypted but with an invalid cert.

@dmcgowan
Copy link
Collaborator

dmcgowan commented Mar 3, 2017

This prevents alternative S3 storage such as Minio, Scality and StorageGRID from running encrypted but with an invalid cert.

Does not prevent it, just currently requires that such solutions trust the certificate at the OS level.

I would be OK with a PR to add an option to use a custom certificate chain, could be added via the http transport like the user agent is doing (https://github.com/docker/distribution/blob/master/registry/storage/driver/s3-aws/s3.go#L418)

@whoshuu
Copy link
Contributor

whoshuu commented Mar 3, 2017

What about a PR adding an option to set InsecureSkipVerify (like it's done here) on the transport? That would obviate the need to provide an interface for custom certificates that exist outside of the OS.

@dmcgowan
Copy link
Collaborator

dmcgowan commented Mar 3, 2017

@whoshuu I don't like adding support for InsecureSkipVerify except for clearly defined development use cases. For registry development there are already plenty of other storage configurations which can be used. For real setups we should always encourage properly setting up TLS, it really isn't much extra work but makes a huge difference is security.

@whoshuu
Copy link
Contributor

whoshuu commented Mar 3, 2017

It's strictly more secure than Secure: false, which is already an option.

The ideal solution would be for every user to know how to properly set up certs on their clients and servers. Understandably that can be difficult for some class of users.

An option to skip the verification step of the server certificate isn't one we would encourage, obviously. There should definitely be a strong push to get users to set up true TLS, but for getting things up and running without sending unencrypted data over the network I think this is a fair compromise.

@pdevine, thoughts?

@pdevine
Copy link
Author

pdevine commented Mar 17, 2017

@whoshuu definitely agree. I think we should add InsecureSkipVerify and also strongly discourage its use.

Some storage, like StorageGRID comes with a baked in CA, and it's impossible to use it with distribution right now, even to test, without having to bake the CA cert or the cert chain into your distribution container image.

@kaovilai
Copy link
Contributor

@dmcgowan

I would be OK with a PR to add an option to use a custom certificate chain, could be added via the http transport like the user agent is doing

Like this?
#3734

@milosgajdos
Copy link
Member

Closing. Addressed in #3841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants