From 3307feafef6e94e98695f3c6ef64c148354456b5 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Wed, 3 Apr 2024 23:16:42 +0200 Subject: [PATCH] cmd/snap-confine, interfaces/udev: device cgroup support for non-strict confinement (#13777) * interfaces/udev: add non-strict flag to snap cgroup device file Add a non-strict=true flag to the snap's cgroup device file, to inform snap-confine that the snap was indeed installed in a non-strict confinement mode (eg. devmode, or classic). This supplements an earlier mechanism in which snapd would not generate any rules tagging devices for a specific snap and can be used as an explicit indicator to avoid mandatory device cgroup even when using bare or core24 and later bases (as well as custom base snaps). Signed-off-by: Maciej Borzecki * cmd/snap-confine: account for non-strict confinement when setting up device cgroup Snaps may be installed in a non-strict confinement mode. In which case, expect an explicit non-strict=true in the per snap /var/lib/snapd/cgroup/snap.*.file. This replaces an earlier mechanism of implicit non-strict confinement when no devices are assigned to the snap. Signed-off-by: Maciej Borzecki * tests/main/security-device-cgroups-required-or-optional: update to check non-strict confinement Update the test to check that --devmode results in a non-strict confinement device cgroup setup. Signed-off-by: Maciej Borzecki --------- Signed-off-by: Maciej Borzecki --- cmd/snap-confine/snap-confine.c | 35 ++++++++++++++---- interfaces/udev/backend.go | 8 +++-- interfaces/udev/backend_test.go | 36 +++++++++++++------ .../task.yaml | 34 +++++++++++++++++- 4 files changed, 94 insertions(+), 19 deletions(-) diff --git a/cmd/snap-confine/snap-confine.c b/cmd/snap-confine/snap-confine.c index 29534678c12..d492e259473 100644 --- a/cmd/snap-confine/snap-confine.c +++ b/cmd/snap-confine/snap-confine.c @@ -627,8 +627,18 @@ static void enter_classic_execution_environment(const sc_invocation *inv, /* max wait time for /var/lib/snapd/cgroup/.devices to appear */ static const size_t DEVICES_FILE_MAX_WAIT = 120; -static bool is_device_cgroup_self_managed(const sc_invocation *inv) +struct sc_device_cgroup_options { + bool self_managed; + bool non_strict; +}; + +static void sc_get_device_cgroup_setup(const sc_invocation *inv, struct sc_device_cgroup_options + *devsetup) { + if (devsetup == NULL) { + die("internal error: devsetup is NULL"); + } + char info_path[PATH_MAX] = { 0 }; sc_must_snprintf(info_path, sizeof info_path, @@ -648,14 +658,22 @@ static bool is_device_cgroup_self_managed(const sc_invocation *inv) die("cannot open %s", info_path); } + sc_error *err SC_CLEANUP(sc_cleanup_error) = NULL; char *self_managed_value SC_CLEANUP(sc_cleanup_string) = NULL; - sc_error *err = NULL; if (sc_infofile_get_key (stream, "self-managed", &self_managed_value, &err) < 0) { sc_die_on_error(err); } + rewind(stream); + + char *non_strict_value SC_CLEANUP(sc_cleanup_string) = NULL; + if (sc_infofile_get_key(stream, "non-strict", &non_strict_value, &err) < + 0) { + sc_die_on_error(err); + } - return sc_streq(self_managed_value, "true"); + devsetup->self_managed = sc_streq(self_managed_value, "true"); + devsetup->non_strict = sc_streq(non_strict_value, "true"); } static sc_device_cgroup_mode device_cgroup_mode_for_snap(sc_invocation *inv) @@ -682,6 +700,7 @@ static sc_device_cgroup_mode device_cgroup_mode_for_snap(sc_invocation *inv) break; } } + return mode; } @@ -712,11 +731,15 @@ static void enter_non_classic_execution_environment(sc_invocation *inv, // Set up a device cgroup, unless the snap has been allowed to manage the // device cgroup by itself. - if (!is_device_cgroup_self_managed(inv)) { + struct sc_device_cgroup_options cgdevopts = { false, false }; + sc_get_device_cgroup_setup(inv, &cgdevopts); + if (cgdevopts.self_managed) { + debug("device cgroup is self-managed by the snap"); + } else if (cgdevopts.non_strict) { + debug("device cgroup skipped, snap in non-strict confinement"); + } else { sc_device_cgroup_mode mode = device_cgroup_mode_for_snap(inv); sc_setup_device_cgroup(inv->security_tag, mode); - } else { - debug("device cgroup is self-managed by the snap"); } /** diff --git a/interfaces/udev/backend.go b/interfaces/udev/backend.go index 5288fc11b24..94afbf9df08 100644 --- a/interfaces/udev/backend.go +++ b/interfaces/udev/backend.go @@ -153,15 +153,19 @@ func (b *Backend) Setup(appSet *interfaces.SnapAppSet, opts interfaces.Confineme var deviceBuf bytes.Buffer deviceBuf.WriteString("# This file is automatically generated.\n") - deviceBuf.WriteString("# snap is allowed to manage own device cgroup.\n") if udevSpec.ControlsDeviceCgroup() { // The spec states that the snap can manage its own device // cgroup (typically applies to container-like snaps), in which // case leave a flag for snap-confine in at a known location. - + deviceBuf.WriteString("# snap is allowed to manage own device cgroup.\n") deviceBuf.WriteString("self-managed=true\n") } + if (opts.DevMode || opts.Classic) && !opts.JailMode { + // Allow devmode + deviceBuf.WriteString("# snap uses non-strict confinement.\n") + deviceBuf.WriteString("non-strict=true\n") + } // the file serves as a checkpoint that udev backend was set up err = osutil.EnsureFileState(selfManageDeviceCgroupPath, &osutil.MemoryFileState{ diff --git a/interfaces/udev/backend_test.go b/interfaces/udev/backend_test.go index bc70cbf4f7a..bf319a489fe 100644 --- a/interfaces/udev/backend_test.go +++ b/interfaces/udev/backend_test.go @@ -332,6 +332,17 @@ func (s *backendSuite) TestCombineSnippetsWithActualSnippets(c *C) { stat, err := os.Stat(fname) c.Assert(err, IsNil) c.Check(stat.Mode(), Equals, os.FileMode(0644)) + + cgroupFname := filepath.Join(dirs.SnapCgroupPolicyDir, "snap.samba.device") + if !opts.DevMode && !opts.Classic { + c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n") + } else { + c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n"+ + "# snap uses non-strict confinement.\n"+ + "non-strict=true\n", + ) + } + s.RemoveSnap(c, snapInfo) } } @@ -348,10 +359,19 @@ func (s *backendSuite) TestControlsDeviceCgroup(c *C) { fname := filepath.Join(dirs.SnapUdevRulesDir, "70-snap.samba.rules") c.Check(fname, testutil.FileAbsent) cgroupFname := filepath.Join(dirs.SnapCgroupPolicyDir, "snap.samba.device") - c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n"+ - "# snap is allowed to manage own device cgroup.\n"+ - "self-managed=true\n", - ) + if !opts.DevMode && !opts.Classic { + c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n"+ + "# snap is allowed to manage own device cgroup.\n"+ + "self-managed=true\n", + ) + } else { + c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n"+ + "# snap is allowed to manage own device cgroup.\n"+ + "self-managed=true\n"+ + "# snap uses non-strict confinement.\n"+ + "non-strict=true\n", + ) + } c.Check(s.udevadmCmd.Calls(), HasLen, 0) s.RemoveSnap(c, snapInfo) c.Check(cgroupFname, testutil.FileAbsent) @@ -407,9 +427,7 @@ func (s *backendSuite) TestDeviceCgroupAlwaysPresent(c *C) { fname := filepath.Join(dirs.SnapUdevRulesDir, "70-snap.samba.rules") snapInfo := s.InstallSnap(c, interfaces.ConfinementOptions{}, "", ifacetest.SambaYamlV1, 0) // device cgroup self manage flag is gone now - c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n"+ - "# snap is allowed to manage own device cgroup.\n", - ) + c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n") // and we have the rules file c.Check(fname, testutil.FileEquals, "# This file is automatically generated.\nsample\n") // and udev was called @@ -634,9 +652,7 @@ func (s *backendSuite) TestPreseed(c *C) { fname := filepath.Join(dirs.SnapUdevRulesDir, "70-snap.samba.rules") s.InstallSnap(c, interfaces.ConfinementOptions{}, "", ifacetest.SambaYamlV1, 0) // device cgroup self manage flag is gone now - c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n"+ - "# snap is allowed to manage own device cgroup.\n", - ) + c.Check(cgroupFname, testutil.FileEquals, "# This file is automatically generated.\n") // and we have the rules file c.Check(fname, testutil.FileEquals, "# This file is automatically generated.\nsample\n") diff --git a/tests/main/security-device-cgroups-required-or-optional/task.yaml b/tests/main/security-device-cgroups-required-or-optional/task.yaml index 515f56af6f6..40436bcc23d 100644 --- a/tests/main/security-device-cgroups-required-or-optional/task.yaml +++ b/tests/main/security-device-cgroups-required-or-optional/task.yaml @@ -12,14 +12,19 @@ systems: - -ubuntu-18.04-32 execute: | + #shellcheck source=tests/lib/systems.sh + . "$TESTSLIB"/systems.sh + echo "Given snap is installed" "$TESTSTOOLS"/snaps-state install-local test-snapd-sh-core20 test -f /var/lib/snapd/cgroup/snap.test-snapd-sh-core20.device + NOMATCH "non-strict=true" < /var/lib/snapd/cgroup/snap.test-snapd-sh-core20.device # XXX explicitly install core24 until there is no release into the stable channel snap install --edge core24 "$TESTSTOOLS"/snaps-state install-local test-snapd-sh-core24 - test -f /var/lib/snapd/cgroup/snap.test-snapd-sh-core20.device + test -f /var/lib/snapd/cgroup/snap.test-snapd-sh-core24.device + NOMATCH "non-strict=true" < /var/lib/snapd/cgroup/snap.test-snapd-sh-core24.device echo "No devices are assigned to either snap" udevadm info "/dev/null" | NOMATCH "E: TAGS=.*snap_test-snapd-sh.*" @@ -42,3 +47,30 @@ execute: | echo "Device is listed as allowed" tests.device-cgroup test-snapd-sh-core24.sh dump | MATCH "c 1:3" + + # drop persistent cgroup information + if is_cgroupv2; then + rm /sys/fs/bpf/snap/snap_test-snapd-sh-core24_sh + test ! -e /sys/fs/bpf/snap/snap_test-snapd-sh-core20_sh + else + rmdir /sys/fs/cgroup/devices/snap.test-snapd-sh-core24.sh + test ! -e /sys/fs/cgroup/devices/snap.test-snapd-sh-core20.sh + fi + + echo "When snaps are installed in devmode" + "$TESTSTOOLS"/snaps-state install-local test-snapd-sh-core20 --devmode + MATCH "non-strict=true" < /var/lib/snapd/cgroup/snap.test-snapd-sh-core20.device + + "$TESTSTOOLS"/snaps-state install-local test-snapd-sh-core24 --devmode + MATCH "non-strict=true" < /var/lib/snapd/cgroup/snap.test-snapd-sh-core24.device + + test-snapd-sh-core20.sh -c 'true' + test-snapd-sh-core24.sh -c 'true' + + if is_cgroupv2; then + test ! -e /sys/fs/bpf/snap/snap_test-snapd-sh-core24_sh + test ! -e /sys/fs/bpf/snap/snap_test-snapd-sh-core20_sh + else + test ! -e /sys/fs/cgroup/devices/snap.test-snapd-sh-core24.sh + test ! -e /sys/fs/cgroup/devices/snap.test-snapd-sh-core20.sh + fi