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

Allow use with invalid SSL certs for testing #56

Closed
dtrudg opened this issue Jan 26, 2018 · 11 comments
Closed

Allow use with invalid SSL certs for testing #56

dtrudg opened this issue Jan 26, 2018 · 11 comments

Comments

@dtrudg
Copy link

dtrudg commented Jan 26, 2018

In some cases, you may want to use sregistry-cli against an sregistry server that has https enabled, but an invalid/untrusted self-signed certificate. Examples include:

  • A test server where you need to run https for credential encryption, but use a self-signed cert
  • A server with different external/internal DNS names, and a cert only valid for the public name
  • Accessing a server via IP address during troubleshooting of DNS issues

A command line flag or environment variable that will result in verify=False being passed to calls to get/post methods from requests would be useful for these situations.

Of course, the server cert in question could be added to the trusted certificates file in use, but this is a pain for small test things.

Currently connecting to a server with untrusted/invalid cert will throw a cryptic exception:

$ sregistry pull shub://sregistry-test.biohpc.swmed.edu/dtrudgian/ubuntu:latest
[client|hub] [database|sqlite:////home2/dtrudgian/.singularity/sregistry.db]
Traceback (most recent call last):
  File "/home2/dtrudgian/.local/lib/python3.4/site-packages/urllib3/connectionpool.py", line 601, in urlopen
    chunked=chunked)
  File "/home2/dtrudgian/.local/lib/python3.4/site-packages/urllib3/connectionpool.py", line 346, in _make_request
    self._validate_conn(conn)
  File "/home2/dtrudgian/.local/lib/python3.4/site-packages/urllib3/connectionpool.py", line 850, in _validate_conn
    conn.connect()
  File "/home2/dtrudgian/.local/lib/python3.4/site-packages/urllib3/connection.py", line 326, in connect
    ssl_context=context)
  File "/home2/dtrudgian/.local/lib/python3.4/site-packages/urllib3/util/ssl_.py", line 329, in ssl_wrap_socket
    return context.wrap_socket(sock, server_hostname=server_hostname)
  File "/cm/shared/apps/python/3.4.x-anaconda/lib/python3.4/ssl.py", line 365, in wrap_socket
    _context=self)
  File "/cm/shared/apps/python/3.4.x-anaconda/lib/python3.4/ssl.py", line 583, in __init__
    self.do_handshake()
  File "/cm/shared/apps/python/3.4.x-anaconda/lib/python3.4/ssl.py", line 810, in do_handshake
    self._sslobj.do_handshake()
ConnectionResetError: [Errno 104] Connection reset by peer

Docker does this by listing insecure registries to trust in a config file: https://docs.docker.com/registry/insecure/

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

This is definitely do-able. We have various variables for the clients to disable https (e.g., SREGISTRY_DOCKER_NOHTTPS - would something like this work? For a test you could try to set SREGISTRY_DOCKER_NOHTTPS and then interact with Docker Hub. Something like:

SREGISTRY_DOCKER_NOHTTPS=true sregistry pull docker://ubuntu:latest

If this works, then we can implement the same for sregistry, either just for the client, and/or add a global setting.

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

and if that method doesn't work we can do the change to add verify=False.

@dtrudg
Copy link
Author

dtrudg commented Jan 26, 2018

Looking at the code, the NOHTTPS vars direct the client to use an http:// connection, not an https:// connection. In this case we still must use https to ensure auth tokens are encrypted on the wire - but need to stop requests/urllib3 verifying the certificate

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

ah ok so maybe it should be a different variable? And set globally? What would you call it?

@dtrudg
Copy link
Author

dtrudg commented Jan 26, 2018

Something like SREGISTRY_HTTPS_NOVERIFY?

It might be good to have it endpoint specific, so you can be sure you are still talking to verified docker hub, or nvidia cloud.... but can disable verification for sregistry URLs - but that may be more complicated than you want. Plus this really should be for limited testing only, never used in production.

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

I would say one global setting, lots of documentation about that it's not for production, and lots of annoying messages printed to the user when it's active, hehe. Should I do that for a first go? Can I add to current PR?

@dtrudg
Copy link
Author

dtrudg commented Jan 26, 2018

Yes - one global setting with a big nasty warning message is probably good here. Current PR is good - if you want I could do a separate PR at some point, but you always get there first!

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

okay gotcha!

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

Okay so in summary:

sregistry

We changed admin variable to is_staff in this PR. If you are good with this PR (and don't want to add any docs) then I'll merge that one.

sregistry-cli

We are working on removing sqlalchemy, in two PRs. I'll add this variable shortly and you can test both in one swoop.

@dtrudg
Copy link
Author

dtrudg commented Jan 26, 2018

Yep 👍

@vsoch
Copy link
Member

vsoch commented Jan 26, 2018

This was fixed and closed with #58 thanks again @dctrud !

@vsoch vsoch closed this as completed Jan 26, 2018
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

2 participants