From f7f4759bde7cc302ecbb39a5589f1d04975dc837 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:01 -0700 Subject: [PATCH 01/68] Build ssl_context as util, wait_up with context --- jupyterhub/utils.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index 6eb6032c..47bc7c5a 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -16,6 +16,7 @@ import os import socket import sys import threading +import ssl import uuid import warnings @@ -70,6 +71,17 @@ def can_connect(ip, port): else: return True + +def make_ssl_context(keyfile, certfile, cafile=None, verify=True, check_hostname=True): + if not keyfile or not certfile: + return None + purpose = ssl.Purpose.SERVER_AUTH if verify else ssl.Purpose.CLIENT_AUTH + ssl_context = ssl.create_default_context(purpose, cafile=cafile) + ssl_context.load_cert_chain(certfile, keyfile) + ssl_context.check_hostname = check_hostname + return ssl_context + + async def exponential_backoff( pass_func, fail_message, @@ -166,11 +178,17 @@ async def wait_for_server(ip, port, timeout=10): ) -async def wait_for_http_server(url, timeout=10): +async def wait_for_http_server(url, timeout=10, ssl_context=None): """Wait for an HTTP Server to respond at url. Any non-5XX response code will do, even 404. """ + loop = ioloop.IOLoop.current() + tic = loop.time() + settings = None + if ssl_context: + settings = {"ssl_options": ssl_context} + AsyncHTTPClient.configure(None, defaults=settings) client = AsyncHTTPClient() async def is_reachable(): try: From a69e906c6e3a21b298f31df36d41c97de794e66a Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:13 -0700 Subject: [PATCH 02/68] Add config and wiring for enabling internal ssl in app --- jupyterhub/app.py | 56 +++++++++++++++++++++++++++++++++++++++++-- jupyterhub/objects.py | 5 ++-- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 3f505c8f..acde4c4d 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -63,6 +63,7 @@ from .utils import ( maybe_future, url_path_join, print_stacks, print_ps_info, + make_ssl_context, ) # classes for config from .auth import Authenticator, PAMAuthenticator @@ -312,6 +313,37 @@ class JupyterHub(Application): When setting this, you should also set ssl_key """ ).tag(config=True) + internal_ssl = Bool(False, + help=""" Turn on server side ssl. This enables end-to-end encryption. + JupyterHub will automatically create the necessary certificate + authority and sign notebook certificates as they're created. + """ + ).tag(config=True) + internal_certs_location = Unicode('out', + help=""" The location to store certificates automatically created by + JupyterHub. + + Use with internal_ssl + """ + ).tag(config=True) + internal_authority_name = Unicode('jupyterhub', + help=""" The name for the internal signing authority + + Use with internal_ssl + """ + ).tag(config=True) + internal_ssl_key = Unicode('', + help=""" The key to be used for internal ssl + """ + ) + internal_ssl_cert = Unicode('', + help=""" The cert to be used for internal ssl + """ + ) + internal_ssl_ca = Unicode('', + help=""" The ca to be used for internal ssl + """ + ) ip = Unicode('', help="""The public facing ip of the whole JupyterHub application (specifically referred to as the proxy). @@ -1138,6 +1170,8 @@ class JupyterHub(Application): ._replace(path=self.hub_prefix) ) self.hub.connect_url = self.hub_connect_url + if self.internal_ssl: + self.hub.proto = 'https' async def init_users(self): """Load users into and from the database""" @@ -1598,6 +1632,12 @@ class JupyterHub(Application): concurrent_spawn_limit=self.concurrent_spawn_limit, spawn_throttle_retry_range=self.spawn_throttle_retry_range, active_server_limit=self.active_server_limit, + internal_ssl=self.internal_ssl, + internal_certs_location=self.internal_certs_location, + internal_authority_name=self.internal_authority_name, + internal_ssl_key=self.internal_ssl_key, + internal_ssl_cert=self.internal_ssl_cert, + internal_ssl_ca=self.internal_ssl_ca, ) # allow configured settings to have priority settings.update(self.tornado_settings) @@ -1829,8 +1869,15 @@ class JupyterHub(Application): loop.stop() return + ssl_context = make_ssl_context( + self.internal_ssl_key, + self.internal_ssl_cert, + cafile=self.internal_ssl_ca, + check_hostname=False + ) + # start the webserver - self.http_server = tornado.httpserver.HTTPServer(self.tornado_application, xheaders=True) + self.http_server = tornado.httpserver.HTTPServer(self.tornado_application, ssl_options=ssl_context, xheaders=True) bind_url = urlparse(self.hub.bind_url) try: if bind_url.scheme.startswith('unix+'): @@ -1880,7 +1927,12 @@ class JupyterHub(Application): tries = 10 if service.managed else 1 for i in range(tries): try: - await Server.from_orm(service.orm.server).wait_up(http=True, timeout=1) + ssl_context = make_ssl_context( + self.internal_ssl_key, + self.internal_ssl_cert, + cafile=self.internal_ssl_ca + ) + await Server.from_orm(service.orm.server).wait_up(http=True, timeout=1, ssl_context=ssl_context) except TimeoutError: if service.managed: status = await service.spawner.poll() diff --git a/jupyterhub/objects.py b/jupyterhub/objects.py index 607e2a11..2ca3e73d 100644 --- a/jupyterhub/objects.py +++ b/jupyterhub/objects.py @@ -157,10 +157,11 @@ class Server(HasTraits): uri=self.base_url, ) - def wait_up(self, timeout=10, http=False): + + def wait_up(self, timeout=10, http=False, ssl_context=None): """Wait for this server to come up""" if http: - return wait_for_http_server(self.url, timeout=timeout) + return wait_for_http_server(self.url, timeout=timeout, ssl_context=ssl_context) else: return wait_for_server(self._connect_ip, self._connect_port, timeout=timeout) From c50cd1ba7f236e67c7cbcd4aa164868c5f20a21e Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:19 -0700 Subject: [PATCH 03/68] Propagate certs to everything that needs them --- jupyterhub/app.py | 1 + jupyterhub/proxy.py | 33 +++++++++++++++++++++++++++++++-- jupyterhub/services/auth.py | 25 +++++++++++++++++++++++++ jupyterhub/services/service.py | 4 ++++ jupyterhub/singleuser.py | 24 ++++++++++++++++++++++++ jupyterhub/spawner.py | 5 +++++ jupyterhub/user.py | 11 +++++++++-- 7 files changed, 99 insertions(+), 4 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index acde4c4d..8d479c23 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1379,6 +1379,7 @@ class JupyterHub(Application): orm_service.admin = spec.get('admin', False) self.db.commit() service = Service(parent=self, + app=self, base_url=self.base_url, db=self.db, orm=orm_service, domain=domain, host=host, diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index fb80495e..23b87ec1 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -41,7 +41,7 @@ from jupyterhub.traitlets import Command from traitlets.config import LoggingConfigurable from .objects import Server from . import utils -from .utils import url_path_join +from .utils import url_path_join, make_ssl_context def _one_at_a_time(method): @@ -391,6 +391,15 @@ class ConfigurableHTTPProxy(Proxy): c.ConfigurableHTTPProxy.should_start = False """ + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + ssl_context = make_ssl_context( + self.app.internal_ssl_key, + self.app.internal_ssl_cert, + cafile=self.app.internal_ssl_ca, + ) + AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) + proxy_process = Any() client = Instance(AsyncHTTPClient, ()) @@ -432,9 +441,22 @@ class ConfigurableHTTPProxy(Proxy): token = utils.new_token() return token - api_url = Unicode('http://127.0.0.1:8001', config=True, + api_url = Unicode(config=True, help="""The ip (or hostname) of the proxy's API endpoint""" ) + + @default('api_url') + def _api_url_default(self): + url = '127.0.0.1:8001' + proto = 'http' + if self.app.internal_ssl: + proto = 'https' + + return "{proto}://{url}".format( + proto=proto, + url=url, + ) + command = Command('configurable-http-proxy', config=True, help="""The command to start the proxy""" ) @@ -541,6 +563,13 @@ class ConfigurableHTTPProxy(Proxy): cmd.extend(['--ssl-key', self.ssl_key]) if self.ssl_cert: cmd.extend(['--ssl-cert', self.ssl_cert]) + if self.app.internal_ssl: + cmd.extend(['--api-ssl-key', self.app.internal_ssl_key]) + cmd.extend(['--api-ssl-cert', self.app.internal_ssl_cert]) + cmd.extend(['--api-ssl-ca', self.app.internal_ssl_ca]) + cmd.extend(['--api-ssl-request-cert']) + cmd.extend(['--api-ssl-reject-unauthorized']) + cmd.extend(['--forward-ssl']) if self.app.statsd_host: cmd.extend([ '--statsd-host', self.app.statsd_host, diff --git a/jupyterhub/services/auth.py b/jupyterhub/services/auth.py index 476ac936..2d97e1b5 100644 --- a/jupyterhub/services/auth.py +++ b/jupyterhub/services/auth.py @@ -196,6 +196,27 @@ class HubAuth(SingletonConfigurable): def _default_login_url(self): return self.hub_host + url_path_join(self.hub_prefix, 'login') + keyfile = Unicode('', + help="""The ssl key to use for requests + + Use with certfile + """ + ).tag(config=True) + + certfile = Unicode('', + help="""The ssl cert to use for requests + + Use with keyfile + """ + ).tag(config=True) + + client_ca = Unicode('', + help="""The ssl certificate authority to use to verify requests + + Use with keyfile and certfile + """ + ).tag(config=True) + cookie_name = Unicode('jupyterhub-services', help="""The name of the cookie I should be looking for""" ).tag(config=True) @@ -277,6 +298,10 @@ class HubAuth(SingletonConfigurable): allow_404 = kwargs.pop('allow_404', False) headers = kwargs.setdefault('headers', {}) headers.setdefault('Authorization', 'token %s' % self.api_token) + if "cert" not in kwargs and self.certfile and self.keyfile: + kwargs["cert"] = (self.certfile, self.keyfile) + if self.client_ca: + kwargs["verify"] = self.client_ca try: r = requests.request(method, url, **kwargs) except requests.ConnectionError as e: diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py index 50bffa15..f4cab84d 100644 --- a/jupyterhub/services/service.py +++ b/jupyterhub/services/service.py @@ -224,6 +224,7 @@ class Service(LoggingConfigurable): domain = Unicode() host = Unicode() hub = Any() + app = Any() proc = Any() # handles on globals: @@ -331,6 +332,9 @@ class Service(LoggingConfigurable): server=self.orm.server, host=self.host, ), + internal_ssl=self.app.internal_ssl, + internal_certs_location=self.app.internal_certs_location, + internal_authority_name=self.app.internal_authority_name, ) self.spawner.start() self.proc = self.spawner.proc diff --git a/jupyterhub/singleuser.py b/jupyterhub/singleuser.py index 468c35da..ef043e01 100755 --- a/jupyterhub/singleuser.py +++ b/jupyterhub/singleuser.py @@ -237,6 +237,27 @@ class SingleUserNotebookApp(NotebookApp): def _default_group(self): return os.environ.get('JUPYTERHUB_GROUP') or '' + keyfile = Unicode('', + help="""The ssl key to use for requests + + Use with certfile + """ + ).tag(config=True) + + certfile = Unicode('', + help="""The ssl cert to use for requests + + Use with keyfile + """ + ).tag(config=True) + + client_ca = Unicode('', + help="""The ssl certificate authority to use to verify requests + + Use with keyfile and certfile + """ + ).tag(config=True) + @observe('user') def _user_changed(self, change): self.log.name = change.new @@ -423,6 +444,9 @@ class SingleUserNotebookApp(NotebookApp): api_url=self.hub_api_url, hub_prefix=self.hub_prefix, base_url=self.base_url, + keyfile=self.keyfile, + certfile=self.certfile, + client_ca=self.client_ca, ) # smoke check if not self.hub_auth.oauth_client_id: diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index ff76b66c..0079a35e 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -158,6 +158,11 @@ class Spawner(LoggingConfigurable): if self.orm_spawner: return self.orm_spawner.name return '' + hub = Any() + authenticator = Any() + internal_ssl = Bool(False) + internal_certs_location = Unicode('') + internal_authority_name = Unicode('') admin_access = Bool(False) api_token = Unicode() oauth_client_id = Unicode() diff --git a/jupyterhub/user.py b/jupyterhub/user.py index f65affc4..7a540727 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -11,7 +11,7 @@ from sqlalchemy import inspect from tornado import gen from tornado.log import app_log -from .utils import maybe_future, url_path_join +from .utils import maybe_future, url_path_join, make_ssl_context from . import orm from ._version import _check_version, __version__ @@ -215,6 +215,9 @@ class User: db=self.db, oauth_client_id=client_id, cookie_options = self.settings.get('cookie_options', {}), + internal_ssl=self.settings.get('internal_ssl'), + internal_certs_location=self.settings.get('internal_certs_location'), + internal_authority_name=self.settings.get('internal_authority_name'), ) # update with kwargs. Mainly for testing. spawn_kwargs.update(kwargs) @@ -493,7 +496,11 @@ class User: db.commit() spawner._waiting_for_response = True try: - resp = await server.wait_up(http=True, timeout=spawner.http_timeout) + key = self.settings['internal_ssl_key'] + cert = self.settings['internal_ssl_cert'] + ca = self.settings['internal_ssl_ca'] + ssl_context = make_ssl_context(key, cert, cafile=ca) + resp = await server.wait_up(http=True, timeout=spawner.http_timeout, ssl_context=ssl_context) except Exception as e: if isinstance(e, TimeoutError): self.log.warning( From c5faf2c5ea8eb4e08358ae6b72eedee0778f444a Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:32 -0700 Subject: [PATCH 04/68] Use certipy to automate cert creation --- jupyterhub/app.py | 14 ++++++++++++++ jupyterhub/spawner.py | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 8d479c23..aad98b87 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -20,6 +20,7 @@ import sys from textwrap import dedent from urllib.parse import unquote, urlparse, urlunparse +from certipy import Certipy if sys.version_info[:2] < (3, 3): raise ValueError("Python < 3.3 not supported: %s" % sys.version) @@ -1682,6 +1683,19 @@ class JupyterHub(Application): cfg = self.config.copy() cfg.JupyterHub.merge(cfg.JupyterHubApp) self.update_config(cfg) + if self.internal_ssl: + cert_store = Certipy(store_dir=self.internal_certs_location) + cert_store.store_load() + if not cert_store.store_get(self.internal_authority_name): + cert_store.create_ca(self.internal_authority_name) + internal_key_pair = cert_store.store_get("localhost") + if not internal_key_pair: + internal_key_pair = cert_store.create_signed_pair("localhost", self.internal_authority_name, alt_names=b"IP:127.0.0.1") + cert_store.store_save() + + self.internal_ssl_key = internal_key_pair.key_file + self.internal_ssl_cert = internal_key_pair.cert_file + self.internal_ssl_ca = internal_key_pair.ca_file self.write_pid_file() def _log_cls(name, cls): diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 0079a35e..51e4640a 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -23,6 +23,7 @@ from async_generator import async_generator, yield_ from sqlalchemy import inspect from tornado.ioloop import PeriodicCallback +from certipy import Certipy from traitlets.config import LoggingConfigurable from traitlets import ( @@ -694,6 +695,23 @@ class Spawner(LoggingConfigurable): default_url = self.format_string(self.default_url) args.append('--NotebookApp.default_url="%s"' % default_url) + if self.internal_ssl: + cert_store = Certipy(store_dir=self.internal_certs_location) + cert_store.store_load() + authority = self.internal_authority_name + internal_key_pair = cert_store.store_get(self.user.name) + if not internal_key_pair: + internal_key_pair = cert_store.create_signed_pair(self.user.name, authority, alt_names=b"DNS:localhost,IP:127.0.0.1") + cert_store.store_save() + key = internal_key_pair.key_file + cert = internal_key_pair.cert_file + ca = internal_key_pair.ca_file + + args.append('--keyfile="%s"' % key) + args.append('--certfile="%s"' % cert) + if ca: + args.append('--client-ca="%s"' % ca) + if self.debug: args.append('--debug') if self.disable_user_config: From 753bd0701f98fc9e7865c843f5bbcc483ffa61dc Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:38 -0700 Subject: [PATCH 05/68] Create and move certs for use with spawned notebooks Add Localhost to trusted alt names Update to match refactored certipy names Add the FQDN to cert alt names for hub Ensure notebooks do not trust each other Drop certs in user's home directory Refactor cert creation and movement Make alt names configurable Make attaching alt names more generic Setup ssl_context for the singleuser hub check --- jupyterhub/app.py | 54 ++++++++++++++++++++++++++++----- jupyterhub/singleuser.py | 9 +++++- jupyterhub/spawner.py | 65 +++++++++++++++++++++++++++++++--------- jupyterhub/user.py | 2 ++ 4 files changed, 108 insertions(+), 22 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index aad98b87..c5338e87 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -327,12 +327,20 @@ class JupyterHub(Application): Use with internal_ssl """ ).tag(config=True) - internal_authority_name = Unicode('jupyterhub', + internal_authority_name = Unicode('hub', help=""" The name for the internal signing authority Use with internal_ssl """ ).tag(config=True) + internal_notebook_authority_name = Unicode('notebook', + help=""" The name for the notebook signing authority. + This authority is separate from internal_authority_name so that + individual notebooks do not trust each other, only the hub and proxy. + + Use with internal_ssl + """ + ).tag(config=True) internal_ssl_key = Unicode('', help=""" The key to be used for internal ssl """ @@ -345,6 +353,16 @@ class JupyterHub(Application): help=""" The ca to be used for internal ssl """ ) + trusted_alt_names = List(Unicode(), + help=""" Names to include in the subject alternative name. + These names will be used for server name verification. This is useful + if JupyterHub is being run behind a reverse proxy or services using ssl + are on different hosts. + + Use with internal_ssl + """ + ).tag(config=True) + ip = Unicode('', help="""The public facing ip of the whole JupyterHub application (specifically referred to as the proxy). @@ -1637,9 +1655,11 @@ class JupyterHub(Application): internal_ssl=self.internal_ssl, internal_certs_location=self.internal_certs_location, internal_authority_name=self.internal_authority_name, + internal_notebook_authority_name=self.internal_notebook_authority_name, internal_ssl_key=self.internal_ssl_key, internal_ssl_cert=self.internal_ssl_cert, internal_ssl_ca=self.internal_ssl_ca, + trusted_alt_names=self.trusted_alt_names, ) # allow configured settings to have priority settings.update(self.tornado_settings) @@ -1685,17 +1705,37 @@ class JupyterHub(Application): self.update_config(cfg) if self.internal_ssl: cert_store = Certipy(store_dir=self.internal_certs_location) - cert_store.store_load() - if not cert_store.store_get(self.internal_authority_name): + joint_ca_file = "{out}/combined-cas.crt".format(out=self.internal_certs_location) + + # The authority for internal components (hub, proxy) + if not cert_store.get(self.internal_authority_name): cert_store.create_ca(self.internal_authority_name) - internal_key_pair = cert_store.store_get("localhost") + + # The authority for individual notebooks + notebook_authority = cert_store.get(self.internal_notebook_authority_name) + if not notebook_authority: + notebook_authority = cert_store.create_ca(self.internal_notebook_authority_name) + + internal_key_pair = cert_store.get("localhost") if not internal_key_pair: - internal_key_pair = cert_store.create_signed_pair("localhost", self.internal_authority_name, alt_names=b"IP:127.0.0.1") - cert_store.store_save() + alt_names = "IP:127.0.0.1,DNS:localhost,{extra_names}" + # In the event the hub needs to be accessed externally, add + # the fqdn and (optionally) rev_proxy to the set of alt_names. + extra_names = [socket.getfqdn()] + self.trusted_alt_names + extra_names = ','.join(["DNS:{}".format(name) for name in extra_names]) + alt_names = alt_names.format(extra_names=extra_names).encode() + internal_key_pair = cert_store.create_signed_pair("localhost", self.internal_authority_name, alt_names=alt_names) + + # Join CA files + with open(internal_key_pair.ca_file) as internal_ca, \ + open(notebook_authority.ca_file) as notebook_ca, \ + open(joint_ca_file, 'w') as combined_ca: + combined_ca.write(internal_ca.read()) + combined_ca.write(notebook_ca.read()) self.internal_ssl_key = internal_key_pair.key_file self.internal_ssl_cert = internal_key_pair.cert_file - self.internal_ssl_ca = internal_key_pair.ca_file + self.internal_ssl_ca = joint_ca_file self.write_pid_file() def _log_cls(name, cls): diff --git a/jupyterhub/singleuser.py b/jupyterhub/singleuser.py index ef043e01..4ad27ad2 100755 --- a/jupyterhub/singleuser.py +++ b/jupyterhub/singleuser.py @@ -43,7 +43,7 @@ from notebook.base.handlers import IPythonHandler from ._version import __version__, _check_version from .log import log_request from .services.auth import HubOAuth, HubOAuthenticated, HubOAuthCallbackHandler -from .utils import url_path_join +from .utils import url_path_join, make_ssl_context # Authenticate requests with the Hub @@ -399,6 +399,13 @@ class SingleUserNotebookApp(NotebookApp): - exit if I can't connect at all - check version and warn on sufficient mismatch """ + ssl_context = make_ssl_context( + self.keyfile, + self.certfile, + cafile=self.client_ca, + ) + AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) + client = AsyncHTTPClient() RETRIES = 5 for i in range(1, RETRIES+1): diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 51e4640a..19eb59c8 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -164,6 +164,7 @@ class Spawner(LoggingConfigurable): internal_ssl = Bool(False) internal_certs_location = Unicode('') internal_authority_name = Unicode('') + internal_notebook_authority_name = Unicode('') admin_access = Bool(False) api_token = Unicode() oauth_client_id = Unicode() @@ -672,6 +673,47 @@ class Spawner(LoggingConfigurable): """ return s.format(**self.template_namespace()) + def create_certs(self): + """Create the certs to be used for internal ssl.""" + cert_store = Certipy(store_dir=self.internal_certs_location) + internal_authority = self.internal_authority_name + notebook_authority = self.internal_notebook_authority_name + internal_key_pair = cert_store.get(internal_authority) + notebook_key_pair = cert_store.create_signed_pair(self.user.name, notebook_authority, alt_names=b"DNS:localhost,IP:127.0.0.1") + return { + "key_file": notebook_key_pair.key_file, + "cert_file": notebook_key_pair.cert_file, + "ca_file": internal_key_pair.ca_file, + } + + def move_certs(self, cert_files): + """Takes dict of cert/ca file paths and moves, sets up proper ownership for them.""" + user = pwd.getpwnam(self.user.name) + uid = user.pw_uid + gid = user.pw_gid + home = user.pw_dir + + # Create dir for user's certs wherever we're starting + out_dir = "{home}/.jupyter".format(home=home) + shutil.rmtree(out_dir, ignore_errors=True) + os.makedirs(out_dir, 0o700, exist_ok=True) + + # Move certs to users dir + shutil.move(cert_files['key_file'], out_dir) + shutil.move(cert_files['cert_file'], out_dir) + shutil.copy(cert_files['ca_file'], out_dir) + + path_tmpl = "{out}/{name}.{ext}" + key = path_tmpl.format(out=out_dir, name=self.user.name, ext="key") + cert = path_tmpl.format(out=out_dir, name=self.user.name, ext="crt") + ca = path_tmpl.format(out=out_dir, name=self.internal_authority_name, ext="crt") + + # Set cert ownership to user + for f in [out_dir, key, cert, ca]: + shutil.chown(f, user=uid, group=gid) + + return [key, cert, ca] + def get_args(self): """Return the arguments to be passed after self.cmd @@ -696,21 +738,16 @@ class Spawner(LoggingConfigurable): args.append('--NotebookApp.default_url="%s"' % default_url) if self.internal_ssl: - cert_store = Certipy(store_dir=self.internal_certs_location) - cert_store.store_load() - authority = self.internal_authority_name - internal_key_pair = cert_store.store_get(self.user.name) - if not internal_key_pair: - internal_key_pair = cert_store.create_signed_pair(self.user.name, authority, alt_names=b"DNS:localhost,IP:127.0.0.1") - cert_store.store_save() - key = internal_key_pair.key_file - cert = internal_key_pair.cert_file - ca = internal_key_pair.ca_file + try: + key, cert, ca = self.move_certs(self.create_certs()) - args.append('--keyfile="%s"' % key) - args.append('--certfile="%s"' % cert) - if ca: - args.append('--client-ca="%s"' % ca) + args.append('--keyfile="%s"' % key) + args.append('--certfile="%s"' % cert) + if ca: + args.append('--client-ca="%s"' % ca) + except Exception as e: + print("Internal SSL, if enabled, will not work.") + raise if self.debug: args.append('--debug') diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 7a540727..282b5445 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -218,6 +218,8 @@ class User: internal_ssl=self.settings.get('internal_ssl'), internal_certs_location=self.settings.get('internal_certs_location'), internal_authority_name=self.settings.get('internal_authority_name'), + internal_notebook_authority_name=self.settings.get('internal_notebook_authority_name'), + trusted_alt_names=self.settings.get('trusted_alt_names'), ) # update with kwargs. Mainly for testing. spawn_kwargs.update(kwargs) From 7c6972df7e82d1b761cf2b3fd02d0ac763ddaedc Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:45 -0700 Subject: [PATCH 06/68] Remove unnecessary flag, forward-ssl Import socket when needed Move pwd import since more than one thing uses it. --- jupyterhub/app.py | 1 + jupyterhub/proxy.py | 1 - jupyterhub/spawner.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index c5338e87..82f86def 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1718,6 +1718,7 @@ class JupyterHub(Application): internal_key_pair = cert_store.get("localhost") if not internal_key_pair: + import socket alt_names = "IP:127.0.0.1,DNS:localhost,{extra_names}" # In the event the hub needs to be accessed externally, add # the fqdn and (optionally) rev_proxy to the set of alt_names. diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 23b87ec1..aefef9fb 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -569,7 +569,6 @@ class ConfigurableHTTPProxy(Proxy): cmd.extend(['--api-ssl-ca', self.app.internal_ssl_ca]) cmd.extend(['--api-ssl-request-cert']) cmd.extend(['--api-ssl-reject-unauthorized']) - cmd.extend(['--forward-ssl']) if self.app.statsd_host: cmd.extend([ '--statsd-host', self.app.statsd_host, diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 19eb59c8..87a71caa 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -14,6 +14,7 @@ import shutil import signal import sys import warnings +import pwd from subprocess import Popen from tempfile import mkdtemp @@ -968,7 +969,6 @@ def set_user_setuid(username, chdir=True): home directory. """ import grp - import pwd user = pwd.getpwnam(username) uid = user.pw_uid gid = user.pw_gid From 3c21e7d45b5509437f0d561ce3d384eecd688f86 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:51 -0700 Subject: [PATCH 07/68] Server cert info into objects and orm --- jupyterhub/app.py | 6 ++++++ jupyterhub/objects.py | 11 +++++++++-- jupyterhub/orm.py | 3 +++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 82f86def..8b92d411 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1148,6 +1148,9 @@ class JupyterHub(Application): hub_args = dict( base_url=self.hub_prefix, public_host=self.subdomain_host, + ssl_cert_file=self.internal_ssl_cert, + ssl_key_file=self.internal_ssl_key, + ssl_ca_file=self.internal_ssl_ca, ) if self.hub_bind_url: # ensure hub_prefix is set on bind_url @@ -1436,6 +1439,9 @@ class JupyterHub(Application): port=port, cookie_name='jupyterhub-services', base_url=service.prefix, + ssl_cert_file=self.internal_ssl_cert, + ssl_key_file=self.internal_ssl_key, + ssl_ca_file=self.internal_ssl_ca, ) self.db.add(server) diff --git a/jupyterhub/objects.py b/jupyterhub/objects.py index 2ca3e73d..e5fa1ef6 100644 --- a/jupyterhub/objects.py +++ b/jupyterhub/objects.py @@ -15,7 +15,7 @@ from .traitlets import URLPrefix from . import orm from .utils import ( url_path_join, can_connect, wait_for_server, - wait_for_http_server, random_port, + wait_for_http_server, random_port, make_ssl_context, ) class Server(HasTraits): @@ -35,6 +35,9 @@ class Server(HasTraits): cookie_name = Unicode('') connect_url = Unicode('') bind_url = Unicode('') + ssl_cert_file = Unicode() + ssl_key_file = Unicode() + ssl_ca_file = Unicode() @default('bind_url') def bind_url_default(self): @@ -123,6 +126,9 @@ class Server(HasTraits): self.port = obj.port self.base_url = obj.base_url self.cookie_name = obj.cookie_name + self.ssl_cert_file = obj.ssl_cert_file + self.ssl_key_file = obj.ssl_key_file + self.ssl_ca_file = obj.ssl_ca_file # setter to pass through to the database @observe('ip', 'proto', 'port', 'base_url', 'cookie_name') @@ -158,8 +164,9 @@ class Server(HasTraits): ) - def wait_up(self, timeout=10, http=False, ssl_context=None): + def wait_up(self, timeout=10, http=False): """Wait for this server to come up""" + ssl_context = make_ssl_context(self.ssl_key_file, self.ssl_cert_file, cafile=self.ssl_ca_file) if http: return wait_for_http_server(self.url, timeout=timeout, ssl_context=ssl_context) else: diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index b90cd4fb..1b4c82d4 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -77,6 +77,9 @@ class Server(Base): port = Column(Integer, default=random_port) base_url = Column(Unicode(255), default='/') cookie_name = Column(Unicode(255), default='cookie') + ssl_cert_file = Column(Unicode(4096), default='') + ssl_key_file = Column(Unicode(4096), default='') + ssl_ca_file = Column(Unicode(4096), default='') def __repr__(self): return "" % (self.ip, self.port) From 25e6b31a5f0f6e7fc4e36f278d64ab6d2f8cc419 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:00:57 -0700 Subject: [PATCH 08/68] Only internal_ssl kwargs if internal_ssl is enabled --- jupyterhub/user.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 282b5445..adef80e9 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -215,12 +215,18 @@ class User: db=self.db, oauth_client_id=client_id, cookie_options = self.settings.get('cookie_options', {}), - internal_ssl=self.settings.get('internal_ssl'), - internal_certs_location=self.settings.get('internal_certs_location'), - internal_authority_name=self.settings.get('internal_authority_name'), - internal_notebook_authority_name=self.settings.get('internal_notebook_authority_name'), trusted_alt_names=self.settings.get('trusted_alt_names'), ) + + if self.settings.get('internal_ssl'): + ssl_kwargs = dict( + internal_ssl=self.settings.get('internal_ssl'), + internal_certs_location=self.settings.get('internal_certs_location'), + internal_authority_name=self.settings.get('internal_authority_name'), + internal_notebook_authority_name=self.settings.get('internal_notebook_authority_name'), + ) + spawn_kwargs.update(ssl_kwargs) + # update with kwargs. Mainly for testing. spawn_kwargs.update(kwargs) spawner = spawner_class(**spawn_kwargs) From a549edfd75a7068d44098a41eb5246ba9666e208 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:04 -0700 Subject: [PATCH 09/68] Testing internal ssl modifications --- jupyterhub/spawner.py | 47 +++++--- jupyterhub/tests/conftest.py | 10 ++ jupyterhub/tests/test_internal_ssl.py | 167 ++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 20 deletions(-) create mode 100644 jupyterhub/tests/test_internal_ssl.py diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 87a71caa..3b6ea077 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -687,31 +687,38 @@ class Spawner(LoggingConfigurable): "ca_file": internal_key_pair.ca_file, } - def move_certs(self, cert_files): + def move_certs(self, key_pair): """Takes dict of cert/ca file paths and moves, sets up proper ownership for them.""" - user = pwd.getpwnam(self.user.name) - uid = user.pw_uid - gid = user.pw_gid - home = user.pw_dir + key = key_pair['key_file'] + cert = key_pair['cert_file'] + ca = key_pair['ca_file'] - # Create dir for user's certs wherever we're starting - out_dir = "{home}/.jupyter".format(home=home) - shutil.rmtree(out_dir, ignore_errors=True) - os.makedirs(out_dir, 0o700, exist_ok=True) + try: + user = pwd.getpwnam(self.user.name) + uid = user.pw_uid + gid = user.pw_gid + home = user.pw_dir - # Move certs to users dir - shutil.move(cert_files['key_file'], out_dir) - shutil.move(cert_files['cert_file'], out_dir) - shutil.copy(cert_files['ca_file'], out_dir) + # Create dir for user's certs wherever we're starting + out_dir = "{home}/.jupyter/certs".format(home=home) + shutil.rmtree(out_dir, ignore_errors=True) + os.makedirs(out_dir, 0o700, exist_ok=True) - path_tmpl = "{out}/{name}.{ext}" - key = path_tmpl.format(out=out_dir, name=self.user.name, ext="key") - cert = path_tmpl.format(out=out_dir, name=self.user.name, ext="crt") - ca = path_tmpl.format(out=out_dir, name=self.internal_authority_name, ext="crt") + # Move certs to users dir + shutil.move(key_pair['key_file'], out_dir) + shutil.move(key_pair['cert_file'], out_dir) + shutil.copy(key_pair['ca_file'], out_dir) - # Set cert ownership to user - for f in [out_dir, key, cert, ca]: - shutil.chown(f, user=uid, group=gid) + path_tmpl = "{out}/{name}.{ext}" + key = path_tmpl.format(out=out_dir, name=self.user.name, ext="key") + cert = path_tmpl.format(out=out_dir, name=self.user.name, ext="crt") + ca = path_tmpl.format(out=out_dir, name=self.internal_authority_name, ext="crt") + + # Set cert ownership to user + for f in [out_dir, key, cert, ca]: + shutil.chown(f, user=uid, group=gid) + except KeyError: + self.log.debug("User {} not found on system, not moving certs".format(self.user.name)) return [key, cert, ca] diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 46f242e0..7928f196 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -116,6 +116,16 @@ def io_loop(request): request.addfinalizer(_close) return io_loop +@fixture(scope='module') +def app(request, io_loop): + """Mock a jupyterhub app for testing""" + ssl_enabled = getattr(request.module, "ssl_enabled", False) + mocked_app = MockHub.instance(log_level=logging.DEBUG, internal_ssl=ssl_enabled) + @gen.coroutine + def make_app(): + yield mocked_app.initialize([]) + yield mocked_app.start() + io_loop.run_sync(make_app) @fixture(autouse=True) def cleanup_after(request, io_loop): diff --git a/jupyterhub/tests/test_internal_ssl.py b/jupyterhub/tests/test_internal_ssl.py new file mode 100644 index 00000000..6f341194 --- /dev/null +++ b/jupyterhub/tests/test_internal_ssl.py @@ -0,0 +1,167 @@ +"""Tests for the SSL enabled REST API.""" + +from concurrent.futures import Future +import json +import time +import sys +from unittest import mock +from urllib.parse import urlparse, quote + +import pytest +from pytest import mark +import requests + +from tornado import gen + +import jupyterhub +from .. import orm +from ..user import User +from ..utils import url_path_join as ujoin +from . import mocking +from .mocking import public_host, public_url +from .utils import async_requests + +ssl_enabled = True + +def check_db_locks(func): + """Decorator that verifies no locks are held on database upon exit. + + This decorator for test functions verifies no locks are held on the + application's database upon exit by creating and dropping a dummy table. + + The decorator relies on an instance of JupyterHubApp being the first + argument to the decorated function. + + Example + ------- + + @check_db_locks + def api_request(app, *api_path, **kwargs): + + """ + def new_func(app, *args, **kwargs): + retval = func(app, *args, **kwargs) + + temp_session = app.session_factory() + temp_session.execute('CREATE TABLE dummy (foo INT)') + temp_session.execute('DROP TABLE dummy') + temp_session.close() + + return retval + + return new_func + +def find_user(db, name): + """Find user in database.""" + return db.query(orm.User).filter(orm.User.name == name).first() + +def add_user(db, app=None, **kwargs): + """Add a user to the database.""" + orm_user = find_user(db, name=kwargs.get('name')) + if orm_user is None: + orm_user = orm.User(**kwargs) + db.add(orm_user) + else: + for attr, value in kwargs.items(): + setattr(orm_user, attr, value) + db.commit() + if app: + user = app.users[orm_user.id] = User(orm_user, app.tornado_settings) + return user + else: + return orm_user + +def auth_header(db, name): + """Return header with user's API authorization token.""" + user = find_user(db, name) + if user is None: + user = add_user(db, name=name) + token = user.new_api_token() + return {'Authorization': 'token %s' % token} + +@check_db_locks +@gen.coroutine +def api_request(app, *api_path, **kwargs): + """Make an API request""" + base_url = app.hub.url + headers = kwargs.setdefault('headers', {}) + + if 'Authorization' not in headers and not kwargs.pop('noauth', False): + headers.update(auth_header(app.db, 'admin')) + + kwargs['cert'] = (app.internal_ssl_cert, app.internal_ssl_key) + kwargs['verify'] = app.internal_ssl_ca + + url = ujoin(base_url, 'api', *api_path) + method = kwargs.pop('method', 'get') + f = getattr(async_requests, method) + resp = yield f(url, **kwargs) + assert "frame-ancestors 'self'" in resp.headers['Content-Security-Policy'] + assert ujoin(app.hub.base_url, "security/csp-report") in resp.headers['Content-Security-Policy'] + assert 'http' not in resp.headers['Content-Security-Policy'] + return resp + +@mark.gen_test +def test_spawn(app): + db = app.db + name = 'wash' + user = add_user(db, app=app, name=name) + options = { + 's': ['value'], + 'i': 5, + } + before_servers = sorted(db.query(orm.Server), key=lambda s: s.url) + r = yield api_request(app, 'users', name, 'server', method='post', + data=json.dumps(options), + ) + assert r.status_code == 201 + assert 'pid' in user.orm_spawners[''].state + app_user = app.users[name] + assert app_user.spawner is not None + spawner = app_user.spawner + assert app_user.spawner.user_options == options + assert not app_user.spawner._spawn_pending + status = yield app_user.spawner.poll() + assert status is None + + assert spawner.server.base_url == ujoin(app.base_url, 'user/%s' % name) + '/' + url = public_url(app, user) + r = yield api_request(app, url) + assert r.status_code == 200 + assert r.text == spawner.server.base_url + + r = yield api_request(app, ujoin(url, 'args')) + assert r.status_code == 200 + argv = r.json() + assert '--port' in ' '.join(argv) + r = yield api_request(app, ujoin(url, 'env')) + env = r.json() + for expected in ['JUPYTERHUB_USER', 'JUPYTERHUB_BASE_URL', 'JUPYTERHUB_API_TOKEN']: + assert expected in env + if app.subdomain_host: + assert env['JUPYTERHUB_HOST'] == app.subdomain_host + + r = yield api_request(app, 'users', name, 'server', method='delete') + assert r.status_code == 204 + + assert 'pid' not in user.orm_spawners[''].state + status = yield app_user.spawner.poll() + assert status == 0 + + # check that we cleaned up after ourselves + assert spawner.server is None + after_servers = sorted(db.query(orm.Server), key=lambda s: s.url) + assert before_servers == after_servers + tokens = list(db.query(orm.APIToken).filter(orm.APIToken.user_id == user.id)) + assert tokens == [] + assert app.users.count_active_users()['pending'] == 0 + +@mark.gen_test +def test_root_api(app): + base_url = app.hub.url + r = yield api_request(app, '') + r.raise_for_status() + expected = { + 'version': jupyterhub.__version__ + } + assert r.json() == expected From 0304dd0040958301755313baa984c7f72c59bd45 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:10 -0700 Subject: [PATCH 10/68] Allow option to specify ssl_context in wait_up --- jupyterhub/objects.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyterhub/objects.py b/jupyterhub/objects.py index e5fa1ef6..97ac4a8d 100644 --- a/jupyterhub/objects.py +++ b/jupyterhub/objects.py @@ -164,9 +164,9 @@ class Server(HasTraits): ) - def wait_up(self, timeout=10, http=False): + def wait_up(self, timeout=10, http=False, ssl_context=None): """Wait for this server to come up""" - ssl_context = make_ssl_context(self.ssl_key_file, self.ssl_cert_file, cafile=self.ssl_ca_file) + ssl_context = ssl_context or make_ssl_context(self.ssl_key_file, self.ssl_cert_file, cafile=self.ssl_ca_file) if http: return wait_for_http_server(self.url, timeout=timeout, ssl_context=ssl_context) else: From 5c39325104ff2bf5cd69bcf7dd6d0e426700f564 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:14 -0700 Subject: [PATCH 11/68] Only import certipy if internal_ssl is turned on --- jupyterhub/app.py | 1 + jupyterhub/spawner.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 8b92d411..86a2ed8c 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1710,6 +1710,7 @@ class JupyterHub(Application): cfg.JupyterHub.merge(cfg.JupyterHubApp) self.update_config(cfg) if self.internal_ssl: + from certipy import Certipy cert_store = Certipy(store_dir=self.internal_certs_location) joint_ca_file = "{out}/combined-cas.crt".format(out=self.internal_certs_location) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 3b6ea077..40ef1d32 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -24,7 +24,6 @@ from async_generator import async_generator, yield_ from sqlalchemy import inspect from tornado.ioloop import PeriodicCallback -from certipy import Certipy from traitlets.config import LoggingConfigurable from traitlets import ( @@ -676,6 +675,7 @@ class Spawner(LoggingConfigurable): def create_certs(self): """Create the certs to be used for internal ssl.""" + from certipy import Certipy cert_store = Certipy(store_dir=self.internal_certs_location) internal_authority = self.internal_authority_name notebook_authority = self.internal_notebook_authority_name From 373c3f82dde073465c6c87b37f9021a7c80979e8 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:24 -0700 Subject: [PATCH 12/68] SSL setup for testing Setup general ssl request, not just to api Basic tests comprised of non-ssl test copies Create the context only when request is http Refactor ssl key, cert, ca names Configure the AsyncHTTPClient at app start Change tests to import existing ones with ssl on Override __new__ in MockHub to turn on SSL --- jupyterhub/app.py | 29 ++- jupyterhub/objects.py | 19 +- jupyterhub/orm.py | 6 +- jupyterhub/proxy.py | 9 - jupyterhub/spawner.py | 27 +-- jupyterhub/tests/conftest.py | 28 +-- jupyterhub/tests/mocking.py | 16 +- jupyterhub/tests/mocksu.py | 21 ++- jupyterhub/tests/test_app.py | 20 +-- jupyterhub/tests/test_internal_ssl.py | 167 ------------------ jupyterhub/tests/test_internal_ssl_api.py | 7 + jupyterhub/tests/test_internal_ssl_app.py | 10 ++ jupyterhub/tests/test_internal_ssl_spawner.py | 7 + jupyterhub/tests/utils.py | 9 + jupyterhub/user.py | 5 +- jupyterhub/utils.py | 6 +- 16 files changed, 153 insertions(+), 233 deletions(-) delete mode 100644 jupyterhub/tests/test_internal_ssl.py create mode 100644 jupyterhub/tests/test_internal_ssl_api.py create mode 100644 jupyterhub/tests/test_internal_ssl_app.py create mode 100644 jupyterhub/tests/test_internal_ssl_spawner.py diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 86a2ed8c..5822d605 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1148,9 +1148,9 @@ class JupyterHub(Application): hub_args = dict( base_url=self.hub_prefix, public_host=self.subdomain_host, - ssl_cert_file=self.internal_ssl_cert, - ssl_key_file=self.internal_ssl_key, - ssl_ca_file=self.internal_ssl_ca, + certfile=self.internal_ssl_cert, + keyfile=self.internal_ssl_key, + cafile=self.internal_ssl_ca, ) if self.hub_bind_url: # ensure hub_prefix is set on bind_url @@ -1439,9 +1439,9 @@ class JupyterHub(Application): port=port, cookie_name='jupyterhub-services', base_url=service.prefix, - ssl_cert_file=self.internal_ssl_cert, - ssl_key_file=self.internal_ssl_key, - ssl_ca_file=self.internal_ssl_ca, + certfile=self.internal_ssl_cert, + keyfile=self.internal_ssl_key, + cafile=self.internal_ssl_ca, ) self.db.add(server) @@ -1732,18 +1732,29 @@ class JupyterHub(Application): extra_names = [socket.getfqdn()] + self.trusted_alt_names extra_names = ','.join(["DNS:{}".format(name) for name in extra_names]) alt_names = alt_names.format(extra_names=extra_names).encode() - internal_key_pair = cert_store.create_signed_pair("localhost", self.internal_authority_name, alt_names=alt_names) + internal_key_pair = cert_store.create_signed_pair( + "localhost", + self.internal_authority_name, + alt_names=alt_names) # Join CA files with open(internal_key_pair.ca_file) as internal_ca, \ - open(notebook_authority.ca_file) as notebook_ca, \ - open(joint_ca_file, 'w') as combined_ca: + open(notebook_authority.ca_file) as notebook_ca, \ + open(joint_ca_file, 'w') as combined_ca: combined_ca.write(internal_ca.read()) combined_ca.write(notebook_ca.read()) self.internal_ssl_key = internal_key_pair.key_file self.internal_ssl_cert = internal_key_pair.cert_file self.internal_ssl_ca = joint_ca_file + + # Configure the AsyncHTTPClient + ssl_context = make_ssl_context( + self.internal_ssl_key, + self.internal_ssl_cert, + cafile=self.internal_ssl_ca, + ) + AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) self.write_pid_file() def _log_cls(name, cls): diff --git a/jupyterhub/objects.py b/jupyterhub/objects.py index 97ac4a8d..d7816c78 100644 --- a/jupyterhub/objects.py +++ b/jupyterhub/objects.py @@ -35,9 +35,9 @@ class Server(HasTraits): cookie_name = Unicode('') connect_url = Unicode('') bind_url = Unicode('') - ssl_cert_file = Unicode() - ssl_key_file = Unicode() - ssl_ca_file = Unicode() + certfile = Unicode() + keyfile = Unicode() + cafile = Unicode() @default('bind_url') def bind_url_default(self): @@ -126,9 +126,9 @@ class Server(HasTraits): self.port = obj.port self.base_url = obj.base_url self.cookie_name = obj.cookie_name - self.ssl_cert_file = obj.ssl_cert_file - self.ssl_key_file = obj.ssl_key_file - self.ssl_ca_file = obj.ssl_ca_file + self.certfile = obj.certfile + self.keyfile = obj.keyfile + self.cafile = obj.cafile # setter to pass through to the database @observe('ip', 'proto', 'port', 'base_url', 'cookie_name') @@ -166,9 +166,12 @@ class Server(HasTraits): def wait_up(self, timeout=10, http=False, ssl_context=None): """Wait for this server to come up""" - ssl_context = ssl_context or make_ssl_context(self.ssl_key_file, self.ssl_cert_file, cafile=self.ssl_ca_file) if http: - return wait_for_http_server(self.url, timeout=timeout, ssl_context=ssl_context) + ssl_context = ssl_context or make_ssl_context( + self.keyfile, self.certfile, cafile=self.cafile) + + return wait_for_http_server( + self.url, timeout=timeout, ssl_context=ssl_context) else: return wait_for_server(self._connect_ip, self._connect_port, timeout=timeout) diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 1b4c82d4..c7d61c02 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -77,9 +77,9 @@ class Server(Base): port = Column(Integer, default=random_port) base_url = Column(Unicode(255), default='/') cookie_name = Column(Unicode(255), default='cookie') - ssl_cert_file = Column(Unicode(4096), default='') - ssl_key_file = Column(Unicode(4096), default='') - ssl_ca_file = Column(Unicode(4096), default='') + certfile = Column(Unicode(4096), default='') + keyfile = Column(Unicode(4096), default='') + cafile = Column(Unicode(4096), default='') def __repr__(self): return "" % (self.ip, self.port) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index aefef9fb..9b6a263f 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -391,15 +391,6 @@ class ConfigurableHTTPProxy(Proxy): c.ConfigurableHTTPProxy.should_start = False """ - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - ssl_context = make_ssl_context( - self.app.internal_ssl_key, - self.app.internal_ssl_cert, - cafile=self.app.internal_ssl_ca, - ) - AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) - proxy_process = Any() client = Instance(AsyncHTTPClient, ()) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 40ef1d32..d8d53e07 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -680,18 +680,23 @@ class Spawner(LoggingConfigurable): internal_authority = self.internal_authority_name notebook_authority = self.internal_notebook_authority_name internal_key_pair = cert_store.get(internal_authority) - notebook_key_pair = cert_store.create_signed_pair(self.user.name, notebook_authority, alt_names=b"DNS:localhost,IP:127.0.0.1") + notebook_key_pair = cert_store.create_signed_pair( + self.user.name, + notebook_authority, + alt_names=b"DNS:localhost,IP:127.0.0.1") return { - "key_file": notebook_key_pair.key_file, - "cert_file": notebook_key_pair.cert_file, - "ca_file": internal_key_pair.ca_file, + "keyfile": notebook_key_pair.key_file, + "certfile": notebook_key_pair.cert_file, + "cafile": internal_key_pair.ca_file, } def move_certs(self, key_pair): - """Takes dict of cert/ca file paths and moves, sets up proper ownership for them.""" - key = key_pair['key_file'] - cert = key_pair['cert_file'] - ca = key_pair['ca_file'] + """Takes dict of cert/ca file paths and moves, sets up proper ownership + for them. + """ + key = key_pair['keyfile'] + cert = key_pair['certfile'] + ca = key_pair['cafile'] try: user = pwd.getpwnam(self.user.name) @@ -705,9 +710,9 @@ class Spawner(LoggingConfigurable): os.makedirs(out_dir, 0o700, exist_ok=True) # Move certs to users dir - shutil.move(key_pair['key_file'], out_dir) - shutil.move(key_pair['cert_file'], out_dir) - shutil.copy(key_pair['ca_file'], out_dir) + shutil.move(key_pair['keyfile'], out_dir) + shutil.move(key_pair['certfile'], out_dir) + shutil.copy(key_pair['cafile'], out_dir) path_tmpl = "{out}/{name}.{ext}" key = path_tmpl.format(out=out_dir, name=self.user.name, ext="key") diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 7928f196..9e11d7c0 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -42,6 +42,7 @@ from ..utils import random_port from . import mocking from .mocking import MockHub +from .utils import ssl_setup from .test_services import mockservice_cmd import jupyterhub.services.service @@ -50,10 +51,25 @@ import jupyterhub.services.service _db = None +@fixture(scope='session') +def ssl_tmpdir(tmpdir_factory): + return tmpdir_factory.mktemp('ssl') + + @fixture(scope='module') -def app(request, io_loop): +def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" mocked_app = MockHub.instance(log_level=logging.DEBUG) + ssl_enabled = getattr(request.module, "ssl_enabled", False) + + if ssl_enabled: + internal_authority_name = 'hub' + external_certs = ssl_setup(str(ssl_tmpdir), internal_authority_name) + mocked_app = MockHub.instance( + log_level=logging.DEBUG, + internal_ssl=True, + internal_authority_name=internal_authority_name, + internal_certs_location=str(ssl_tmpdir)) @gen.coroutine def make_app(): @@ -116,16 +132,6 @@ def io_loop(request): request.addfinalizer(_close) return io_loop -@fixture(scope='module') -def app(request, io_loop): - """Mock a jupyterhub app for testing""" - ssl_enabled = getattr(request.module, "ssl_enabled", False) - mocked_app = MockHub.instance(log_level=logging.DEBUG, internal_ssl=ssl_enabled) - @gen.coroutine - def make_app(): - yield mocked_app.initialize([]) - yield mocked_app.start() - io_loop.run_sync(make_app) @fixture(autouse=True) def cleanup_after(request, io_loop): diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 792779e5..c57b1e57 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -48,7 +48,7 @@ from ..objects import Server from ..spawner import LocalProcessSpawner from ..singleuser import SingleUserNotebookApp from ..utils import random_port, url_path_join -from .utils import async_requests +from .utils import async_requests, ssl_setup from pamela import PAMError @@ -216,6 +216,20 @@ class MockHub(JupyterHub): last_activity_interval = 2 log_datefmt = '%M:%S' + def __new__(cls, *args, **kwargs): + try: + # Turn on internalSSL if the options exist + internal_authority_name = 'hub' + cert_location = kwargs['internal_certs_location'] + external_certs = ssl_setup(cert_location, internal_authority_name) + kwargs['internal_ssl'] = True + kwargs['internal_authority_name'] = internal_authority_name + kwargs['ssl_cert'] = external_certs.cert_file + kwargs['ssl_key'] = external_certs.key_file + except KeyError: + pass + return super().__new__(cls, *args, **kwargs) + @default('subdomain_host') def _subdomain_host_default(self): return os.environ.get('JUPYTERHUB_TEST_SUBDOMAIN_HOST', '') diff --git a/jupyterhub/tests/mocksu.py b/jupyterhub/tests/mocksu.py index 817e823d..b8c9be05 100644 --- a/jupyterhub/tests/mocksu.py +++ b/jupyterhub/tests/mocksu.py @@ -14,9 +14,11 @@ Handlers and their purpose include: import argparse import json import sys +import os from tornado import web, httpserver, ioloop from .mockservice import EnvHandler +from ..utils import make_ssl_context class EchoHandler(web.RequestHandler): def get(self): @@ -34,7 +36,19 @@ def main(args): (r'.*', EchoHandler), ]) - server = httpserver.HTTPServer(app) + ssl_context = None + if args.keyfile and args.certfile and args.client_ca: + key = args.keyfile.strip('"') + cert = args.certfile.strip('"') + ca = args.client_ca.strip('"') + + ssl_context = make_ssl_context( + key, + cert, + cafile = ca, + check_hostname = False) + + server = httpserver.HTTPServer(app, ssl_options=ssl_context) server.listen(args.port) try: ioloop.IOLoop.instance().start() @@ -44,5 +58,8 @@ def main(args): if __name__ == '__main__': parser = argparse.ArgumentParser() parser.add_argument('--port', type=int) + parser.add_argument('--keyfile', type=str) + parser.add_argument('--certfile', type=str) + parser.add_argument('--client-ca', type=str) args, extra = parser.parse_known_args() - main(args) \ No newline at end of file + main(args) diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 1a220219..86ce82f9 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -72,7 +72,7 @@ def test_init_tokens(): 'also-super-secret': 'gordon', 'boagasdfasdf': 'chell', } - app = MockHub(db_url=db_file, api_tokens=tokens) + app = MockHub(db_url=db_file, api_tokens=tokens, internal_certs_location=td) yield app.initialize([]) db = app.db for token, username in tokens.items(): @@ -82,7 +82,7 @@ def test_init_tokens(): assert user.name == username # simulate second startup, reloading same tokens: - app = MockHub(db_url=db_file, api_tokens=tokens) + app = MockHub(db_url=db_file, api_tokens=tokens, internal_certs_location=td) yield app.initialize([]) db = app.db for token, username in tokens.items(): @@ -93,7 +93,7 @@ def test_init_tokens(): # don't allow failed token insertion to create users: tokens['short'] = 'gman' - app = MockHub(db_url=db_file, api_tokens=tokens) + app = MockHub(db_url=db_file, api_tokens=tokens, internal_certs_location=td) with pytest.raises(ValueError): yield app.initialize([]) assert orm.User.find(app.db, 'gman') is None @@ -101,7 +101,7 @@ def test_init_tokens(): def test_write_cookie_secret(tmpdir): secret_path = str(tmpdir.join('cookie_secret')) - hub = MockHub(cookie_secret_file=secret_path) + hub = MockHub(cookie_secret_file=secret_path, internal_certs_location=str(tmpdir)) hub.init_secrets() assert os.path.exists(secret_path) assert os.stat(secret_path).st_mode & 0o600 @@ -113,7 +113,7 @@ def test_cookie_secret_permissions(tmpdir): secret_path = str(secret_file) secret = os.urandom(COOKIE_SECRET_BYTES) secret_file.write(binascii.b2a_hex(secret)) - hub = MockHub(cookie_secret_file=secret_path) + hub = MockHub(cookie_secret_file=secret_path, internal_certs_location=str(tmpdir)) # raise with public secret file os.chmod(secret_path, 0o664) @@ -131,13 +131,13 @@ def test_cookie_secret_content(tmpdir): secret_file.write('not base 64: uñiço∂e') secret_path = str(secret_file) os.chmod(secret_path, 0o660) - hub = MockHub(cookie_secret_file=secret_path) + hub = MockHub(cookie_secret_file=secret_path, internal_certs_location=str(tmpdir)) with pytest.raises(SystemExit): hub.init_secrets() def test_cookie_secret_env(tmpdir): - hub = MockHub(cookie_secret_file=str(tmpdir.join('cookie_secret'))) + hub = MockHub(cookie_secret_file=str(tmpdir.join('cookie_secret')), internal_certs_location=str(tmpdir)) with patch.dict(os.environ, {'JPY_COOKIE_SECRET': 'not hex'}): with pytest.raises(ValueError): @@ -150,12 +150,12 @@ def test_cookie_secret_env(tmpdir): @pytest.mark.gen_test -def test_load_groups(): +def test_load_groups(tmpdir): to_load = { 'blue': ['cyclops', 'rogue', 'wolverine'], 'gold': ['storm', 'jean-grey', 'colossus'], } - hub = MockHub(load_groups=to_load) + hub = MockHub(load_groups=to_load, internal_certs_location=str(tmpdir)) hub.init_db() yield hub.init_users() yield hub.init_groups() @@ -178,7 +178,7 @@ def test_resume_spawners(tmpdir, request): request.addfinalizer(p.stop) @gen.coroutine def new_hub(): - app = MockHub() + app = MockHub(internal_certs_location=str(tmpdir)) app.config.ConfigurableHTTPProxy.should_start = False app.config.ConfigurableHTTPProxy.auth_token = 'unused' yield app.initialize([]) diff --git a/jupyterhub/tests/test_internal_ssl.py b/jupyterhub/tests/test_internal_ssl.py deleted file mode 100644 index 6f341194..00000000 --- a/jupyterhub/tests/test_internal_ssl.py +++ /dev/null @@ -1,167 +0,0 @@ -"""Tests for the SSL enabled REST API.""" - -from concurrent.futures import Future -import json -import time -import sys -from unittest import mock -from urllib.parse import urlparse, quote - -import pytest -from pytest import mark -import requests - -from tornado import gen - -import jupyterhub -from .. import orm -from ..user import User -from ..utils import url_path_join as ujoin -from . import mocking -from .mocking import public_host, public_url -from .utils import async_requests - -ssl_enabled = True - -def check_db_locks(func): - """Decorator that verifies no locks are held on database upon exit. - - This decorator for test functions verifies no locks are held on the - application's database upon exit by creating and dropping a dummy table. - - The decorator relies on an instance of JupyterHubApp being the first - argument to the decorated function. - - Example - ------- - - @check_db_locks - def api_request(app, *api_path, **kwargs): - - """ - def new_func(app, *args, **kwargs): - retval = func(app, *args, **kwargs) - - temp_session = app.session_factory() - temp_session.execute('CREATE TABLE dummy (foo INT)') - temp_session.execute('DROP TABLE dummy') - temp_session.close() - - return retval - - return new_func - -def find_user(db, name): - """Find user in database.""" - return db.query(orm.User).filter(orm.User.name == name).first() - -def add_user(db, app=None, **kwargs): - """Add a user to the database.""" - orm_user = find_user(db, name=kwargs.get('name')) - if orm_user is None: - orm_user = orm.User(**kwargs) - db.add(orm_user) - else: - for attr, value in kwargs.items(): - setattr(orm_user, attr, value) - db.commit() - if app: - user = app.users[orm_user.id] = User(orm_user, app.tornado_settings) - return user - else: - return orm_user - -def auth_header(db, name): - """Return header with user's API authorization token.""" - user = find_user(db, name) - if user is None: - user = add_user(db, name=name) - token = user.new_api_token() - return {'Authorization': 'token %s' % token} - -@check_db_locks -@gen.coroutine -def api_request(app, *api_path, **kwargs): - """Make an API request""" - base_url = app.hub.url - headers = kwargs.setdefault('headers', {}) - - if 'Authorization' not in headers and not kwargs.pop('noauth', False): - headers.update(auth_header(app.db, 'admin')) - - kwargs['cert'] = (app.internal_ssl_cert, app.internal_ssl_key) - kwargs['verify'] = app.internal_ssl_ca - - url = ujoin(base_url, 'api', *api_path) - method = kwargs.pop('method', 'get') - f = getattr(async_requests, method) - resp = yield f(url, **kwargs) - assert "frame-ancestors 'self'" in resp.headers['Content-Security-Policy'] - assert ujoin(app.hub.base_url, "security/csp-report") in resp.headers['Content-Security-Policy'] - assert 'http' not in resp.headers['Content-Security-Policy'] - return resp - -@mark.gen_test -def test_spawn(app): - db = app.db - name = 'wash' - user = add_user(db, app=app, name=name) - options = { - 's': ['value'], - 'i': 5, - } - before_servers = sorted(db.query(orm.Server), key=lambda s: s.url) - r = yield api_request(app, 'users', name, 'server', method='post', - data=json.dumps(options), - ) - assert r.status_code == 201 - assert 'pid' in user.orm_spawners[''].state - app_user = app.users[name] - assert app_user.spawner is not None - spawner = app_user.spawner - assert app_user.spawner.user_options == options - assert not app_user.spawner._spawn_pending - status = yield app_user.spawner.poll() - assert status is None - - assert spawner.server.base_url == ujoin(app.base_url, 'user/%s' % name) + '/' - url = public_url(app, user) - r = yield api_request(app, url) - assert r.status_code == 200 - assert r.text == spawner.server.base_url - - r = yield api_request(app, ujoin(url, 'args')) - assert r.status_code == 200 - argv = r.json() - assert '--port' in ' '.join(argv) - r = yield api_request(app, ujoin(url, 'env')) - env = r.json() - for expected in ['JUPYTERHUB_USER', 'JUPYTERHUB_BASE_URL', 'JUPYTERHUB_API_TOKEN']: - assert expected in env - if app.subdomain_host: - assert env['JUPYTERHUB_HOST'] == app.subdomain_host - - r = yield api_request(app, 'users', name, 'server', method='delete') - assert r.status_code == 204 - - assert 'pid' not in user.orm_spawners[''].state - status = yield app_user.spawner.poll() - assert status == 0 - - # check that we cleaned up after ourselves - assert spawner.server is None - after_servers = sorted(db.query(orm.Server), key=lambda s: s.url) - assert before_servers == after_servers - tokens = list(db.query(orm.APIToken).filter(orm.APIToken.user_id == user.id)) - assert tokens == [] - assert app.users.count_active_users()['pending'] == 0 - -@mark.gen_test -def test_root_api(app): - base_url = app.hub.url - r = yield api_request(app, '') - r.raise_for_status() - expected = { - 'version': jupyterhub.__version__ - } - assert r.json() == expected diff --git a/jupyterhub/tests/test_internal_ssl_api.py b/jupyterhub/tests/test_internal_ssl_api.py new file mode 100644 index 00000000..5c9e4026 --- /dev/null +++ b/jupyterhub/tests/test_internal_ssl_api.py @@ -0,0 +1,7 @@ +"""Tests for the SSL enabled REST API.""" + +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + +from jupyterhub.tests.test_api import * +ssl_enabled = True diff --git a/jupyterhub/tests/test_internal_ssl_app.py b/jupyterhub/tests/test_internal_ssl_app.py new file mode 100644 index 00000000..bef84d99 --- /dev/null +++ b/jupyterhub/tests/test_internal_ssl_app.py @@ -0,0 +1,10 @@ +"""Test the JupyterHub entry point with internal ssl""" + +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + +import sys +import jupyterhub.tests.mocking +from .utils import ssl_setup + +from jupyterhub.tests.test_app import * diff --git a/jupyterhub/tests/test_internal_ssl_spawner.py b/jupyterhub/tests/test_internal_ssl_spawner.py new file mode 100644 index 00000000..b45a0a0e --- /dev/null +++ b/jupyterhub/tests/test_internal_ssl_spawner.py @@ -0,0 +1,7 @@ +"""Tests for process spawning with internal_ssl""" + +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + +from jupyterhub.tests.test_spawner import * +ssl_enabled = True diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py index 58d9222a..ea169bcf 100644 --- a/jupyterhub/tests/utils.py +++ b/jupyterhub/tests/utils.py @@ -1,6 +1,8 @@ from concurrent.futures import ThreadPoolExecutor import requests +from certipy import Certipy + class _AsyncRequests: """Wrapper around requests to return a Future from request methods @@ -16,3 +18,10 @@ class _AsyncRequests: # async_requests.get = requests.get returning a Future, etc. async_requests = _AsyncRequests() +def ssl_setup(cert_dir, authority_name): + # Set up the external certs with the same authority as the internal + # one so that certificate trust works regardless of chosen endpoint. + cert_store = Certipy(store_dir=cert_dir) + internal_authority = cert_store.create_ca(authority_name) + external_certs = cert_store.create_signed_pair('external', authority_name) + return external_certs diff --git a/jupyterhub/user.py b/jupyterhub/user.py index adef80e9..1d1815d2 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -508,7 +508,10 @@ class User: cert = self.settings['internal_ssl_cert'] ca = self.settings['internal_ssl_ca'] ssl_context = make_ssl_context(key, cert, cafile=ca) - resp = await server.wait_up(http=True, timeout=spawner.http_timeout, ssl_context=ssl_context) + resp = await server.wait_up( + http=True, + timeout=spawner.http_timeout, + ssl_context=ssl_context) except Exception as e: if isinstance(e, TimeoutError): self.log.warning( diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index 47bc7c5a..778cc357 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -72,7 +72,11 @@ def can_connect(ip, port): return True -def make_ssl_context(keyfile, certfile, cafile=None, verify=True, check_hostname=True): +def make_ssl_context( + keyfile, certfile, cafile=None, + verify=True, check_hostname=True): + """Setup context for starting an https server or making requests over ssl. + """ if not keyfile or not certfile: return None purpose = ssl.Purpose.SERVER_AUTH if verify else ssl.Purpose.CLIENT_AUTH From 01b27645fb92df4fa5264b03341bd8e426d1b228 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:31 -0700 Subject: [PATCH 13/68] Set http[s] as appropriate for the singleuser url --- jupyterhub/user.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 1d1815d2..6f42a983 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -422,7 +422,8 @@ class User: pass else: # >= 0.7 returns (ip, port) - url = 'http://%s:%i' % url + proto = 'https' if self.settings['internal_ssl'] else 'http' + url = '%s://%s:%i' % ((proto,) + url) urlinfo = urlparse(url) server.proto = urlinfo.scheme server.ip = urlinfo.hostname From fa3437c09a7642bd742e63dc904c8f89a95641f0 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:35 -0700 Subject: [PATCH 14/68] Add db migration for ssl changes to servers --- .../26fc487e2b43_ssl_info_into_server.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py diff --git a/jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py b/jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py new file mode 100644 index 00000000..08c77b3b --- /dev/null +++ b/jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py @@ -0,0 +1,28 @@ +"""SSL info into server + +Revision ID: 26fc487e2b43 +Revises: 896818069c98 +Create Date: 2018-05-17 10:33:52.022786 + +""" + +# revision identifiers, used by Alembic. +revision = '26fc487e2b43' +down_revision = '896818069c98' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('servers', sa.Column('certfile', sa.Unicode(4096))) + op.add_column('servers', sa.Column('keyfile', sa.Unicode(4096))) + op.add_column('servers', sa.Column('cafile', sa.Unicode(4096))) + + +def downgrade(): + op.drop_column('servers', 'certfile') + op.drop_column('servers', 'keyfile') + op.drop_column('servers', 'cafile') From 1fc75086aa8618be9543affb3ebbb252126002a2 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:41 -0700 Subject: [PATCH 15/68] Remove vague try-catch --- jupyterhub/spawner.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index d8d53e07..4c28c567 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -751,16 +751,12 @@ class Spawner(LoggingConfigurable): args.append('--NotebookApp.default_url="%s"' % default_url) if self.internal_ssl: - try: - key, cert, ca = self.move_certs(self.create_certs()) + key, cert, ca = self.move_certs(self.create_certs()) - args.append('--keyfile="%s"' % key) - args.append('--certfile="%s"' % cert) - if ca: - args.append('--client-ca="%s"' % ca) - except Exception as e: - print("Internal SSL, if enabled, will not work.") - raise + args.append('--keyfile="%s"' % key) + args.append('--certfile="%s"' % cert) + if ca: + args.append('--client-ca="%s"' % ca) if self.debug: args.append('--debug') From 5de870be41accbfad21a9f5e2eeef1527797178d Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 7 Jun 2018 16:01:49 -0700 Subject: [PATCH 16/68] Fix docstring --- jupyterhub/spawner.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 4c28c567..f5cd0838 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -691,9 +691,7 @@ class Spawner(LoggingConfigurable): } def move_certs(self, key_pair): - """Takes dict of cert/ca file paths and moves, sets up proper ownership - for them. - """ + """Takes dict of cert paths, moves and sets ownership for them.""" key = key_pair['keyfile'] cert = key_pair['certfile'] ca = key_pair['cafile'] From d429433bb2fa61d8fce33fce747f0ec0e5b5f213 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Mon, 23 Jul 2018 13:41:34 -0700 Subject: [PATCH 17/68] Add Certipy to requirements now that its in PyPI --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 33ce10b8..5efd0a9d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,3 +9,4 @@ python-dateutil SQLAlchemy>=1.1 requests prometheus_client>=0.0.21 +certipy From 6000a84ffc7ec06904240935982ee0d09c4fbf3f Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 26 Jul 2018 14:29:58 -0700 Subject: [PATCH 18/68] Remove certs from the Server orm --- .../26fc487e2b43_ssl_info_into_server.py | 28 ------------------- jupyterhub/app.py | 3 -- jupyterhub/objects.py | 3 -- jupyterhub/orm.py | 3 -- 4 files changed, 37 deletions(-) delete mode 100644 jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py diff --git a/jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py b/jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py deleted file mode 100644 index 08c77b3b..00000000 --- a/jupyterhub/alembic/versions/26fc487e2b43_ssl_info_into_server.py +++ /dev/null @@ -1,28 +0,0 @@ -"""SSL info into server - -Revision ID: 26fc487e2b43 -Revises: 896818069c98 -Create Date: 2018-05-17 10:33:52.022786 - -""" - -# revision identifiers, used by Alembic. -revision = '26fc487e2b43' -down_revision = '896818069c98' -branch_labels = None -depends_on = None - -from alembic import op -import sqlalchemy as sa - - -def upgrade(): - op.add_column('servers', sa.Column('certfile', sa.Unicode(4096))) - op.add_column('servers', sa.Column('keyfile', sa.Unicode(4096))) - op.add_column('servers', sa.Column('cafile', sa.Unicode(4096))) - - -def downgrade(): - op.drop_column('servers', 'certfile') - op.drop_column('servers', 'keyfile') - op.drop_column('servers', 'cafile') diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 5822d605..3a817777 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1439,9 +1439,6 @@ class JupyterHub(Application): port=port, cookie_name='jupyterhub-services', base_url=service.prefix, - certfile=self.internal_ssl_cert, - keyfile=self.internal_ssl_key, - cafile=self.internal_ssl_ca, ) self.db.add(server) diff --git a/jupyterhub/objects.py b/jupyterhub/objects.py index d7816c78..7058246d 100644 --- a/jupyterhub/objects.py +++ b/jupyterhub/objects.py @@ -126,9 +126,6 @@ class Server(HasTraits): self.port = obj.port self.base_url = obj.base_url self.cookie_name = obj.cookie_name - self.certfile = obj.certfile - self.keyfile = obj.keyfile - self.cafile = obj.cafile # setter to pass through to the database @observe('ip', 'proto', 'port', 'base_url', 'cookie_name') diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index c7d61c02..b90cd4fb 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -77,9 +77,6 @@ class Server(Base): port = Column(Integer, default=random_port) base_url = Column(Unicode(255), default='/') cookie_name = Column(Unicode(255), default='cookie') - certfile = Column(Unicode(4096), default='') - keyfile = Column(Unicode(4096), default='') - cafile = Column(Unicode(4096), default='') def __repr__(self): return "" % (self.ip, self.port) From 3adbfe315e254bd2c9cfc55e3cbc83326adc53dc Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 26 Jul 2018 17:05:50 -0700 Subject: [PATCH 19/68] Pass certfile info via env instead of args --- jupyterhub/singleuser.py | 33 ++++++++++++--------------------- jupyterhub/spawner.py | 15 +++++++-------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/jupyterhub/singleuser.py b/jupyterhub/singleuser.py index 4ad27ad2..5048d948 100755 --- a/jupyterhub/singleuser.py +++ b/jupyterhub/singleuser.py @@ -237,27 +237,6 @@ class SingleUserNotebookApp(NotebookApp): def _default_group(self): return os.environ.get('JUPYTERHUB_GROUP') or '' - keyfile = Unicode('', - help="""The ssl key to use for requests - - Use with certfile - """ - ).tag(config=True) - - certfile = Unicode('', - help="""The ssl cert to use for requests - - Use with keyfile - """ - ).tag(config=True) - - client_ca = Unicode('', - help="""The ssl certificate authority to use to verify requests - - Use with keyfile and certfile - """ - ).tag(config=True) - @observe('user') def _user_changed(self, change): self.log.name = change.new @@ -266,6 +245,18 @@ class SingleUserNotebookApp(NotebookApp): hub_prefix = Unicode('/hub/').tag(config=True) + @default('keyfile') + def _keyfile_default(self): + return os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_KEYFILE') or '' + + @default('certfile') + def _certfile_default(self): + return os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CERTFILE') or '' + + @default('client_ca') + def _client_ca_default(self): + return os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA') or '' + @default('hub_prefix') def _hub_prefix_default(self): base_url = os.environ.get('JUPYTERHUB_BASE_URL') or '/' diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index f5cd0838..98a4cab8 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -633,6 +633,13 @@ class Spawner(LoggingConfigurable): if self.cpu_guarantee: env['CPU_GUARANTEE'] = str(self.cpu_guarantee) + if self.internal_ssl: + key, cert, ca = self.move_certs(self.create_certs()) + + env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = key + env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = cert + env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = ca + return env def template_namespace(self): @@ -748,14 +755,6 @@ class Spawner(LoggingConfigurable): default_url = self.format_string(self.default_url) args.append('--NotebookApp.default_url="%s"' % default_url) - if self.internal_ssl: - key, cert, ca = self.move_certs(self.create_certs()) - - args.append('--keyfile="%s"' % key) - args.append('--certfile="%s"' % cert) - if ca: - args.append('--client-ca="%s"' % ca) - if self.debug: args.append('--debug') if self.disable_user_config: From dd4df873b43eedbdd25bd0f8a867b5af2e8df2fe Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Fri, 27 Jul 2018 16:41:33 -0700 Subject: [PATCH 20/68] Move internal_ssl init into an init function --- jupyterhub/app.py | 95 ++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 46 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 3a817777..e21f930a 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1114,6 +1114,54 @@ class JupyterHub(Application): # store the loaded trait value self.cookie_secret = secret + def init_internal_ssl(self): + """Create the certs needed to turn on internal SSL""" + if self.internal_ssl: + from certipy import Certipy + cert_store = Certipy(store_dir=self.internal_certs_location) + joint_ca_name = 'combined-cas' + + # The authority for internal components (hub, proxy) + if not cert_store.get(self.internal_authority_name): + cert_store.create_ca(self.internal_authority_name) + + # The authority for individual notebooks + notebook_authority = cert_store.get(self.internal_notebook_authority_name) + if not notebook_authority: + notebook_authority = cert_store.create_ca(self.internal_notebook_authority_name) + + internal_key_pair = cert_store.get("localhost") + if not internal_key_pair: + import socket + alt_names = "IP:127.0.0.1,DNS:localhost,{extra_names}" + # In the event the hub needs to be accessed externally, add + # the fqdn and (optionally) rev_proxy to the set of alt_names. + extra_names = [socket.getfqdn()] + self.trusted_alt_names + extra_names = ','.join(["DNS:{}".format(name) for name in extra_names]) + alt_names = alt_names.format(extra_names=extra_names).encode() + internal_key_pair = cert_store.create_signed_pair( + "localhost", + self.internal_authority_name, + alt_names=alt_names) + + authorities = [self.internal_authority_name, + self.internal_notebook_authority_name] + joint_ca_file = cert_store.create_ca_bundle(authorities, + joint_ca_name) + + self.internal_ssl_key = internal_key_pair.key_file + self.internal_ssl_cert = internal_key_pair.cert_file + self.internal_ssl_ca = joint_ca_file + + # Configure the AsyncHTTPClient. This will affect anything using + # AsyncHTTPClient. + ssl_context = make_ssl_context( + self.internal_ssl_key, + self.internal_ssl_cert, + cafile=self.internal_ssl_ca, + ) + AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) + def init_db(self): """Create the database connection""" @@ -1706,52 +1754,6 @@ class JupyterHub(Application): cfg = self.config.copy() cfg.JupyterHub.merge(cfg.JupyterHubApp) self.update_config(cfg) - if self.internal_ssl: - from certipy import Certipy - cert_store = Certipy(store_dir=self.internal_certs_location) - joint_ca_file = "{out}/combined-cas.crt".format(out=self.internal_certs_location) - - # The authority for internal components (hub, proxy) - if not cert_store.get(self.internal_authority_name): - cert_store.create_ca(self.internal_authority_name) - - # The authority for individual notebooks - notebook_authority = cert_store.get(self.internal_notebook_authority_name) - if not notebook_authority: - notebook_authority = cert_store.create_ca(self.internal_notebook_authority_name) - - internal_key_pair = cert_store.get("localhost") - if not internal_key_pair: - import socket - alt_names = "IP:127.0.0.1,DNS:localhost,{extra_names}" - # In the event the hub needs to be accessed externally, add - # the fqdn and (optionally) rev_proxy to the set of alt_names. - extra_names = [socket.getfqdn()] + self.trusted_alt_names - extra_names = ','.join(["DNS:{}".format(name) for name in extra_names]) - alt_names = alt_names.format(extra_names=extra_names).encode() - internal_key_pair = cert_store.create_signed_pair( - "localhost", - self.internal_authority_name, - alt_names=alt_names) - - # Join CA files - with open(internal_key_pair.ca_file) as internal_ca, \ - open(notebook_authority.ca_file) as notebook_ca, \ - open(joint_ca_file, 'w') as combined_ca: - combined_ca.write(internal_ca.read()) - combined_ca.write(notebook_ca.read()) - - self.internal_ssl_key = internal_key_pair.key_file - self.internal_ssl_cert = internal_key_pair.cert_file - self.internal_ssl_ca = joint_ca_file - - # Configure the AsyncHTTPClient - ssl_context = make_ssl_context( - self.internal_ssl_key, - self.internal_ssl_cert, - cafile=self.internal_ssl_ca, - ) - AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) self.write_pid_file() def _log_cls(name, cls): @@ -1782,6 +1784,7 @@ class JupyterHub(Application): self.init_pycurl() self.init_secrets() + self.init_internal_ssl() self.init_db() self.init_hub() self.init_proxy() From e082b923e00307e0755d934f48fe28ce8c7e4ebe Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Fri, 27 Jul 2018 16:44:24 -0700 Subject: [PATCH 21/68] Clarify output directory name for user certs --- jupyterhub/spawner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 98a4cab8..cc47df42 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -710,7 +710,7 @@ class Spawner(LoggingConfigurable): home = user.pw_dir # Create dir for user's certs wherever we're starting - out_dir = "{home}/.jupyter/certs".format(home=home) + out_dir = "{home}/.jupyterhub/jupyterhub-certs".format(home=home) shutil.rmtree(out_dir, ignore_errors=True) os.makedirs(out_dir, 0o700, exist_ok=True) From 9607edcc23138b8cf6f8da1ea2326fbe2e577d19 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Fri, 27 Jul 2018 17:00:44 -0700 Subject: [PATCH 22/68] Return a dict instead of a tuple from move_certs --- jupyterhub/spawner.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index cc47df42..d1c55da9 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -634,11 +634,11 @@ class Spawner(LoggingConfigurable): env['CPU_GUARANTEE'] = str(self.cpu_guarantee) if self.internal_ssl: - key, cert, ca = self.move_certs(self.create_certs()) + files = self.move_certs(self.create_certs()) - env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = key - env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = cert - env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = ca + env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = files['keyfile'] + env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = files['certfile'] + env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = files['cafile'] return env @@ -730,7 +730,7 @@ class Spawner(LoggingConfigurable): except KeyError: self.log.debug("User {} not found on system, not moving certs".format(self.user.name)) - return [key, cert, ca] + return {"keyfile": key, "certfile": cert, "cafile": ca} def get_args(self): """Return the arguments to be passed after self.cmd From 3c0503348185e6cc2e7d3ebee1642fc6f71af185 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Tue, 4 Sep 2018 15:08:12 -0700 Subject: [PATCH 23/68] Update cert generation to use Certipy's new API To better accommodate external certificate management as well as building of trust, Certipy was refactored. This included general improvements to file and record handling. In the process, some of Certipy's APIs changed slightly, but should be more stable now going forward. --- jupyterhub/app.py | 94 ++++++++++++++++------- jupyterhub/spawner.py | 67 ++++++++++------ jupyterhub/tests/mocking.py | 4 +- jupyterhub/tests/test_internal_ssl_app.py | 1 - jupyterhub/tests/utils.py | 7 +- requirements.txt | 2 +- 6 files changed, 118 insertions(+), 57 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index e21f930a..65a01d6b 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -20,8 +20,6 @@ import sys from textwrap import dedent from urllib.parse import unquote, urlparse, urlunparse -from certipy import Certipy - if sys.version_info[:2] < (3, 3): raise ValueError("Python < 3.3 not supported: %s" % sys.version) @@ -327,6 +325,12 @@ class JupyterHub(Application): Use with internal_ssl """ ).tag(config=True) + recreate_internal_certs = Bool(False, + help=""" Recreate all certificates used within JupyterHub on restart. + + Use with internal_ssl + """ + ).tag(config=True) internal_authority_name = Unicode('hub', help=""" The name for the internal signing authority @@ -341,6 +345,20 @@ class JupyterHub(Application): Use with internal_ssl """ ).tag(config=True) + external_authorities = Dict( + help=""" A dict of common name to paths to external CA certificates. + + This option is useful when there exist certificates that need to be + trusted, but are outside of Certipy's control. For example, when + running a proxy in lieu of ConfigurableHTTPProxy, the signing CA that + it uses for SSL can be imported and its trust properly propagated. + + The dict uses common names for keys that map to the file path of the + certificate. Common names need to be unique. + + Use with internal_ssl + """ + ).tag(config=True) internal_ssl_key = Unicode('', help=""" The key to be used for internal ssl """ @@ -1117,40 +1135,60 @@ class JupyterHub(Application): def init_internal_ssl(self): """Create the certs needed to turn on internal SSL""" if self.internal_ssl: - from certipy import Certipy - cert_store = Certipy(store_dir=self.internal_certs_location) - joint_ca_name = 'combined-cas' + from certipy import Certipy, CertNotFoundError + certipy = Certipy(store_dir=self.internal_certs_location, + remove_existing=self.recreate_internal_certs) + joint_ca_name = "combined-cas.crt" # The authority for internal components (hub, proxy) - if not cert_store.get(self.internal_authority_name): - cert_store.create_ca(self.internal_authority_name) + try: + certipy.store.get_record(self.internal_authority_name) + except CertNotFoundError: + certipy.create_ca(self.internal_authority_name) # The authority for individual notebooks - notebook_authority = cert_store.get(self.internal_notebook_authority_name) - if not notebook_authority: - notebook_authority = cert_store.create_ca(self.internal_notebook_authority_name) + try: + authority_record = certipy.store.get_record( + self.internal_notebook_authority_name) + except CertNotFoundError: + authority_record = certipy.create_ca( + self.internal_notebook_authority_name) - internal_key_pair = cert_store.get("localhost") - if not internal_key_pair: + # The signed certs used by hub-internal components + try: + internal_key_pair = certipy.store.get_record("hub-internal") + except CertNotFoundError: import socket - alt_names = "IP:127.0.0.1,DNS:localhost,{extra_names}" + alt_names = ["IP:127.0.0.1", "DNS:localhost"] # In the event the hub needs to be accessed externally, add # the fqdn and (optionally) rev_proxy to the set of alt_names. - extra_names = [socket.getfqdn()] + self.trusted_alt_names - extra_names = ','.join(["DNS:{}".format(name) for name in extra_names]) - alt_names = alt_names.format(extra_names=extra_names).encode() - internal_key_pair = cert_store.create_signed_pair( - "localhost", - self.internal_authority_name, - alt_names=alt_names) + alt_names += (["DNS:" + socket.getfqdn()] + + self.trusted_alt_names) + internal_key_pair = certipy.create_signed_pair( + "hub-internal", + self.internal_authority_name, + alt_names=alt_names + ) - authorities = [self.internal_authority_name, - self.internal_notebook_authority_name] - joint_ca_file = cert_store.create_ca_bundle(authorities, - joint_ca_name) + external_authorities = [] + # Add provided, external authority info into Certipy. This will + # allow these certs to be added to the joint trust bundle. + for cn, file_path in self.external_authorities.items(): + certipy.store.add_record( + cn, files={"cert": file_path} + ) + external_authorities.append(cn) - self.internal_ssl_key = internal_key_pair.key_file - self.internal_ssl_cert = internal_key_pair.cert_file + authorities = ([self.internal_authority_name, + self.internal_notebook_authority_name] + + external_authorities) + + joint_ca_file = certipy.create_ca_bundle( + joint_ca_name, ca_names=authorities + ) + + self.internal_ssl_key = internal_key_pair['files']['key'] + self.internal_ssl_cert = internal_key_pair['files']['cert'] self.internal_ssl_ca = joint_ca_file # Configure the AsyncHTTPClient. This will affect anything using @@ -1160,7 +1198,9 @@ class JupyterHub(Application): self.internal_ssl_cert, cafile=self.internal_ssl_ca, ) - AsyncHTTPClient.configure(None, defaults={"ssl_options" : ssl_context}) + AsyncHTTPClient.configure( + None, defaults={"ssl_options" : ssl_context} + ) def init_db(self): """Create the database connection""" diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index d1c55da9..c867eba1 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -634,11 +634,11 @@ class Spawner(LoggingConfigurable): env['CPU_GUARANTEE'] = str(self.cpu_guarantee) if self.internal_ssl: - files = self.move_certs(self.create_certs()) + paths = self.move_certs(self.create_certs()) - env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = files['keyfile'] - env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = files['certfile'] - env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = files['cafile'] + env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = paths['keyfile'] + env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = paths['certfile'] + env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = paths['cafile'] return env @@ -680,28 +680,48 @@ class Spawner(LoggingConfigurable): """ return s.format(**self.template_namespace()) - def create_certs(self): + def create_certs(self, alt_names=None, override=False): """Create the certs to be used for internal ssl.""" from certipy import Certipy - cert_store = Certipy(store_dir=self.internal_certs_location) + default_names = ["DNS:localhost", "IP:127.0.0.1"] + alt_names = alt_names or [] + + if not override: + alt_names = default_names + alt_names + + certipy = Certipy(store_dir=self.internal_certs_location) internal_authority = self.internal_authority_name notebook_authority = self.internal_notebook_authority_name - internal_key_pair = cert_store.get(internal_authority) - notebook_key_pair = cert_store.create_signed_pair( - self.user.name, - notebook_authority, - alt_names=b"DNS:localhost,IP:127.0.0.1") - return { - "keyfile": notebook_key_pair.key_file, - "certfile": notebook_key_pair.cert_file, - "cafile": internal_key_pair.ca_file, + internal_key_pair = certipy.store.get_record(internal_authority) + notebook_key_pair = certipy.create_signed_pair( + self.user.name, + notebook_authority, + alt_names=alt_names, + overwrite=True + ) + paths = { + "keyfile": notebook_key_pair['files']['key'], + "certfile": notebook_key_pair['files']['cert'], + "cafile": internal_key_pair['files']['cert'], } - def move_certs(self, key_pair): """Takes dict of cert paths, moves and sets ownership for them.""" - key = key_pair['keyfile'] - cert = key_pair['certfile'] - ca = key_pair['cafile'] + try: + user = pwd.getpwnam(self.user.name) + uid = user.pw_uid + gid = user.pw_gid + for f in ['keyfile', 'certfile']: + shutil.chown(paths[f], user=uid, group=gid) + except KeyError: + self.log.info("User {} not found on system, " + "unable to change ownership".format(self.user.name)) + + return paths + + def move_certs(self, paths): + key = paths['keyfile'] + cert = paths['certfile'] + ca = paths['cafile'] try: user = pwd.getpwnam(self.user.name) @@ -715,9 +735,9 @@ class Spawner(LoggingConfigurable): os.makedirs(out_dir, 0o700, exist_ok=True) # Move certs to users dir - shutil.move(key_pair['keyfile'], out_dir) - shutil.move(key_pair['certfile'], out_dir) - shutil.copy(key_pair['cafile'], out_dir) + shutil.move(paths['keyfile'], out_dir) + shutil.move(paths['certfile'], out_dir) + shutil.copy(paths['cafile'], out_dir) path_tmpl = "{out}/{name}.{ext}" key = path_tmpl.format(out=out_dir, name=self.user.name, ext="key") @@ -728,7 +748,8 @@ class Spawner(LoggingConfigurable): for f in [out_dir, key, cert, ca]: shutil.chown(f, user=uid, group=gid) except KeyError: - self.log.debug("User {} not found on system, not moving certs".format(self.user.name)) + self.log.info("User {} not found on system, " + "not moving certs".format(self.user.name)) return {"keyfile": key, "certfile": cert, "cafile": ca} diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index c57b1e57..db3f6472 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -224,8 +224,8 @@ class MockHub(JupyterHub): external_certs = ssl_setup(cert_location, internal_authority_name) kwargs['internal_ssl'] = True kwargs['internal_authority_name'] = internal_authority_name - kwargs['ssl_cert'] = external_certs.cert_file - kwargs['ssl_key'] = external_certs.key_file + kwargs['ssl_cert'] = external_certs['files']['cert'] + kwargs['ssl_key'] = external_certs['files']['key'] except KeyError: pass return super().__new__(cls, *args, **kwargs) diff --git a/jupyterhub/tests/test_internal_ssl_app.py b/jupyterhub/tests/test_internal_ssl_app.py index bef84d99..47cfeb1f 100644 --- a/jupyterhub/tests/test_internal_ssl_app.py +++ b/jupyterhub/tests/test_internal_ssl_app.py @@ -5,6 +5,5 @@ import sys import jupyterhub.tests.mocking -from .utils import ssl_setup from jupyterhub.tests.test_app import * diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py index ea169bcf..fe8baa71 100644 --- a/jupyterhub/tests/utils.py +++ b/jupyterhub/tests/utils.py @@ -21,7 +21,8 @@ async_requests = _AsyncRequests() def ssl_setup(cert_dir, authority_name): # Set up the external certs with the same authority as the internal # one so that certificate trust works regardless of chosen endpoint. - cert_store = Certipy(store_dir=cert_dir) - internal_authority = cert_store.create_ca(authority_name) - external_certs = cert_store.create_signed_pair('external', authority_name) + certipy = Certipy(store_dir=cert_dir) + internal_authority = certipy.create_ca(authority_name, overwrite=True) + external_certs = certipy.create_signed_pair('external', authority_name, + overwrite=True) return external_certs diff --git a/requirements.txt b/requirements.txt index 5efd0a9d..92eb07ac 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,4 +9,4 @@ python-dateutil SQLAlchemy>=1.1 requests prometheus_client>=0.0.21 -certipy +certipy>=0.1.0 From 2a0e5d90e66ffa0c1bd22193acf85cc66744ffee Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Tue, 4 Sep 2018 15:22:49 -0700 Subject: [PATCH 24/68] Add the ability to generate JupyterHub's certificates This is used to be able to access JupyterHub's CA information and (manually) move it to components that need them (like externally managed proxies). --- jupyterhub/app.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 65a01d6b..32c0e50b 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -102,6 +102,8 @@ flags = { "set log level to logging.DEBUG (maximize logging output)"), 'generate-config': ({'JupyterHub': {'generate_config': True}}, "generate default config file"), + 'generate-certs': ({'JupyterHub': {'generate_certs': True}}, + "generate certificates used for internal ssl"), 'no-db': ({'JupyterHub': {'db_url': 'sqlite:///:memory:'}}, "disable persisting state database to disk" ), @@ -252,6 +254,9 @@ class JupyterHub(Application): generate_config = Bool(False, help="Generate default config file", ).tag(config=True) + generate_certs = Bool(False, + help="Generate certs used for internal ssl", + ).tag(config=True) answer_yes = Bool(False, help="Answer yes to any questions (e.g. confirm overwrite)" ).tag(config=True) @@ -1781,7 +1786,7 @@ class JupyterHub(Application): @catch_config_error async def initialize(self, *args, **kwargs): super().initialize(*args, **kwargs) - if self.generate_config or self.subapp: + if self.generate_config or self.generate_certs or self.subapp: return self.load_config_file(self.config_file) self.init_logging() @@ -1983,6 +1988,19 @@ class JupyterHub(Application): loop.stop() return + if self.generate_certs: + self.load_config_file(self.config_file) + if not self.internal_ssl: + self.log.warn("You'll need to enable `internal_ssl` " + "in the `jupyterhub_config` file to use " + "these certs.") + self.internal_ssl = True + self.init_internal_ssl() + self.log.info("Certificates written to directory `{}`".format( + self.internal_certs_location)) + loop.stop() + return + ssl_context = make_ssl_context( self.internal_ssl_key, self.internal_ssl_cert, From 84deb1fa7a848e81322c4f6383742c662f25e82e Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Tue, 4 Sep 2018 15:50:45 -0700 Subject: [PATCH 25/68] Update doc strings for create_certs and move_certs --- jupyterhub/spawner.py | 49 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index c867eba1..25d5b93b 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -681,7 +681,33 @@ class Spawner(LoggingConfigurable): return s.format(**self.template_namespace()) def create_certs(self, alt_names=None, override=False): - """Create the certs to be used for internal ssl.""" + """Create and set ownership for the certs to be used for internal ssl + + Keyword Arguments: + alt_names (list): a list of alternative names to identify the + server by, see: + https://en.wikipedia.org/wiki/Subject_Alternative_Name + + override: override the default_names with the provided alt_names + + Returns: + dict: Path to cert files and CA + + This method creates certs for use with the singleuser notebook. It + enables SSL and ensures that the notebook can perform bi-directional + SSL auth with the hub (verification based on CA). + + If the singleuser host has a name or ip other than localhost, + an appropriate alternative name(s) must be passed for ssl verification + by the hub to work. For example, for Jupyter hosts with an IP of + 10.10.10.10 or DNS name of jupyter.example.com, this would be: + + alt_names=["IP:10.10.10.10"] + alt_names=["DNS:jupyter.example.com"] + + respectively. The list can contain both the IP and DNS names to refer + to the host by either IP or DNS name (note the `default_names` below). + """ from certipy import Certipy default_names = ["DNS:localhost", "IP:127.0.0.1"] alt_names = alt_names or [] @@ -705,7 +731,6 @@ class Spawner(LoggingConfigurable): "cafile": internal_key_pair['files']['cert'], } - """Takes dict of cert paths, moves and sets ownership for them.""" try: user = pwd.getpwnam(self.user.name) uid = user.pw_uid @@ -719,6 +744,26 @@ class Spawner(LoggingConfigurable): return paths def move_certs(self, paths): + """Takes cert paths, moves and sets ownership for them + + Arguments: + paths (dict): a list of paths for key, cert, and CA + + Returns: + dict: a list (potentially altered) of paths for key, cert, + and CA + + `.move_certs` is called after certs for the singleuser notebook have + been created by create_certs. + + By default, certs are created in a standard, central location defined + by `internal_certs_location`. For a local, single-host deployment of + JupyterHub, this should suffice. If, however, singleuser notebooks + are spawned on other hosts, `.move_certs` should be overridden to move + these files appropriately. This could mean using `scp` to copy them + to another host, moving them to a volume mounted in a docker container, + or exporting them as a secret in kubernetes. + """ key = paths['keyfile'] cert = paths['certfile'] ca = paths['cafile'] From ca33ccd66ded93dda098bf07761c2068dfa24352 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Tue, 4 Sep 2018 15:51:26 -0700 Subject: [PATCH 26/68] Add longer internal_ssl documentation to main docs --- docs/source/reference/proxy.md | 9 +++++++++ docs/source/reference/spawners.md | 27 +++++++++++++++++++++++++++ docs/source/reference/websecurity.md | 17 +++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/docs/source/reference/proxy.md b/docs/source/reference/proxy.md index bd4bdf6f..550886ad 100644 --- a/docs/source/reference/proxy.md +++ b/docs/source/reference/proxy.md @@ -62,6 +62,15 @@ These methods **may** be coroutines. `c.Proxy.should_start` is a configurable flag that determines whether the Hub should call these methods when the Hub itself starts and stops. +## Encryption + +When using `internal_ssl` to encrypt traffic behind the proxy, at minimum, +your `Proxy` will need client ssl certificates which the `Hub` must be made +aware of. These can be provided to the hub via the `jupyterhub_config.py` file +by providing a `dict` of named paths to the `external_authorities` option. The +hub will include all certificates provided in that `dict` in the trust bundle +utilized by all internal components. + ### Purely external proxies Probably most custom proxies will be externally managed, diff --git a/docs/source/reference/spawners.md b/docs/source/reference/spawners.md index 40b50eb8..8d7f809e 100644 --- a/docs/source/reference/spawners.md +++ b/docs/source/reference/spawners.md @@ -223,3 +223,30 @@ in the single-user notebook server when a guarantee is being provided. **The spawner's underlying system or cluster is responsible for enforcing these limits and providing these guarantees.** If these values are set to `None`, no limits or guarantees are provided, and no environment values are set. + +### Encryption + +Communication between the `Proxy`, `Hub`, and `Notebook` can be secured by +turning on `internal_ssl` in `jupyterhub_config.py`. For a custom spawner to +utilize these certs, there are two methods of interest on the base `Spawner` +class: `.create_certs` and `.move_certs`. + +The first method, `.create_certs` will sign a key-cert pair using an internally +trusted authority for notebooks. During this process, `.create_certs` can +apply `ip` and `dns` name information to the cert via an `alt_names` `kwarg`. +This is used for certificate authentication (verification). Without proper +verification, the `Notebook` will be unable to communicate with the `Hub` and +vice versa when `internal_ssl` is enabled. For example, given a deployment +using the `DockerSpawner` which will start containers with `ips` from the +`docker` subnet pool, the `DockerSpawner` would need to instead choose a +container `ip` prior to starting and pass that to `.create_certs` (TODO: edit). + +In general though, this method will not need to be changed and the default +`ip`/`dns` (localhost) info will suffice. + +When `.create_certs` is run, it will `.create_certs` in a default, central +location specified by `c.JupyterHub.internal_certs_location`. For `Spawners` +that need access to these certs elsewhere (i.e. on another host altogether), +the `.move_certs` method can be overridden to move the certs appropriately. +Again, using `DockerSpawner` as an example, this would entail moving certs +to a directory that will get mounted into the container this spawner starts. diff --git a/docs/source/reference/websecurity.md b/docs/source/reference/websecurity.md index 981fcecb..6c823bd2 100644 --- a/docs/source/reference/websecurity.md +++ b/docs/source/reference/websecurity.md @@ -99,6 +99,23 @@ single-user server, and not the environment(s) in which the user's kernel(s) may run. Installing additional packages in the kernel environment does not pose additional risk to the web application's security. +### Encrypt internal connections with SSL/TLS + +By default, all communication on the server, between the proxy, hub, and single +-user notebooks is performed unencrypted. Setting the `internal_ssl` flag in +`jupyterhub_config.py` secures the aforementioned routes. Turning this +feature on does require that the enabled `Spawner` can use the certificates +generated by the `Hub` (the default `LocalProcessSpawner` can, for instance). + +It is also important to note that this encryption **does not** (yet) cover the +`zmq tcp` sockets between the Notebook client and kernel. While users cannot +submit arbitrary commands to another user's kernel, they can bind to these +sockets and listen. When serving untrusted users, this eavesdropping can be +mitigated by setting `KernelManager.transport` to `ipc`. This applies standard +Unix permissions to the communication sockets thereby restricting +communication to the socket owner. The `internal_ssl` option will eventually +extend to securing the `tcp` sockets as well. + ## Security audits We recommend that you do periodic reviews of your deployment's security. It's From 67f19a65b7b24cda3aa9ce40edd10b1df6fa206c Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Wed, 12 Sep 2018 17:46:39 -0700 Subject: [PATCH 27/68] Use Certipy's trust graph to set up internal_ssl With changes to CHP requiring a second, different authority, the complexity of managing trust within JupyterHub has risen. To solve this, Certipy now has a feature to specify what components should trust what and builds trust bundles accordingly. --- jupyterhub/app.py | 120 ++++++++++++++++++--------------- jupyterhub/proxy.py | 60 ++++++++++++++++- jupyterhub/services/service.py | 2 +- jupyterhub/spawner.py | 18 ++--- jupyterhub/tests/conftest.py | 4 +- jupyterhub/tests/mocking.py | 4 +- jupyterhub/user.py | 13 ++-- requirements.txt | 2 +- 8 files changed, 142 insertions(+), 81 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 32c0e50b..e2a237d2 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -336,34 +336,54 @@ class JupyterHub(Application): Use with internal_ssl """ ).tag(config=True) - internal_authority_name = Unicode('hub', - help=""" The name for the internal signing authority + external_ssl_authorities = Dict( + default_value={}, + help=""" Dict authority:dict(files). Specify the key, cert, and/or + ca file for an authority. This is useful for externally managed + proxies that wish to use internal_ssl. + + The files dict has this format (you must specify at least a cert): + + { + 'key': '/path/to/key.key', + 'cert': '/path/to/cert.crt', + 'ca': '/path/to/ca.crt' + } + + The authorities you can override: 'hub-ca', 'notebooks-ca', + 'proxy-api-ca', 'proxy-client-ca', and 'services-ca'. Use with internal_ssl """ ).tag(config=True) - internal_notebook_authority_name = Unicode('notebook', - help=""" The name for the notebook signing authority. - This authority is separate from internal_authority_name so that - individual notebooks do not trust each other, only the hub and proxy. + internal_ssl_authorities = Dict( + default_value={ + 'hub-ca': None, + 'notebooks-ca': None, + 'proxy-api-ca': None, + 'proxy-client-ca': None, + 'services-ca': None, + }, + help=""" Dict authority:dict(files). When creating the various + CAs needed for internal_ssl, these are the names that will be used + for each authority. Use with internal_ssl """ - ).tag(config=True) - external_authorities = Dict( - help=""" A dict of common name to paths to external CA certificates. - - This option is useful when there exist certificates that need to be - trusted, but are outside of Certipy's control. For example, when - running a proxy in lieu of ConfigurableHTTPProxy, the signing CA that - it uses for SSL can be imported and its trust properly propagated. - - The dict uses common names for keys that map to the file path of the - certificate. Common names need to be unique. + ) + internal_ssl_components_trust = Dict( + help=""" Dict component:list(components). This dict specifies the + relationships of components secured by internal_ssl. + """ + ) + internal_trust_bundles = Dict( + help=""" Dict component:path. These are the paths to the trust bundles + that each component should have. They will be set during + `init_internal_ssl`. Use with internal_ssl """ - ).tag(config=True) + ) internal_ssl_key = Unicode('', help=""" The key to be used for internal ssl """ @@ -1138,26 +1158,37 @@ class JupyterHub(Application): self.cookie_secret = secret def init_internal_ssl(self): - """Create the certs needed to turn on internal SSL""" + """Create the certs needed to turn on internal SSL. + + + """ + if self.internal_ssl: from certipy import Certipy, CertNotFoundError certipy = Certipy(store_dir=self.internal_certs_location, remove_existing=self.recreate_internal_certs) - joint_ca_name = "combined-cas.crt" - # The authority for internal components (hub, proxy) - try: - certipy.store.get_record(self.internal_authority_name) - except CertNotFoundError: - certipy.create_ca(self.internal_authority_name) + # Here we define how trust should be laid out per each component + self.internal_ssl_components_trust = { + 'hub-ca': list(self.internal_ssl_authorities.keys()), + 'proxy-api-ca': ['hub-ca', 'services-ca', 'notebooks-ca'], + 'proxy-client-ca': ['hub-ca', 'notebooks-ca'], + 'notebooks-ca': ['hub-ca', 'proxy-client-ca'], + 'services-ca': ['hub-ca', 'proxy-api-ca'], + } - # The authority for individual notebooks - try: - authority_record = certipy.store.get_record( - self.internal_notebook_authority_name) - except CertNotFoundError: - authority_record = certipy.create_ca( - self.internal_notebook_authority_name) + hub_name = 'hub-ca' + + # If any external CAs were specified in external_ssl_authorities + # add records of them to Certipy's store. + self.internal_ssl_authorities.update(self.external_ssl_authorities) + for authority, files in self.internal_ssl_authorities.items(): + if files: + certipy.store.add_record( + authority, is_ca=True, files=files) + + self.internal_trust_bundles = certipy.trust_from_graph( + self.internal_ssl_components_trust) # The signed certs used by hub-internal components try: @@ -1171,30 +1202,13 @@ class JupyterHub(Application): + self.trusted_alt_names) internal_key_pair = certipy.create_signed_pair( "hub-internal", - self.internal_authority_name, + hub_name, alt_names=alt_names ) - external_authorities = [] - # Add provided, external authority info into Certipy. This will - # allow these certs to be added to the joint trust bundle. - for cn, file_path in self.external_authorities.items(): - certipy.store.add_record( - cn, files={"cert": file_path} - ) - external_authorities.append(cn) - - authorities = ([self.internal_authority_name, - self.internal_notebook_authority_name] - + external_authorities) - - joint_ca_file = certipy.create_ca_bundle( - joint_ca_name, ca_names=authorities - ) - self.internal_ssl_key = internal_key_pair['files']['key'] self.internal_ssl_cert = internal_key_pair['files']['cert'] - self.internal_ssl_ca = joint_ca_file + self.internal_ssl_ca = self.internal_trust_bundles[hub_name] # Configure the AsyncHTTPClient. This will affect anything using # AsyncHTTPClient. @@ -1750,8 +1764,8 @@ class JupyterHub(Application): active_server_limit=self.active_server_limit, internal_ssl=self.internal_ssl, internal_certs_location=self.internal_certs_location, - internal_authority_name=self.internal_authority_name, - internal_notebook_authority_name=self.internal_notebook_authority_name, + internal_authorities=self.internal_ssl_authorities, + internal_trust_bundles=self.internal_trust_bundles, internal_ssl_key=self.internal_ssl_key, internal_ssl_cert=self.internal_ssl_cert, internal_ssl_ca=self.internal_ssl_ca, diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 9b6a263f..03a824ec 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -121,6 +121,44 @@ class Proxy(LoggingConfigurable): if the proxy is to be started by the Hub """ + def create_certs(self, name, ca_name, alt_names=None, override=False): + """Create certificates to be used with internal_ssl. + """ + + from certipy import Certipy, CertNotFoundError + default_names = ["DNS:localhost", "IP:127.0.0.1"] + alt_names = alt_names or [] + + if not override: + alt_names = default_names + alt_names + + certipy = Certipy(store_dir=self.app.internal_certs_location) + record = None + + try: + record = certipy.store.get_record(name) + except CertNotFoundError: + record = certipy.create_signed_pair( + name, + ca_name, + alt_names=alt_names, + overwrite=True + ) + + paths = { + "keyfile": record['files']['key'], + "certfile": record['files']['cert'], + "cafile": record['files']['cert'], + } + + return paths + + + def move_certs(self, paths): + """Move certificates created by create_certs. + """ + return paths + def validate_routespec(self, routespec): """Validate a routespec @@ -555,11 +593,27 @@ class ConfigurableHTTPProxy(Proxy): if self.ssl_cert: cmd.extend(['--ssl-cert', self.ssl_cert]) if self.app.internal_ssl: - cmd.extend(['--api-ssl-key', self.app.internal_ssl_key]) - cmd.extend(['--api-ssl-cert', self.app.internal_ssl_cert]) - cmd.extend(['--api-ssl-ca', self.app.internal_ssl_ca]) + certs = {} + trust_bundles = {} + proxy_api = 'proxy-api' + proxy_client = 'proxy-client' + for component in [proxy_api, proxy_client]: + ca_name = component + '-ca' + trust_bundles[component] = \ + self.app.internal_trust_bundles[ca_name] + certs[component] = self.move_certs( + self.create_certs(component, ca_name)) + cmd.extend(['--api-ssl-key', certs[proxy_api]['keyfile']]) + cmd.extend(['--api-ssl-cert', certs[proxy_api]['certfile']]) + cmd.extend(['--api-ssl-ca', trust_bundles[proxy_api]]) cmd.extend(['--api-ssl-request-cert']) cmd.extend(['--api-ssl-reject-unauthorized']) + + cmd.extend(['--client-ssl-key', certs[proxy_client]['keyfile']]) + cmd.extend(['--client-ssl-cert', certs[proxy_client]['certfile']]) + cmd.extend(['--client-ssl-ca', trust_bundles[proxy_client]]) + cmd.extend(['--client-ssl-request-cert']) + cmd.extend(['--client-ssl-reject-unauthorized']) if self.app.statsd_host: cmd.extend([ '--statsd-host', self.app.statsd_host, diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py index f4cab84d..9ddd4b45 100644 --- a/jupyterhub/services/service.py +++ b/jupyterhub/services/service.py @@ -334,7 +334,7 @@ class Service(LoggingConfigurable): ), internal_ssl=self.app.internal_ssl, internal_certs_location=self.app.internal_certs_location, - internal_authority_name=self.app.internal_authority_name, + internal_trust_bundles=self.app.internal_trust_bundles, ) self.spawner.start() self.proc = self.spawner.proc diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 25d5b93b..1116cb8c 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -162,9 +162,8 @@ class Spawner(LoggingConfigurable): hub = Any() authenticator = Any() internal_ssl = Bool(False) + internal_trust_bundles = Dict() internal_certs_location = Unicode('') - internal_authority_name = Unicode('') - internal_notebook_authority_name = Unicode('') admin_access = Bool(False) api_token = Unicode() oauth_client_id = Unicode() @@ -716,19 +715,17 @@ class Spawner(LoggingConfigurable): alt_names = default_names + alt_names certipy = Certipy(store_dir=self.internal_certs_location) - internal_authority = self.internal_authority_name - notebook_authority = self.internal_notebook_authority_name - internal_key_pair = certipy.store.get_record(internal_authority) + notebook_component = 'notebooks-ca' notebook_key_pair = certipy.create_signed_pair( self.user.name, - notebook_authority, + notebook_component, alt_names=alt_names, overwrite=True ) paths = { "keyfile": notebook_key_pair['files']['key'], "certfile": notebook_key_pair['files']['cert'], - "cafile": internal_key_pair['files']['cert'], + "cafile": self.internal_trust_bundles[notebook_component] } try: @@ -784,10 +781,9 @@ class Spawner(LoggingConfigurable): shutil.move(paths['certfile'], out_dir) shutil.copy(paths['cafile'], out_dir) - path_tmpl = "{out}/{name}.{ext}" - key = path_tmpl.format(out=out_dir, name=self.user.name, ext="key") - cert = path_tmpl.format(out=out_dir, name=self.user.name, ext="crt") - ca = path_tmpl.format(out=out_dir, name=self.internal_authority_name, ext="crt") + key = os.path.join(out_dir, os.path.basename(paths['keyfile'])) + cert = os.path.join(out_dir, os.path.basename(paths['certfile'])) + ca = os.path.join(out_dir, os.path.basename(paths['cafile'])) # Set cert ownership to user for f in [out_dir, key, cert, ca]: diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 9e11d7c0..2f2e7009 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -63,12 +63,10 @@ def app(request, io_loop, ssl_tmpdir): ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: - internal_authority_name = 'hub' - external_certs = ssl_setup(str(ssl_tmpdir), internal_authority_name) + external_certs = ssl_setup(str(ssl_tmpdir), 'hub-ca') mocked_app = MockHub.instance( log_level=logging.DEBUG, internal_ssl=True, - internal_authority_name=internal_authority_name, internal_certs_location=str(ssl_tmpdir)) @gen.coroutine diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index db3f6472..f82e51c8 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -219,11 +219,9 @@ class MockHub(JupyterHub): def __new__(cls, *args, **kwargs): try: # Turn on internalSSL if the options exist - internal_authority_name = 'hub' cert_location = kwargs['internal_certs_location'] - external_certs = ssl_setup(cert_location, internal_authority_name) + external_certs = ssl_setup(cert_location, 'hub-ca') kwargs['internal_ssl'] = True - kwargs['internal_authority_name'] = internal_authority_name kwargs['ssl_cert'] = external_certs['files']['cert'] kwargs['ssl_key'] = external_certs['files']['key'] except KeyError: diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 6f42a983..2013d74d 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -221,9 +221,10 @@ class User: if self.settings.get('internal_ssl'): ssl_kwargs = dict( internal_ssl=self.settings.get('internal_ssl'), - internal_certs_location=self.settings.get('internal_certs_location'), - internal_authority_name=self.settings.get('internal_authority_name'), - internal_notebook_authority_name=self.settings.get('internal_notebook_authority_name'), + internal_trust_bundles=self.settings.get( + 'internal_trust_bundles'), + internal_certs_location=self.settings.get( + 'internal_certs_location'), ) spawn_kwargs.update(ssl_kwargs) @@ -505,9 +506,9 @@ class User: db.commit() spawner._waiting_for_response = True try: - key = self.settings['internal_ssl_key'] - cert = self.settings['internal_ssl_cert'] - ca = self.settings['internal_ssl_ca'] + key = self.settings.get('internal_ssl_key') + cert = self.settings.get('internal_ssl_cert') + ca = self.settings.get('internal_ssl_ca') ssl_context = make_ssl_context(key, cert, cafile=ca) resp = await server.wait_up( http=True, diff --git a/requirements.txt b/requirements.txt index 92eb07ac..9b7688a6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,4 +9,4 @@ python-dateutil SQLAlchemy>=1.1 requests prometheus_client>=0.0.21 -certipy>=0.1.0 +certipy>=0.1.2 From a81972067ae2ebc210e96ce4739844303c5ab7f1 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Wed, 12 Sep 2018 17:48:31 -0700 Subject: [PATCH 28/68] Stop servers that don't get cleaned up Running all tests (including internal_ssl monkey-patched ones) leaves behind some spawned servers. Stop them. --- jupyterhub/tests/test_spawner.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index 3dd3276c..b1f27627 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -253,6 +253,7 @@ def test_shell_cmd(db, tmpdir, request): r.raise_for_status() env = r.json() assert env['TESTVAR'] == 'foo' + yield s.stop() def test_inherit_overwrite(): @@ -406,3 +407,4 @@ def test_spawner_routing(app, name): r.raise_for_status() assert r.url == url assert r.text == urlparse(url).path + yield user.stop() From a13f4197d47488fe6fc24e9b599c02675730da54 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Fri, 28 Sep 2018 16:23:17 -0700 Subject: [PATCH 29/68] Move proxy cert creation into .init_internal_ssl --- docs/source/reference/proxy.md | 11 +++--- jupyterhub/app.py | 27 +++++++++++++- jupyterhub/proxy.py | 66 ++++++++-------------------------- 3 files changed, 47 insertions(+), 57 deletions(-) diff --git a/docs/source/reference/proxy.md b/docs/source/reference/proxy.md index 550886ad..ad2be315 100644 --- a/docs/source/reference/proxy.md +++ b/docs/source/reference/proxy.md @@ -66,10 +66,13 @@ Hub should call these methods when the Hub itself starts and stops. When using `internal_ssl` to encrypt traffic behind the proxy, at minimum, your `Proxy` will need client ssl certificates which the `Hub` must be made -aware of. These can be provided to the hub via the `jupyterhub_config.py` file -by providing a `dict` of named paths to the `external_authorities` option. The -hub will include all certificates provided in that `dict` in the trust bundle -utilized by all internal components. +aware of. These can be generated with the command `jupyterhub --generate-certs` +which will write them to the `internal_certs_location` in folders named +`proxy_api` and `proxy_client`. Alternatively, these can be provided to the +hub via the `jupyterhub_config.py` file by providing a `dict` of named paths +to the `external_authorities` option. The hub will include all certificates +provided in that `dict` in the trust bundle utilized by all internal +components. ### Purely external proxies diff --git a/jupyterhub/app.py b/jupyterhub/app.py index e2a237d2..e7b559df 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -396,6 +396,11 @@ class JupyterHub(Application): help=""" The ca to be used for internal ssl """ ) + internal_proxy_certs = Dict( + help=""" Dict component:dict(cert files). This dict contains the certs + generated for both the proxy API and proxy client. + """ + ) trusted_alt_names = List(Unicode(), help=""" Names to include in the subject alternative name. These names will be used for server name verification. This is useful @@ -1190,12 +1195,13 @@ class JupyterHub(Application): self.internal_trust_bundles = certipy.trust_from_graph( self.internal_ssl_components_trust) + default_alt_names = ["IP:127.0.0.1", "DNS:localhost"] # The signed certs used by hub-internal components try: internal_key_pair = certipy.store.get_record("hub-internal") except CertNotFoundError: import socket - alt_names = ["IP:127.0.0.1", "DNS:localhost"] + alt_names = list(default_alt_names) # In the event the hub needs to be accessed externally, add # the fqdn and (optionally) rev_proxy to the set of alt_names. alt_names += (["DNS:" + socket.getfqdn()] @@ -1206,6 +1212,25 @@ class JupyterHub(Application): alt_names=alt_names ) + # Create the proxy certs + proxy_api = 'proxy-api' + proxy_client = 'proxy-client' + for component in [proxy_api, proxy_client]: + ca_name = component + '-ca' + + record = certipy.create_signed_pair( + component, + ca_name, + alt_names=default_alt_names, + overwrite=True + ) + + self.internal_proxy_certs[component] = { + "keyfile": record['files']['key'], + "certfile": record['files']['cert'], + "cafile": record['files']['cert'], + } + self.internal_ssl_key = internal_key_pair['files']['key'] self.internal_ssl_cert = internal_key_pair['files']['cert'] self.internal_ssl_ca = self.internal_trust_bundles[hub_name] diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 03a824ec..4adc1b3b 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -121,44 +121,6 @@ class Proxy(LoggingConfigurable): if the proxy is to be started by the Hub """ - def create_certs(self, name, ca_name, alt_names=None, override=False): - """Create certificates to be used with internal_ssl. - """ - - from certipy import Certipy, CertNotFoundError - default_names = ["DNS:localhost", "IP:127.0.0.1"] - alt_names = alt_names or [] - - if not override: - alt_names = default_names + alt_names - - certipy = Certipy(store_dir=self.app.internal_certs_location) - record = None - - try: - record = certipy.store.get_record(name) - except CertNotFoundError: - record = certipy.create_signed_pair( - name, - ca_name, - alt_names=alt_names, - overwrite=True - ) - - paths = { - "keyfile": record['files']['key'], - "certfile": record['files']['cert'], - "cafile": record['files']['cert'], - } - - return paths - - - def move_certs(self, paths): - """Move certificates created by create_certs. - """ - return paths - def validate_routespec(self, routespec): """Validate a routespec @@ -593,25 +555,25 @@ class ConfigurableHTTPProxy(Proxy): if self.ssl_cert: cmd.extend(['--ssl-cert', self.ssl_cert]) if self.app.internal_ssl: - certs = {} - trust_bundles = {} proxy_api = 'proxy-api' proxy_client = 'proxy-client' - for component in [proxy_api, proxy_client]: - ca_name = component + '-ca' - trust_bundles[component] = \ - self.app.internal_trust_bundles[ca_name] - certs[component] = self.move_certs( - self.create_certs(component, ca_name)) - cmd.extend(['--api-ssl-key', certs[proxy_api]['keyfile']]) - cmd.extend(['--api-ssl-cert', certs[proxy_api]['certfile']]) - cmd.extend(['--api-ssl-ca', trust_bundles[proxy_api]]) + api_key = self.app.internal_proxy_certs[proxy_api]['keyfile'] + api_cert = self.app.internal_proxy_certs[proxy_api]['certfile'] + api_ca = self.app.internal_trust_bundles[proxy_api + '-ca'] + + client_key = self.app.internal_proxy_certs[proxy_client]['keyfile'] + client_cert = self.app.internal_proxy_certs[proxy_client]['certfile'] + client_ca = self.app.internal_trust_bundles[proxy_client + '-ca'] + + cmd.extend(['--api-ssl-key', api_key]) + cmd.extend(['--api-ssl-cert', api_cert]) + cmd.extend(['--api-ssl-ca', api_ca]) cmd.extend(['--api-ssl-request-cert']) cmd.extend(['--api-ssl-reject-unauthorized']) - cmd.extend(['--client-ssl-key', certs[proxy_client]['keyfile']]) - cmd.extend(['--client-ssl-cert', certs[proxy_client]['certfile']]) - cmd.extend(['--client-ssl-ca', trust_bundles[proxy_client]]) + cmd.extend(['--client-ssl-key', client_key]) + cmd.extend(['--client-ssl-cert', client_cert]) + cmd.extend(['--client-ssl-ca', client_ca]) cmd.extend(['--client-ssl-request-cert']) cmd.extend(['--client-ssl-reject-unauthorized']) if self.app.statsd_host: From 371ef6cad83f81ee5856da3217e1429539e53b23 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 15:15:03 -0700 Subject: [PATCH 30/68] Spawn under name 'service' if no username exists --- jupyterhub/spawner.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 1116cb8c..386105fb 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -714,10 +714,11 @@ class Spawner(LoggingConfigurable): if not override: alt_names = default_names + alt_names + common_name = self.user.name or 'service' certipy = Certipy(store_dir=self.internal_certs_location) notebook_component = 'notebooks-ca' notebook_key_pair = certipy.create_signed_pair( - self.user.name, + common_name, notebook_component, alt_names=alt_names, overwrite=True From d1aeff7bbf023f75017fbc67e2f25b0ff88592e4 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 15:15:54 -0700 Subject: [PATCH 31/68] Fix issue where Mockub was not seeing ssl_enabled flag --- jupyterhub/tests/conftest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 2f2e7009..168560c9 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -59,7 +59,7 @@ def ssl_tmpdir(tmpdir_factory): @fixture(scope='module') def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" - mocked_app = MockHub.instance(log_level=logging.DEBUG) + mocked_app = None ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: @@ -68,6 +68,8 @@ def app(request, io_loop, ssl_tmpdir): log_level=logging.DEBUG, internal_ssl=True, internal_certs_location=str(ssl_tmpdir)) + else: + mocked_app = MockHub.instance(log_level=logging.DEBUG) @gen.coroutine def make_app(): From 88b2954c90eaefe88d7706c133201c971c88ce31 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 15:16:56 -0700 Subject: [PATCH 32/68] Missed change in mocksu to pick up certs from env --- jupyterhub/tests/mocksu.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/jupyterhub/tests/mocksu.py b/jupyterhub/tests/mocksu.py index b8c9be05..690ca495 100644 --- a/jupyterhub/tests/mocksu.py +++ b/jupyterhub/tests/mocksu.py @@ -37,11 +37,11 @@ def main(args): ]) ssl_context = None - if args.keyfile and args.certfile and args.client_ca: - key = args.keyfile.strip('"') - cert = args.certfile.strip('"') - ca = args.client_ca.strip('"') + key = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_KEYFILE') or '' + cert = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CERTFILE') or '' + ca = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA') or '' + if key and cert and ca: ssl_context = make_ssl_context( key, cert, @@ -58,8 +58,5 @@ def main(args): if __name__ == '__main__': parser = argparse.ArgumentParser() parser.add_argument('--port', type=int) - parser.add_argument('--keyfile', type=str) - parser.add_argument('--certfile', type=str) - parser.add_argument('--client-ca', type=str) args, extra = parser.parse_known_args() main(args) From 34d59f66d91613d6562b77d614ca17f38d2c5278 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 15:17:49 -0700 Subject: [PATCH 33/68] Setup mock services to use certs from env --- jupyterhub/tests/conftest.py | 5 ++++- jupyterhub/tests/mockservice.py | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 168560c9..2a7ca96f 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -178,7 +178,10 @@ def _mockservice(request, app, url=False): 'admin': True, } if url: - spec['url'] = 'http://127.0.0.1:%i' % random_port() + if app.internal_ssl: + spec['url'] = 'https://127.0.0.1:%i' % random_port() + else: + spec['url'] = 'http://127.0.0.1:%i' % random_port() io_loop = app.io_loop diff --git a/jupyterhub/tests/mockservice.py b/jupyterhub/tests/mockservice.py index 375bfff2..dc7e9836 100644 --- a/jupyterhub/tests/mockservice.py +++ b/jupyterhub/tests/mockservice.py @@ -23,6 +23,7 @@ import requests from tornado import web, httpserver, ioloop from jupyterhub.services.auth import HubAuthenticated, HubOAuthenticated, HubOAuthCallbackHandler +from jupyterhub.utils import make_ssl_context class EchoHandler(web.RequestHandler): @@ -85,7 +86,19 @@ def main(): (r'.*', EchoHandler), ], cookie_secret=os.urandom(32)) - server = httpserver.HTTPServer(app) + ssl_context = None + key = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_KEYFILE') or '' + cert = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CERTFILE') or '' + ca = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA') or '' + + if key and cert and ca: + ssl_context = make_ssl_context( + key, + cert, + cafile = ca, + check_hostname = False) + + server = httpserver.HTTPServer(app, ssl_options=ssl_context) server.listen(url.port, url.hostname) try: ioloop.IOLoop.instance().start() From 99f2905cab39a62a61f1e8a510b9675a3182a0fa Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 15:19:22 -0700 Subject: [PATCH 34/68] Use certs if available for test_api --- jupyterhub/tests/test_api.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index e68e5d04..550eb2c4 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -96,6 +96,9 @@ def api_request(app, *api_path, **kwargs): url = ujoin(base_url, 'api', *api_path) method = kwargs.pop('method', 'get') f = getattr(async_requests, method) + if app.internal_ssl: + kwargs['cert'] = (app.internal_ssl_cert, app.internal_ssl_key) + kwargs["verify"] = app.internal_ssl_ca resp = yield f(url, **kwargs) assert "frame-ancestors 'self'" in resp.headers['Content-Security-Policy'] assert ujoin(app.hub.base_url, "security/csp-report") in resp.headers['Content-Security-Policy'] @@ -1571,7 +1574,11 @@ def test_get_service(app, mockservice_url): def test_root_api(app): base_url = app.hub.url url = ujoin(base_url, 'api') - r = yield async_requests.get(url) + kwargs = {} + if app.internal_ssl: + kwargs['cert'] = (app.internal_ssl_cert, app.internal_ssl_key) + kwargs["verify"] = app.internal_ssl_ca + r = yield async_requests.get(url, **kwargs) r.raise_for_status() expected = { 'version': jupyterhub.__version__ From c5102452e471941cee7d723578547248f221a8c0 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 17:23:55 -0700 Subject: [PATCH 35/68] Move turning ssl on into __init__ --- jupyterhub/tests/mocking.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index f82e51c8..f7eebefc 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -217,16 +217,19 @@ class MockHub(JupyterHub): log_datefmt = '%M:%S' def __new__(cls, *args, **kwargs): + return super().__new__(cls, *args, **kwargs) + + def __init__(self, *args, **kwargs): try: # Turn on internalSSL if the options exist cert_location = kwargs['internal_certs_location'] - external_certs = ssl_setup(cert_location, 'hub-ca') + self.external_certs = ssl_setup(cert_location, 'hub-ca') kwargs['internal_ssl'] = True - kwargs['ssl_cert'] = external_certs['files']['cert'] - kwargs['ssl_key'] = external_certs['files']['key'] + kwargs['ssl_cert'] = self.external_certs['files']['cert'] + kwargs['ssl_key'] = self.external_certs['files']['key'] except KeyError: pass - return super().__new__(cls, *args, **kwargs) + super().__init__(*args, **kwargs) @default('subdomain_host') def _subdomain_host_default(self): From 080ff7043e4cadef22e1c4757556898d552891b7 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 17:24:21 -0700 Subject: [PATCH 36/68] Set appropriate protocol for bind_url --- jupyterhub/tests/mocking.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index f7eebefc..3d24990d 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -237,11 +237,12 @@ class MockHub(JupyterHub): @default('bind_url') def _default_bind_url(self): + proto = 'https' if self.internal_ssl else 'http' if self.subdomain_host: port = urlparse(self.subdomain_host).port else: port = random_port() - return 'http://127.0.0.1:%i/@/space%%20word/' % port + return '%s://127.0.0.1:%i/@/space%%20word/' % (proto, port) @default('ip') def _ip_default(self): From 32bd8aa105a6f1d21f442eae33b7073ee105cbba Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 17:26:10 -0700 Subject: [PATCH 37/68] Verify the self-signed certs for the proxy --- jupyterhub/tests/mocking.py | 5 +++++ jupyterhub/tests/utils.py | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 3d24990d..89445863 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -215,6 +215,7 @@ class MockHub(JupyterHub): db_file = None last_activity_interval = 2 log_datefmt = '%M:%S' + external_certs = None def __new__(cls, *args, **kwargs): return super().__new__(cls, *args, **kwargs) @@ -324,12 +325,16 @@ class MockHub(JupyterHub): def login_user(self, name): """Login a user by name, returning her cookies.""" base_url = public_url(self) + external_ca = None + if self.internal_ssl: + external_ca = self.external_certs['files']['ca'] r = yield async_requests.post(base_url + 'hub/login', data={ 'username': name, 'password': name, }, allow_redirects=False, + verify=external_ca, ) r.raise_for_status() assert r.cookies diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py index fe8baa71..84c208a7 100644 --- a/jupyterhub/tests/utils.py +++ b/jupyterhub/tests/utils.py @@ -22,7 +22,9 @@ def ssl_setup(cert_dir, authority_name): # Set up the external certs with the same authority as the internal # one so that certificate trust works regardless of chosen endpoint. certipy = Certipy(store_dir=cert_dir) + alt_names = ['DNS:localhost', 'IP:127.0.0.1'] internal_authority = certipy.create_ca(authority_name, overwrite=True) external_certs = certipy.create_signed_pair('external', authority_name, - overwrite=True) + overwrite=True, + alt_names=alt_names) return external_certs From ca336924595cdcba1fc45796cdebf4409450bcdf Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 17:27:06 -0700 Subject: [PATCH 38/68] Only test internal_ssl if the value is set in request --- jupyterhub/tests/test_app.py | 58 +++++++++++++++++------ jupyterhub/tests/test_internal_ssl_app.py | 1 + 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 86ce82f9..759ccafe 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -64,7 +64,7 @@ def test_generate_config(): @pytest.mark.gen_test -def test_init_tokens(): +def test_init_tokens(request): with TemporaryDirectory() as td: db_file = os.path.join(td, 'jupyterhub.sqlite') tokens = { @@ -72,7 +72,11 @@ def test_init_tokens(): 'also-super-secret': 'gordon', 'boagasdfasdf': 'chell', } - app = MockHub(db_url=db_file, api_tokens=tokens, internal_certs_location=td) + kwargs = {'db_url': db_file, 'api_tokens': tokens} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = td + app = MockHub(**kwargs) yield app.initialize([]) db = app.db for token, username in tokens.items(): @@ -82,7 +86,7 @@ def test_init_tokens(): assert user.name == username # simulate second startup, reloading same tokens: - app = MockHub(db_url=db_file, api_tokens=tokens, internal_certs_location=td) + app = MockHub(**kwargs) yield app.initialize([]) db = app.db for token, username in tokens.items(): @@ -93,27 +97,35 @@ def test_init_tokens(): # don't allow failed token insertion to create users: tokens['short'] = 'gman' - app = MockHub(db_url=db_file, api_tokens=tokens, internal_certs_location=td) + app = MockHub(**kwargs) with pytest.raises(ValueError): yield app.initialize([]) assert orm.User.find(app.db, 'gman') is None -def test_write_cookie_secret(tmpdir): +def test_write_cookie_secret(tmpdir, request): secret_path = str(tmpdir.join('cookie_secret')) - hub = MockHub(cookie_secret_file=secret_path, internal_certs_location=str(tmpdir)) + kwargs = {'cookie_secret_file': secret_path} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) hub.init_secrets() assert os.path.exists(secret_path) assert os.stat(secret_path).st_mode & 0o600 assert not os.stat(secret_path).st_mode & 0o177 -def test_cookie_secret_permissions(tmpdir): +def test_cookie_secret_permissions(tmpdir, request): secret_file = tmpdir.join('cookie_secret') secret_path = str(secret_file) secret = os.urandom(COOKIE_SECRET_BYTES) secret_file.write(binascii.b2a_hex(secret)) - hub = MockHub(cookie_secret_file=secret_path, internal_certs_location=str(tmpdir)) + kwargs = {'cookie_secret_file': secret_path} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) # raise with public secret file os.chmod(secret_path, 0o664) @@ -126,18 +138,26 @@ def test_cookie_secret_permissions(tmpdir): assert hub.cookie_secret == secret -def test_cookie_secret_content(tmpdir): +def test_cookie_secret_content(tmpdir, request): secret_file = tmpdir.join('cookie_secret') secret_file.write('not base 64: uñiço∂e') secret_path = str(secret_file) os.chmod(secret_path, 0o660) - hub = MockHub(cookie_secret_file=secret_path, internal_certs_location=str(tmpdir)) + kwargs = {'cookie_secret_file': secret_path} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) with pytest.raises(SystemExit): hub.init_secrets() -def test_cookie_secret_env(tmpdir): - hub = MockHub(cookie_secret_file=str(tmpdir.join('cookie_secret')), internal_certs_location=str(tmpdir)) +def test_cookie_secret_env(tmpdir, request): + kwargs = {'cookie_secret_file': str(tmpdir.join('cookie_secret'))} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) with patch.dict(os.environ, {'JPY_COOKIE_SECRET': 'not hex'}): with pytest.raises(ValueError): @@ -150,12 +170,16 @@ def test_cookie_secret_env(tmpdir): @pytest.mark.gen_test -def test_load_groups(tmpdir): +def test_load_groups(tmpdir, request): to_load = { 'blue': ['cyclops', 'rogue', 'wolverine'], 'gold': ['storm', 'jean-grey', 'colossus'], } - hub = MockHub(load_groups=to_load, internal_certs_location=str(tmpdir)) + kwargs = {'load_groups': to_load} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + hub = MockHub(**kwargs) hub.init_db() yield hub.init_users() yield hub.init_groups() @@ -178,7 +202,11 @@ def test_resume_spawners(tmpdir, request): request.addfinalizer(p.stop) @gen.coroutine def new_hub(): - app = MockHub(internal_certs_location=str(tmpdir)) + kwargs = {} + ssl_enabled = getattr(request.module, "ssl_enabled", False) + if ssl_enabled: + kwargs['internal_certs_location'] = str(tmpdir) + app = MockHub(**kwargs) app.config.ConfigurableHTTPProxy.should_start = False app.config.ConfigurableHTTPProxy.auth_token = 'unused' yield app.initialize([]) diff --git a/jupyterhub/tests/test_internal_ssl_app.py b/jupyterhub/tests/test_internal_ssl_app.py index 47cfeb1f..1dff4aeb 100644 --- a/jupyterhub/tests/test_internal_ssl_app.py +++ b/jupyterhub/tests/test_internal_ssl_app.py @@ -7,3 +7,4 @@ import sys import jupyterhub.tests.mocking from jupyterhub.tests.test_app import * +ssl_enabled = True From 2cd6a9e720107b9173054f3508eeff35a2a1f2a5 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Thu, 27 Sep 2018 17:27:33 -0700 Subject: [PATCH 39/68] Supply certs to individual async_requests --- jupyterhub/tests/test_api.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 550eb2c4..051b79e4 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -576,15 +576,19 @@ def test_spawn(app): assert spawner.server.base_url == ujoin(app.base_url, 'user/%s' % name) + '/' url = public_url(app, user) - r = yield async_requests.get(url) + kwargs = {} + if app.internal_ssl: + kwargs['cert'] = (app.internal_ssl_cert, app.internal_ssl_key) + kwargs["verify"] = app.internal_ssl_ca + r = yield async_requests.get(url, **kwargs) assert r.status_code == 200 assert r.text == spawner.server.base_url - r = yield async_requests.get(ujoin(url, 'args')) + r = yield async_requests.get(ujoin(url, 'args'), **kwargs) assert r.status_code == 200 argv = r.json() assert '--port' in ' '.join(argv) - r = yield async_requests.get(ujoin(url, 'env')) + r = yield async_requests.get(ujoin(url, 'env'), **kwargs) env = r.json() for expected in ['JUPYTERHUB_USER', 'JUPYTERHUB_BASE_URL', 'JUPYTERHUB_API_TOKEN']: assert expected in env From 2ff067be6dbf53c66400ad04a65d255870b21f49 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Mon, 1 Oct 2018 11:37:33 -0700 Subject: [PATCH 40/68] Formatting change only --- jupyterhub/tests/mocksu.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/jupyterhub/tests/mocksu.py b/jupyterhub/tests/mocksu.py index 690ca495..51c6321f 100644 --- a/jupyterhub/tests/mocksu.py +++ b/jupyterhub/tests/mocksu.py @@ -43,10 +43,11 @@ def main(args): if key and cert and ca: ssl_context = make_ssl_context( - key, - cert, - cafile = ca, - check_hostname = False) + key, + cert, + cafile = ca, + check_hostname = False + ) server = httpserver.HTTPServer(app, ssl_options=ssl_context) server.listen(args.port) From b7b5cf2f2d4ae6cde0832a1a0297bd2f5f990f17 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Mon, 1 Oct 2018 11:38:26 -0700 Subject: [PATCH 41/68] Fix spawner tests for running with internal_ssl --- jupyterhub/tests/test_spawner.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index b1f27627..b6709acf 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -402,8 +402,12 @@ def test_spawner_routing(app, name): yield user.spawn() yield wait_for_spawner(user.spawner) yield app.proxy.add_user(user) + kwargs = {'allow_redirects': False} + if app.internal_ssl: + kwargs['cert'] = (app.internal_ssl_cert, app.internal_ssl_key) + kwargs["verify"] = app.internal_ssl_ca url = url_path_join(public_url(app, user), "test/url") - r = yield async_requests.get(url, allow_redirects=False) + r = yield async_requests.get(url, **kwargs) r.raise_for_status() assert r.url == url assert r.text == urlparse(url).path From 76a6959cf0e0f1ed13be71fd6074e0aa397fad51 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Mon, 1 Oct 2018 16:38:25 -0700 Subject: [PATCH 42/68] Test to ensure connections with improper certs fail --- .../tests/test_internal_ssl_connections.py | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 jupyterhub/tests/test_internal_ssl_connections.py diff --git a/jupyterhub/tests/test_internal_ssl_connections.py b/jupyterhub/tests/test_internal_ssl_connections.py new file mode 100644 index 00000000..42b306aa --- /dev/null +++ b/jupyterhub/tests/test_internal_ssl_connections.py @@ -0,0 +1,77 @@ +"""Tests for jupyterhub internal_ssl connections""" + +import time +from subprocess import check_output +import sys +from urllib.parse import urlparse + +import pytest + +import jupyterhub +from tornado import gen +from unittest import mock +from requests.exceptions import SSLError + +from .utils import async_requests +from .test_api import add_user + +ssl_enabled = True + + +@gen.coroutine +def wait_for_spawner(spawner, timeout=10): + """Wait for an http server to show up + + polling at shorter intervals for early termination + """ + deadline = time.monotonic() + timeout + def wait(): + return spawner.server.wait_up(timeout=1, http=True) + while time.monotonic() < deadline: + status = yield spawner.poll() + assert status is None + try: + yield wait() + except TimeoutError: + continue + else: + break + yield wait() + + +@pytest.mark.gen_test +def test_connection_hub_wrong_certs(app): + """Connecting to the internal hub url fails without correct certs""" + with pytest.raises(SSLError): + kwargs = {'verify': False} + r = yield async_requests.get(app.hub.url, **kwargs) + r.raise_for_status() + + +@pytest.mark.gen_test +def test_connection_proxy_api_wrong_certs(app): + """Connecting to the proxy api fails without correct certs""" + with pytest.raises(SSLError): + kwargs = {'verify': False} + r = yield async_requests.get(app.proxy.api_url, **kwargs) + r.raise_for_status() + + +@pytest.mark.gen_test +def test_connection_notebook_wrong_certs(app): + """Connecting to a notebook fails without correct certs""" + with mock.patch.dict( + app.config.LocalProcessSpawner, + {'cmd': [sys.executable, '-m', 'jupyterhub.tests.mocksu']} + ): + user = add_user(app.db, app, name='foo') + yield user.spawn() + yield wait_for_spawner(user.spawner) + spawner = user.spawner + status = yield spawner.poll() + assert status is None + + with pytest.raises(SSLError): + kwargs = {'verify': False} + r = yield async_requests.get(spawner.server.url, **kwargs) + r.raise_for_status() From c3176b0ca35d88f522bdef80545d3fafc8bb27f9 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 28 Sep 2018 11:02:05 +0200 Subject: [PATCH 43/68] Do not set ownership in `create_certs` Most Authenticators do not have local users, so this doesn't make sense at this stage --- jupyterhub/spawner.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index b777028f..cde0bdfa 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -744,19 +744,8 @@ class Spawner(LoggingConfigurable): paths = { "keyfile": notebook_key_pair['files']['key'], "certfile": notebook_key_pair['files']['cert'], - "cafile": self.internal_trust_bundles[notebook_component] + "cafile": self.internal_trust_bundles[notebook_component], } - - try: - user = pwd.getpwnam(self.user.name) - uid = user.pw_uid - gid = user.pw_gid - for f in ['keyfile', 'certfile']: - shutil.chown(paths[f], user=uid, group=gid) - except KeyError: - self.log.info("User {} not found on system, " - "unable to change ownership".format(self.user.name)) - return paths def move_certs(self, paths): From 50f1decee7168c8dfb494cbfb21fb3ac7fe07fbd Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 28 Sep 2018 11:08:43 +0200 Subject: [PATCH 44/68] move local-process move_certs implementation to LocalProcessSpawner --- jupyterhub/spawner.py | 86 +++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index cde0bdfa..3b6510fc 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -749,14 +749,19 @@ class Spawner(LoggingConfigurable): return paths def move_certs(self, paths): - """Takes cert paths, moves and sets ownership for them + """Takes certificate paths and makes them available to the notebook server Arguments: - paths (dict): a list of paths for key, cert, and CA + paths (dict): a list of paths for key, cert, and CA. + These paths will be resolvable and readable by the Hub process, + but not necessarily by the notebook server. Returns: dict: a list (potentially altered) of paths for key, cert, - and CA + and CA. + These paths should be resolvable and readable + by the notebook server to be launched. + `.move_certs` is called after certs for the singleuser notebook have been created by create_certs. @@ -769,38 +774,7 @@ class Spawner(LoggingConfigurable): to another host, moving them to a volume mounted in a docker container, or exporting them as a secret in kubernetes. """ - key = paths['keyfile'] - cert = paths['certfile'] - ca = paths['cafile'] - - try: - user = pwd.getpwnam(self.user.name) - uid = user.pw_uid - gid = user.pw_gid - home = user.pw_dir - - # Create dir for user's certs wherever we're starting - out_dir = "{home}/.jupyterhub/jupyterhub-certs".format(home=home) - shutil.rmtree(out_dir, ignore_errors=True) - os.makedirs(out_dir, 0o700, exist_ok=True) - - # Move certs to users dir - shutil.move(paths['keyfile'], out_dir) - shutil.move(paths['certfile'], out_dir) - shutil.copy(paths['cafile'], out_dir) - - key = os.path.join(out_dir, os.path.basename(paths['keyfile'])) - cert = os.path.join(out_dir, os.path.basename(paths['certfile'])) - ca = os.path.join(out_dir, os.path.basename(paths['cafile'])) - - # Set cert ownership to user - for f in [out_dir, key, cert, ca]: - shutil.chown(f, user=uid, group=gid) - except KeyError: - self.log.info("User {} not found on system, " - "not moving certs".format(self.user.name)) - - return {"keyfile": key, "certfile": cert, "cafile": ca} + raise NotImplementedError() def get_args(self): """Return the arguments to be passed after self.cmd @@ -1205,6 +1179,48 @@ class LocalProcessSpawner(Spawner): env = self.user_env(env) return env + async def move_certs(self, paths): + """Takes cert paths, moves and sets ownership for them + + Arguments: + paths (dict): a list of paths for key, cert, and CA + + Returns: + dict: a list (potentially altered) of paths for key, cert, + and CA + + Stage certificates into a private home directory + and make them readable by the user. + """ + key = paths['keyfile'] + cert = paths['certfile'] + ca = paths['cafile'] + + user = pwd.getpwnam(self.user.name) + uid = user.pw_uid + gid = user.pw_gid + home = user.pw_dir + + # Create dir for user's certs wherever we're starting + out_dir = "{home}/.jupyterhub/jupyterhub-certs".format(home=home) + shutil.rmtree(out_dir, ignore_errors=True) + os.makedirs(out_dir, 0o700, exist_ok=True) + + # Move certs to users dir + shutil.move(paths['keyfile'], out_dir) + shutil.move(paths['certfile'], out_dir) + shutil.copy(paths['cafile'], out_dir) + + key = os.path.join(out_dir, os.path.basename(paths['keyfile'])) + cert = os.path.join(out_dir, os.path.basename(paths['certfile'])) + ca = os.path.join(out_dir, os.path.basename(paths['cafile'])) + + # Set cert ownership to user + for f in [out_dir, key, cert, ca]: + shutil.chown(f, user=uid, group=gid) + + return {"keyfile": key, "certfile": cert, "cafile": ca} + async def start(self): """Start the single-user server.""" self.port = random_port() From 5fbd4f2d4e8f51094e29f29e1749a90a888fe690 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 28 Sep 2018 13:57:41 +0200 Subject: [PATCH 45/68] call make/move certs at a higher level mostly to allow them to be async --- jupyterhub/spawner.py | 15 +++++++-------- jupyterhub/user.py | 13 +++++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 3b6510fc..343b565f 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -168,6 +168,7 @@ class Spawner(LoggingConfigurable): internal_ssl = Bool(False) internal_trust_bundles = Dict() internal_certs_location = Unicode('') + cert_paths = Dict() admin_access = Bool(False) api_token = Unicode() oauth_client_id = Unicode() @@ -650,12 +651,10 @@ class Spawner(LoggingConfigurable): if self.cpu_guarantee: env['CPU_GUARANTEE'] = str(self.cpu_guarantee) - if self.internal_ssl: - paths = self.move_certs(self.create_certs()) - - env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = paths['keyfile'] - env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = paths['certfile'] - env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = paths['cafile'] + if self.cert_paths: + env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = self.cert_paths['keyfile'] + env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = self.cert_paths['certfile'] + env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = self.cert_paths['cafile'] return env @@ -697,7 +696,7 @@ class Spawner(LoggingConfigurable): """ return s.format(**self.template_namespace()) - def create_certs(self, alt_names=None, override=False): + async def create_certs(self, alt_names=None, override=False): """Create and set ownership for the certs to be used for internal ssl Keyword Arguments: @@ -748,7 +747,7 @@ class Spawner(LoggingConfigurable): } return paths - def move_certs(self, paths): + async def move_certs(self, paths): """Takes certificate paths and makes them available to the notebook server Arguments: diff --git a/jupyterhub/user.py b/jupyterhub/user.py index c9e71bef..7d0bb801 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -440,6 +440,11 @@ class User: try: # run optional preparation work to bootstrap the notebook await maybe_future(spawner.run_pre_spawn_hook()) + if self.settings.get('internal_ssl'): + self.log.debug("Creating internal SSL certs for %s", spawner._log_name) + hub_paths = await maybe_future(spawner.create_certs()) + spawner.cert_paths = await maybe_future(spawner.move_certs(hub_paths)) + self.log.debug("Calling Spawner.start for %s", spawner._log_name) f = maybe_future(spawner.start()) # commit any changes in spawner.start (always commit db changes before yield) db.commit() @@ -536,11 +541,11 @@ class User: spawner.orm_spawner.state = spawner.get_state() db.commit() spawner._waiting_for_response = True + key = self.settings.get('internal_ssl_key') + cert = self.settings.get('internal_ssl_cert') + ca = self.settings.get('internal_ssl_ca') + ssl_context = make_ssl_context(key, cert, cafile=ca) try: - key = self.settings.get('internal_ssl_key') - cert = self.settings.get('internal_ssl_cert') - ca = self.settings.get('internal_ssl_ca') - ssl_context = make_ssl_context(key, cert, cafile=ca) resp = await server.wait_up( http=True, timeout=spawner.http_timeout, From 94bb9ed00da9600e0ec2ef85cf924b9713f00ff8 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 28 Sep 2018 13:58:28 +0200 Subject: [PATCH 46/68] remove NOTEBOOK from internal ssl env --- jupyterhub/singleuser.py | 6 +++--- jupyterhub/spawner.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/jupyterhub/singleuser.py b/jupyterhub/singleuser.py index 8be9cd86..84038173 100755 --- a/jupyterhub/singleuser.py +++ b/jupyterhub/singleuser.py @@ -247,15 +247,15 @@ class SingleUserNotebookApp(NotebookApp): @default('keyfile') def _keyfile_default(self): - return os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_KEYFILE') or '' + return os.environ.get('JUPYTERHUB_SSL_KEYFILE') or '' @default('certfile') def _certfile_default(self): - return os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CERTFILE') or '' + return os.environ.get('JUPYTERHUB_SSL_CERTFILE') or '' @default('client_ca') def _client_ca_default(self): - return os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA') or '' + return os.environ.get('JUPYTERHUB_SSL_CLIENT_CA') or '' @default('hub_prefix') def _hub_prefix_default(self): diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 343b565f..8218a10e 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -652,9 +652,9 @@ class Spawner(LoggingConfigurable): env['CPU_GUARANTEE'] = str(self.cpu_guarantee) if self.cert_paths: - env['JUPYTERHUB_NOTEBOOK_SSL_KEYFILE'] = self.cert_paths['keyfile'] - env['JUPYTERHUB_NOTEBOOK_SSL_CERTFILE'] = self.cert_paths['certfile'] - env['JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA'] = self.cert_paths['cafile'] + env['JUPYTERHUB_SSL_KEYFILE'] = self.cert_paths['keyfile'] + env['JUPYTERHUB_SSL_CERTFILE'] = self.cert_paths['certfile'] + env['JUPYTERHUB_SSL_CLIENT_CA'] = self.cert_paths['cafile'] return env From febb7c32c1366edbfd4be290cd9cc33ce631eed6 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 2 Oct 2018 11:21:38 +0200 Subject: [PATCH 47/68] make alt names attributes on Spawner instead of args to create_certs --- jupyterhub/spawner.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 8218a10e..1eaf15d2 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -696,7 +696,22 @@ class Spawner(LoggingConfigurable): """ return s.format(**self.template_namespace()) - async def create_certs(self, alt_names=None, override=False): + ssl_alt_names = List( + Unicode(), + config=True, + help="""List of SSL alt names + + May be set in config if all spawners should have the same value(s), + or set at runtime by Spawner that know their names. + """ + ) + ssl_alt_names_include_local = Bool( + True, + config=True, + help="""Whether to include DNS:localhost, IP:127.0.0.1 in alt names""", + ) + + async def create_certs(self): """Create and set ownership for the certs to be used for internal ssl Keyword Arguments: @@ -726,9 +741,9 @@ class Spawner(LoggingConfigurable): """ from certipy import Certipy default_names = ["DNS:localhost", "IP:127.0.0.1"] - alt_names = alt_names or [] + alt_names = self.ssl_alt_names or [] - if not override: + if self.ssl_alt_names_include_local: alt_names = default_names + alt_names common_name = self.user.name or 'service' From 4b3f9e5f4232134ca1b7f1b390987537f0802886 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 2 Oct 2018 11:23:26 +0200 Subject: [PATCH 48/68] more descriptive 'internal-ssl' certs location and update/clarify ssl-related docstrings --- jupyterhub/app.py | 56 +++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 86c8110d..fbabbb67 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -324,37 +324,40 @@ class JupyterHub(Application): """ ).tag(config=True) internal_ssl = Bool(False, - help=""" Turn on server side ssl. This enables end-to-end encryption. + help="""Enable SSL for all internal communication + + This enables end-to-end encryption between all JupyterHub components. JupyterHub will automatically create the necessary certificate authority and sign notebook certificates as they're created. """ ).tag(config=True) - internal_certs_location = Unicode('out', - help=""" The location to store certificates automatically created by + internal_certs_location = Unicode('internal-ssl', + help="""The location to store certificates automatically created by JupyterHub. Use with internal_ssl """ ).tag(config=True) recreate_internal_certs = Bool(False, - help=""" Recreate all certificates used within JupyterHub on restart. + help="""Recreate all certificates used within JupyterHub on restart. + + Note: enabling this feature requires restarting all notebook servers. Use with internal_ssl """ ).tag(config=True) external_ssl_authorities = Dict( - default_value={}, - help=""" Dict authority:dict(files). Specify the key, cert, and/or + help="""Dict authority:dict(files). Specify the key, cert, and/or ca file for an authority. This is useful for externally managed proxies that wish to use internal_ssl. - The files dict has this format (you must specify at least a cert): + The files dict has this format (you must specify at least a cert):: - { - 'key': '/path/to/key.key', - 'cert': '/path/to/cert.crt', - 'ca': '/path/to/ca.crt' - } + { + 'key': '/path/to/key.key', + 'cert': '/path/to/cert.crt', + 'ca': '/path/to/ca.crt' + } The authorities you can override: 'hub-ca', 'notebooks-ca', 'proxy-api-ca', 'proxy-client-ca', and 'services-ca'. @@ -370,7 +373,7 @@ class JupyterHub(Application): 'proxy-client-ca': None, 'services-ca': None, }, - help=""" Dict authority:dict(files). When creating the various + help="""Dict authority:dict(files). When creating the various CAs needed for internal_ssl, these are the names that will be used for each authority. @@ -378,29 +381,26 @@ class JupyterHub(Application): """ ) internal_ssl_components_trust = Dict( - help=""" Dict component:list(components). This dict specifies the + help="""Dict component:list(components). This dict specifies the relationships of components secured by internal_ssl. """ ) internal_trust_bundles = Dict( - help=""" Dict component:path. These are the paths to the trust bundles + help="""Dict component:path. These are the paths to the trust bundles that each component should have. They will be set during `init_internal_ssl`. Use with internal_ssl """ ) - internal_ssl_key = Unicode('', - help=""" The key to be used for internal ssl - """ + internal_ssl_key = Unicode( + help="""The key to be used for internal ssl""" ) - internal_ssl_cert = Unicode('', - help=""" The cert to be used for internal ssl - """ + internal_ssl_cert = Unicode( + help="""The cert to be used for internal ssl""" ) - internal_ssl_ca = Unicode('', - help=""" The ca to be used for internal ssl - """ + internal_ssl_ca = Unicode( + help="""The certificate authority to be used for internal ssl""" ) internal_proxy_certs = Dict( help=""" Dict component:dict(cert files). This dict contains the certs @@ -408,7 +408,8 @@ class JupyterHub(Application): """ ) trusted_alt_names = List(Unicode(), - help=""" Names to include in the subject alternative name. + help="""Names to include in the subject alternative name. + These names will be used for server name verification. This is useful if JupyterHub is being run behind a reverse proxy or services using ssl are on different hosts. @@ -1183,10 +1184,7 @@ class JupyterHub(Application): self.cookie_secret = secret def init_internal_ssl(self): - """Create the certs needed to turn on internal SSL. - - - """ + """Create the certs needed to turn on internal SSL.""" if self.internal_ssl: from certipy import Certipy, CertNotFoundError From 7656adc8b0204d29e0781c95f8d213d351a13f17 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 2 Oct 2018 13:11:40 +0200 Subject: [PATCH 49/68] expand logging of ssl cert creation --- jupyterhub/app.py | 21 +++++++++++++++++---- jupyterhub/spawner.py | 5 +++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index fbabbb67..0ec69d33 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1207,6 +1207,7 @@ class JupyterHub(Application): self.internal_ssl_authorities.update(self.external_ssl_authorities) for authority, files in self.internal_ssl_authorities.items(): if files: + self.log.info("Adding CA for %s", authority) certipy.store.add_record( authority, is_ca=True, files=files) @@ -1224,23 +1225,35 @@ class JupyterHub(Application): # the fqdn and (optionally) rev_proxy to the set of alt_names. alt_names += (["DNS:" + socket.getfqdn()] + self.trusted_alt_names) + self.log.info( + "Adding CA for %s: %s", + "hub-internal", + ";".join(alt_names), + ) internal_key_pair = certipy.create_signed_pair( "hub-internal", hub_name, - alt_names=alt_names + alt_names=alt_names, ) + else: + self.log.info("Using existing hub-internal CA") # Create the proxy certs proxy_api = 'proxy-api' proxy_client = 'proxy-client' for component in [proxy_api, proxy_client]: ca_name = component + '-ca' - + alt_names = default_alt_names + self.trusted_alt_names + self.log.info( + "Generating signed signed pair for %s: %s", + component, + ';'.join(alt_names), + ) record = certipy.create_signed_pair( component, ca_name, - alt_names=default_alt_names, - overwrite=True + alt_names=alt_names, + overwrite=True, ) self.internal_proxy_certs[component] = { diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 1eaf15d2..cab08af0 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -746,6 +746,11 @@ class Spawner(LoggingConfigurable): if self.ssl_alt_names_include_local: alt_names = default_names + alt_names + self.log.info("Creating certs for %s: %s", + self._log_name, + ';'.join(alt_names), + ) + common_name = self.user.name or 'service' certipy = Certipy(store_dir=self.internal_certs_location) notebook_component = 'notebooks-ca' From 58f6659e40cf5d090c56501a392641432e882ca9 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 2 Oct 2018 18:16:47 +0200 Subject: [PATCH 50/68] implement .move_certs in dummy MockSpawner --- jupyterhub/spawner.py | 2 +- jupyterhub/tests/mocking.py | 4 ++++ jupyterhub/tests/mockservice.py | 6 +++--- jupyterhub/tests/mocksu.py | 6 +++--- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index cab08af0..1b8ba923 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -793,7 +793,7 @@ class Spawner(LoggingConfigurable): to another host, moving them to a volume mounted in a docker container, or exporting them as a secret in kubernetes. """ - raise NotImplementedError() + return paths def get_args(self): """Return the arguments to be passed after self.cmd diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 165c2206..fede2166 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -95,6 +95,10 @@ class MockSpawner(LocalProcessSpawner): def _cmd_default(self): return [sys.executable, '-m', 'jupyterhub.tests.mocksu'] + def move_certs(self, paths): + """Return the paths unmodified""" + return paths + use_this_api_token = None def start(self): if self.use_this_api_token: diff --git a/jupyterhub/tests/mockservice.py b/jupyterhub/tests/mockservice.py index dc7e9836..a087417e 100644 --- a/jupyterhub/tests/mockservice.py +++ b/jupyterhub/tests/mockservice.py @@ -87,9 +87,9 @@ def main(): ], cookie_secret=os.urandom(32)) ssl_context = None - key = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_KEYFILE') or '' - cert = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CERTFILE') or '' - ca = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA') or '' + key = os.environ.get('JUPYTERHUB_SSL_KEYFILE') or '' + cert = os.environ.get('JUPYTERHUB_SSL_CERTFILE') or '' + ca = os.environ.get('JUPYTERHUB_SSL_CLIENT_CA') or '' if key and cert and ca: ssl_context = make_ssl_context( diff --git a/jupyterhub/tests/mocksu.py b/jupyterhub/tests/mocksu.py index fc9f1bb9..d6c2b08d 100644 --- a/jupyterhub/tests/mocksu.py +++ b/jupyterhub/tests/mocksu.py @@ -37,9 +37,9 @@ def main(args): ]) ssl_context = None - key = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_KEYFILE') or '' - cert = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CERTFILE') or '' - ca = os.environ.get('JUPYTERHUB_NOTEBOOK_SSL_CLIENT_CA') or '' + key = os.environ.get('JUPYTERHUB_SSL_KEYFILE') or '' + cert = os.environ.get('JUPYTERHUB_SSL_CERTFILE') or '' + ca = os.environ.get('JUPYTERHUB_SSL_CLIENT_CA') or '' if key and cert and ca: ssl_context = make_ssl_context( From a19812489407014f7bccba6ce31d4f5eae37583a Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 2 Oct 2018 18:40:01 +0200 Subject: [PATCH 51/68] ssl tests need CHP master for now --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8c03233a..a91fc73a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,7 +20,8 @@ before_install: - set -e - nvm install 6; nvm use 6 - npm install - - npm install -g configurable-http-proxy + # temporary: install CHP master for ssl tests + - npm install -g https://github.com/configurable-http-proxy/archive/master.tar.gz - | # setup database if [[ $JUPYTERHUB_TEST_DB_URL == mysql* ]]; then From dd074956245b4c89f059dd070211dc3373219f1b Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Fri, 5 Oct 2018 15:59:10 -0700 Subject: [PATCH 52/68] Fix public_url call with ssl testing enabled --- jupyterhub/tests/test_api.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 60980178..ad0c8e55 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -627,8 +627,11 @@ def test_spawn_handler(app): # verify that request params got passed down # implemented in MockSpawner + kwargs = {} + if app.external_certs: + kwargs['verify'] = app.external_certs['files']['ca'] url = public_url(app, user) - r = yield async_requests.get(ujoin(url, 'env')) + r = yield async_requests.get(ujoin(url, 'env'), **kwargs) env = r.json() assert 'HANDLER_ARGS' in env assert env['HANDLER_ARGS'] == 'foo=bar' From bcebf0ee7b247847e8e0d287fb8e92f6ad243c84 Mon Sep 17 00:00:00 2001 From: Thomas Mendoza Date: Tue, 9 Oct 2018 16:25:23 -0700 Subject: [PATCH 53/68] Set `change-origin` so certs behind proxy work --- jupyterhub/proxy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 53eed31a..e2dc7ef2 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -578,6 +578,7 @@ class ConfigurableHTTPProxy(Proxy): client_cert = self.app.internal_proxy_certs[proxy_client]['certfile'] client_ca = self.app.internal_trust_bundles[proxy_client + '-ca'] + cmd.extend(['--change-origin']) cmd.extend(['--api-ssl-key', api_key]) cmd.extend(['--api-ssl-cert', api_cert]) cmd.extend(['--api-ssl-ca', api_ca]) From 7c0e113fbcb8edd6a6b9d0a365f677d829c29634 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 12 Oct 2018 16:21:35 +0200 Subject: [PATCH 54/68] Revert "Set `change-origin` so certs behind proxy work" This reverts commit bcebf0ee7b247847e8e0d287fb8e92f6ad243c84. Setting change-origin introduces CORS problems --- jupyterhub/proxy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 6e8494cb..ee0f83c7 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -583,7 +583,6 @@ class ConfigurableHTTPProxy(Proxy): client_cert = self.app.internal_proxy_certs[proxy_client]['certfile'] client_ca = self.app.internal_trust_bundles[proxy_client + '-ca'] - cmd.extend(['--change-origin']) cmd.extend(['--api-ssl-key', api_key]) cmd.extend(['--api-ssl-cert', api_cert]) cmd.extend(['--api-ssl-ca', api_ca]) From 67f21bb5184bad14ac71f06ff39e86ebec11b892 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 12 Oct 2018 16:26:42 +0200 Subject: [PATCH 55/68] ssl tests can use configproxy --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index a91fc73a..8c03233a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,8 +20,7 @@ before_install: - set -e - nvm install 6; nvm use 6 - npm install - # temporary: install CHP master for ssl tests - - npm install -g https://github.com/configurable-http-proxy/archive/master.tar.gz + - npm install -g configurable-http-proxy - | # setup database if [[ $JUPYTERHUB_TEST_DB_URL == mysql* ]]; then From 28c6377db756de2243e43102a42845f0d5d711c4 Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 12 Oct 2018 17:05:59 +0200 Subject: [PATCH 56/68] avoid modifying headers in-place can have consequences if args are re-used --- jupyterhub/tests/test_api.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index ad0c8e55..d4e91cd0 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -91,7 +91,10 @@ def api_request(app, *api_path, **kwargs): headers = kwargs.setdefault('headers', {}) if 'Authorization' not in headers and not kwargs.pop('noauth', False): - headers.update(auth_header(app.db, 'admin')) + # make a copy to avoid modifying arg in-place + kwargs['headers'] = h = {} + h.update(headers) + h.update(auth_header(app.db, 'admin')) url = ujoin(base_url, 'api', *api_path) method = kwargs.pop('method', 'get') From b72d887dd7e46759e6c0c0cce938f006b531927d Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 15 Oct 2018 15:33:59 +0200 Subject: [PATCH 57/68] register cleanup before start avoids leaving lingering proxy if app fails to start --- jupyterhub/tests/conftest.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 2a7ca96f..f731e019 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -71,12 +71,10 @@ def app(request, io_loop, ssl_tmpdir): else: mocked_app = MockHub.instance(log_level=logging.DEBUG) - @gen.coroutine - def make_app(): - yield mocked_app.initialize([]) - yield mocked_app.start() - io_loop.run_sync(make_app) + async def make_app(): + await mocked_app.initialize([]) + await mocked_app.start() def fin(): # disconnect logging during cleanup because pytest closes captured FDs prematurely @@ -85,6 +83,7 @@ def app(request, io_loop, ssl_tmpdir): mocked_app.stop() request.addfinalizer(fin) + io_loop.run_sync(make_app) return mocked_app From d64853a6f5e12b968c3818dbf789903b9cbc11cb Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 15 Oct 2018 15:35:21 +0200 Subject: [PATCH 58/68] fix ssl tmpdir in tests must be module-scoped, not session-scoped, or it will get reused inconsistently --- jupyterhub/tests/conftest.py | 16 ++++++++-------- jupyterhub/tests/mocking.py | 16 +++++----------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index f731e019..d7b5f2da 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -51,7 +51,7 @@ import jupyterhub.services.service _db = None -@fixture(scope='session') +@fixture(scope='module') def ssl_tmpdir(tmpdir_factory): return tmpdir_factory.mktemp('ssl') @@ -61,16 +61,16 @@ def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" mocked_app = None ssl_enabled = getattr(request.module, "ssl_enabled", False) - + kwargs = dict(log_level=logging.DEBUG) if ssl_enabled: - external_certs = ssl_setup(str(ssl_tmpdir), 'hub-ca') - mocked_app = MockHub.instance( - log_level=logging.DEBUG, + kwargs.update( + dict( internal_ssl=True, - internal_certs_location=str(ssl_tmpdir)) - else: - mocked_app = MockHub.instance(log_level=logging.DEBUG) + internal_certs_location=str(ssl_tmpdir), + ) + ) + mocked_app = MockHub.instance(**kwargs) async def make_app(): await mocked_app.initialize([]) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index fede2166..cba109ea 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -222,20 +222,14 @@ class MockHub(JupyterHub): last_activity_interval = 2 log_datefmt = '%M:%S' external_certs = None - - def __new__(cls, *args, **kwargs): - return super().__new__(cls, *args, **kwargs) + log_level = 10 def __init__(self, *args, **kwargs): - try: - # Turn on internalSSL if the options exist + if 'internal_certs_location' in kwargs: cert_location = kwargs['internal_certs_location'] - self.external_certs = ssl_setup(cert_location, 'hub-ca') - kwargs['internal_ssl'] = True - kwargs['ssl_cert'] = self.external_certs['files']['cert'] - kwargs['ssl_key'] = self.external_certs['files']['key'] - except KeyError: - pass + kwargs['external_certs'] = certs = ssl_setup(cert_location, 'hub-ca') + kwargs['ssl_cert'] = certs['files']['cert'] + kwargs['ssl_key'] = certs['files']['key'] super().__init__(*args, **kwargs) @default('subdomain_host') From f3c2a15e53b82c800c244cd28689fb755fc5ff0b Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 15 Oct 2018 16:15:46 +0200 Subject: [PATCH 59/68] ensure AsyncIOMainLoop is registered in tests --- jupyterhub/tests/conftest.py | 7 +++++++ jupyterhub/tests/mocking.py | 1 + 2 files changed, 8 insertions(+) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index d7b5f2da..47d413da 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -27,6 +27,7 @@ Fixtures to add functionality or spawning behavior # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +import asyncio from getpass import getuser import logging import os @@ -34,6 +35,7 @@ from pytest import fixture, raises from subprocess import TimeoutExpired from tornado import ioloop, gen from tornado.httpclient import HTTPError +from tornado.platform.asyncio import AsyncIOMainLoop from unittest import mock from .. import orm @@ -121,8 +123,13 @@ def db(): @fixture(scope='module') def io_loop(request): """Same as pytest-tornado.io_loop, but re-scoped to module-level""" + ioloop.IOLoop.configure(AsyncIOMainLoop) + aio_loop = asyncio.new_event_loop() + asyncio.set_event_loop(aio_loop) io_loop = ioloop.IOLoop() io_loop.make_current() + assert asyncio.get_event_loop() is aio_loop + assert io_loop.asyncio_loop is aio_loop def _close(): io_loop.clear_current() diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index cba109ea..33ca7ebe 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -392,6 +392,7 @@ class StubSingleUserSpawner(MockSpawner): evt = threading.Event() print(args, env) def _run(): + asyncio.set_event_loop(asyncio.new_event_loop()) io_loop = IOLoop() io_loop.make_current() io_loop.add_callback(lambda : evt.set()) From 1f3165859fb685ead849b23c5bf808252df94417 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 16 Oct 2018 15:45:20 +0200 Subject: [PATCH 60/68] avoid unnecessarily recreating proxy certs --- jupyterhub/app.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 0ec69d33..bd9bf239 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -16,6 +16,7 @@ from operator import itemgetter import os import re import signal +import socket import sys from textwrap import dedent from urllib.parse import unquote, urlparse, urlunparse @@ -1219,7 +1220,6 @@ class JupyterHub(Application): try: internal_key_pair = certipy.store.get_record("hub-internal") except CertNotFoundError: - import socket alt_names = list(default_alt_names) # In the event the hub needs to be accessed externally, add # the fqdn and (optionally) rev_proxy to the set of alt_names. @@ -1244,17 +1244,21 @@ class JupyterHub(Application): for component in [proxy_api, proxy_client]: ca_name = component + '-ca' alt_names = default_alt_names + self.trusted_alt_names - self.log.info( - "Generating signed signed pair for %s: %s", - component, - ';'.join(alt_names), - ) - record = certipy.create_signed_pair( - component, - ca_name, - alt_names=alt_names, - overwrite=True, - ) + try: + record = certipy.store.get_record(component) + except CertNotFoundError: + self.log.info( + "Generating signed pair for %s: %s", + component, + ';'.join(alt_names), + ) + record = certipy.create_signed_pair( + component, + ca_name, + alt_names=alt_names, + ) + else: + self.log.info("Using existing %s CA", component) self.internal_proxy_certs[component] = { "keyfile": record['files']['key'], From 9a45f4a8c9517325bdd5259d301190ee64671cc9 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 16 Oct 2018 15:45:49 +0200 Subject: [PATCH 61/68] add user- prefix to user cert dirs avoids possible conflict e.g. if a user had the name 'hub-internal' --- jupyterhub/spawner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index 1b8ba923..b2b4da25 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -755,7 +755,7 @@ class Spawner(LoggingConfigurable): certipy = Certipy(store_dir=self.internal_certs_location) notebook_component = 'notebooks-ca' notebook_key_pair = certipy.create_signed_pair( - common_name, + 'user-' + common_name, notebook_component, alt_names=alt_names, overwrite=True From eb7648abc203d316ac718a10c06a0b892ddf864b Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 16 Oct 2018 15:46:50 +0200 Subject: [PATCH 62/68] consolidate trusted alt names - trust subdomain_host by default - JupyterHub.trusted_alt_names is inherited by Spawners by default. Do we need Spawner.ssl_alt_names to be separately configurable? --- jupyterhub/app.py | 2 ++ jupyterhub/spawner.py | 11 ++++++++++- jupyterhub/utils.py | 6 ++---- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index bd9bf239..74862da1 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1216,6 +1216,8 @@ class JupyterHub(Application): self.internal_ssl_components_trust) default_alt_names = ["IP:127.0.0.1", "DNS:localhost"] + if self.subdomain_host: + default_alt_names.append("DNS:%s" % urlparse(self.subdomain_host).hostname) # The signed certs used by hub-internal components try: internal_key_pair = certipy.store.get_record("hub-internal") diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index b2b4da25..ad1c4c6d 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -28,7 +28,7 @@ from tornado.ioloop import PeriodicCallback from traitlets.config import LoggingConfigurable from traitlets import ( Any, Bool, Dict, Instance, Integer, Float, List, Unicode, Union, - observe, validate, + default, observe, validate, ) from .objects import Server @@ -696,6 +696,8 @@ class Spawner(LoggingConfigurable): """ return s.format(**self.template_namespace()) + trusted_alt_names = List(Unicode()) + ssl_alt_names = List( Unicode(), config=True, @@ -705,6 +707,13 @@ class Spawner(LoggingConfigurable): or set at runtime by Spawner that know their names. """ ) + + @default('ssl_alt_names') + def _default_ssl_alt_names(self): + # by default, use trusted_alt_names + # inherited from global app + return list(self.trusted_alt_names) + ssl_alt_names_include_local = Bool( True, config=True, diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index 770610f4..3947eaea 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -189,11 +189,9 @@ async def wait_for_http_server(url, timeout=10, ssl_context=None): """ loop = ioloop.IOLoop.current() tic = loop.time() - settings = None - if ssl_context: - settings = {"ssl_options": ssl_context} - AsyncHTTPClient.configure(None, defaults=settings) client = AsyncHTTPClient() + if ssl_context: + client.ssl_options = ssl_context async def is_reachable(): try: r = await client.fetch(url, follow_redirects=False) From e921354544935c43743bc626c0442ab98a795e15 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 16 Oct 2018 15:56:32 +0200 Subject: [PATCH 63/68] run internal-ssl tests with external http to cover any protocol mismatches --- jupyterhub/tests/mocking.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 33ca7ebe..f867139f 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -227,9 +227,7 @@ class MockHub(JupyterHub): def __init__(self, *args, **kwargs): if 'internal_certs_location' in kwargs: cert_location = kwargs['internal_certs_location'] - kwargs['external_certs'] = certs = ssl_setup(cert_location, 'hub-ca') - kwargs['ssl_cert'] = certs['files']['cert'] - kwargs['ssl_key'] = certs['files']['key'] + kwargs['external_certs'] = ssl_setup(cert_location, 'hub-ca') super().__init__(*args, **kwargs) @default('subdomain_host') @@ -238,12 +236,11 @@ class MockHub(JupyterHub): @default('bind_url') def _default_bind_url(self): - proto = 'https' if self.internal_ssl else 'http' if self.subdomain_host: port = urlparse(self.subdomain_host).port else: port = random_port() - return '%s://127.0.0.1:%i/@/space%%20word/' % (proto, port) + return 'http://127.0.0.1:%i/@/space%%20word/' % (port,) @default('ip') def _ip_default(self): From 15788bec672e0c8fc84f1f82fe0103a35c8ca299 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 16 Oct 2018 15:56:54 +0200 Subject: [PATCH 64/68] ensure user's own subdomain is in trusted alt names --- jupyterhub/spawner.py | 3 ++- jupyterhub/user.py | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index ad1c4c6d..90bcf006 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -750,7 +750,8 @@ class Spawner(LoggingConfigurable): """ from certipy import Certipy default_names = ["DNS:localhost", "IP:127.0.0.1"] - alt_names = self.ssl_alt_names or [] + alt_names = [] + alt_names.extend(self.ssl_alt_names) if self.ssl_alt_names_include_local: alt_names = default_names + alt_names diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 947dd3ca..74003ed2 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -229,6 +229,12 @@ class User: client_id = 'jupyterhub-user-%s' % quote(self.name) if server_name: client_id = '%s-%s' % (client_id, quote(server_name)) + + trusted_alt_names = [] + trusted_alt_names.extend(self.settings.get('trusted_alt_names', [])) + if self.settings.get('subdomain_host'): + trusted_alt_names.append('DNS:' + self.domain) + spawn_kwargs = dict( user=self, orm_spawner=orm_spawner, @@ -239,7 +245,7 @@ class User: db=self.db, oauth_client_id=client_id, cookie_options = self.settings.get('cookie_options', {}), - trusted_alt_names=self.settings.get('trusted_alt_names'), + trusted_alt_names=trusted_alt_names, ) if self.settings.get('internal_ssl'): From 301fed30b2aa8b33177c1e3abc785bc1e30689e5 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 17 Oct 2018 10:38:19 +0200 Subject: [PATCH 65/68] Delete users in MockHub avoids pollution from one test module to the next --- jupyterhub/tests/mocking.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index f867139f..9ba425af 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -282,6 +282,13 @@ class MockHub(JupyterHub): self.db.expire(service) return super().init_services() + def init_db(self): + """Ensure we start with a clean user list""" + super().init_db() + for user in self.db.query(orm.User): + self.db.delete(user) + self.db.commit() + @gen.coroutine def initialize(self, argv=None): self.pid_file = NamedTemporaryFile(delete=False).name From b0116ee5393cde72e73cb1c5364737b767a3b5d6 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 17 Oct 2018 11:02:06 +0200 Subject: [PATCH 66/68] avoid cleaning users when we are testing resume --- jupyterhub/tests/mocking.py | 11 +++++++---- jupyterhub/tests/test_app.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 9ba425af..368d3cf4 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -40,7 +40,7 @@ from tornado import gen from tornado.concurrent import Future from tornado.ioloop import IOLoop -from traitlets import default +from traitlets import Bool, default from ..app import JupyterHub from ..auth import PAMAuthenticator @@ -282,12 +282,15 @@ class MockHub(JupyterHub): self.db.expire(service) return super().init_services() + clean_users = Bool(True) + def init_db(self): """Ensure we start with a clean user list""" super().init_db() - for user in self.db.query(orm.User): - self.db.delete(user) - self.db.commit() + if self.clean_users: + for user in self.db.query(orm.User): + self.db.delete(user) + self.db.commit() @gen.coroutine def initialize(self, argv=None): diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 759ccafe..87001f2c 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -206,7 +206,7 @@ def test_resume_spawners(tmpdir, request): ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: kwargs['internal_certs_location'] = str(tmpdir) - app = MockHub(**kwargs) + app = MockHub(clean_users=False, **kwargs) app.config.ConfigurableHTTPProxy.should_start = False app.config.ConfigurableHTTPProxy.auth_token = 'unused' yield app.initialize([]) From e3852141213249650de0920c82c225f87f18f3fa Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 17 Oct 2018 13:04:42 +0200 Subject: [PATCH 67/68] empty groups, too --- jupyterhub/tests/mocking.py | 6 ++++-- jupyterhub/tests/test_app.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 368d3cf4..2c553e34 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -282,14 +282,16 @@ class MockHub(JupyterHub): self.db.expire(service) return super().init_services() - clean_users = Bool(True) + test_clean_db = Bool(True) def init_db(self): """Ensure we start with a clean user list""" super().init_db() - if self.clean_users: + if self.test_clean_db: for user in self.db.query(orm.User): self.db.delete(user) + for group in self.db.query(orm.Group): + self.db.delete(group) self.db.commit() @gen.coroutine diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 87001f2c..c758489d 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -206,7 +206,7 @@ def test_resume_spawners(tmpdir, request): ssl_enabled = getattr(request.module, "ssl_enabled", False) if ssl_enabled: kwargs['internal_certs_location'] = str(tmpdir) - app = MockHub(clean_users=False, **kwargs) + app = MockHub(test_clean_db=False, **kwargs) app.config.ConfigurableHTTPProxy.should_start = False app.config.ConfigurableHTTPProxy.auth_token = 'unused' yield app.initialize([]) From 7a055e65db9d0323dc43b98aedcc6d9faea76881 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 17 Oct 2018 13:05:20 +0200 Subject: [PATCH 68/68] Catch and print errors stopping hub in case it failed to fully start --- jupyterhub/tests/conftest.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 47d413da..e44dd60c 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -31,12 +31,14 @@ import asyncio from getpass import getuser import logging import os -from pytest import fixture, raises +import sys from subprocess import TimeoutExpired +from unittest import mock + +from pytest import fixture, raises from tornado import ioloop, gen from tornado.httpclient import HTTPError from tornado.platform.asyncio import AsyncIOMainLoop -from unittest import mock from .. import orm from .. import crypto @@ -82,7 +84,10 @@ def app(request, io_loop, ssl_tmpdir): # disconnect logging during cleanup because pytest closes captured FDs prematurely mocked_app.log.handlers = [] MockHub.clear_instance() - mocked_app.stop() + try: + mocked_app.stop() + except Exception as e: + print("Error stopping Hub: %s" % e, file=sys.stderr) request.addfinalizer(fin) io_loop.run_sync(make_app)