diff --git a/CHANGELOG.md b/CHANGELOG.md index ec61c8c7..76f2ea7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). 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. + The `SERVER_PORT` variable is still supported for backward compatibility, although `SERVER_PORT_DEFAULT` takes precedence if set. ### Deprecated @@ -26,6 +26,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: permissions on the container's authorized keys file are now set explicitly, fixing possible errors caused by applying unsuitable defaults from the process. ## [1.6.3] diff --git a/src/hooks/ssh/SshHook.cpp b/src/hooks/ssh/SshHook.cpp index f48039e3..fc65a15c 100644 --- a/src/hooks/ssh/SshHook.cpp +++ b/src/hooks/ssh/SshHook.cpp @@ -241,10 +241,12 @@ void SshHook::generateAuthorizedKeys(const boost::filesystem::path& userKeyFile, auto matches = boost::smatch{}; auto re = boost::regex{"^(ecdsa-.*)$"}; + sarus::common::createFileIfNecessary(authorizedKeysFile, uidOfUser, gidOfUser); + // write public key to "authorized_keys" file while(std::getline(ss, line)) { if(boost::regex_match(line, matches, re)) { - auto ofs = std::ofstream{ authorizedKeysFile.c_str() }; + auto ofs = std::ofstream{authorizedKeysFile.c_str(), std::ios::out | std::ios::app}; ofs << matches[1] << std::endl; ofs.close(); log("Successfully generated \"authorized_keys\" file", sarus::common::LogLevel::INFO); @@ -252,6 +254,12 @@ void SshHook::generateAuthorizedKeys(const boost::filesystem::path& userKeyFile, } } + // set permissions + boost::filesystem::permissions(authorizedKeysFile, + boost::filesystem::owner_read | boost::filesystem::owner_write | + boost::filesystem::group_read | + boost::filesystem::others_read); + auto message = boost::format("Failed to parse key from %s") % userKeyFile; SARUS_THROW_ERROR(message.str()); } diff --git a/src/hooks/ssh/test/test_SSHHook.cpp b/src/hooks/ssh/test/test_SSHHook.cpp index c76b8205..37f80b17 100644 --- a/src/hooks/ssh/test/test_SSHHook.cpp +++ b/src/hooks/ssh/test/test_SSHHook.cpp @@ -231,10 +231,20 @@ class Helper { } void checkContainerHasClientKeys() const { - CHECK(boost::filesystem::exists(expectedHomeDirInContainer / ".ssh/id_dropbear")); - CHECK(sarus::common::getOwner(expectedHomeDirInContainer / ".ssh/id_dropbear") == idsOfUser); - CHECK(boost::filesystem::exists(expectedHomeDirInContainer / ".ssh/authorized_keys")); - CHECK(sarus::common::getOwner(expectedHomeDirInContainer / ".ssh/authorized_keys") == idsOfUser); + auto userKeyFile = expectedHomeDirInContainer / ".ssh/id_dropbear"; + auto authorizedKeysFile = expectedHomeDirInContainer / ".ssh/authorized_keys"; + + CHECK(boost::filesystem::exists(userKeyFile)); + CHECK(sarus::common::getOwner(userKeyFile) == idsOfUser); + CHECK(boost::filesystem::exists(authorizedKeysFile)); + CHECK(sarus::common::getOwner(authorizedKeysFile) == idsOfUser); + + auto expectedAuthKeysPermissions = + boost::filesystem::owner_read | boost::filesystem::owner_write | + boost::filesystem::group_read | + boost::filesystem::others_read; + auto status = boost::filesystem::status(authorizedKeysFile); + CHECK(status.permissions() == expectedAuthKeysPermissions); } boost::optional getSshDaemonPid() const { @@ -661,7 +671,7 @@ TEST(SSHHookTestGroup, testDefaultServerPortOverridesDeprecatedVar) { Helper helper{}; helper.setRootIds(); - helper.setupTestEnvironment(); + helper.setupTestEnvironment(); // "SERVER_PORT_DEFAULT" is set here sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort)); // generate + check SSH keys in local repository @@ -681,7 +691,7 @@ TEST(SSHHookTestGroup, testDeprecatedServerPort) { Helper helper{}; helper.setRootIds(); - helper.setupTestEnvironment(); + helper.setupTestEnvironment(); // "SERVER_PORT_DEFAULT" is set here sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort)); unsetenv("SERVER_PORT_DEFAULT");