Skip to content

Commit 97a70c3

Browse files
SaviqSaviq
committed
[apparmor] explicitly allow qemu-img image paths
1 parent 51d21da commit 97a70c3

File tree

5 files changed

+64
-25
lines changed

5 files changed

+64
-25
lines changed

‎include/multipass/process/qemuimg_process_spec.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace multipass
2929
class QemuImgProcessSpec : public ProcessSpec
3030
{
3131
public:
32-
explicit QemuImgProcessSpec(const QStringList& args);
32+
explicit QemuImgProcessSpec(const QStringList& args, const QString& source_image, const QString& target_image = {});
3333

3434
QString program() const override;
3535
QStringList arguments() const override;
@@ -38,6 +38,8 @@ class QemuImgProcessSpec : public ProcessSpec
3838

3939
private:
4040
const QStringList args;
41+
const QString source_image;
42+
const QString target_image;
4143
};
4244

4345
} // namespace multipass

‎src/daemon/daemon.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,8 @@ mp::InstanceStatus::Status grpc_instance_status_for(const mp::VirtualMachine::St
606606
mp::MemorySize get_image_size(const mp::VMImage& image)
607607
{
608608
QStringList qemuimg_parameters{{"info", image.image_path}};
609-
auto qemuimg_process = mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters));
609+
auto qemuimg_process =
610+
mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters, image.image_path));
610611
auto process_state = qemuimg_process->execute();
611612

612613
if (!process_state.completed_successfully())

‎src/platform/backends/shared/linux/backend_utils.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ void mp::backend::resize_instance_image(const MemorySize& disk_space, const mp::
133133
{
134134
auto disk_size = QString::number(disk_space.in_bytes()); // format documented in `man qemu-img` (look for "size")
135135
QStringList qemuimg_parameters{{"resize", image_path, disk_size}};
136-
auto qemuimg_process = mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters));
136+
auto qemuimg_process =
137+
mp::platform::make_process(std::make_unique<mp::QemuImgProcessSpec>(qemuimg_parameters, "", image_path));
137138

138139
auto process_state = qemuimg_process->execute(mp::backend::image_resize_timeout);
139140
if (!process_state.completed_successfully())
@@ -150,7 +151,8 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
150151
// TODO: we could support converting from other the image formats that qemu-img can deal with
151152
const auto qcow2_path{image_path + ".qcow2"};
152153

153-
auto qemuimg_info_spec = std::make_unique<mp::QemuImgProcessSpec>(QStringList{"info", "--output=json", image_path});
154+
auto qemuimg_info_spec =
155+
std::make_unique<mp::QemuImgProcessSpec>(QStringList{"info", "--output=json", image_path}, image_path);
154156
auto qemuimg_info_process = MP_PROCFACTORY.create_process(std::move(qemuimg_info_spec));
155157

156158
auto process_state = qemuimg_info_process->execute();
@@ -167,7 +169,7 @@ mp::Path mp::backend::convert_to_qcow_if_necessary(const mp::Path& image_path)
167169
if (image_record["format"].toString() == "raw")
168170
{
169171
auto qemuimg_convert_spec = std::make_unique<mp::QemuImgProcessSpec>(
170-
QStringList{"convert", "-p", "-O", "qcow2", image_path, qcow2_path});
172+
QStringList{"convert", "-p", "-O", "qcow2", image_path, qcow2_path}, image_path, qcow2_path);
171173
auto qemuimg_convert_process = MP_PROCFACTORY.create_process(std::move(qemuimg_convert_spec));
172174
process_state = qemuimg_convert_process->execute(mp::backend::image_resize_timeout);
173175

‎src/process/qemuimg_process_spec.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
namespace mp = multipass;
2323
namespace mu = multipass::utils;
2424

25-
mp::QemuImgProcessSpec::QemuImgProcessSpec(const QStringList& args) : args{args}
25+
mp::QemuImgProcessSpec::QemuImgProcessSpec(const QStringList& args, const QString& source_image,
26+
const QString& target_image)
27+
: args{args}, source_image{source_image}, target_image{target_image}
2628
{
2729
}
2830

@@ -52,8 +54,8 @@ profile %1 flags=(attach_disconnected) {
5254
# CLASSIC ONLY: need to specify required libs from core snap
5355
/{,var/lib/snapd/}snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
5456
55-
# Subdirectory containing disk image(s)
56-
%5/** rwk,
57+
# Images
58+
%5
5759
5860
# Allow multipassd send qemu-img signals
5961
signal (receive) peer=%6,
@@ -62,14 +64,13 @@ profile %1 flags=(attach_disconnected) {
6264

6365
/* Customisations depending on if running inside snap or not */
6466
QString root_dir; // root directory: either "" or $SNAP
65-
QString image_dir;
67+
QString images;
6668
QString extra_capabilities;
6769
QString signal_peer; // who can send kill signal to qemu-img
6870

6971
try
7072
{
7173
root_dir = mu::snap_dir();
72-
image_dir = mu::snap_common_dir(); // FIXME - am guessing we work inside this directory
7374
signal_peer = "snap.multipass.multipassd"; // only multipassd can send qemu-img signals
7475
}
7576
catch (mp::SnapEnvironmentException&)
@@ -78,9 +79,13 @@ profile %1 flags=(attach_disconnected) {
7879
"capability dac_read_search,\n capability dac_override,"; // FIXME - unclear why this is required when
7980
// not snap confined
8081
signal_peer = "unconfined";
81-
image_dir = ""; // FIXME - Do not know where disk images might be, as passed by argument
8282
}
8383

84-
return profile_template.arg(apparmor_profile_name(), extra_capabilities, root_dir, program(), image_dir,
85-
signal_peer);
84+
if (!source_image.isEmpty())
85+
images.append(QString(" %1 rk,\n").arg(source_image));
86+
87+
if (!target_image.isEmpty())
88+
images.append(QString(" %1 rwk,\n").arg(target_image));
89+
90+
return profile_template.arg(apparmor_profile_name(), extra_capabilities, root_dir, program(), images, signal_peer);
8691
}

