Skip to content

Commit

Permalink
[daemon] Change permissions for instances to root only
Browse files Browse the repository at this point in the history
  • Loading branch information
Sploder12 committed Oct 3, 2024
1 parent 950779c commit 2193d7b
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 4 deletions.
3 changes: 2 additions & 1 deletion include/multipass/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ class Platform : public Singleton<Platform>
virtual bool is_backend_supported(const QString& backend) const; // temporary (?)
virtual int chown(const char* path, unsigned int uid, unsigned int gid) const;
virtual int chmod(const char* path, unsigned int mode) const;
virtual bool set_permissions(const multipass::Path path, const QFileDevice::Permissions permissions) const;
virtual bool set_permissions(const multipass::Path& path, const QFileDevice::Permissions permissions) const;
virtual bool set_root_as_owner(const multipass::Path& path) const;
virtual bool link(const char* target, const char* link) const;
virtual bool symlink(const char* target, const char* link, bool is_dir) const;
virtual int utime(const char* path, int atime, int mtime) const;
Expand Down
7 changes: 5 additions & 2 deletions src/daemon/default_vm_image_vault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,9 @@ QString mp::DefaultVMImageVault::extract_image_from(const VMImage& source_image,
const ProgressMonitor& monitor,
const mp::Path& dest_dir)
{
MP_UTILS.make_dir(dest_dir);
MP_UTILS.make_dir(dest_dir, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner);
MP_PLATFORM.set_root_as_owner(dest_dir);

QFileInfo file_info{source_image.image_path};
const auto image_name = file_info.fileName().remove(".xz");
const auto image_path = QDir(dest_dir).filePath(image_name);
Expand All @@ -655,7 +657,8 @@ QString mp::DefaultVMImageVault::extract_image_from(const VMImage& source_image,

mp::VMImage mp::DefaultVMImageVault::image_instance_from(const VMImage& prepared_image, const mp::Path& dest_dir)
{
MP_UTILS.make_dir(dest_dir);
MP_UTILS.make_dir(dest_dir, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner);
MP_PLATFORM.set_root_as_owner(dest_dir);

return {mp::vault::copy(prepared_image.image_path, dest_dir),
prepared_image.id,
Expand Down
7 changes: 7 additions & 0 deletions src/iso/cloud_init_iso.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <multipass/cloud_init_iso.h>
#include <multipass/file_ops.h>
#include <multipass/format.h>
#include <multipass/platform.h>
#include <multipass/yaml_node_utils.h>

#include <QFile>
Expand Down Expand Up @@ -536,6 +537,12 @@ void mp::CloudInitIso::write_to(const Path& path)
throw std::runtime_error{fmt::format(
"Failed to open file for writing during cloud-init generation: {}; path: {}", f.errorString(), path)};

if (!MP_PLATFORM.set_permissions(path, QFile::ReadOwner | QFile::WriteOwner))
throw std::runtime_error{fmt::format("Failed to set permissions during cloud-init generation: path: {}", path)};

if (!MP_PLATFORM.set_root_as_owner(path))
throw std::runtime_error{fmt::format("Failed to set owner during cloud-init generation: path: {}", path)};

const uint32_t num_reserved_bytes = 32768u;
const uint32_t num_reserved_blocks = num_blocks(num_reserved_bytes);
f.seek(num_reserved_bytes);
Expand Down
7 changes: 6 additions & 1 deletion src/platform/platform_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,16 @@ int mp::platform::Platform::chmod(const char* path, unsigned int mode) const
return ::chmod(path, mode);
}

bool mp::platform::Platform::set_permissions(const mp::Path path, const QFileDevice::Permissions permissions) const
bool mp::platform::Platform::set_permissions(const mp::Path& path, const QFileDevice::Permissions permissions) const
{
return QFile::setPermissions(path, permissions);
}

bool mp::platform::Platform::set_root_as_owner(const mp::Path& path) const
{
return this->chown(path.toStdString().c_str(), 0, 0) == 0;
}

bool mp::platform::Platform::symlink(const char* target, const char* link, bool is_dir) const
{
return ::symlink(target, link) == 0;
Expand Down
8 changes: 8 additions & 0 deletions src/utils/vm_image_vault_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

#include <multipass/format.h>
#include <multipass/platform.h>
#include <multipass/vm_image_host.h>
#include <multipass/vm_image_vault.h>
#include <multipass/xz_image_decoder.h>
Expand Down Expand Up @@ -45,6 +46,10 @@ QString mp::vault::copy(const QString& file_name, const QDir& output_dir)
const auto source_name = info.fileName();
auto new_path = output_dir.filePath(source_name);
QFile::copy(file_name, new_path);

MP_PLATFORM.set_permissions(new_path, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner);
MP_PLATFORM.set_root_as_owner(new_path);

return new_path;
}

Expand Down Expand Up @@ -90,6 +95,9 @@ QString mp::vault::extract_image(const mp::Path& image_path, const mp::ProgressM

xz_decoder.decode_to(new_image_path, monitor);

MP_PLATFORM.set_permissions(new_image_path, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner);
MP_PLATFORM.set_root_as_owner(new_image_path);

mp::vault::delete_file(image_path);

return new_image_path;
Expand Down
5 changes: 5 additions & 0 deletions tests/mock_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ class MockPlatform : public platform::Platform
MOCK_METHOD(bool, is_alias_supported, (const std::string&, const std::string&), (const, override));
MOCK_METHOD(int, chmod, (const char*, unsigned int), (const, override));
MOCK_METHOD(int, chown, (const char*, unsigned int, unsigned int), (const, override));
MOCK_METHOD(bool,
set_permissions,
(const multipass::Path& path, const QFileDevice::Permissions),
(const, override));
MOCK_METHOD(bool, set_root_as_owner, (const multipass::Path& path), (const, override));
MOCK_METHOD(bool, link, (const char*, const char*), (const, override));
MOCK_METHOD(bool, symlink, (const char*, const char*, bool), (const, override));
MOCK_METHOD(int, utime, (const char*, int, int), (const, override));
Expand Down
5 changes: 5 additions & 0 deletions tests/test_base_virtual_machine_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ TEST_F(BaseFactory, networks_throws)
// at this time. Instead, just make sure an ISO image is created and has the expected path.
TEST_F(BaseFactory, creates_cloud_init_iso_image)
{
auto [mock_platform, platform_guard] = mpt::MockPlatform::inject();

ON_CALL(*mock_platform, set_permissions).WillByDefault(Return(true));
ON_CALL(*mock_platform, set_root_as_owner).WillByDefault(Return(true));

MockBaseFactory factory;
const std::string name{"foo"};
const YAML::Node metadata{YAML::Load({fmt::format("name: {}", name)})}, vendor_data{metadata}, user_data{metadata},
Expand Down
7 changes: 7 additions & 0 deletions tests/test_cloud_init_iso.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "common.h"
#include "mock_file_ops.h"
#include "mock_platform.h"
#include "temp_dir.h"

#include <multipass/cloud_init_iso.h>
Expand Down Expand Up @@ -51,9 +52,15 @@ struct CloudInitIso : public Test
CloudInitIso()
{
iso_path = QDir{temp_dir.path()}.filePath("test.iso");

ON_CALL(mock_platform, set_permissions).WillByDefault(Return(true));
ON_CALL(mock_platform, set_root_as_owner).WillByDefault(Return(true));
}
mpt::TempDir temp_dir;
QString iso_path;

const mpt::MockPlatform::GuardedMock attr{mpt::MockPlatform::inject<NiceMock>()};
mpt::MockPlatform& mock_platform = *attr.first;
};

TEST_F(CloudInitIso, check_contains_false)
Expand Down

0 comments on commit 2193d7b

Please sign in to comment.