Merge #1228

1228: [utils] resolve symlinks to snap directories (Fixes #1223) r=ricab a=Saviq



Co-authored-by: Michał Sawicz <[email protected]>

src/platform/backends/qemu/dnsmasq_process_spec.cpp

@@ -91,7 +91,7 @@ profile %1 flags=(attach_disconnected) {
   %3/{usr/,}lib/@{multiarch}/{,**/}*.so* rm,
 
   # CLASSIC ONLY: need to specify required libs from core snap
-  /snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
+  /{,var/lib/snapd/}snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
 
   %5/dnsmasq.leases rw,           # Leases file
   %5/dnsmasq.hosts r,             # Hosts file

src/platform/backends/qemu/qemu_vm_process_spec.cpp

@@ -234,7 +234,7 @@ profile %1 flags=(attach_disconnected) {
   %4/{,usr/}lib/{,@{multiarch}/}{,**/}*.so* rm,
 
   # CLASSIC ONLY: need to specify required libs from core snap
-  /snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
+  /{,var/lib/snapd/}snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
 
   # Disk images
   %6 rwk,  # QCow2 filesystem image

src/platform/backends/shared/linux/qemuimg_process_spec.cpp

@@ -50,7 +50,7 @@ profile %1 flags=(attach_disconnected) {
   %3/{usr/,}lib/@{multiarch}/{,**/}*.so* rm,
 
   # CLASSIC ONLY: need to specify required libs from core snap
-  /snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
+  /{,var/lib/snapd/}snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
 
   # Subdirectory containing disk image(s)
   %5/** rwk,

src/platform/backends/shared/sshfs_server_process_spec.cpp

@@ -110,7 +110,7 @@ profile %1 flags=(attach_disconnected) {
     %3/{usr/,}lib/** rm,
 
     # CLASSIC ONLY: need to specify required libs from core snap
-    /snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
+    /{,var/lib/snapd/}snap/core18/*/{,usr/}lib/@{multiarch}/{,**/}*.so* rm,
 
     # allow full access just to this user-specified source directory on the host
     %4/ rw,

src/utils/snap_utils.cpp

@@ -17,6 +17,8 @@
 
 #include <multipass/snap_utils.h>
 
+#include <QFileInfo>
+
 namespace mu = multipass::utils;
 
 bool mu::is_snap()
@@ -28,10 +30,12 @@ bool mu::is_snap()
 
 QByteArray mu::snap_dir()
 {
-    return qgetenv("SNAP"); // Inside snap, this can be trusted.
+    auto snap_dir = qgetenv("SNAP");                         // Inside snap, this can be trusted.
+    return QFileInfo(snap_dir).canonicalFilePath().toUtf8(); // To resolve any symlinks
 }
 
 QByteArray mu::snap_common_dir()
 {
-    return qgetenv("SNAP_COMMON"); // Inside snap, this can be trusted
+    auto snap_common = qgetenv("SNAP_COMMON");                  // Inside snap, this can be trusted
+    return QFileInfo(snap_common).canonicalFilePath().toUtf8(); // To resolve any symlinks
 }

tests/linux/test_qemuimg_process_spec.cpp

@@ -20,6 +20,9 @@
 #include "tests/mock_environment_helpers.h"
 #include <gmock/gmock.h>
 
+#include <QFile>
+#include <QTemporaryDir>
+
 namespace mp = multipass;
 namespace mpt = multipass::test;
 using namespace testing;
@@ -62,12 +65,29 @@ TEST(TestQemuImgProcessSpec, no_apparmor_profile_identifier)
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_snap_correct)
 {
-    mpt::SetEnvScope e("SNAP", "/something");
-    mpt::SetEnvScope e2("SNAP_COMMON", "/snap/common");
+    QTemporaryDir snap_dir, common_dir;
+    mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
+    mpt::SetEnvScope e2("SNAP_COMMON", common_dir.path().toUtf8());
+    mp::QemuImgProcessSpec spec({});
+
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/** rwk,").arg(common_dir.path())));
+}
+
+TEST(TestQemuImgProcessSpec, apparmor_profile_running_as_symlinked_snap_correct)
+{
+    QTemporaryDir snap_dir, snap_link_dir, common_dir, common_link_dir;
+    snap_link_dir.remove();
+    common_link_dir.remove();
+    QFile::link(snap_dir.path(), snap_link_dir.path());
+    QFile::link(common_dir.path(), common_link_dir.path());
+
+    mpt::SetEnvScope e("SNAP", snap_link_dir.path().toUtf8());
+    mpt::SetEnvScope e2("SNAP_COMMON", common_link_dir.path().toUtf8());
     mp::QemuImgProcessSpec spec({});
 
-    EXPECT_TRUE(spec.apparmor_profile().contains("/something/usr/bin/qemu-img ixr,"));
-    EXPECT_TRUE(spec.apparmor_profile().contains("/snap/common/** rwk,"));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-img ixr,").arg(snap_dir.path())));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/** rwk,").arg(common_dir.path())));
 }
 
 TEST(TestQemuImgProcessSpec, apparmor_profile_not_running_as_snap_correct)

tests/linux/test_snap_utils.cpp

@@ -17,6 +17,7 @@
 
 #include <multipass/snap_utils.h>
 
+#include <QFile>
 #include <QTemporaryDir>
 
 #include <gmock/gmock.h>
@@ -59,6 +60,16 @@ TEST(SnapUtils, test_snap_dir_null_if_not_set)
     EXPECT_EQ(QByteArray(), mu::snap_dir());
 }
 
+TEST(SnapUtils, test_snap_dir_resolves_links)
+{
+    QTemporaryDir snap_dir, link_dir;
+    link_dir.remove();
+    QFile::link(snap_dir.path(), link_dir.path());
+    mpt::SetEnvScope env("SNAP", link_dir.path().toUtf8());
+
+    EXPECT_EQ(snap_dir.path(), mu::snap_dir());
+}
+
 TEST(SnapUtils, test_snap_common_dir_read_ok)
 {
     QTemporaryDir snap_dir;
@@ -73,3 +84,13 @@ TEST(SnapUtils, test_snap_common_dir_null_if_not_set)
 
     EXPECT_EQ(QByteArray(), mu::snap_common_dir());
 }
+
+TEST(SnapUtils, test_snap_common_resolves_links)
+{
+    QTemporaryDir common_dir, link_dir;
+    link_dir.remove();
+    QFile::link(common_dir.path(), link_dir.path());
+    mpt::SetEnvScope env("SNAP_COMMON", link_dir.path().toUtf8());
+
+    EXPECT_EQ(common_dir.path(), mu::snap_common_dir());
+}

tests/qemu/test_dnsmasq_process_spec.cpp

@@ -21,6 +21,9 @@
 #include <gmock/gmock.h>
 #include <multipass/ip_address.h>
 
+#include <QFile>
+#include <QTemporaryDir>
+
 namespace mp = multipass;
 namespace mpt = multipass::test;
 using namespace testing;
@@ -70,11 +73,25 @@ TEST_F(TestDnsmasqProcessSpec, apparmor_profile_identifier)
 
 TEST_F(TestDnsmasqProcessSpec, apparmor_profile_running_as_snap_correct)
 {
-    mpt::SetEnvScope e1("SNAP", "/something");
+    QTemporaryDir snap_dir;
+
+    mpt::SetEnvScope e1("SNAP", snap_dir.path().toUtf8());
     mp::DNSMasqProcessSpec spec(data_dir, bridge_name, pid_file_path, subnet);
 
     EXPECT_TRUE(spec.apparmor_profile().contains("signal (receive) peer=snap.multipass.multipassd"));
-    EXPECT_TRUE(spec.apparmor_profile().contains("/something/usr/sbin/dnsmasq ixr,"));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/sbin/dnsmasq ixr,").arg(snap_dir.path())));
+}
+
+TEST_F(TestDnsmasqProcessSpec, apparmor_profile_running_as_symlinked_snap_correct)
+{
+    QTemporaryDir snap_dir, link_dir;
+    link_dir.remove();
+    QFile::link(snap_dir.path(), link_dir.path());
+
+    mpt::SetEnvScope e1("SNAP", link_dir.path().toUtf8());
+    mp::DNSMasqProcessSpec spec(data_dir, bridge_name, pid_file_path, subnet);
+
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/sbin/dnsmasq ixr,").arg(snap_dir.path())));
 }
 
 TEST_F(TestDnsmasqProcessSpec, apparmor_profile_not_running_as_snap_correct)

tests/qemu/test_qemu_vm_process_spec.cpp

@@ -20,6 +20,8 @@
 #include "tests/mock_environment_helpers.h"
 #include <gmock/gmock.h>
 
+#include <QTemporaryDir>
+
 namespace mp = multipass;
 namespace mpt = multipass::test;
 using namespace testing;
@@ -180,12 +182,27 @@ TEST_F(TestQemuVMProcessSpec, apparmor_profile_identifier)
 
 TEST_F(TestQemuVMProcessSpec, apparmor_profile_running_as_snap_correct)
 {
-    mpt::SetEnvScope e("SNAP", "/something");
+    QTemporaryDir snap_dir;
+
+    mpt::SetEnvScope e("SNAP", snap_dir.path().toUtf8());
     mp::QemuVMProcessSpec spec(desc, tap_device_name, mp::nullopt);
 
     EXPECT_TRUE(spec.apparmor_profile().contains("signal (receive) peer=snap.multipass.multipassd"));
-    EXPECT_TRUE(spec.apparmor_profile().contains("/something/qemu/* r,"));
-    EXPECT_TRUE(spec.apparmor_profile().contains("/something/usr/bin/qemu-system-"));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/qemu/* r,").arg(snap_dir.path())));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-system-").arg(snap_dir.path())));
+}
+
+TEST_F(TestQemuVMProcessSpec, apparmor_profile_running_as_symlinked_snap_correct)
+{
+    QTemporaryDir snap_dir, link_dir;
+    link_dir.remove();
+    QFile::link(snap_dir.path(), link_dir.path());
+
+    mpt::SetEnvScope e("SNAP", link_dir.path().toUtf8());
+    mp::QemuVMProcessSpec spec(desc, tap_device_name, mp::nullopt);
+
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/qemu/* r,").arg(snap_dir.path())));
+    EXPECT_TRUE(spec.apparmor_profile().contains(QString("%1/usr/bin/qemu-system-").arg(snap_dir.path())));
 }
 
 TEST_F(TestQemuVMProcessSpec, apparmor_profile_not_running_as_snap_correct)