Skip to content

Commit

Permalink
SSH Hook: ensure container authorized keys file has correct permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
Madeeks committed Feb 27, 2024
1 parent b4e5d7a commit b03976d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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]
Expand Down
10 changes: 9 additions & 1 deletion src/hooks/ssh/SshHook.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,17 +241,25 @@ 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);
return;
}
}

// 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());
}
Expand Down
22 changes: 16 additions & 6 deletions src/hooks/ssh/test/test_SSHHook.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<pid_t> getSshDaemonPid() const {
Expand Down Expand Up @@ -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
Expand All @@ -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");

Expand Down

0 comments on commit b03976d

Please sign in to comment.