From 9b0248e0c3afe95855ec126855ed8450cc7e05fb Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Wed, 12 Sep 2018 16:46:59 -0700 Subject: [PATCH 1/6] Normalize unix usernames to be under 32char JupyterHub usernames do not have this restriction, causing user creation to fail when usernames are too large. --- tests/test_normalize.py | 24 ++++++++++++++++++++++++ tljh/jupyterhub_config.py | 15 ++++++++++----- tljh/normalize.py | 24 ++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 tests/test_normalize.py create mode 100644 tljh/normalize.py diff --git a/tests/test_normalize.py b/tests/test_normalize.py new file mode 100644 index 0000000..9676365 --- /dev/null +++ b/tests/test_normalize.py @@ -0,0 +1,24 @@ +""" +Test functions for normalizing various kinds of values +""" +from tljh.normalize import generate_system_username + + +def test_generate_username(): + """ + Test generating system usernames from hub usernames + """ + usernames = { + # Very short + 'jupyter-test': 'jupyter-test', + # Very long + 'jupyter-aelie9sohjeequ9iemeipuimuoshahz4aitugiuteeg4ohioh5yuiha6aei7te5z': 'jupyter-aelie9sohjeequ9iem-4b726', + # 26 characters, just below our cutoff for hashing + 'jupyter-abcdefghijklmnopq': 'jupyter-abcdefghijklmnopq', + # 27 characters, just above our cutoff for hashing + 'jupyter-abcdefghijklmnopqr': 'jupyter-abcdefghijklmnopqr-e375e', + + } + for hub_user, system_user in usernames.items(): + assert generate_system_username(hub_user) == system_user + assert len(system_user) <= 32 \ No newline at end of file diff --git a/tljh/jupyterhub_config.py b/tljh/jupyterhub_config.py index 5ad957b..75b52d6 100644 --- a/tljh/jupyterhub_config.py +++ b/tljh/jupyterhub_config.py @@ -7,17 +7,23 @@ import yaml from glob import glob from systemdspawner import SystemdSpawner -from tljh import user, configurer +from tljh import configurer, user from tljh.config import INSTALL_PREFIX, USER_ENV_PREFIX, CONFIG_DIR +from tljh.normalize import generate_system_username -class CustomSpawner(SystemdSpawner): +class UserCreatingSpawner(SystemdSpawner): + """ + SystemdSpawner with user creation on spawn. + + FIXME: Remove this somehow? + """ def start(self): """ Perform system user activities before starting server """ # FIXME: Move this elsewhere? Into the Authenticator? - system_username = 'jupyter-' + self.user.name + system_username = generate_system_username('jupyter-' + self.user.name) user.ensure_user(system_username) user.ensure_user_group(system_username, 'jupyterhub-users') if self.user.admin: @@ -26,8 +32,7 @@ class CustomSpawner(SystemdSpawner): user.remove_user_group(system_username, 'jupyterhub-admins') return super().start() - -c.JupyterHub.spawner_class = CustomSpawner +c.JupyterHub.spawner_class = UserCreatingSpawner # leave users running when the Hub restarts c.JupyterHub.cleanup_servers = False diff --git a/tljh/normalize.py b/tljh/normalize.py new file mode 100644 index 0000000..bba2b41 --- /dev/null +++ b/tljh/normalize.py @@ -0,0 +1,24 @@ +""" +Functions to normalize various inputs +""" +import hashlib + + +def generate_system_username(username): + """ + Generate a posix username from given username. + + If username < 26 char, we just return it. + Else, we hash the username, truncate username at + 26 char, append a '-' and first add 5char of hash. + This makes sure our usernames are always under 32char. + """ + + if len(username) < 26: + return username + + userhash = hashlib.sha256(username.encode('utf-8')).hexdigest() + return '{username_trunc}-{hash}'.format( + username_trunc=username[:26], + hash=userhash[:5] + ) \ No newline at end of file From 37ea7eaa6f9a61d34f1a8581615db55027d4337c Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Wed, 12 Sep 2018 16:48:55 -0700 Subject: [PATCH 2/6] Add docs explaining truncation of username --- docs/topic/security.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/topic/security.rst b/docs/topic/security.rst index 6734205..dd6a508 100644 --- a/docs/topic/security.rst +++ b/docs/topic/security.rst @@ -18,7 +18,10 @@ permissions. #. The unix user account created for a JupyterHub user named ```` is ``jupyter-``. This prefix helps prevent clashes with users that already exist - otherwise a user named ``root`` can trivially gain full root - access to your server. + access to your server. If the username (including the ``jupyter-`` prefix) + is longer than 26 characters, it is truncated at 26 characters & a 5 charcter + hash is appeneded to it. This keeps usernames under the linux username limit + of 32 characters while also reducing chances of collision. #. A home directory is created for the user under ``/home/jupyter-``. From dffc3c09167048dcbd5f66dec2809dffe6bcd6f2 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Wed, 12 Sep 2018 16:52:48 -0700 Subject: [PATCH 3/6] Add integration test for long usernames --- integration-tests/test_hub.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/integration-tests/test_hub.py b/integration-tests/test_hub.py index 9d63380..a4e8704 100644 --- a/integration-tests/test_hub.py +++ b/integration-tests/test_hub.py @@ -8,6 +8,7 @@ import asyncio import pwd import grp import sys +from tljh.normalize import generate_system_username # Use sudo to invoke it, since this is how users invoke it. @@ -112,4 +113,31 @@ async def test_user_admin_remove(): await u.ensure_server() # Assert that the user does *not* have admin rights - assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem \ No newline at end of file + assert f'jupyter-{username}' in grp.getgrnam('jupyterhub-admins').gr_mem + + +@pytest.mark.asyncio +async def test_long_username(): + """ + User with a long name logs in, and we check if their name is properly truncated. + """ + # This *must* be localhost, not an IP + # aiohttp throws away cookies if we are connecting to an IP! + hub_url = 'http://localhost' + username = secrets.token_hex(32) + + assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'set', 'auth.type', 'dummyauthenticator.DummyAuthenticator')).wait() + assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'reload')).wait() + + # FIXME: wait for reload to finish & hub to come up + # Should be part of tljh-config reload + await asyncio.sleep(1) + async with User(username, hub_url, partial(login_dummy, password='')) as u: + await u.login() + await u.ensure_server() + + # Assert that the user exists + system_username = generate_system_username(f'jupyter-{username}') + assert pwd.getpwnam(system_username) is not None + + await u.stop_server() \ No newline at end of file From 9f538775e02c833b1267a52f774bb5283d0fc64f Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Fri, 14 Sep 2018 10:48:35 -0700 Subject: [PATCH 4/6] Print hub logs if hubtraf fails in integration tests --- integration-tests/test_hub.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/integration-tests/test_hub.py b/integration-tests/test_hub.py index a4e8704..30965d2 100644 --- a/integration-tests/test_hub.py +++ b/integration-tests/test_hub.py @@ -132,12 +132,21 @@ async def test_long_username(): # FIXME: wait for reload to finish & hub to come up # Should be part of tljh-config reload await asyncio.sleep(1) - async with User(username, hub_url, partial(login_dummy, password='')) as u: - await u.login() - await u.ensure_server() + try: + async with User(username, hub_url, partial(login_dummy, password='')) as u: + await u.login() + await u.ensure_server() - # Assert that the user exists - system_username = generate_system_username(f'jupyter-{username}') - assert pwd.getpwnam(system_username) is not None + # Assert that the user exists + system_username = generate_system_username(f'jupyter-{username}') + assert pwd.getpwnam(system_username) is not None - await u.stop_server() \ No newline at end of file + await u.stop_server() + except: + # If we have any errors, print jupyterhub logs before exiting + subprocess.check_call([ + 'journalctl', + '-u', 'jupyterhub', + '--no-pager' + ]) + raise \ No newline at end of file From f1378754b683a97dc457343e9892f0c9adedfb7a Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Fri, 14 Sep 2018 11:55:01 -0700 Subject: [PATCH 5/6] Import subprocess before using it --- integration-tests/test_hub.py | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/test_hub.py b/integration-tests/test_hub.py index 30965d2..1f3228e 100644 --- a/integration-tests/test_hub.py +++ b/integration-tests/test_hub.py @@ -8,6 +8,7 @@ import asyncio import pwd import grp import sys +import subprocess from tljh.normalize import generate_system_username From 4a3762d38d1352f2e89bdce510f147d3966d5510 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Fri, 14 Sep 2018 12:38:13 -0700 Subject: [PATCH 6/6] Pass in truncated username to systemdspawner --- tljh/jupyterhub_config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tljh/jupyterhub_config.py b/tljh/jupyterhub_config.py index 75b52d6..62c6db4 100644 --- a/tljh/jupyterhub_config.py +++ b/tljh/jupyterhub_config.py @@ -24,6 +24,9 @@ class UserCreatingSpawner(SystemdSpawner): """ # FIXME: Move this elsewhere? Into the Authenticator? system_username = generate_system_username('jupyter-' + self.user.name) + + # FIXME: This is a hack. Allow setting username directly instead + self.username_template = system_username user.ensure_user(system_username) user.ensure_user_group(system_username, 'jupyterhub-users') if self.user.admin: