diff --git a/.gitignore b/.gitignore index b44889fc..2de5db2f 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ cloudflared.exe !cmd/cloudflared/ .DS_Store *-session.log +ssh_server_tests/.env diff --git a/Makefile b/Makefile index da63e656..02770a7a 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,10 @@ container: test: vet go test -v -race $(VERSION_FLAGS) ./... +.PHONY: test-ssh-server +test-ssh-server: + docker-compose -f ssh_server_tests/docker-compose.yml up + .PHONY: cloudflared-deb cloudflared-deb: cloudflared mkdir -p $(PACKAGE_DIR) diff --git a/ssh_server_tests/Dockerfile b/ssh_server_tests/Dockerfile new file mode 100644 index 00000000..d827b0fe --- /dev/null +++ b/ssh_server_tests/Dockerfile @@ -0,0 +1,14 @@ +FROM python:3-buster + +RUN wget https://bin.equinox.io/c/VdrWdbjqyF/cloudflared-stable-linux-amd64.deb \ + && dpkg -i cloudflared-stable-linux-amd64.deb + +RUN pip install pexpect + +COPY tests.py . +COPY ssh /root/.ssh +RUN chmod 600 /root/.ssh/id_rsa + +ARG SSH_HOSTNAME +RUN bash -c 'sed -i "s/{{hostname}}/${SSH_HOSTNAME}/g" /root/.ssh/authorized_keys_config' +RUN bash -c 'sed -i "s/{{hostname}}/${SSH_HOSTNAME}/g" /root/.ssh/short_lived_cert_config' diff --git a/ssh_server_tests/README.md b/ssh_server_tests/README.md new file mode 100644 index 00000000..391d9040 --- /dev/null +++ b/ssh_server_tests/README.md @@ -0,0 +1,23 @@ +# Cloudflared SSH server smoke tests + +Runs several tests in a docker container against a server that is started out of band of these tests. +Cloudflared token also needs to be retrieved out of band. +SSH server hostname and user need to be configured in a docker environment file + + +## Running tests + +* Build cloudflared: +make cloudflared + +* Start server: +sudo ./cloudflared tunnel --hostname HOSTNAME --ssh-server + +* Fetch token: +./cloudflared access login HOSTNAME + +* Create docker env file: +echo "SSH_HOSTNAME=HOSTNAME\nSSH_USER=USERNAME\n" > ssh_server_tests/.env + +* Run tests: +make test-ssh-server diff --git a/ssh_server_tests/docker-compose.yml b/ssh_server_tests/docker-compose.yml new file mode 100644 index 00000000..dc4cac5a --- /dev/null +++ b/ssh_server_tests/docker-compose.yml @@ -0,0 +1,19 @@ +version: "3.1" + +services: + ssh_test: + build: + context: . + args: + - SSH_HOSTNAME=${SSH_HOSTNAME} + volumes: + - "~/.cloudflared/:/root/.cloudflared" + env_file: + - .env + environment: + - AUTHORIZED_KEYS_SSH_CONFIG=/root/.ssh/authorized_keys_config + - SHORT_LIVED_CERT_SSH_CONFIG=/root/.ssh/short_lived_cert_config + - REMOTE_SCP_FILENAME=scp_test.txt + - ROOT_ONLY_TEST_FILE_PATH=~/permission_test.txt + + entrypoint: "python tests.py" diff --git a/ssh_server_tests/ssh/authorized_keys_config b/ssh_server_tests/ssh/authorized_keys_config new file mode 100644 index 00000000..288c0b70 --- /dev/null +++ b/ssh_server_tests/ssh/authorized_keys_config @@ -0,0 +1,5 @@ +Host * + AddressFamily inet + +Host {{hostname}} + ProxyCommand /usr/local/bin/cloudflared access ssh --hostname %h diff --git a/ssh_server_tests/ssh/id_rsa b/ssh_server_tests/ssh/id_rsa new file mode 100644 index 00000000..16e178b1 --- /dev/null +++ b/ssh_server_tests/ssh/id_rsa @@ -0,0 +1,49 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAACFwAAAAdzc2gtcn +NhAAAAAwEAAQAAAgEAvi26NDQ8cYTTztqPe9ZgF5HR/rIo5FoDgL5NbbZKW6h0txP9Fd8s +id9Bgmo+aGkeM327tPVVMQ6UFmdRksOCIDWQDjNLF8b6S+Fu95tvMKSbGreRoR32OvgZKV +I6KmOsF4z4GIv9naPplZswtKEUhSSI+/gPdAs9wfwalqZ77e82QJ727bYMeC3lzuoT+KBI +dYufJ4OQhLtpHrqhB5sn7s6+oCv/u85GSln5SIC18Hi2t9lW4tgb5tH8P0kEDDWGfPS5ok +qGi4kFTvwBXOCS2r4dhi5hRkpP7PqG4np0OCfvK5IRRJ27fCnj0loc+puZJAxnPMbuJr64 +vwxRx78PM/V0PDUsl0P6aR/vbe0XmF9FGqbWf2Tar1p4r6C9/bMzcDz8seYT8hzLIHP3+R +l1hdlsTLm+1EzhaExKId+tjXegKGG4nU24h6qHEnRxLQDMwEsdkfj4E1pVypZJXVyNj99D +o84vi0EUnu7R4HmQb/C+Pu7qMDtLT3Zk7O5Mg4LQ+cTz9V0noYEAyG46nAB4U/nJzBnV1J ++aAdpioHmUAYhLYlQ9Kiy7LCJi92g9Wqa4wxMKxBbO5ZeH++p2p2lUi/oQNqx/2dLYFmy0 +wxvJHbZIhAaFbOeCvHg1ucIAQznli2jOr2qoB+yKRRPAp/3NXnZg1v7ce2CkwiAD52wjtC +kAAAdILMJUeyzCVHsAAAAHc3NoLXJzYQAAAgEAvi26NDQ8cYTTztqPe9ZgF5HR/rIo5FoD +gL5NbbZKW6h0txP9Fd8sid9Bgmo+aGkeM327tPVVMQ6UFmdRksOCIDWQDjNLF8b6S+Fu95 +tvMKSbGreRoR32OvgZKVI6KmOsF4z4GIv9naPplZswtKEUhSSI+/gPdAs9wfwalqZ77e82 +QJ727bYMeC3lzuoT+KBIdYufJ4OQhLtpHrqhB5sn7s6+oCv/u85GSln5SIC18Hi2t9lW4t +gb5tH8P0kEDDWGfPS5okqGi4kFTvwBXOCS2r4dhi5hRkpP7PqG4np0OCfvK5IRRJ27fCnj +0loc+puZJAxnPMbuJr64vwxRx78PM/V0PDUsl0P6aR/vbe0XmF9FGqbWf2Tar1p4r6C9/b +MzcDz8seYT8hzLIHP3+Rl1hdlsTLm+1EzhaExKId+tjXegKGG4nU24h6qHEnRxLQDMwEsd +kfj4E1pVypZJXVyNj99Do84vi0EUnu7R4HmQb/C+Pu7qMDtLT3Zk7O5Mg4LQ+cTz9V0noY +EAyG46nAB4U/nJzBnV1J+aAdpioHmUAYhLYlQ9Kiy7LCJi92g9Wqa4wxMKxBbO5ZeH++p2 +p2lUi/oQNqx/2dLYFmy0wxvJHbZIhAaFbOeCvHg1ucIAQznli2jOr2qoB+yKRRPAp/3NXn +Zg1v7ce2CkwiAD52wjtCkAAAADAQABAAACAQCbnVsyAFQ9J00Rg/HIiUATyTQlzq57O9SF +8jH1RiZOHedzLx32WaleH5rBFiJ+2RTnWUjQ57aP77fpJR2wk93UcT+w/vPBPwXsNUjRvx +Qan3ZzRCYbyiKDWiNslmYV7X0RwD36CAK8jTVDP7t48h2SXLTiSLaMY+5i3uD6yLu7k/O2 +qNyw4jgN1rCmwQ8acD0aQec3NAZ7NcbsaBX/3Uutsup0scwOZtlJWZoLY5Z8cKpCgcsAz4 +j1NHnNZvey7dFgSffj/ktdvf7kBH0w/GnuJ4aNF0Jte70u0kiw5TZYBQVFh74tgUu6a6SJ +qUbxIYUL5EJNjxGsDn+phHEemw3aMv0CwZG6Tqaionlna7bLsl9Bg1HTGclczVWx8uqC+M +6agLmkhYCHG0rVj8h5smjXAQXtmvIDVYDOlJZZoF9VAOCj6QfmJUH1NAGpCs1HDHbeOxGA +OLCh4d3F4rScPqhGdtSt4W13VFIvXn2Qqoz9ufepZsee1SZqpcerxywx2wN9ZAzu+X8lTN +i+TA2B3vWpqqucOEsp4JwDN+VMKZqKUGUDWcm/eHSaG6wq0q734LUlgM85TjaIg8QsNtWV +giB1nWwsYIuH4rsFNFGEwURYdGBcw6idH0GZ7I4RaIB5F9oOza1d601E0APHYrtnx9yOiK +nOtJ+5ZmVZovaDRfu1aQAAAQBU/EFaNUzoVhO04pS2L6BlByt963bOIsSJhdlEzek5AAli +eaf1S/PD6xWCc0IGY+GZE0HPbhsKYanjqOpWldcA2T7fzf4oz4vFBfUkPYo/MLSlLCYsDd +IH3wBkCssnfR5EkzNgxnOvq646Nl64BMvxwSIXGPktdq9ZALxViwricSRzCFURnh5vLHWU +wBzSgAA0UlZ9E64GtAv066+AoZCp83GhTLRC4o0naE2e/K4op4BCFHLrZ8eXmDRK3NJj80 +Vkn+uhrk+SHmbjIhmS57Pv9p8TWyRvemph/nMUuZGKBUu2X+JQxggck0KigIrXjsmciCsM +BIM3mYDDfjYbyVhTAAABAQDkV8O1bWUsAIqk7RU+iDZojN5kaO+zUvj1TafX8QX1sY6pu4 +Z2cfSEka1532BaehM95bQm7BCPw4cYg56XidmCQTZ9WaWqxVrOo48EKXUtZMZx6nKFOKlq +MT2XTMnGT9n7kFCfEjSVkAjuJ9ZTFLOaoXAaVRnxeHQwOKaup5KKP9GSzNIw328U+96s3V +WKHeT4pMjHBccgW/qX/tRRidZw5in5uBC9Ew5y3UACFTkNOnhUwVfyUNbBZJ2W36msQ3KD +AN7nOrQHqhd3NFyCEy2ovIAKVBacr/VEX6EsRUshIehJzz8EY9f3kXL7WT2QDoz2giPeBJ +HJdEpXt43UpszjAAABAQDVNpqNdHUlCs9XnbIvc6ZRrNh79wt65YFfvh/QEuA33KnA6Ri6 +EgnV5IdUWXS/UFaYcm2udydrBpVIVifSYl3sioHBylpri23BEy38PKwVXvghUtfpN6dWGn +NZUG25fQPtIzqi+lo953ZjIj+Adi17AeVv4P4NiLrZeM9lXfWf2pEPOecxXs1IwAf9IiDQ +WepAwRLsu42eEnHA+DSJPZUkSbISfM5X345k0g6EVATX/yLL3CsqClPzPtsqjh6rbEfFg3 +2OfIMcWV77gOlGWGQ+bUHc8kV6xJqV9QVacLWzfLvIqHF0wQMf8WLOVHEzkfiq4VjwhVqr +/+FFvljm5nSDAAAAEW1pa2VAQzAyWTUwVEdKR0g4AQ== +-----END OPENSSH PRIVATE KEY----- diff --git a/ssh_server_tests/ssh/id_rsa.pub b/ssh_server_tests/ssh/id_rsa.pub new file mode 100644 index 00000000..024f80ee --- /dev/null +++ b/ssh_server_tests/ssh/id_rsa.pub @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC+Lbo0NDxxhNPO2o971mAXkdH+sijkWgOAvk1ttkpbqHS3E/0V3yyJ30GCaj5oaR4zfbu09VUxDpQWZ1GSw4IgNZAOM0sXxvpL4W73m28wpJsat5GhHfY6+BkpUjoqY6wXjPgYi/2do+mVmzC0oRSFJIj7+A90Cz3B/BqWpnvt7zZAnvbttgx4LeXO6hP4oEh1i58ng5CEu2keuqEHmyfuzr6gK/+7zkZKWflIgLXweLa32Vbi2Bvm0fw/SQQMNYZ89LmiSoaLiQVO/AFc4JLavh2GLmFGSk/s+obienQ4J+8rkhFEnbt8KePSWhz6m5kkDGc8xu4mvri/DFHHvw8z9XQ8NSyXQ/ppH+9t7ReYX0UaptZ/ZNqvWnivoL39szNwPPyx5hPyHMsgc/f5GXWF2WxMub7UTOFoTEoh362Nd6AoYbidTbiHqocSdHEtAMzASx2R+PgTWlXKlkldXI2P30Ojzi+LQRSe7tHgeZBv8L4+7uowO0tPdmTs7kyDgtD5xPP1XSehgQDIbjqcAHhT+cnMGdXUn5oB2mKgeZQBiEtiVD0qLLssImL3aD1aprjDEwrEFs7ll4f76nanaVSL+hA2rH/Z0tgWbLTDG8kdtkiEBoVs54K8eDW5wgBDOeWLaM6vaqgH7IpFE8Cn/c1edmDW/tx7YKTCIAPnbCO0KQ== mike@C02Y50TGJGH8 diff --git a/ssh_server_tests/ssh/short_lived_cert_config b/ssh_server_tests/ssh/short_lived_cert_config new file mode 100644 index 00000000..e29356fe --- /dev/null +++ b/ssh_server_tests/ssh/short_lived_cert_config @@ -0,0 +1,11 @@ +Host * + AddressFamily inet + +Host {{hostname}} + ProxyCommand bash -c '/usr/local/bin/cloudflared access ssh-gen --hostname %h; ssh -F /root/.ssh/short_lived_cert_config -tt %r@cfpipe-{{hostname}} >&2 <&1' + +Host cfpipe-{{hostname}} + HostName {{hostname}} + ProxyCommand /usr/local/bin/cloudflared access ssh --hostname %h + IdentityFile ~/.cloudflared/{{hostname}}-cf_key + CertificateFile ~/.cloudflared/{{hostname}}-cf_key-cert.pub diff --git a/ssh_server_tests/tests.py b/ssh_server_tests/tests.py new file mode 100644 index 00000000..f79f3794 --- /dev/null +++ b/ssh_server_tests/tests.py @@ -0,0 +1,195 @@ +""" +Cloudflared Integration tests +""" + +import unittest +import subprocess +import os +import tempfile +from contextlib import contextmanager + +from pexpect import pxssh + + +class TestSSHBase(unittest.TestCase): + """ + SSH test base class containing constants and helper funcs + """ + + HOSTNAME = os.environ["SSH_HOSTNAME"] + SSH_USER = os.environ["SSH_USER"] + SSH_TARGET = f"{SSH_USER}@{HOSTNAME}" + AUTHORIZED_KEYS_SSH_CONFIG = os.environ["AUTHORIZED_KEYS_SSH_CONFIG"] + SHORT_LIVED_CERT_SSH_CONFIG = os.environ["SHORT_LIVED_CERT_SSH_CONFIG"] + SSH_OPTIONS = {"StrictHostKeyChecking": "no"} + + @classmethod + def get_ssh_command(cls, pty=True): + """ + Return ssh command arg list. If pty is true, a PTY is forced for the session. + """ + cmd = [ + "ssh", + "-o", + "StrictHostKeyChecking=no", + "-F", + cls.AUTHORIZED_KEYS_SSH_CONFIG, + cls.SSH_TARGET, + ] + if not pty: + cmd += ["-T"] + else: + cmd += ["-tt"] + + return cmd + + @classmethod + @contextmanager + def ssh_session_manager(cls, *args, **kwargs): + """ + Context manager for interacting with a pxssh session. + Disables pty echo on the remote server and ensures session is terminated afterward. + """ + session = pxssh.pxssh(options=cls.SSH_OPTIONS) + + session.login( + cls.HOSTNAME, + username=cls.SSH_USER, + original_prompt=r"[#@$]", + ssh_config=kwargs.get("ssh_config", cls.AUTHORIZED_KEYS_SSH_CONFIG), + ssh_tunnels=kwargs.get("ssh_tunnels", {}), + ) + try: + session.sendline("stty -echo") + session.prompt() + yield session + finally: + session.logout() + + @staticmethod + def get_command_output(session, cmd): + """ + Executes command on remote ssh server and waits for prompt. + Returns command output + """ + session.sendline(cmd) + session.prompt() + return session.before.decode().strip() + + def exec_command(self, cmd, shell=False): + """ + Executes command locally. Raises Assertion error for non-zero return code. + Returns stdout and stderr + """ + proc = subprocess.Popen( + cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE, shell=shell + ) + raw_out, raw_err = proc.communicate() + + out = raw_out.decode() + err = raw_err.decode() + self.assertEqual(proc.returncode, 0, msg=f"stdout: {out} stderr: {err}") + return out.strip(), err.strip() + + +class TestSSHCommandExec(TestSSHBase): + """ + Tests inline ssh command exec + """ + + # Name of file to be downloaded over SCP on remote server. + REMOTE_SCP_FILENAME = os.environ["REMOTE_SCP_FILENAME"] + + @classmethod + def get_scp_base_command(cls): + return [ + "scp", + "-o", + "StrictHostKeyChecking=no", + "-v", + "-F", + cls.AUTHORIZED_KEYS_SSH_CONFIG, + ] + + @unittest.skip( + "This creates files on the remote. Should be skipped until server is dockerized." + ) + def test_verbose_scp_sink_mode(self): + with tempfile.NamedTemporaryFile() as fl: + self.exec_command( + self.get_scp_base_command() + [fl.name, f"{self.SSH_TARGET}:"] + ) + + def test_verbose_scp_source_mode(self): + with tempfile.TemporaryDirectory() as tmpdirname: + self.exec_command( + self.get_scp_base_command() + + [f"{self.SSH_TARGET}:{self.REMOTE_SCP_FILENAME}", tmpdirname] + ) + local_filename = os.path.join(tmpdirname, self.REMOTE_SCP_FILENAME) + + self.assertTrue(os.path.exists(local_filename)) + self.assertTrue(os.path.getsize(local_filename) > 0) + + def test_pty_command(self): + base_cmd = self.get_ssh_command() + + out, _ = self.exec_command(base_cmd + ["whoami"]) + self.assertEqual(out.strip().lower(), self.SSH_USER.lower()) + + out, _ = self.exec_command(base_cmd + ["tty"]) + self.assertNotEqual(out, "not a tty") + + def test_non_pty_command(self): + base_cmd = self.get_ssh_command(pty=False) + + out, _ = self.exec_command(base_cmd + ["whoami"]) + self.assertEqual(out.strip().lower(), self.SSH_USER.lower()) + + out, _ = self.exec_command(base_cmd + ["tty"]) + self.assertEqual(out, "not a tty") + + +class TestSSHShell(TestSSHBase): + """ + Tests interactive SSH shell + """ + + # File path to a file on the remote server with root only read privileges. + ROOT_ONLY_TEST_FILE_PATH = os.environ["ROOT_ONLY_TEST_FILE_PATH"] + + def test_ssh_pty(self): + with self.ssh_session_manager() as session: + + # Test shell launched as correct user + username = self.get_command_output(session, "whoami") + self.assertEqual(username.lower(), self.SSH_USER.lower()) + + # Test USER env variable set + user_var = self.get_command_output(session, "echo $USER") + self.assertEqual(user_var.lower(), self.SSH_USER.lower()) + + # Test HOME env variable set to true user home. + home_env = self.get_command_output(session, "echo $HOME") + pwd = self.get_command_output(session, "pwd") + self.assertEqual(pwd, home_env) + + # Test shell launched in correct user home dir. + self.assertIn(username, pwd) + + # Ensure shell launched with correct user's permissions and privs. + # Cant read root owned 0700 files. + output = self.get_command_output( + session, f"cat {self.ROOT_ONLY_TEST_FILE_PATH}" + ) + self.assertIn("Permission denied", output) + + def test_short_lived_cert_auth(self): + with self.ssh_session_manager( + ssh_config=self.SHORT_LIVED_CERT_SSH_CONFIG + ) as session: + username = self.get_command_output(session, "whoami") + self.assertEqual(username.lower(), self.SSH_USER.lower()) + + +unittest.main() diff --git a/sshserver/sshserver_unix.go b/sshserver/sshserver_unix.go index 99fd3f93..c6b6cd4a 100644 --- a/sshserver/sshserver_unix.go +++ b/sshserver/sshserver_unix.go @@ -88,7 +88,7 @@ func New(logManager sshlog.Manager, logger *logrus.Logger, version, address stri } // AUTH-2050: This is a temporary workaround of a timing issue in the tunnel muxer to allow further testing. - // TODO: Remove this + // TODO: Remove this sshServer.ConnCallback = func(conn net.Conn) net.Conn { time.Sleep(10 * time.Millisecond) return conn @@ -216,7 +216,16 @@ func (s *SSHServer) connectionHandler(session ssh.Session) { // Write stdin to shell go func() { - defer shellInput.Close() + + /* + Only close shell stdin for non-pty sessions because they have distinct stdin, stdout, and stderr. + This is done to prevent commands like SCP from hanging after all data has been sent. + PTY sessions share one file for all three streams and the shell process closes it. + Closing it here also closes shellOutput and causes an error on copy(). + */ + if !isPty { + defer shellInput.Close() + } if _, err := io.Copy(shellInput, session); err != nil { s.logger.WithError(err).Error("Failed to write incoming command to pty") }