[WIP] many: non-setuid snap-confine, caps v4 #15094

bboozzoo · 2025-02-18T11:17:47Z

This is a work-in-progress branch of non-setuid snap-confine. Originally based on #14970 and attempts to address the comments made in that PR.

github-actions · 2025-02-18T13:09:20Z

Wed Feb 26 13:23:33 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/13542856525

Failures:

Executing:

openstack:fedora-41-64:tests/main/snap-confine-undesired-mode-group
google-core:ubuntu-core-18-64:tests/core/kernel-base-gadget-pair-single-reboot:kernel_gadget
google-core:ubuntu-core-18-64:tests/main/snap-confine-undesired-mode-group
google-core:ubuntu-core-20-64:tests/main/snap-confine-undesired-mode-group
google:ubuntu-20.04-64:tests/main/interfaces-network-control
google:ubuntu-20.04-64:tests/main/snap-confine-undesired-mode-group
google:ubuntu-24.10-64:tests/main/snap-confine-undesired-mode-group

Restoring:

google-core:ubuntu-core-20-64:tests/main/snap-quota

codecov · 2025-02-18T14:06:22Z

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 78.06%. Comparing base (a272aac) to head (c304154).
Report is 54 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/snap-update-ns/system.go	40.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #15094      +/-   ##
==========================================
- Coverage   78.07%   78.06%   -0.01%     
==========================================
  Files        1182     1185       +3     
  Lines      157743   158161     +418     
==========================================
+ Hits       123154   123476     +322     
- Misses      26943    27010      +67     
- Partials     7646     7675      +29

Flag	Coverage Δ
unittests	`78.06% <57.14%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bboozzoo · 2025-02-19T11:14:56Z

cmd/libsnap-confine-private/device-cgroup-support.c

@@ -717,8 +717,8 @@ static int sc_udev_open_cgroup_v1(const char *security_tag, int flags, sc_cgroup
            if (fchownat(devices_fd, security_tag_relpath, 0, 0, AT_SYMLINK_NOFOLLOW) < 0) {
                die("cannot set root ownership on %s/%s/%s", cgroup_path, devices_relpath, security_tag_relpath);
            }
-            if (fchmodat(devices_fd, security_tag_relpath, 0755, 0) < 0) {
-                die("cannot set 0755 permissions on %s/%s/%s", cgroup_path, devices_relpath, security_tag_relpath);
+            if (fchmodat(devices_fd, security_tag_relpath, 0700, 0) < 0) {


to be checked

bboozzoo · 2025-02-19T11:15:12Z

cmd/libsnap-confine-private/cgroup-support.c

        die("cannot create cgroup hierarchy %s/%s", parent, name);
    }
-    (void)sc_set_effective_identity(old);
+    if (errno == 0) {
+        if (fchownat(parent_fd, name, 0, 0, 0) < 0 || fchmodat(parent_fd, name, 0700, 0) < 0) {


permissions to be checked

bboozzoo · 2025-02-19T11:23:12Z

cmd/libsnap-confine-private/utils.c

@@ -268,3 +277,31 @@ static bool _sc_is_in_container(const char *p) {
 }

 bool sc_is_in_container(void) { return _sc_is_in_container(run_systemd_container); }
+
+int sc_ensure_mkdirat(int fd, const char *name, mode_t mode, uid_t uid, uid_t gid) {
+    if (mkdirat(fd, name, mode) < 0) {


use 0o000 mode and chmod later on

bboozzoo · 2025-02-25T15:46:15Z

cmd/snap-confine/snap-confine.c

+            /* TODO:nonsetuid: workaround PR_SET_KEEPCAPS */
+            sc_identity old = sc_set_effective_identity(sc_root_group_identity());
+            sc_cgroup_freezer_join(inv->snap_instance, getpid());
+            (void)sc_set_effective_identity(old);


Since snap-confine is eventually going to be run under an ordinary user, make sure that the snaps' cgroup directories are created as the root user and group. Also, since we do not have direct control over the ownership of the leaf files defining the cgroup (they are automatically created using the effective user of the process that creates the cgroup), remove the execute permission from the parent directory, so that only the root user will be able to alter the snap's cgroup configuration.

Create a wrapper that works well with 64 bit capabilities.

Read the process state (for the time being, that's just the current working directory) as the real user who started the application, because if we do this operation while we are setuid root, it would fail on some filesystems (like SSHFS and NFS) which restrict access to the root user.

Move the AppArmor check up, right after checking that the effective user is root. Remove the check on the effective user being 0, since we assert it right above. Avoid calling getuid() and use the freshly obtained `real_uid` instead.

When needed, add code to explicitly adjust the group ownership. Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>