Skip to content

Commit

Permalink
[permissions] Remove all but top-level permission change
Browse files Browse the repository at this point in the history
  • Loading branch information
Sploder12 committed Dec 19, 2024
1 parent 73f969e commit 16b981f
Show file tree
Hide file tree
Showing 14 changed files with 2 additions and 101 deletions.
9 changes: 2 additions & 7 deletions include/multipass/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,13 @@ class Utils : public Singleton<Utils>
public:
Utils(const Singleton<Utils>::PrivatePass&) noexcept;

static constexpr QFileDevice::Permissions default_permissions =
QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner;

virtual qint64 filesystem_bytes_available(const QString& data_directory) const;
virtual void exit(int code);
virtual std::string contents_of(const multipass::Path& file_path) const;
virtual void make_file_with_content(const std::string& file_name, const std::string& content,
const bool& overwrite = false);
virtual Path make_dir(const QDir& a_dir,
const QString& name,
QFileDevice::Permissions permissions = default_permissions);
virtual Path make_dir(const QDir& dir, QFileDevice::Permissions permissions = default_permissions);
virtual Path make_dir(const QDir& a_dir, const QString& name, QFileDevice::Permissions permissions = {0});
virtual Path make_dir(const QDir& dir, QFileDevice::Permissions permissions = {0});

// command and process helpers
virtual std::string run_cmd_for_output(const QString& cmd, const QStringList& args,
Expand Down
3 changes: 0 additions & 3 deletions src/cert/client_cert_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include <QFile>
#include <QSaveFile>

#include <multipass/utils/permission_utils.h>
#include <stdexcept>

namespace mp = multipass;
Expand Down Expand Up @@ -77,8 +76,6 @@ void mp::ClientCertStore::add_cert(const std::string& pem_cert)
if (!MP_FILEOPS.open(file, QIODevice::WriteOnly))
throw std::runtime_error("failed to create file to store certificate");

MP_PERMISSIONS.restrict_permissions(file.fileName().toStdU16String());

// QIODevice::Append is not supported in QSaveFile, so must write out all of the
// existing clients certs each time.
for (const auto& saved_cert : authenticated_client_certs)
Expand Down
2 changes: 0 additions & 2 deletions src/daemon/default_vm_image_vault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
#include <QtConcurrent/QtConcurrent>

#include <exception>
#include <multipass/utils/permission_utils.h>

namespace mp = multipass;
namespace mpl = multipass::logging;
Expand Down Expand Up @@ -671,7 +670,6 @@ QString mp::DefaultVMImageVault::extract_image_from(const VMImage& source_image,
const mp::Path& dest_dir)
{
MP_UTILS.make_dir(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 Down
4 changes: 0 additions & 4 deletions src/iso/cloud_init_iso.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
#include <multipass/cloud_init_iso.h>
#include <multipass/file_ops.h>
#include <multipass/format.h>
#include <multipass/platform.h>
#include <multipass/utils/permission_utils.h>
#include <multipass/yaml_node_utils.h>

#include <QFile>
Expand Down Expand Up @@ -538,8 +536,6 @@ 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)};

MP_PERMISSIONS.restrict_permissions(path.toStdU16String());

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
3 changes: 0 additions & 3 deletions src/platform/backends/lxd/lxd_vm_image_vault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
#include <QTemporaryDir>

#include <chrono>
#include <multipass/utils/permission_utils.h>
#include <thread>

namespace mp = multipass;
Expand Down Expand Up @@ -94,8 +93,6 @@ QString post_process_downloaded_image(const QString& image_path, const mp::Progr
mp::vault::delete_file(original_image_path);
}

MP_PERMISSIONS.restrict_permissions(new_image_path.toStdU16String());

return new_image_path;
}

Expand Down
7 changes: 0 additions & 7 deletions src/utils/vm_image_vault_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
*/

#include <multipass/format.h>
#include <multipass/platform.h>
#include <multipass/utils/permission_utils.h>
#include <multipass/vm_image_host.h>
#include <multipass/vm_image_vault.h>
#include <multipass/xz_image_decoder.h>
Expand Down Expand Up @@ -47,9 +45,6 @@ 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_PERMISSIONS.restrict_permissions(new_path.toStdU16String());

return new_path;
}

Expand Down Expand Up @@ -97,8 +92,6 @@ QString mp::vault::extract_image(const mp::Path& image_path, const mp::ProgressM

mp::vault::delete_file(image_path);

MP_PERMISSIONS.restrict_permissions(new_image_path.toStdU16String());

return new_image_path;
}