‎tests/test_qemuimg_process_spec.cpp‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,36 +29,36 @@ using namespace testing;
2929

3030
TEST(TestQemuImgProcessSpec, program_correct)
3131
{
32-
mp::QemuImgProcessSpec spec({});
32+
mp::QemuImgProcessSpec spec({}, "");
3333

3434
EXPECT_EQ(spec.program(), "qemu-img");
3535
}
3636

3737
TEST(TestQemuImgProcessSpec, default_arguments_correct)
3838
{
39-
mp::QemuImgProcessSpec spec({});
39+
mp::QemuImgProcessSpec spec({}, "");
4040

4141
EXPECT_EQ(spec.arguments(), QStringList());
4242
}
4343

4444
TEST(TestQemuImgProcessSpec, arguments_set_correctly)
4545
{
4646
QStringList args{"-one", "--two"};
47-
mp::QemuImgProcessSpec spec(args);
47+
mp::QemuImgProcessSpec spec(args, "");
4848

4949
EXPECT_EQ(spec.arguments(), args);
5050
}
5151

5252
TEST(TestQemuImgProcessSpec, apparmor_profile_has_correct_name)
5353
{
54-
mp::QemuImgProcessSpec spec({});
54+
mp::QemuImgProcessSpec spec({}, "");
5555

5656
EXPECT_TRUE(spec.apparmor_profile().contains("profile multipass.qemu-img"));
5757
}
5858

5959
TEST(TestQemuImgProcessSpec, no_apparmor_profile_identifier)
6060
{
61-
mp::QemuImgProcessSpec spec({});
61+
mp::QemuImgProcessSpec spec({}, "");
6262

6363
EXPECT_EQ(spec.identifier(), "");
6464
}
@@ -67,33 +67,62 @@ TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_correct)
6767
{
6868
const QByteArray snap_name{"multipass"};
6969
QTemporaryDir snap_dir, common_dir;
70+
QString source_image{"/source/image/file"};
7071

7172
mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
72-
mpt::SetEnvScope e2("SNAP_COMMON", common_dir.path().toUtf8());
7373
mpt::SetEnvScope e3("SNAP_NAME", snap_name);
74-
mp::QemuImgProcessSpec spec({});
74+
mp::QemuImgProcessSpec spec({}, source_image);
7575

7676
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
77-
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/** rwk,").arg(common_dir.path())));
77+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
78+
}
79+
80+
TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_with_target_correct)
81+
{
82+
const QByteArray snap_name{"multipass"};
83+
QTemporaryDir snap_dir, common_dir;
84+
QString source_image{"/source/image/file"}, target_image{"/target/image/file"};
85+
86+
mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
87+
mpt::SetEnvScope e3("SNAP_NAME", snap_name);
88+
mp::QemuImgProcessSpec spec({}, source_image, target_image);
89+
90+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
91+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
92+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(target_image)));
93+
}
94+
95+
TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_with_only_target_correct)
96+
{
97+
const QByteArray snap_name{"multipass"};
98+
QTemporaryDir snap_dir, common_dir;
99+
QString target_image{"/target/image/file"};
100+
101+
mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
102+
mpt::SetEnvScope e3("SNAP_NAME", snap_name);
103+
mp::QemuImgProcessSpec spec({}, "", target_image);
104+
105+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
106+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rwk,").arg(target_image)));
78107
}
79108

80109
TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_symlinked_snap_correct)
81110
{
82111
const QByteArray snap_name{"multipass"};
83112
QTemporaryDir snap_dir, snap_link_dir, common_dir, common_link_dir;
113+
QString source_image{"/source/image/file"};
84114

85115
snap_link_dir.remove();
86116
common_link_dir.remove();
87117
QFile::link(snap_dir.path(), snap_link_dir.path());
88118
QFile::link(common_dir.path(), common_link_dir.path());
89119

90120
mpt::SetEnvScope e("SNAP", snap_link_dir.path().toUtf8());
91-
mpt::SetEnvScope e2("SNAP_COMMON", common_link_dir.path().toUtf8());
92121
mpt::SetEnvScope e3("SNAP_NAME", snap_name);
93-
mp::QemuImgProcessSpec spec({});
122+
mp::QemuImgProcessSpec spec({}, source_image);
94123

95124
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
96-
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/** rwk,").arg(common_dir.path())));
125+
EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1 rk,").arg(source_image)));
97126
}
98127

99128
TEST(TestQemuImgProcessSpec, apparmor_profile_not_running_as_snap_correct)
@@ -102,7 +131,7 @@ TEST(TestQemuImgProcessSpec, apparmor_profile_not_running_as_snap_correct)
102131

103132
mpt::UnsetEnvScope e("SNAP");
104133
mpt::SetEnvScope e2("SNAP_NAME", snap_name);
105-
mp::QemuImgProcessSpec spec({});
134+
mp::QemuImgProcessSpec spec({}, "");
106135

107136
EXPECT_TRUE(spec.apparmor_profile().contains("capability dac_read_search,"));
108137
EXPECT_TRUE(spec.apparmor_profile().contains(" /usr/bin/qemu-img ixr,")); // space wanted

0 commit comments

Comments
 (0)