Skip to content

Commit

Permalink
cmd/snap-confine, interfaces/udev: device cgroup support for non-stri…
Browse files Browse the repository at this point in the history 
…ct 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Signed-off-by: Maciej Borzecki <[email protected]>
  • Loading branch information
@bboozzoo
bboozzoo authored Apr 3, 2024
1 parent fe5d801 commit 3307fea
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 19 deletions.
35 changes: 29 additions & 6 deletions cmd/snap-confine/snap-confine.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,18 @@ static void enter_classic_execution_environment(const sc_invocation *inv,
/* max wait time for /var/lib/snapd/cgroup/<snap>.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,
Expand All @@ -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)
Expand All @@ -682,6 +700,7 @@ static sc_device_cgroup_mode device_cgroup_mode_for_snap(sc_invocation *inv)
break;
}
}

return mode;
}

Expand Down Expand Up @@ -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");
}

/**
Expand Down
8 changes: 6 additions & 2 deletions interfaces/udev/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
36 changes: 26 additions & 10 deletions interfaces/udev/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")

Expand Down
34 changes: 33 additions & 1 deletion tests/main/security-device-cgroups-required-or-optional/task.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*"
Expand All @@ -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

0 comments on commit 3307fea

Please sign in to comment.