From ab3ca1a93f2c8af991fff3d19a137e5af07da71d Mon Sep 17 00:00:00 2001
From: Ryan Lee <ryan.lee@canonical.com>
Date: Tue, 11 Feb 2025 11:48:41 -0800
Subject: [PATCH] profiles: fix non-user-namespace-related sandbox bypass in
 unshare profile

The unshare-userns-restrict profile contained a cx transition to
transition to a profile that allows most things while denying
capabilities:

audit allow cx /** -> unpriv,

However, this transition does not stack the unshare//unpriv profile
against any other profile the target binary might have had. As a result,
the lack of stacking resulted in a non-namespace-related sandboxing
bypass in which attachments of other profiles that should have confined
the target binary do not get applied. Instead, we adopt a stack similar
to the one in bwrap-userns-restrict, with the exception that unshare
does not use no-new-privs and therefore only needs a two-layer stack
instead of a three-layer stack.

Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
---
 .../profiles/extras/unshare-userns-restrict   | 65 +++++++++++--------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/profiles/apparmor/profiles/extras/unshare-userns-restrict b/profiles/apparmor/profiles/extras/unshare-userns-restrict
index ea19c05f8..8a6df50a4 100644
--- a/profiles/apparmor/profiles/extras/unshare-userns-restrict
+++ b/profiles/apparmor/profiles/extras/unshare-userns-restrict
@@ -18,10 +18,13 @@ abi <abi/4.0>,
 include <tunables/global>
 
 profile unshare /usr/bin/unshare flags=(attach_disconnected mediate_deleted) {
-  # not allow all, to allow for cix transition
-  # and to limit executable mapping to just unshare
+  # not allow all, to allow for pix stack on systems that don't support
+  # rule priority.
+  #
+  # sadly we have to allow 'm' every where to allow children to work under
+  # profile stacking atm.
   allow capability,
-  allow file rwlk /{**,},
+  allow file rwmlk /{**,},
   allow network,
   allow unix,
   allow ptrace,
@@ -33,33 +36,41 @@ profile unshare /usr/bin/unshare flags=(attach_disconnected mediate_deleted) {
   allow umount,
   allow pivot_root,
   allow dbus,
-  audit allow cx /** -> unpriv,
-
-  allow file m /usr/lib/@{multiarch}/libc.so.6,
-  allow file m /usr/bin/unshare,
+  # This will stack a target profile against unpriv_unshare
+  # Most of the comments for the pix transition in bwrap-userns-restrict
+  # also apply here, with the exception of unshare not using no-new-privs
+  # Thus, we only need a two-layer stack instead of a three-layer stack
+  audit allow pix /** -> &unpriv_unshare,
 
   # the local include should not be used without understanding the userns
   # restriction.
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/unshare-userns-restrict>
-
-  profile unpriv flags=(attach_disconnected mediate_deleted) {
-    # not allow all, to allow for pix stack
-    allow file rwlkm /{**,},
-    allow network,
-    allow unix,
-    allow ptrace,
-    allow signal,
-    allow mqueue,
-    allow io_uring,
-    allow userns,
-    allow mount,
-    allow umount,
-    allow pivot_root,
-    allow dbus,
-
-    allow pix /** -> &unshare//unpriv,
-
-    audit deny capability,
-  }
+}
+
+profile unpriv_unshare flags=(attach_disconnected mediate_deleted) {
+  # not allow all, to allow for pix stack
+  allow file rwlkm /{**,},
+  allow network,
+  allow unix,
+  allow ptrace,
+  allow signal,
+  allow mqueue,
+  allow io_uring,
+  allow userns,
+  allow mount,
+  allow umount,
+  allow pivot_root,
+  allow dbus,
+
+  # Maintain the stack against itself for further transitions
+  # If done recursively the stack will remove any duplicate
+  allow pix /** -> &unpriv_unshare,
+
+  audit deny capability,
+
+  # the local include should not be used without understanding the userns
+  # restriction.
+  # Site-specific additions and overrides. See local/README for details.
+  include if exists <local/unpriv_unshare>
 }