Expand Down
4 changes: 0 additions & 4 deletions tests/lxd/test_lxd_image_vault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "tests/common.h"
#include "tests/mock_image_host.h"
#include "tests/mock_logger.h"
#include "tests/mock_permission_utils.h"
#include "tests/mock_process_factory.h"
#include "tests/stub_url_downloader.h"
#include "tests/temp_dir.h"
Expand Down Expand Up @@ -61,9 +60,6 @@ struct LXDImageVault : public Test

mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject();
std::unique_ptr<NiceMock<mpt::MockNetworkAccessManager>> mock_network_access_manager;
const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection =
mpt::MockPermissionUtils::inject<NiceMock>();
mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first;
std::vector<mp::VMImageHost*> hosts;
NiceMock<mpt::MockImageHost> host;
QUrl base_url{"unix:///[email protected]"};
Expand Down
1 change: 0 additions & 1 deletion tests/test_alias_dict.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,6 @@ struct DaemonAliasTestsuite

const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection =
mpt::MockPermissionUtils::inject<NiceMock>();
mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first;
};

TEST_P(DaemonAliasTestsuite, purge_removes_purged_instance_aliases_and_scripts)
Expand Down
4 changes: 0 additions & 4 deletions tests/test_base_virtual_machine_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "common.h"
#include "mock_logger.h"
#include "mock_permission_utils.h"
#include "mock_platform.h"
#include "stub_ssh_key_provider.h"
#include "stub_url_downloader.h"
Expand Down Expand Up @@ -124,9 +123,6 @@ 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_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));

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
13 changes: 0 additions & 13 deletions tests/test_client_cert_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "file_operations.h"
#include "mock_file_ops.h"
#include "mock_logger.h"
#include "mock_permission_utils.h"
#include "mock_utils.h"
#include "temp_dir.h"

Expand Down Expand Up @@ -100,9 +99,6 @@ TEST_F(ClientCertStore, add_cert_throws_on_invalid_data)

TEST_F(ClientCertStore, add_cert_stores_certificate)
{
auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));

mp::ClientCertStore cert_store{temp_dir.path()};
EXPECT_NO_THROW(cert_store.add_cert(cert_data));

Expand Down Expand Up @@ -155,9 +151,6 @@ TEST_F(ClientCertStore, addCertAlreadyExistingDoesNotAddAgain)

TEST_F(ClientCertStore, addCertWithExistingCertPersistsCerts)
{
auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));

const QDir dir{cert_dir};
const auto cert_path = dir.filePath("multipass_client_certs.pem");
mpt::make_file_with_content(cert_path, cert_data);
Expand Down Expand Up @@ -198,9 +191,6 @@ TEST_F(ClientCertStore, openingFileForWritingFailsAndThrows)
auto [mock_file_ops, guard] = mpt::MockFileOps::inject();
EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(false));

auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
EXPECT_CALL(*mock_permission_utils, set_permissions(_, _));

mp::ClientCertStore cert_store{temp_dir.path()};

MP_EXPECT_THROW_THAT(cert_store.add_cert(cert_data), std::runtime_error,
Expand All @@ -216,9 +206,6 @@ TEST_F(ClientCertStore, writingFileFailsAndThrows)
EXPECT_CALL(*mock_file_ops, write(_, _)).WillOnce(Return(-1));
EXPECT_CALL(*mock_file_ops, commit).WillOnce(Return(false));

auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));

mp::ClientCertStore cert_store{temp_dir.path()};

MP_EXPECT_THROW_THAT(cert_store.add_cert(cert_data), std::runtime_error,
Expand Down
1 change: 0 additions & 1 deletion tests/test_client_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ struct TestClientCommon : public mpt::DaemonTestFixture

const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection =
mpt::MockPermissionUtils::inject<NiceMock>();
mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first;

const std::string server_address{"localhost:50052"};
mpt::TempDir temp_dir;
Expand Down
38 changes: 0 additions & 38 deletions tests/test_cloud_init_iso.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

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

#include <multipass/cloud_init_iso.h>
Expand Down Expand Up @@ -71,14 +70,9 @@ struct CloudInitIso : public Test
CloudInitIso()
{
iso_path = QDir{temp_dir.path()}.filePath("test.iso");
std_iso_path = std::filesystem::path(iso_path.toStdU16String());
}
mpt::TempDir temp_dir;
QString iso_path;
std::filesystem::path std_iso_path;

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

TEST_F(CloudInitIso, check_contains_false)
Expand Down Expand Up @@ -140,8 +134,6 @@ TEST_F(CloudInitIso, check_index_operator_found_key)

TEST_F(CloudInitIso, creates_iso_file)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso iso;
iso.add_file("test", "test data");
iso.write_to(iso_path);
Expand All @@ -153,8 +145,6 @@ TEST_F(CloudInitIso, creates_iso_file)

TEST_F(CloudInitIso, reads_iso_file_failed_to_open_file)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.write_to(iso_path);

Expand All @@ -168,8 +158,6 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_open_file)

