Skip to content

Commit b03976d

Browse files
committed
SSH Hook: ensure container authorized keys file has correct permissions
1 parent b4e5d7a commit b03976d

File tree

3 files changed

+27
-8
lines changed

3 files changed

+27
-8
lines changed

CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1616
In particular, a warning about adding libraries into the container has been moved to a higher verbosity level
1717
(i.e. it will only be displayed when using the `--verbose` or `--debug` global command-line options).
1818
- 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.
19-
The `SERVER_PORT` variable is still supported for backward compatibility reasons, although `SERVER_PORT_DEFAULT` takes precedence if set.
19+
The `SERVER_PORT` variable is still supported for backward compatibility, although `SERVER_PORT_DEFAULT` takes precedence if set.
2020

2121
### Deprecated
2222

@@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
2626
### Fixed
2727

2828
- Glibc hook: fixed detection of the container's glibc version, which was causing a shell-init error on some systems
29+
- 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.
2930

3031

3132
## [1.6.3]

src/hooks/ssh/SshHook.cpp

+9-1
Original file line numberDiff line numberDiff line change
@@ -241,17 +241,25 @@ void SshHook::generateAuthorizedKeys(const boost::filesystem::path& userKeyFile,
241241
auto matches = boost::smatch{};
242242
auto re = boost::regex{"^(ecdsa-.*)$"};
243243

244+
sarus::common::createFileIfNecessary(authorizedKeysFile, uidOfUser, gidOfUser);
245+
244246
// write public key to "authorized_keys" file
245247
while(std::getline(ss, line)) {
246248
if(boost::regex_match(line, matches, re)) {
247-
auto ofs = std::ofstream{ authorizedKeysFile.c_str() };
249+
auto ofs = std::ofstream{authorizedKeysFile.c_str(), std::ios::out | std::ios::app};
248250
ofs << matches[1] << std::endl;
249251
ofs.close();
250252
log("Successfully generated \"authorized_keys\" file", sarus::common::LogLevel::INFO);
251253
return;
252254
}
253255
}
254256

257+
// set permissions
258+
boost::filesystem::permissions(authorizedKeysFile,
259+
boost::filesystem::owner_read | boost::filesystem::owner_write |
260+
boost::filesystem::group_read |
261+
boost::filesystem::others_read);
262+
255263
auto message = boost::format("Failed to parse key from %s") % userKeyFile;
256264
SARUS_THROW_ERROR(message.str());
257265
}

src/hooks/ssh/test/test_SSHHook.cpp

+16-6
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,20 @@ class Helper {
231231
}
232232

233233
void checkContainerHasClientKeys() const {
234-
CHECK(boost::filesystem::exists(expectedHomeDirInContainer / ".ssh/id_dropbear"));
235-
CHECK(sarus::common::getOwner(expectedHomeDirInContainer / ".ssh/id_dropbear") == idsOfUser);
236-
CHECK(boost::filesystem::exists(expectedHomeDirInContainer / ".ssh/authorized_keys"));
237-
CHECK(sarus::common::getOwner(expectedHomeDirInContainer / ".ssh/authorized_keys") == idsOfUser);
234+
auto userKeyFile = expectedHomeDirInContainer / ".ssh/id_dropbear";
235+
auto authorizedKeysFile = expectedHomeDirInContainer / ".ssh/authorized_keys";
236+
237+
CHECK(boost::filesystem::exists(userKeyFile));
238+
CHECK(sarus::common::getOwner(userKeyFile) == idsOfUser);
239+
CHECK(boost::filesystem::exists(authorizedKeysFile));
240+
CHECK(sarus::common::getOwner(authorizedKeysFile) == idsOfUser);
241+
242+
auto expectedAuthKeysPermissions =
243+
boost::filesystem::owner_read | boost::filesystem::owner_write |
244+
boost::filesystem::group_read |
245+
boost::filesystem::others_read;
246+
auto status = boost::filesystem::status(authorizedKeysFile);
247+
CHECK(status.permissions() == expectedAuthKeysPermissions);
238248
}
239249

240250
boost::optional<pid_t> getSshDaemonPid() const {
@@ -661,7 +671,7 @@ TEST(SSHHookTestGroup, testDefaultServerPortOverridesDeprecatedVar) {
661671
Helper helper{};
662672

663673
helper.setRootIds();
664-
helper.setupTestEnvironment();
674+
helper.setupTestEnvironment(); // "SERVER_PORT_DEFAULT" is set here
665675
sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort));
666676

667677
// generate + check SSH keys in local repository
@@ -681,7 +691,7 @@ TEST(SSHHookTestGroup, testDeprecatedServerPort) {
681691
Helper helper{};
682692

683693
helper.setRootIds();
684-
helper.setupTestEnvironment();
694+
helper.setupTestEnvironment(); // "SERVER_PORT_DEFAULT" is set here
685695
sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort));
686696
unsetenv("SERVER_PORT_DEFAULT");
687697

0 commit comments

Comments
 (0)