From b4e5d7ab63dd3fc0dd7ae96535063ff27333fea8 Mon Sep 17 00:00:00 2001 From: Alberto Madonna Date: Mon, 26 Feb 2024 20:33:15 +0100 Subject: [PATCH] SSH hook: deprecated the use of the `SERVER_PORT` env var in hook config file --- CHANGELOG.md | 8 +++++- doc/config/ssh-hook.rst | 4 +++ src/hooks/ssh/SshHook.cpp | 12 ++++++++- src/hooks/ssh/SshHook.hpp | 2 +- src/hooks/ssh/test/test_SSHHook.cpp | 41 +++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a71fcfc6..ec61c8c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - MPI hook: verbosity levels for log messages about ABI compatibility and library replacements have been slightly adjusted. In particular, a warning about adding libraries into the container has been moved to a higher verbosity level (i.e. it will only be displayed when using the `--verbose` or `--debug` global command-line options). +- SSH hook: the default port used by the Dropbear server is now set through the `SERVER_PORT_DEFAULT` environment variable in the hook JSON configuration file. + The `SERVER_PORT` variable is still supported for backward compatibility reasons, although `SERVER_PORT_DEFAULT` takes precedence if set. + +### Deprecated + +- SSH hook: usage of the `SERVER_PORT` environment variable in the hook JSON configuration file has been deprecated. + Support for it will be removed in a future release. ### 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/doc/config/ssh-hook.rst b/doc/config/ssh-hook.rst index ea20ea29..7d2b556c 100644 --- a/doc/config/ssh-hook.rst +++ b/doc/config/ssh-hook.rst @@ -43,6 +43,10 @@ environment variables must be defined: daemon that could be running on the host. This value can be overridden by setting the ``com.hooks.ssh.port`` annotation for the container. + ``SERVER_PORT_DEFAULT`` takes precedence over the deprecated ``SERVER_PORT`` + environment variable, which serves the same purpose. + Support for ``SERVER_PORT`` will be removed in a future release. + The following optional environment variables can also be defined: * ``OVERLAY_MOUNT_HOME_SSH``: When set to ``False`` (case-insensitive), an overlay diff --git a/src/hooks/ssh/SshHook.cpp b/src/hooks/ssh/SshHook.cpp index f5870a05..f48039e3 100644 --- a/src/hooks/ssh/SshHook.cpp +++ b/src/hooks/ssh/SshHook.cpp @@ -81,7 +81,17 @@ 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_DEFAULT")); + try { + serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT")); + } catch (sarus::common::Error&) {} + try { + serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT_DEFAULT")); + } catch (sarus::common::Error& e) { + if (serverPort == 0) { + SARUS_RETHROW_ERROR(e, "At least one of the environment variables SERVER_PORT_DEFAULT (preferred)" + " or SERVER_PORT (deprecated) must be defined.") + } + } try { auto envJoinNamespaces = sarus::common::getEnvironmentVariable("JOIN_NAMESPACES"); joinNamespaces = (boost::algorithm::to_upper_copy(envJoinNamespaces) == std::string("TRUE")); diff --git a/src/hooks/ssh/SshHook.hpp b/src/hooks/ssh/SshHook.hpp index 12d9b529..7b6a8ca7 100644 --- a/src/hooks/ssh/SshHook.hpp +++ b/src/hooks/ssh/SshHook.hpp @@ -69,7 +69,7 @@ class SshHook { pid_t pidOfContainer; uid_t uidOfUser; gid_t gidOfUser; - std::uint16_t serverPort; + std::uint16_t serverPort = 0; bool overlayMountHostDotSsh = true; }; diff --git a/src/hooks/ssh/test/test_SSHHook.cpp b/src/hooks/ssh/test/test_SSHHook.cpp index 0a459277..c76b8205 100644 --- a/src/hooks/ssh/test/test_SSHHook.cpp +++ b/src/hooks/ssh/test/test_SSHHook.cpp @@ -656,6 +656,47 @@ TEST(SSHHookTestGroup, testDefaultServerPort) { helper.checkContainerHasSshBinary(); } +TEST(SSHHookTestGroup, testDefaultServerPortOverridesDeprecatedVar) { + std::uint16_t expectedPort = 29476; + Helper helper{}; + + helper.setRootIds(); + helper.setupTestEnvironment(); + sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort)); + + // 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(); +} + +TEST(SSHHookTestGroup, testDeprecatedServerPort) { + std::uint16_t expectedPort = 44184; + Helper helper{}; + + helper.setRootIds(); + helper.setupTestEnvironment(); + sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort)); + unsetenv("SERVER_PORT_DEFAULT"); + + // 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); +} + TEST(SSHHookTestGroup, testCustomServerPort) { std::uint16_t expectedPort = 57864; Helper helper{};