TEST_F(CloudInitIso, reads_iso_file_failed_to_read_single_bytes)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.write_to(iso_path);

Expand All @@ -188,8 +176,6 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_read_single_bytes)

TEST_F(CloudInitIso, reads_iso_file_failed_to_check_it_has_Joliet_volume_descriptor)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.write_to(iso_path);

Expand All @@ -213,8 +199,6 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_check_it_has_Joliet_volume_descrip

TEST_F(CloudInitIso, reads_iso_file_Joliet_volume_descriptor_malformed)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.write_to(iso_path);

Expand All @@ -240,8 +224,6 @@ TEST_F(CloudInitIso, reads_iso_file_Joliet_volume_descriptor_malformed)

TEST_F(CloudInitIso, reads_iso_file_failed_to_read_array)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.write_to(iso_path);

Expand All @@ -263,8 +245,6 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_read_array)

TEST_F(CloudInitIso, reads_iso_file_failed_to_check_root_dir_record_data)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.write_to(iso_path);

Expand Down Expand Up @@ -292,8 +272,6 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_check_root_dir_record_data)

TEST_F(CloudInitIso, reads_iso_file_failed_to_read_vec)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
// At least one actual file entry is need to reach the read_bytes_to_vec call
original_iso.add_file("test1", "test data1");
Expand All @@ -319,8 +297,6 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_read_vec)

TEST_F(CloudInitIso, reads_iso_file_encoded_file_name_is_not_even_length)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
// At least one actual file entry is need to reach the convert_u16_name_back call
original_iso.add_file("test1", "test data1");
Expand Down Expand Up @@ -356,8 +332,6 @@ TEST_F(CloudInitIso, reads_iso_file_encoded_file_name_is_not_even_length)

TEST_F(CloudInitIso, reads_iso_file_with_random_string_data)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;

original_iso.add_file("test1", "test data1");
Expand Down Expand Up @@ -395,8 +369,6 @@ timezone: Europe/Amsterdam
content: "multipass/version/1.14.0-dev.1209+g5b2c7f7d # written by Multipass\nmultipass/driver/qemu-8.0.4 # written by Multipass\nmultipass/host/ubuntu-23.10 # written by Multipass\nmultipass/alias/default # written by Multipass\n"
)";

EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;

original_iso.add_file("meta-data", default_meta_data_content);
Expand All @@ -412,8 +384,6 @@ timezone: Europe/Amsterdam

TEST_F(CloudInitIso, updateCloudInitWithNewNonEmptyExtraInterfaces)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);

mp::CloudInitIso original_iso;

original_iso.add_file("meta-data", default_meta_data_content);
Expand All @@ -440,8 +410,6 @@ TEST_F(CloudInitIso, updateCloudInitWithNewNonEmptyExtraInterfaces)

TEST_F(CloudInitIso, updateCloudInitWithNewEmptyExtraInterfaces)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);

mp::CloudInitIso original_iso;
original_iso.add_file("meta-data", default_meta_data_content);
original_iso.add_file("network-config", "dummy_data");
Expand All @@ -465,8 +433,6 @@ TEST_F(CloudInitIso, updateCloneCloudInitSrcFileWithExtraInterfaces)
const std::string src_network_config_data_content =
fmt::format(network_config_data_content_template, "00:00:00:00:00:00", "00:00:00:00:00:01");

EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);

mp::CloudInitIso original_iso;
original_iso.add_file("meta-data", src_meta_data_content);
original_iso.add_file("network-config", src_network_config_data_content);
Expand All @@ -492,8 +458,6 @@ TEST_F(CloudInitIso, updateCloneCloudInitSrcFileWithExtraInterfaces)

TEST_F(CloudInitIso, addExtraInterfaceToCloudInit)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);

mp::CloudInitIso original_iso;
original_iso.add_file("meta-data", default_meta_data_content);
original_iso.write_to(iso_path);
Expand All @@ -505,8 +469,6 @@ TEST_F(CloudInitIso, addExtraInterfaceToCloudInit)

TEST_F(CloudInitIso, getInstanceIdFromCloudInit)
{
EXPECT_CALL(mock_permission_utils, restrict_permissions(_));

mp::CloudInitIso original_iso;
original_iso.add_file("meta-data", default_meta_data_content);
original_iso.write_to(iso_path);
Expand Down
Loading

0 comments on commit 16b981f

Please sign in to comment.