From 0e2817c4be59650ab85c54c4cf8545afb2997915 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:08:55 -0700 Subject: [PATCH 01/13] Try specifying UsePrivilegeSeparation to No --- jupyter_sshd_proxy/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyter_sshd_proxy/__init__.py b/jupyter_sshd_proxy/__init__.py index a2c777d..724f70a 100644 --- a/jupyter_sshd_proxy/__init__.py +++ b/jupyter_sshd_proxy/__init__.py @@ -25,8 +25,10 @@ def setup_sshd() -> Dict[str, Any]: # Last login info is from /var/log/lastlog, which is transient in containerized systems '-o', 'PrintLastLog no', '-o', f'AuthorizedKeysFile {AUTHORIZED_KEYS_PATH}', - '-o', f'LogLevel {SSHD_LOG_LEVEL}' + '-o', f'LogLevel {SSHD_LOG_LEVEL}', + '-o', 'UsePrivilegeSeparation no', ] + print(shlex.join(cmd)) return { "command": cmd, From 006c0c1ae612d0b594758728b3c24541c462eeb2 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:12:55 -0700 Subject: [PATCH 02/13] Remove -v from test local commands --- tests/test_ssh.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 013c719..48aab95 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -55,7 +55,7 @@ def get_ssh_client_options(random_port, token, authorized_keys_path): def test_ssh_command_execution(jupyter_server): cmd = [ - 'ssh', '-v', + 'ssh', ] + [f"-o={o}" for o in get_ssh_client_options(*jupyter_server)] + ['127.0.0.1', 'hostname'] out = subprocess.check_output(cmd).decode().strip() @@ -65,7 +65,7 @@ def test_ssh_command_execution(jupyter_server): def test_ssh_interactive(jupyter_server): cmd = [ - 'ssh', '-v', + 'ssh', ] + [f"-o={o}" for o in get_ssh_client_options(*jupyter_server)] + ['127.0.0.1', 'hostname'] proc = pexpect.spawn(shlex.join(cmd), echo=False) From c409f350542326256f748a5dda65561b6f96064e Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:15:05 -0700 Subject: [PATCH 03/13] chmod directory containing keys before trying to test --- tests/test_ssh.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 48aab95..07e183b 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -30,6 +30,7 @@ def jupyter_server(random_port): ] env = os.environ.copy() with tempfile.TemporaryDirectory() as temp_dir: + os.chmod(temp_dir, 0o700) authorized_keys_path = os.path.join(temp_dir, 'authorized_keys') subprocess.check_call(['ssh-keygen', '-f', authorized_keys_path, '-q', '-N', '']) From 17e2e22df0e3c154005fedc296f8d3eecd668f13 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:21:56 -0700 Subject: [PATCH 04/13] Fix interactive test --- tests/test_ssh.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 07e183b..75afc01 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -65,13 +65,15 @@ def test_ssh_command_execution(jupyter_server): def test_ssh_interactive(jupyter_server): + # Explicitly call /bin/sh without any args, so we can run without any prompts cmd = [ 'ssh', - ] + [f"-o={o}" for o in get_ssh_client_options(*jupyter_server)] + ['127.0.0.1', 'hostname'] + ] + [f"-o={o}" for o in get_ssh_client_options(*jupyter_server)] + ['127.0.0.1', '/bin/sh'] proc = pexpect.spawn(shlex.join(cmd), echo=False) proc.sendline('hostname') assert proc.readline().decode().strip() == socket.gethostname() + proc.sendline("exit") proc.wait() assert proc.exitstatus == 0 From b54c3df5ff7169185d26ed76b89e9c6d388f6363 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:24:57 -0700 Subject: [PATCH 05/13] Show verbose output from websocat --- tests/test_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 75afc01..a92f577 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -48,7 +48,7 @@ def jupyter_server(random_port): def get_ssh_client_options(random_port, token, authorized_keys_path): return [ - f'ProxyCommand=websocat --binary -H="Authorization: token {token}" asyncstdio: ws://%h:{random_port}/sshd/', + f'ProxyCommand=websocat -v --binary -H="Authorization: token {token}" asyncstdio: ws://%h:{random_port}/sshd/', f'User={getpass.getuser()}', f'IdentityFile={authorized_keys_path}', 'StrictHostKeyChecking=no' # FIXME: Validate this correctly later From 423ec932b15001c96ff0305d56980a0299167201 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:26:47 -0700 Subject: [PATCH 06/13] Wait longer for jupyter_server to be up --- tests/test_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index a92f577..91101ea 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -38,7 +38,7 @@ def jupyter_server(random_port): proc = subprocess.Popen(c, env=env) # Should healthcheck instead but HEY - time.sleep(1) + time.sleep(10) yield (random_port, token, authorized_keys_path) From c012b074a6aa94fa5d85bb24c8a5c9b51ba99948 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:30:15 -0700 Subject: [PATCH 07/13] Create temp dir with keys in cwd --- tests/test_ssh.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 91101ea..e932a68 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -29,7 +29,8 @@ def jupyter_server(random_port): '--no-browser' ] env = os.environ.copy() - with tempfile.TemporaryDirectory() as temp_dir: + dir_prefix = os.path.join(os.getcwd(), "tmp-") + with tempfile.TemporaryDirectory(prefix=dir_prefix) as temp_dir: os.chmod(temp_dir, 0o700) authorized_keys_path = os.path.join(temp_dir, 'authorized_keys') subprocess.check_call(['ssh-keygen', '-f', authorized_keys_path, '-q', '-N', '']) From ba43d6d628e0b4afd45c0bffdadc2e802350c68d Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:34:35 -0700 Subject: [PATCH 08/13] Check only stdout for command test --- tests/test_ssh.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index e932a68..5f4b76f 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -60,9 +60,10 @@ def test_ssh_command_execution(jupyter_server): 'ssh', ] + [f"-o={o}" for o in get_ssh_client_options(*jupyter_server)] + ['127.0.0.1', 'hostname'] - out = subprocess.check_output(cmd).decode().strip() + proc = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) + print(proc.stderr) - assert out == socket.gethostname() + assert proc.stdout.decode().strip() == socket.gethostname() def test_ssh_interactive(jupyter_server): From 7f4f3b47f570ff7b4901de97fe8d5774146d6b21 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:35:55 -0700 Subject: [PATCH 09/13] Remove verbose command for websocat --- tests/test_ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 5f4b76f..501cddc 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -49,7 +49,7 @@ def jupyter_server(random_port): def get_ssh_client_options(random_port, token, authorized_keys_path): return [ - f'ProxyCommand=websocat -v --binary -H="Authorization: token {token}" asyncstdio: ws://%h:{random_port}/sshd/', + f'ProxyCommand=websocat --binary -H="Authorization: token {token}" asyncstdio: ws://%h:{random_port}/sshd/', f'User={getpass.getuser()}', f'IdentityFile={authorized_keys_path}', 'StrictHostKeyChecking=no' # FIXME: Validate this correctly later From 8d6cf3d2d05889360e46f9c2cdd036e6e49978b9 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:45:53 -0700 Subject: [PATCH 10/13] Wait for jupyter server to be fully up before we test --- tests/test_ssh.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 501cddc..9bf976b 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -8,6 +8,8 @@ import getpass import time import socket +from urllib.request import urlopen, Request +from urllib.error import URLError @pytest.fixture def random_port(): @@ -38,8 +40,18 @@ def jupyter_server(random_port): env['JUPYTER_SSHD_PROXY_AUTHORIZED_KEYS_PATH'] = authorized_keys_path + '.pub' proc = subprocess.Popen(c, env=env) - # Should healthcheck instead but HEY - time.sleep(10) + # Wait for server to be fully up before we yield + req = Request(f"http://127.0.0.1:{random_port}/api/status", headers={"Authorization": f"token {token}"}) + while True: + try: + resp = urlopen(req) + if resp.status == 200: + break + except URLError as e: + if not isinstance(e.reason, ConnectionRefusedError): + raise + print("Waiting for jupyter server to come up...") + time.sleep(1) yield (random_port, token, authorized_keys_path) From 3a1b03c20cbb05a99a42d23fe94e5cf404f6fb41 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:47:57 -0700 Subject: [PATCH 11/13] Don't use UsePrivilegeSeparation no Don't think this is necessary --- jupyter_sshd_proxy/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jupyter_sshd_proxy/__init__.py b/jupyter_sshd_proxy/__init__.py index 724f70a..68ade73 100644 --- a/jupyter_sshd_proxy/__init__.py +++ b/jupyter_sshd_proxy/__init__.py @@ -25,8 +25,7 @@ def setup_sshd() -> Dict[str, Any]: # Last login info is from /var/log/lastlog, which is transient in containerized systems '-o', 'PrintLastLog no', '-o', f'AuthorizedKeysFile {AUTHORIZED_KEYS_PATH}', - '-o', f'LogLevel {SSHD_LOG_LEVEL}', - '-o', 'UsePrivilegeSeparation no', + '-o', f'LogLevel {SSHD_LOG_LEVEL}' ] print(shlex.join(cmd)) From 92d991e55783ed508f52de1993653ed0db0fa48d Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:50:01 -0700 Subject: [PATCH 12/13] Add comment about why we don't use /tmp --- tests/test_ssh.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_ssh.py b/tests/test_ssh.py index 9bf976b..233ecd6 100644 --- a/tests/test_ssh.py +++ b/tests/test_ssh.py @@ -31,6 +31,12 @@ def jupyter_server(random_port): '--no-browser' ] env = os.environ.copy() + + # sshd requires that the path to the authorized keys (and every ancestor) is fully owned + # by the user who is trying to log in (or root), and mode is not group or world writeable. + # Since that's not necessarily true for `/tmp`, we can not put our keys there for tests. + # Create them instead in cwd, which we assume matches this description instead. We + # clean up after ourselves. dir_prefix = os.path.join(os.getcwd(), "tmp-") with tempfile.TemporaryDirectory(prefix=dir_prefix) as temp_dir: os.chmod(temp_dir, 0o700) From 84cf0e9ca55bf4afd84a7531d45fd05924836611 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 25 Oct 2024 19:53:14 -0700 Subject: [PATCH 13/13] Remove printing sshd command --- jupyter_sshd_proxy/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyter_sshd_proxy/__init__.py b/jupyter_sshd_proxy/__init__.py index 68ade73..6ce9442 100644 --- a/jupyter_sshd_proxy/__init__.py +++ b/jupyter_sshd_proxy/__init__.py @@ -28,7 +28,6 @@ def setup_sshd() -> Dict[str, Any]: '-o', f'LogLevel {SSHD_LOG_LEVEL}' ] - print(shlex.join(cmd)) return { "command": cmd, "raw_socket_proxy": True,