diff --git a/CHANGELOG.md b/CHANGELOG.md index c97c282d..a71fcfc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Added + +- SSH hook: added support for the `com.hooks.ssh.port` OCI annotation, which allows to customize the port used by the Dropbear server. + ### Changed - MPI hook: verbosity levels for log messages about ABI compatibility and library replacements have been slightly adjusted. @@ -15,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Glibc hook: fixed detection of the container's glibc version, which was causing a shell-init error on some systems +- SSH hook: the `SERVER_PORT` environment variable in the JSON configuration file has been renamed to `SERVER_PORT_DEFAULT` ## [1.6.3] diff --git a/CI/src/integration_tests/test_ssh_hook.py b/CI/src/integration_tests/test_ssh_hook.py index 3b282756..b121e616 100644 --- a/CI/src/integration_tests/test_ssh_hook.py +++ b/CI/src/integration_tests/test_ssh_hook.py @@ -20,7 +20,7 @@ def generate_hook_config(): "HOOK_BASE_DIR=/home", "PASSWD_FILE=" + os.environ["CMAKE_INSTALL_PREFIX"] + "/etc/passwd", "DROPBEAR_DIR=" + os.environ["CMAKE_INSTALL_PREFIX"] + "/dropbear", - "SERVER_PORT=15263" + "SERVER_PORT_DEFAULT=15263" ], "args": [ "ssh_hook", diff --git a/CI/src/integration_tests_for_virtual_cluster/test_ssh_hook.py b/CI/src/integration_tests_for_virtual_cluster/test_ssh_hook.py index 3bf26c83..89d13506 100755 --- a/CI/src/integration_tests_for_virtual_cluster/test_ssh_hook.py +++ b/CI/src/integration_tests_for_virtual_cluster/test_ssh_hook.py @@ -58,7 +58,7 @@ def _generate_ssh_hook_config(cls): f"HOOK_BASE_DIR={cls.OCI_HOOKS_BASE_DIR}", f"PASSWD_FILE={cls.SARUS_INSTALL_PATH}/etc/passwd", f"DROPBEAR_DIR={cls.SARUS_INSTALL_PATH}/dropbear", - "SERVER_PORT=15263" + "SERVER_PORT_DEFAULT=15263" ], "args": [ "ssh_hook", @@ -101,26 +101,18 @@ def test_ssh_hook(self): # check SSH from node0 to node1 hostname_node1 = get_hostname_of_node1(self.image_with_musl) - hostname_node1_through_ssh = get_hostname_of_node1_though_ssh(self.image_with_musl, hostname_node1) - self.assertEqual(hostname_node1, hostname_node1_through_ssh) - hostname_node1_through_ssh = get_hostname_of_node1_though_ssh(self.image_with_glibc, hostname_node1) - self.assertEqual(hostname_node1, hostname_node1_through_ssh) + self._check_ssh_hook(hostname_node1) # check SSH from node0 to node1 with non-standard $HOME # in some systems the home from the passwd file (copied from the host) # might not be present in the container. The hook has to deduce it from # the passwd file and create it. with custom_home_in_sarus_passwd_file("/users/test"): - hostname_node1_through_ssh = get_hostname_of_node1_though_ssh(self.image_with_musl, hostname_node1) - self.assertEqual(hostname_node1, hostname_node1_through_ssh) - hostname_node1_through_ssh = get_hostname_of_node1_though_ssh(self.image_with_glibc, hostname_node1) - self.assertEqual(hostname_node1, hostname_node1_through_ssh) + self._check_ssh_hook(hostname_node1) + + # check SSH from node0 to node1 with server port set from Sarus CLI + self._check_ssh_hook(hostname_node1, ["--annotation=com.hooks.ssh.port=33333"]) - # check SSH goes into container (not host) - prettyname = get_prettyname_of_node1_through_ssh(self.image_with_musl, hostname_node1) - self.assertEqual(prettyname, "Alpine Linux v3.14") - prettyname = get_prettyname_of_node1_through_ssh(self.image_with_glibc, hostname_node1) - self.assertEqual(prettyname, "Debian GNU/Linux 10 (buster)") def test_ssh_keys_generation(self): ssh_dir = os.environ['HOME'] + "/.oci-hooks/ssh" @@ -143,6 +135,26 @@ def test_ssh_keys_generation(self): fingerprint_changed = subprocess.check_output(fingerprint_command).decode() self.assertNotEqual(fingerprint, fingerprint_changed) + def _check_ssh_hook(self, hostname_node1, sarus_run_extra_opts=[]): + hostname_node1_through_ssh = get_hostname_of_node1_though_ssh(self.image_with_musl, + hostname_node1, + sarus_run_extra_opts) + self.assertEqual(hostname_node1, hostname_node1_through_ssh) + hostname_node1_through_ssh = get_hostname_of_node1_though_ssh(self.image_with_glibc, + hostname_node1, + sarus_run_extra_opts) + self.assertEqual(hostname_node1, hostname_node1_through_ssh) + + # check SSH goes into container (not host) + prettyname = get_prettyname_of_node1_through_ssh(self.image_with_musl, + hostname_node1, + sarus_run_extra_opts) + self.assertEqual(prettyname, "Alpine Linux v3.14") + prettyname = get_prettyname_of_node1_through_ssh(self.image_with_glibc, + hostname_node1, + sarus_run_extra_opts) + self.assertEqual(prettyname, "Debian GNU/Linux 10 (buster)") + def get_hostname_of_node1(image): command = [ @@ -158,7 +170,7 @@ def get_hostname_of_node1(image): return out[0] -def get_hostname_of_node1_though_ssh(image, hostname_node1): +def get_hostname_of_node1_though_ssh(image, hostname_node1, sarus_run_extra_opts=[]): command = [ "sh", "-c", @@ -174,12 +186,12 @@ def get_hostname_of_node1_though_ssh(image, hostname_node1): out = util.run_command_in_container_with_slurm(image=image, command=command, options_of_srun_command=["-N2"], - options_of_run_command=["--ssh"]) + options_of_run_command=["--ssh"]+sarus_run_extra_opts) assert len(out) == 1 # expect one single line of output return out[0] -def get_prettyname_of_node1_through_ssh(image, hostname_node1): +def get_prettyname_of_node1_through_ssh(image, hostname_node1, sarus_run_extra_opts=[]): command = [ "sh", "-c", @@ -195,7 +207,7 @@ def get_prettyname_of_node1_through_ssh(image, hostname_node1): out = util.run_command_in_container_with_slurm(image=image, command=command, options_of_srun_command=["-N2"], - options_of_run_command=["--ssh"]) + options_of_run_command=["--ssh"]+sarus_run_extra_opts) assert len(out) == 1 # expect one single line of output return re.match(r".*PRETTY_NAME=\"([^\"]+)\" .*", out[0]).group(1) diff --git a/doc/config/ssh-hook.rst b/doc/config/ssh-hook.rst index f1ec7f8d..ea20ea29 100644 --- a/doc/config/ssh-hook.rst +++ b/doc/config/ssh-hook.rst @@ -38,9 +38,10 @@ environment variables must be defined: * ``DROPBEAR_DIR``: Absolute path to the location of the custom SSH software. -* ``SERVER_PORT``: TCP port on which the SSH daemon will listen. This must be an unused - port and is typically set to a value different than 22 in order to avoid clashes with an SSH - daemon that could be running on the host. +* ``SERVER_PORT_DEFAULT``: Default TCP port on which the SSH daemon will listen. This must be an unused + port and is typically set to a value different than 22 in order to avoid clashes with an OpenSSH + daemon that could be running on the host. This value can be overridden by setting the + ``com.hooks.ssh.port`` annotation for the container. The following optional environment variables can also be defined: @@ -100,6 +101,10 @@ Dropbear daemon PIDfile inside the container (the default path is ``/opt/oci-hoo The ``com.hooks.ssh.pidfile_host`` annotation can be used to copy the PIDfile of the Dropbear daemon to the specified path on the host. +The ``com.hooks.ssh.port`` annotation can be used to set an arbitrary port for the Dropbear server +and client, overriding the value from the ``SERVER_PORT_DEFAULT`` environment variable set in the hook +configuration file. + .. important:: The SSH hook currently does not implement a poststop functionality and requires the use of a private PID namespace to cleanup the Dropbear daemon. diff --git a/doc/user/user_guide.rst b/doc/user/user_guide.rst index 52f15b30..7531dd36 100644 --- a/doc/user/user_guide.rst +++ b/doc/user/user_guide.rst @@ -1226,12 +1226,13 @@ SSH connection within containers -------------------------------- Sarus also comes with a hook which enables support for SSH connections within -containers. +containers, leveraging the `Dropbear SSH software `_. -When Sarus is configured to use this hook, you must first run the command -``sarus ssh-keygen`` to generate the SSH keys that will be used by the SSH -daemons and the SSH clients in the containers. It is sufficient to generate -the keys just once, as they are persistent between sessions. +When Sarus is configured to use this hook, before attempting SSH connections +to/from containers, the ``sarus ssh-keygen`` command must be run in order to +generate the keys that will be used by the SSH daemons and the SSH clients +inside containers. +It is sufficient to generate the keys just once, as they are persistent between sessions. It is then possible to execute a container passing the ``--ssh`` option to :program:`sarus run`, e.g. ``sarus run --ssh ``. Using the @@ -1272,6 +1273,9 @@ Dropbear daemon PIDfile inside the container. The ``com.hooks.ssh.pidfile_host`` annotation can be used to copy the PIDfile of the Dropbear daemon in the host. +The ``com.hooks.ssh.port`` annotation can be used to set an arbitrary port for the Dropbear server +and client, overriding the value from the default hook configuration. + .. warning:: The SSH hook currently does not implement a poststop functionality and requires the use of a :ref:`private PID namespace ` for @@ -1514,7 +1518,13 @@ Configure Visual Studio Code to access the remote Sarus container: 11. Select "Connect" to connect the IDE to the remote container environment. +.. important:: + + In order to establish connections through "Remote - SSH" extension, + the ``scp`` program must be available within the container. + This is required by Visual Studio Code to send and establish the VS Code + Server into the remote container. + For more details about "Remote - SSH" Visual Studio Code extension, you can refer to `this tutorial `_ from the official Visual Studio Code documentation. - diff --git a/src/hooks/ssh/SshHook.cpp b/src/hooks/ssh/SshHook.cpp index 10176d83..f5870a05 100644 --- a/src/hooks/ssh/SshHook.cpp +++ b/src/hooks/ssh/SshHook.cpp @@ -81,7 +81,7 @@ void SshHook::startSshDaemon() { dropbearRelativeDirInContainer = boost::filesystem::path("/opt/oci-hooks/ssh/dropbear"); dropbearDirInHost = sarus::common::getEnvironmentVariable("DROPBEAR_DIR"); - serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT")); + serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT_DEFAULT")); try { auto envJoinNamespaces = sarus::common::getEnvironmentVariable("JOIN_NAMESPACES"); joinNamespaces = (boost::algorithm::to_upper_copy(envJoinNamespaces) == std::string("TRUE")); @@ -139,6 +139,9 @@ void SshHook::parseConfigJSONOfBundle() { if(json["annotations"].HasMember("com.hooks.ssh.pidfile_host")) { pidfileHost = boost::filesystem::path(json["annotations"]["com.hooks.ssh.pidfile_host"].GetString()); } + if(json["annotations"].HasMember("com.hooks.ssh.port")) { + serverPort = std::stoi(json["annotations"]["com.hooks.ssh.port"].GetString()); + } } try { diff --git a/src/hooks/ssh/test/test_SSHHook.cpp b/src/hooks/ssh/test/test_SSHHook.cpp index 7b180390..0a459277 100644 --- a/src/hooks/ssh/test/test_SSHHook.cpp +++ b/src/hooks/ssh/test/test_SSHHook.cpp @@ -104,7 +104,12 @@ class Helper { sarus::common::setEnvironmentVariable("HOOK_BASE_DIR", sshKeysBaseDir.string()); sarus::common::setEnvironmentVariable("PASSWD_FILE", passwdFile.string()); sarus::common::setEnvironmentVariable("DROPBEAR_DIR", dropbearDirInHost.getPath().string()); - sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(serverPort)); + sarus::common::setEnvironmentVariable("SERVER_PORT_DEFAULT", std::to_string(serverPortDefault)); + + if (!userSshKeyPath.empty()) { + std::ofstream userSshKeyFile{userSshKeyPath.string()}; + userSshKeyFile << userSshKey; + } createConfigJSON(); @@ -146,10 +151,12 @@ class Helper { doc["process"]["env"].PushBack(rj::Value{var.c_str(), allocator}, allocator); } rapidjson::Value extraAnnotations(rapidjson::kObjectType); - extraAnnotations.AddMember( - "com.hooks.ssh.authorize_ssh_key", - rj::Value{(sshKeysDirInHost / "user_key.pub").c_str(), allocator}, - allocator); + if (boost::filesystem::exists(userSshKeyPath)) { + extraAnnotations.AddMember( + "com.hooks.ssh.authorize_ssh_key", + rj::Value{userSshKeyPath.c_str(), allocator}, + allocator); + } if (!dropbearPidFileInContainerRelative.empty()) { extraAnnotations.AddMember( "com.hooks.ssh.pidfile_container", @@ -163,6 +170,12 @@ class Helper { rj::Value{dropbearPidFileInHost.getPath().c_str(), allocator}, allocator); } + if (serverPort > 0) { + extraAnnotations.AddMember( + "com.hooks.ssh.port", + rj::Value{std::to_string(serverPort).c_str(), allocator}, + allocator); + } if (!doc.HasMember("annotations")) { doc.AddMember("annotations", extraAnnotations, allocator); } else { @@ -202,9 +215,8 @@ class Helper { environmentVariablesInContainer.push_back(variable); } - void generateUserSshKeyFile() { - std::ofstream userSshKeyFile{(sshKeysDirInHost / "user_key.pub").string()}; - userSshKeyFile << userSshKey; + void enableUserSshKeyPath() { + userSshKeyPath = homeDirInHost / "user_key.pub"; } void checkHostHasSshKeys() const { @@ -241,6 +253,26 @@ class Helper { return {}; } + boost::optional getSshDaemonPort() const { + auto out = sarus::common::executeCommand("ps ax -o args"); + std::stringstream ss{out}; + std::string line; + + boost::smatch matches; + boost::regex pattern("^ */opt/oci-hooks/ssh/dropbear/bin/dropbear.*-p ([0-9]+).*$"); + + while(std::getline(ss, line)) { + if(boost::regex_match(line, matches, pattern)) { + return std::stoi(matches[1]); + } + } + return {}; + } + + void checkDefaultSshDaemonPort() const { + CHECK(getSshDaemonPort() == serverPortDefault); + } + void checkContainerHasSshBinary() const { auto targetFile = boost::filesystem::path(rootfsDir / "usr/bin/ssh"); CHECK(boost::filesystem::exists(targetFile)); @@ -248,7 +280,7 @@ class Helper { auto expectedScript = boost::format{ "#!/bin/sh\n" "/opt/oci-hooks/ssh/dropbear/bin/dbclient -y -p %s $*\n" - } % serverPort; + } % (serverPort > 0 ? serverPort : serverPortDefault); auto actualScript = sarus::common::readFile(targetFile); CHECK_EQUAL(actualScript, expectedScript.str()); @@ -349,6 +381,10 @@ class Helper { dropbearPidFileInHost = sarus::common::PathRAII{PidFile}; } + void setCustomServerPort(const std::uint16_t portNumber) { + serverPort = portNumber; + } + bool containerMountsDotSsh() { auto overlayDir = bundleDir / "overlay"; if (!boost::filesystem::exists(overlayDir) || !boost::filesystem::is_directory(overlayDir)) { @@ -382,10 +418,12 @@ class Helper { boost::filesystem::path dropbearDirInContainer = rootfsDir / "opt/oci-hooks/ssh/dropbear"; boost::filesystem::path dropbearPidFileInContainerRelative = "/var/run/dropbear/dropbear.pid"; sarus::common::PathRAII dropbearPidFileInHost = sarus::common::PathRAII(""); - std::uint16_t serverPort = 11111; + std::uint16_t serverPortDefault = 11111; + std::uint16_t serverPort = 0; std::vector rootfsFolders = {"etc", "dev", "bin", "sbin", "usr", "lib", "lib64"}; // necessary to chroot into rootfs std::vector environmentVariablesInContainer; std::string userSshKey{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAvAIP2SI2ON23c6ZP1c7gQf17P25npZLgHSxfwqRKNWh27p user@test"}; + boost::filesystem::path userSshKeyPath; }; TEST_GROUP(SSHHookTestGroup) { @@ -466,12 +504,12 @@ TEST(SSHHookTestGroup, testInjectKeyUsingAnnotations) { Helper helper{}; helper.setRootIds(); + helper.enableUserSshKeyPath(); helper.setupTestEnvironment(); // generate + check SSH keys in local repository helper.setUserIds(); // keygen is executed with user privileges SshHook{}.generateSshKeys(true); - helper.generateUserSshKeyFile(); helper.setRootIds(); helper.checkHostHasSshKeys(); @@ -494,7 +532,6 @@ TEST(SSHHookTestGroup, testDefaultDropbearPidFiles) { // generate + check SSH keys in local repository helper.setUserIds(); // keygen is executed with user privileges SshHook{}.generateSshKeys(true); - helper.generateUserSshKeyFile(); helper.setRootIds(); helper.checkHostHasSshKeys(); @@ -518,7 +555,6 @@ TEST(SSHHookTestGroup, testDropbearPidFileInHost) { // generate + check SSH keys in local repository helper.setUserIds(); // keygen is executed with user privileges SshHook{}.generateSshKeys(true); - helper.generateUserSshKeyFile(); helper.setRootIds(); helper.checkHostHasSshKeys(); @@ -544,7 +580,6 @@ TEST(SSHHookTestGroup, testDropbearPidFilesInCustomPaths) { // generate + check SSH keys in local repository helper.setUserIds(); // keygen is executed with user privileges SshHook{}.generateSshKeys(true); - helper.generateUserSshKeyFile(); helper.setRootIds(); helper.checkHostHasSshKeys(); @@ -570,7 +605,6 @@ TEST(SSHHookTestGroup, testDefaultMountsDotSshAsOverlayFs) { // generate + check SSH keys in local repository helper.setUserIds(); // keygen is executed with user privileges SshHook{}.generateSshKeys(true); - helper.generateUserSshKeyFile(); helper.setRootIds(); @@ -592,7 +626,6 @@ TEST(SSHHookTestGroup, testEnvVarDisableMountsDotSshAsOverlayFs) { // generate + check SSH keys in local repository helper.setUserIds(); // keygen is executed with user privileges SshHook{}.generateSshKeys(true); - helper.generateUserSshKeyFile(); helper.setRootIds(); @@ -604,6 +637,46 @@ TEST(SSHHookTestGroup, testEnvVarDisableMountsDotSshAsOverlayFs) { sarus::common::setEnvironmentVariable("OVERLAY_MOUNT_HOME_SSH", ""); } +TEST(SSHHookTestGroup, testDefaultServerPort) { + Helper helper{}; + + helper.setRootIds(); + helper.setupTestEnvironment(); + + // generate + check SSH keys in local repository + helper.setUserIds(); // keygen is executed with user privileges + SshHook{}.generateSshKeys(true); + helper.setRootIds(); + helper.checkHostHasSshKeys(); + + // start sshd + helper.writeContainerStateToStdin(); + SshHook{}.startSshDaemon(); + helper.checkDefaultSshDaemonPort(); + helper.checkContainerHasSshBinary(); +} + +TEST(SSHHookTestGroup, testCustomServerPort) { + std::uint16_t expectedPort = 57864; + Helper helper{}; + + helper.setRootIds(); + helper.setCustomServerPort(expectedPort); + helper.setupTestEnvironment(); + + // generate + check SSH keys in local repository + helper.setUserIds(); // keygen is executed with user privileges + SshHook{}.generateSshKeys(true); + helper.setRootIds(); + helper.checkHostHasSshKeys(); + + // start sshd + helper.writeContainerStateToStdin(); + SshHook{}.startSshDaemon(); + CHECK(helper.getSshDaemonPort() == expectedPort); + helper.checkContainerHasSshBinary(); +} + }}}} // namespace SARUS_UNITTEST_MAIN_FUNCTION();