From c46beb976acfce7da9025de3797184b1f4ff22e5 Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Thu, 19 Nov 2020 11:59:03 +0100 Subject: [PATCH 01/15] Moving ssl tests to testing matrix --- .github/workflows/test.yml | 8 ++++++++ jupyterhub/tests/conftest.py | 6 +++++- jupyterhub/tests/test_internal_ssl_api.py | 6 ------ jupyterhub/tests/test_internal_ssl_app.py | 7 ------- jupyterhub/tests/test_internal_ssl_spawner.py | 6 ------ 5 files changed, 13 insertions(+), 20 deletions(-) delete mode 100644 jupyterhub/tests/test_internal_ssl_api.py delete mode 100644 jupyterhub/tests/test_internal_ssl_app.py delete mode 100644 jupyterhub/tests/test_internal_ssl_spawner.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 671bf5d6..2255fc78 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -77,6 +77,10 @@ jobs: # Tests everything when the user instances are started with # jupyter_server instead of notebook. # + # ssl: + # Tests everything using internal SSL connections instead of + # unencrypted HTTP + # # main_dependencies: # Tests everything when the we use the latest available dependencies # from: ipytraitlets. @@ -91,6 +95,7 @@ jobs: db: mysql - python: "3.8" db: postgres + ssl: enabled - python: "3.8" jupyter_server: jupyter_server - python: "3.9" @@ -109,6 +114,9 @@ jobs: echo "MYSQL_HOST=127.0.0.1" >> $GITHUB_ENV echo "JUPYTERHUB_TEST_DB_URL=mysql+mysqlconnector://root@127.0.0.1:3306/jupyterhub" >> $GITHUB_ENV fi + if [ "${{ matrix.ssl }}" == "enabled" ]; then + echo "SSL_ENABLED=1" >> $GITHUB_ENV + fi if [ "${{ matrix.db }}" == "postgres" ]; then echo "PGHOST=127.0.0.1" >> $GITHUB_ENV echo "PGUSER=test_user" >> $GITHUB_ENV diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 7126baa7..000de730 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -74,7 +74,11 @@ def ssl_tmpdir(tmpdir_factory): def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" mocked_app = None - ssl_enabled = getattr(request.module, "ssl_enabled", False) + if 'SSL_ENABLED' in os.environ: + ssl_env_var = bool(os.environ['SSL_ENABLED']) + else: + ssl_env_var = False + ssl_enabled = getattr(request.module, "ssl_enabled", ssl_env_var) kwargs = dict() if ssl_enabled: kwargs.update(dict(internal_ssl=True, internal_certs_location=str(ssl_tmpdir))) diff --git a/jupyterhub/tests/test_internal_ssl_api.py b/jupyterhub/tests/test_internal_ssl_api.py deleted file mode 100644 index 17349ae6..00000000 --- a/jupyterhub/tests/test_internal_ssl_api.py +++ /dev/null @@ -1,6 +0,0 @@ -"""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 deleted file mode 100644 index cae0519b..00000000 --- a/jupyterhub/tests/test_internal_ssl_app.py +++ /dev/null @@ -1,7 +0,0 @@ -"""Test the JupyterHub entry point with internal ssl""" -# Copyright (c) Jupyter Development Team. -# Distributed under the terms of the Modified BSD License. -import jupyterhub.tests.mocking -from jupyterhub.tests.test_app import * - -ssl_enabled = True diff --git a/jupyterhub/tests/test_internal_ssl_spawner.py b/jupyterhub/tests/test_internal_ssl_spawner.py deleted file mode 100644 index 85ea5ccd..00000000 --- a/jupyterhub/tests/test_internal_ssl_spawner.py +++ /dev/null @@ -1,6 +0,0 @@ -"""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 From 4862831f71a207cdbc830b58facc0dd93ac33d65 Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Thu, 19 Nov 2020 12:08:10 +0100 Subject: [PATCH 02/15] Trying with different configuration --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2255fc78..c314bcf1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -93,9 +93,10 @@ jobs: subdomain: subdomain - python: "3.7" db: mysql + - python: "3.7" + ssl: ssl_enabled - python: "3.8" db: postgres - ssl: enabled - python: "3.8" jupyter_server: jupyter_server - python: "3.9" @@ -114,7 +115,7 @@ jobs: echo "MYSQL_HOST=127.0.0.1" >> $GITHUB_ENV echo "JUPYTERHUB_TEST_DB_URL=mysql+mysqlconnector://root@127.0.0.1:3306/jupyterhub" >> $GITHUB_ENV fi - if [ "${{ matrix.ssl }}" == "enabled" ]; then + if [ "${{ matrix.ssl }}" == "ssl_enabled" ]; then echo "SSL_ENABLED=1" >> $GITHUB_ENV fi if [ "${{ matrix.db }}" == "postgres" ]; then From 313f050c424d85e20de7144942b56e1e73f726a3 Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Thu, 19 Nov 2020 12:58:38 +0100 Subject: [PATCH 03/15] Reduced ssl on for active tests only --- .github/workflows/test.yml | 4 ++-- jupyterhub/tests/conftest.py | 6 +----- jupyterhub/tests/test_api.py | 3 ++- jupyterhub/tests/test_app.py | 2 ++ jupyterhub/tests/test_spawner.py | 3 +++ 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c314bcf1..d1bbbc93 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -94,7 +94,7 @@ jobs: - python: "3.7" db: mysql - python: "3.7" - ssl: ssl_enabled + ssl: ssl - python: "3.8" db: postgres - python: "3.8" @@ -115,7 +115,7 @@ jobs: echo "MYSQL_HOST=127.0.0.1" >> $GITHUB_ENV echo "JUPYTERHUB_TEST_DB_URL=mysql+mysqlconnector://root@127.0.0.1:3306/jupyterhub" >> $GITHUB_ENV fi - if [ "${{ matrix.ssl }}" == "ssl_enabled" ]; then + if [ "${{ matrix.ssl }}" == "ssl" ]; then echo "SSL_ENABLED=1" >> $GITHUB_ENV fi if [ "${{ matrix.db }}" == "postgres" ]; then diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 000de730..7126baa7 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -74,11 +74,7 @@ def ssl_tmpdir(tmpdir_factory): def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" mocked_app = None - if 'SSL_ENABLED' in os.environ: - ssl_env_var = bool(os.environ['SSL_ENABLED']) - else: - ssl_env_var = False - ssl_enabled = getattr(request.module, "ssl_enabled", ssl_env_var) + ssl_enabled = getattr(request.module, "ssl_enabled", False) kwargs = dict() if ssl_enabled: kwargs.update(dict(internal_ssl=True, internal_certs_location=str(ssl_tmpdir))) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index fa6a20d1..7bedbb2c 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1,5 +1,6 @@ """Tests for the REST API.""" import json +import os import re import sys import uuid @@ -27,7 +28,7 @@ from .utils import async_requests from .utils import auth_header from .utils import find_user - +ssl_enabled = os.environ.get('SSL_ENABLED', False) # -------------------- # Authentication tests # -------------------- diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index accdd430..47d23a6a 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -20,6 +20,8 @@ from ..app import JupyterHub from .mocking import MockHub from .test_api import add_user +ssl_enabled = os.environ.get('SSL_ENABLED', False) + def test_help_all(): out = check_output([sys.executable, '-m', 'jupyterhub', '--help-all']).decode( diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index 99b84393..d7bc1301 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -41,6 +41,9 @@ while True: except KeyboardInterrupt: print("interrupted") """ +import os + +ssl_enabled = os.environ.get('SSL_ENABLED', False) def setup(): From 09ff03ca4ff00784f7c2a6174e8629be134846a8 Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Thu, 19 Nov 2020 13:10:48 +0100 Subject: [PATCH 04/15] Superfluous import statement --- jupyterhub/tests/test_spawner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index d7bc1301..c98cd9de 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -41,7 +41,6 @@ while True: except KeyboardInterrupt: print("interrupted") """ -import os ssl_enabled = os.environ.get('SSL_ENABLED', False) From 0472ef05331dad790314d1cb83f6ddbc8571da5b Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Fri, 20 Nov 2020 15:27:50 +0100 Subject: [PATCH 05/15] Central internal_ssl switch --- jupyterhub/tests/conftest.py | 4 +++- jupyterhub/tests/test_api.py | 2 -- jupyterhub/tests/test_app.py | 2 -- jupyterhub/tests/test_spawner.py | 2 -- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 7126baa7..96e9a827 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -74,7 +74,9 @@ def ssl_tmpdir(tmpdir_factory): def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" mocked_app = None - ssl_enabled = getattr(request.module, "ssl_enabled", False) + ssl_enabled = request.module.getattr( + 'ssl_enabled', os.environ.get('SSL_ENABLED', False) + ) kwargs = dict() if ssl_enabled: kwargs.update(dict(internal_ssl=True, internal_certs_location=str(ssl_tmpdir))) diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index 7bedbb2c..2ee3c5e9 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -1,6 +1,5 @@ """Tests for the REST API.""" import json -import os import re import sys import uuid @@ -28,7 +27,6 @@ from .utils import async_requests from .utils import auth_header from .utils import find_user -ssl_enabled = os.environ.get('SSL_ENABLED', False) # -------------------- # Authentication tests # -------------------- diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 47d23a6a..accdd430 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -20,8 +20,6 @@ from ..app import JupyterHub from .mocking import MockHub from .test_api import add_user -ssl_enabled = os.environ.get('SSL_ENABLED', False) - def test_help_all(): out = check_output([sys.executable, '-m', 'jupyterhub', '--help-all']).decode( diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index c98cd9de..99b84393 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -42,8 +42,6 @@ while True: print("interrupted") """ -ssl_enabled = os.environ.get('SSL_ENABLED', False) - def setup(): logging.basicConfig(level=logging.DEBUG) From 164447717fc27cbd6d1cef74b0acf6d0e944529c Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Fri, 20 Nov 2020 15:30:23 +0100 Subject: [PATCH 06/15] Fix formulation --- jupyterhub/tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyterhub/tests/conftest.py b/jupyterhub/tests/conftest.py index 96e9a827..f4cad6cf 100644 --- a/jupyterhub/tests/conftest.py +++ b/jupyterhub/tests/conftest.py @@ -74,8 +74,8 @@ def ssl_tmpdir(tmpdir_factory): def app(request, io_loop, ssl_tmpdir): """Mock a jupyterhub app for testing""" mocked_app = None - ssl_enabled = request.module.getattr( - 'ssl_enabled', os.environ.get('SSL_ENABLED', False) + ssl_enabled = getattr( + request.module, 'ssl_enabled', os.environ.get('SSL_ENABLED', False) ) kwargs = dict() if ssl_enabled: From e02345a4e8b0ae889fda4eca0cc66783e490fc85 Mon Sep 17 00:00:00 2001 From: Omar Richardson Date: Thu, 26 Nov 2020 09:24:44 +0100 Subject: [PATCH 07/15] WIP: Moved ssl options to new method --- jupyterhub/proxy.py | 55 ++++++++++++++++++---------------- jupyterhub/tests/test_proxy.py | 18 +++++++++-- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 5b8d386a..654a7bdf 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -557,6 +557,34 @@ class ConfigurableHTTPProxy(Proxy): else: raise RuntimeError("Failed to stop proxy at pid=%s", pid) + def _append_ssl_options(self, cmd): + if self.ssl_key: + cmd.extend(['--ssl-key', self.ssl_key]) + if self.ssl_cert: + cmd.extend(['--ssl-cert', self.ssl_cert]) + if self.app.internal_ssl: + proxy_api = 'proxy-api' + proxy_client = 'proxy-client' + 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', 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']) + def _write_pid_file(self): """write pid for proxy to a file""" self.log.debug("Writing proxy pid file: %s", self.pid_file) @@ -600,32 +628,7 @@ class ConfigurableHTTPProxy(Proxy): cmd.append('--host-routing') if self.debug: cmd.extend(['--log-level', 'debug']) - if self.ssl_key: - cmd.extend(['--ssl-key', self.ssl_key]) - if self.ssl_cert: - cmd.extend(['--ssl-cert', self.ssl_cert]) - if self.app.internal_ssl: - proxy_api = 'proxy-api' - proxy_client = 'proxy-client' - 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', 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']) + self._append_ssl_options(cmd) if self.app.statsd_host: cmd.extend( [ diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index e912bc62..85800b59 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -27,7 +27,7 @@ def disable_check_routes(app): app.last_activity_callback.start() -async def test_external_proxy(request): +async def test_external_proxy(request, tmpdir_factory): auth_token = 'secret!' proxy_ip = '127.0.0.1' proxy_port = 54321 @@ -35,8 +35,16 @@ async def test_external_proxy(request): cfg.ConfigurableHTTPProxy.auth_token = auth_token cfg.ConfigurableHTTPProxy.api_url = 'http://%s:%i' % (proxy_ip, proxy_port) cfg.ConfigurableHTTPProxy.should_start = False - - app = MockHub.instance(config=cfg) + internal_ssl = os.environ.get('SSL_ENABLED', False) + kwargs = {'config': cfg} + if internal_ssl: + kwargs.update( + dict( + internal_ssl=True, + internal_certs_location=str(tmpdir_factory.mktemp('ssl')), + ) + ) + app = MockHub.instance(**kwargs) # disable last_activity polling to avoid check_routes being called during the test, # which races with some of our test conditions app.last_activity_interval = 0 @@ -62,6 +70,10 @@ async def test_external_proxy(request): str(proxy_port), '--log-level=debug', ] + if app.internal_ssl: + cfg.ConfigurableHTTPProxy._append_ssl_options(cmd) + app.log.warn("Doing stuff here") + app.log.warn(cmd) if app.subdomain_host: cmd.append('--host-routing') proxy = Popen(cmd, env=env) From 40176a667f61fc59e211332af31499b4e022865b Mon Sep 17 00:00:00 2001 From: 0mar Date: Thu, 26 Nov 2020 12:22:43 +0100 Subject: [PATCH 08/15] Attempt to patch proxy, unsuccessful --- jupyterhub/tests/test_proxy.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index 85800b59..2038d4f0 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -38,13 +38,11 @@ async def test_external_proxy(request, tmpdir_factory): internal_ssl = os.environ.get('SSL_ENABLED', False) kwargs = {'config': cfg} if internal_ssl: - kwargs.update( - dict( - internal_ssl=True, - internal_certs_location=str(tmpdir_factory.mktemp('ssl')), - ) - ) + kwargs['internal_ssl'] = True + kwargs['internal_certs_location'] = str(tmpdir_factory.mktemp('ssl')) + app = MockHub.instance(**kwargs) + app.proxy = app.proxy_class(app=app) # disable last_activity polling to avoid check_routes being called during the test, # which races with some of our test conditions app.last_activity_interval = 0 @@ -71,12 +69,12 @@ async def test_external_proxy(request, tmpdir_factory): '--log-level=debug', ] if app.internal_ssl: - cfg.ConfigurableHTTPProxy._append_ssl_options(cmd) + app.proxy._append_ssl_options(cmd) app.log.warn("Doing stuff here") app.log.warn(cmd) if app.subdomain_host: cmd.append('--host-routing') - proxy = Popen(cmd, env=env) + proxy = Popen(cmd, env=env) # Todo: Update test to use proxy instance def _cleanup_proxy(): if proxy.poll() is None: From 2a06c8a94c3e04361b8af7615714cebf8c831274 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 27 Nov 2020 13:26:32 +0100 Subject: [PATCH 09/15] WIP: Attempt to access SSL parameters, failing due to self-signed certificate error --- jupyterhub/proxy.py | 35 ++++++------------------------- jupyterhub/tests/test_proxy.py | 29 ++++++++++++++++--------- jupyterhub/tests/test_services.py | 13 +++++++++++- jupyterhub/utils.py | 29 +++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 40 deletions(-) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 654a7bdf..ff9b8f53 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -44,6 +44,7 @@ from .metrics import CHECK_ROUTES_DURATION_SECONDS from .metrics import PROXY_POLL_DURATION_SECONDS from .objects import Server from .utils import exponential_backoff +from .utils import get_ssl_options from .utils import make_ssl_context from .utils import url_path_join from jupyterhub.traitlets import Command @@ -557,34 +558,6 @@ class ConfigurableHTTPProxy(Proxy): else: raise RuntimeError("Failed to stop proxy at pid=%s", pid) - def _append_ssl_options(self, cmd): - if self.ssl_key: - cmd.extend(['--ssl-key', self.ssl_key]) - if self.ssl_cert: - cmd.extend(['--ssl-cert', self.ssl_cert]) - if self.app.internal_ssl: - proxy_api = 'proxy-api' - proxy_client = 'proxy-client' - 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', 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']) - def _write_pid_file(self): """write pid for proxy to a file""" self.log.debug("Writing proxy pid file: %s", self.pid_file) @@ -628,7 +601,11 @@ class ConfigurableHTTPProxy(Proxy): cmd.append('--host-routing') if self.debug: cmd.extend(['--log-level', 'debug']) - self._append_ssl_options(cmd) + if self.ssl_key: + cmd.extend(['--ssl-key', self.ssl_key]) + if self.ssl_cert: + cmd.extend(['--ssl-cert', self.ssl_cert]) + cmd += get_ssl_options(self.app) if self.app.statsd_host: cmd.extend( [ diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index 2038d4f0..b894644e 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -10,6 +10,7 @@ import pytest from traitlets.config import Config from .. import orm +from ..utils import get_ssl_options from ..utils import url_path_join as ujoin from ..utils import wait_for_http_server from .mocking import MockHub @@ -33,25 +34,27 @@ async def test_external_proxy(request, tmpdir_factory): proxy_port = 54321 cfg = Config() cfg.ConfigurableHTTPProxy.auth_token = auth_token - cfg.ConfigurableHTTPProxy.api_url = 'http://%s:%i' % (proxy_ip, proxy_port) - cfg.ConfigurableHTTPProxy.should_start = False internal_ssl = os.environ.get('SSL_ENABLED', False) + protocol = 'http' kwargs = {'config': cfg} if internal_ssl: kwargs['internal_ssl'] = True kwargs['internal_certs_location'] = str(tmpdir_factory.mktemp('ssl')) - + protocol = 'https' + cfg.ConfigurableHTTPProxy.api_url = '%s://%s:%i' % (protocol, proxy_ip, proxy_port) + cfg.ConfigurableHTTPProxy.should_start = False app = MockHub.instance(**kwargs) - app.proxy = app.proxy_class(app=app) # disable last_activity polling to avoid check_routes being called during the test, # which races with some of our test conditions app.last_activity_interval = 0 def fin(): MockHub.clear_instance() + app.log.warn("Calling finalizer") app.http_server.stop() request.addfinalizer(fin) + await app.initialize([]) # configures and starts proxy process env = os.environ.copy() @@ -69,12 +72,17 @@ async def test_external_proxy(request, tmpdir_factory): '--log-level=debug', ] if app.internal_ssl: - app.proxy._append_ssl_options(cmd) + # app.internal_proxy_certs + cmd += get_ssl_options(app) app.log.warn("Doing stuff here") app.log.warn(cmd) if app.subdomain_host: cmd.append('--host-routing') + app.log.warn("Finished setting up command") + proxy = Popen(cmd, env=env) # Todo: Update test to use proxy instance + app.log.warn(proxy.communicate()) + app.log.warn("Defined proxy") def _cleanup_proxy(): if proxy.poll() is None: @@ -84,12 +92,13 @@ async def test_external_proxy(request, tmpdir_factory): request.addfinalizer(_cleanup_proxy) def wait_for_proxy(): - return wait_for_http_server('http://%s:%i' % (proxy_ip, proxy_port)) + return wait_for_http_server('%s://%s:%i' % (protocol, proxy_ip, proxy_port)) + + app.log.warn("Starting to wait for proxy") await wait_for_proxy() - - await app.initialize([]) await app.start() + app.log.warn("Have started app") assert app.proxy.proxy_process is None # test if api service has a root route '/' @@ -149,7 +158,7 @@ async def test_external_proxy(request, tmpdir_factory): '--api-port', str(proxy_port), '--default-target', - 'http://%s:%i' % (app.hub_ip, app.hub_port), + '%s://%s:%i' % (protocol, app.hub_ip, app.hub_port), ] if app.subdomain_host: cmd.append('--host-routing') @@ -157,7 +166,7 @@ async def test_external_proxy(request, tmpdir_factory): await wait_for_proxy() # tell the hub where the new proxy is - new_api_url = 'http://{}:{}'.format(proxy_ip, proxy_port) + new_api_url = '{}://{}:{}'.format(protocol, proxy_ip, proxy_port) r = await api_request( app, 'proxy', diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index 127a9f45..fcb2d1ce 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -11,6 +11,7 @@ from async_generator import asynccontextmanager from async_generator import yield_ from tornado.ioloop import IOLoop +from ..utils import make_ssl_context from ..utils import maybe_future from ..utils import random_port from ..utils import url_path_join @@ -33,8 +34,18 @@ async def external_service(app, name='mockservice'): 'JUPYTERHUB_SERVICE_URL': 'http://127.0.0.1:%i' % random_port(), } proc = Popen(mockservice_cmd, env=env) + ssl_context = None + if app.internal_ssl: + ssl_context = make_ssl_context( + app.internal_ssl_key, + app.internal_ssl_cert, + cafile=app.internal_ssl_ca, + check_hostname=False, + ) try: - await wait_for_http_server(env['JUPYTERHUB_SERVICE_URL']) + await wait_for_http_server( + env['JUPYTERHUB_SERVICE_URL'], ssl_context=ssl_context + ) await yield_(env) finally: proc.terminate() diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index 73235914..ab648bc9 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -345,6 +345,35 @@ def compare_token(compare, token): return False +def get_ssl_options(app): + cmd = [] + if app.internal_ssl: + proxy_api = 'proxy-api' + proxy_client = 'proxy-client' + api_key = app.internal_proxy_certs[proxy_api][ + 'keyfile' + ] # Check content in next test and just patch manulaly or in the config of the file + api_cert = app.internal_proxy_certs[proxy_api]['certfile'] + api_ca = app.internal_trust_bundles[proxy_api + '-ca'] + + client_key = app.internal_proxy_certs[proxy_client]['keyfile'] + client_cert = app.internal_proxy_certs[proxy_client]['certfile'] + client_ca = 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', 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']) + return cmd + + def url_path_join(*pieces): """Join components of url into a relative url. From 184d87ff2a76684dfc151ac276f9a7a835f86e9c Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 27 Nov 2020 17:00:09 +0100 Subject: [PATCH 10/15] Skip SSL-free tests if not on SSL matrix --- jupyterhub/tests/test_proxy.py | 3 ++- jupyterhub/tests/test_services.py | 3 +++ jupyterhub/tests/utils.py | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index e912bc62..60f2d0e0 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -9,12 +9,12 @@ from urllib.parse import urlparse import pytest from traitlets.config import Config -from .. import orm from ..utils import url_path_join as ujoin from ..utils import wait_for_http_server from .mocking import MockHub from .test_api import add_user from .test_api import api_request +from .utils import skip_if_ssl @pytest.fixture @@ -27,6 +27,7 @@ def disable_check_routes(app): app.last_activity_callback.start() +@skip_if_ssl async def test_external_proxy(request): auth_token = 'secret!' proxy_ip = '127.0.0.1' diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index 127a9f45..22ee175b 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -17,6 +17,7 @@ from ..utils import url_path_join from ..utils import wait_for_http_server from .mocking import public_url from .utils import async_requests +from .utils import skip_if_ssl mockservice_path = os.path.dirname(os.path.abspath(__file__)) mockservice_py = os.path.join(mockservice_path, 'mockservice.py') @@ -63,6 +64,7 @@ async def test_managed_service(mockservice): assert service.proc.poll() is None +@skip_if_ssl async def test_proxy_service(app, mockservice_url): service = mockservice_url name = service.name @@ -76,6 +78,7 @@ async def test_proxy_service(app, mockservice_url): assert r.text.endswith(path) +@skip_if_ssl async def test_external_service(app): name = 'external' async with external_service(app, name=name) as env: diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py index 09aeb196..2c8210b2 100644 --- a/jupyterhub/tests/utils.py +++ b/jupyterhub/tests/utils.py @@ -1,6 +1,8 @@ import asyncio +import os from concurrent.futures import ThreadPoolExecutor +import pytest import requests from certipy import Certipy @@ -52,6 +54,9 @@ def ssl_setup(cert_dir, authority_name): return external_certs +skip_if_ssl = pytest.mark.skipif(os.environ.get('SSL_ENABLED', False)) + + def check_db_locks(func): """Decorator that verifies no locks are held on database upon exit. From 74766e4786e3dd13b34377edef37bad3ffc4f0f7 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 27 Nov 2020 17:18:40 +0100 Subject: [PATCH 11/15] Resolving merge conflichts --- jupyterhub/tests/test_services.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/jupyterhub/tests/test_services.py b/jupyterhub/tests/test_services.py index 1e7982ee..22ee175b 100644 --- a/jupyterhub/tests/test_services.py +++ b/jupyterhub/tests/test_services.py @@ -11,7 +11,6 @@ from async_generator import asynccontextmanager from async_generator import yield_ from tornado.ioloop import IOLoop -from ..utils import make_ssl_context from ..utils import maybe_future from ..utils import random_port from ..utils import url_path_join @@ -35,18 +34,8 @@ async def external_service(app, name='mockservice'): 'JUPYTERHUB_SERVICE_URL': 'http://127.0.0.1:%i' % random_port(), } proc = Popen(mockservice_cmd, env=env) - ssl_context = None - if app.internal_ssl: - ssl_context = make_ssl_context( - app.internal_ssl_key, - app.internal_ssl_cert, - cafile=app.internal_ssl_ca, - check_hostname=False, - ) try: - await wait_for_http_server( - env['JUPYTERHUB_SERVICE_URL'], ssl_context=ssl_context - ) + await wait_for_http_server(env['JUPYTERHUB_SERVICE_URL']) await yield_(env) finally: proc.terminate() From 2cfe4474ac6e68d87bd3ced895e6ad44a71bea98 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 27 Nov 2020 17:26:44 +0100 Subject: [PATCH 12/15] Submitting reason for skiptest --- jupyterhub/tests/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py index 2c8210b2..7089dafc 100644 --- a/jupyterhub/tests/utils.py +++ b/jupyterhub/tests/utils.py @@ -54,7 +54,9 @@ def ssl_setup(cert_dir, authority_name): return external_certs -skip_if_ssl = pytest.mark.skipif(os.environ.get('SSL_ENABLED', False)) +skip_if_ssl = pytest.mark.skipif( + os.environ.get('SSL_ENABLED', False), reason="Does not use internal SSL" +) def check_db_locks(func): From cc73ab711e4db5caff9bb807e8ad7aba8bb1b037 Mon Sep 17 00:00:00 2001 From: 0mar Date: Fri, 27 Nov 2020 17:50:47 +0100 Subject: [PATCH 13/15] Disabled ssl testing --- jupyterhub/tests/test_services_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jupyterhub/tests/test_services_auth.py b/jupyterhub/tests/test_services_auth.py index 867d1d97..fb657385 100644 --- a/jupyterhub/tests/test_services_auth.py +++ b/jupyterhub/tests/test_services_auth.py @@ -35,6 +35,7 @@ from .utils import AsyncSession # mock for sending monotonic counter way into the future monotonic_future = mock.patch('time.monotonic', lambda: sys.maxsize) +ssl_enabled = False def test_expiring_dict(): From 671ef0d5ef8f93019b47ae78ba02b987c51f6bda Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 1 Dec 2020 10:30:44 +0100 Subject: [PATCH 14/15] Moved ssl options to proxy --- jupyterhub/proxy.py | 32 ++++++++++++++++++++++++++++++-- jupyterhub/utils.py | 29 ----------------------------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index ff9b8f53..b8d7886c 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -44,7 +44,6 @@ from .metrics import CHECK_ROUTES_DURATION_SECONDS from .metrics import PROXY_POLL_DURATION_SECONDS from .objects import Server from .utils import exponential_backoff -from .utils import get_ssl_options from .utils import make_ssl_context from .utils import url_path_join from jupyterhub.traitlets import Command @@ -575,6 +574,34 @@ class ConfigurableHTTPProxy(Proxy): self.log.debug("PID file %s already removed", self.pid_file) pass + def _get_ssl_options(self): + """List of cmd proxy options to use internal SSL""" + cmd = [] + proxy_api = 'proxy-api' + proxy_client = 'proxy-client' + api_key = self.app.internal_proxy_certs[proxy_api][ + 'keyfile' + ] # Check content in next test and just patch manulaly or in the config of the file + 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', 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']) + return cmd + async def start(self): """Start the proxy process""" # check if there is a previous instance still around @@ -605,7 +632,8 @@ class ConfigurableHTTPProxy(Proxy): cmd.extend(['--ssl-key', self.ssl_key]) if self.ssl_cert: cmd.extend(['--ssl-cert', self.ssl_cert]) - cmd += get_ssl_options(self.app) + if self.app.internal_ssl: + cmd.extend(self._get_ssl_options()) if self.app.statsd_host: cmd.extend( [ diff --git a/jupyterhub/utils.py b/jupyterhub/utils.py index ab648bc9..73235914 100644 --- a/jupyterhub/utils.py +++ b/jupyterhub/utils.py @@ -345,35 +345,6 @@ def compare_token(compare, token): return False -def get_ssl_options(app): - cmd = [] - if app.internal_ssl: - proxy_api = 'proxy-api' - proxy_client = 'proxy-client' - api_key = app.internal_proxy_certs[proxy_api][ - 'keyfile' - ] # Check content in next test and just patch manulaly or in the config of the file - api_cert = app.internal_proxy_certs[proxy_api]['certfile'] - api_ca = app.internal_trust_bundles[proxy_api + '-ca'] - - client_key = app.internal_proxy_certs[proxy_client]['keyfile'] - client_cert = app.internal_proxy_certs[proxy_client]['certfile'] - client_ca = 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', 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']) - return cmd - - def url_path_join(*pieces): """Join components of url into a relative url. From 970b25d017a2816260b7c40efcd2d37f0ccf32c7 Mon Sep 17 00:00:00 2001 From: 0mar Date: Tue, 1 Dec 2020 10:49:10 +0100 Subject: [PATCH 15/15] Added docstrings --- jupyterhub/tests/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jupyterhub/tests/utils.py b/jupyterhub/tests/utils.py index 7089dafc..85286b60 100644 --- a/jupyterhub/tests/utils.py +++ b/jupyterhub/tests/utils.py @@ -54,6 +54,7 @@ def ssl_setup(cert_dir, authority_name): return external_certs +"""Skip tests that don't work under internal-ssl when testing under internal-ssl""" skip_if_ssl = pytest.mark.skipif( os.environ.get('SSL_ENABLED', False), reason="Does not use internal SSL" )