[PATCH 0/5] Fixes to AppArmor policy, passt/pasta profile split

[PATCH 0/5] Fixes to AppArmor policy, passt/pasta profile split

[Stefano Brivio]

These assume that distributions with AppArmor support will install
pasta and pasta.avx2 as hard links, and I prepared matching changes
for the Debian package. This particular aspect is
distribution-specific, so I'm not changing the symlink install from
the Makefile anyway.

Stefano Brivio (5):
  apparmor: Use abstractions/nameservice to deal with symlinked
    resolv.conf
  apparmor: Explicitly pass options we use while remounting root
    filesystem
  apparmor: Allow read-only access to uid_map
  apparmor: Allow pasta to remount /proc, access entries under its own
    copy
  apparmor: Add pasta's own profile

 contrib/apparmor/abstractions/passt |  7 ++++---
 contrib/apparmor/abstractions/pasta |  9 +++++++++
 contrib/apparmor/usr.bin.passt      | 12 ++----------
 contrib/apparmor/usr.bin.pasta      | 27 +++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 13 deletions(-)
 create mode 100644 contrib/apparmor/usr.bin.pasta

-- 
2.39.2

[PATCH 1/5] apparmor: Use abstractions/nameservice to deal with symlinked resolv.conf

[Stefano Brivio]

While abstractions/nameservice appeared too broad and overkill for
our simple need (read-only resolv.conf access), it properly deals
with symlinked resolv.conf files generated by systemd-resolved,
NetworkManager or suchlike.

If we just grant read-only access to /etc/resolv.conf, we'll fail to
read nameserver information in rather common configurations, because
AppArmor won't follow the symlink.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/apparmor/abstractions/passt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/apparmor/abstractions/passt b/contrib/apparmor/abstractions/passt
index 6e2a6f3..a16eb6e 100644
--- a/contrib/apparmor/abstractions/passt
+++ b/contrib/apparmor/abstractions/passt
@@ -15,8 +15,7 @@
 
   include <abstractions/base>
 
-  # Alternatively: include <abstractions/nameservice>
-  @{etc_ro}/resolv.conf			r,	# get_dns(), conf.c
+  include <abstractions/nameservice>		# get_dns(), conf.c
 
   capability net_bind_service,			# isolation.c, conf.c
   capability setuid,
-- 
@@ -15,8 +15,7 @@
 
   include <abstractions/base>
 
-  # Alternatively: include <abstractions/nameservice>
-  @{etc_ro}/resolv.conf			r,	# get_dns(), conf.c
+  include <abstractions/nameservice>		# get_dns(), conf.c
 
   capability net_bind_service,			# isolation.c, conf.c
   capability setuid,
-- 
2.39.2

[PATCH 2/5] apparmor: Explicitly pass options we use while remounting root filesystem

[Stefano Brivio]

As a result of AppArmor commit d4b0fef10a4a ("parser: fix rule flag
generation change_mount type rules"), we can't expect anymore to
get permission to mount() / read-write, with MS_REC | MS_UNBINDABLE
("runbindable", in AppArmor terms), if we don't explicitly pass those
flags as options. It used to work by mistake.

Now, the reasonable expectation would be that we could just change the
existing rule into:

  mount options=(rw, runbindable) "" -> /,

...but this now fails to load too, I think as a result of AppArmor
commit 9d3f8c6cc05d ("parser: fix parsing of source as mount point
for propagation type flags"). It works with 'rw' alone, but
'runbindable' is indeed a propagation type flag.

Skip the source specification, it doesn't add anything meaningful to
the rule anyway.

Reported-by: Paul Holzinger <pholzing@redhat.com>
Link: https://github.com/containers/podman/pull/19751
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/apparmor/abstractions/passt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/apparmor/abstractions/passt b/contrib/apparmor/abstractions/passt
index a16eb6e..d778222 100644
--- a/contrib/apparmor/abstractions/passt
+++ b/contrib/apparmor/abstractions/passt
@@ -26,7 +26,7 @@
   capability sys_ptrace,
 
   /					r,	# isolate_prefork(), isolation.c
-  mount		""	-> "/",
+  mount options=(rw, runbindable) /,
   mount		""	-> "/tmp/",
   pivot_root	"/tmp/" -> "/tmp/",
   umount	"/",
-- 
@@ -26,7 +26,7 @@
   capability sys_ptrace,
 
   /					r,	# isolate_prefork(), isolation.c
-  mount		""	-> "/",
+  mount options=(rw, runbindable) /,
   mount		""	-> "/tmp/",
   pivot_root	"/tmp/" -> "/tmp/",
   umount	"/",
-- 
2.39.2

[PATCH 3/5] apparmor: Allow read-only access to uid_map

[Stefano Brivio]

Starting with commit 770d1a4502dd ("isolation: Initially Keep
CAP_SETFCAP if running as UID 0 in non-init"), the lack of this rule
became more apparent as pasta needs to access uid_map in procfs even
as non-root.

However, both passt and pasta needs this, in case they are started as
root, so add this directly to passt's abstraction (which is sourced
by pasta's profile too).

Fixes: 770d1a4502dd ("isolation: Initially Keep CAP_SETFCAP if running as UID 0 in non-init")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/apparmor/abstractions/passt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/apparmor/abstractions/passt b/contrib/apparmor/abstractions/passt
index d778222..6bb25e0 100644
--- a/contrib/apparmor/abstractions/passt
+++ b/contrib/apparmor/abstractions/passt
@@ -31,6 +31,8 @@
   pivot_root	"/tmp/" -> "/tmp/",
   umount	"/",
 
+  owner @{PROC}/@{pid}/uid_map		r,	# conf_ugid()
+
   network netlink raw,				# nl_sock_init_do(), netlink.c
 
   network inet stream,				# tcp.c
-- 
@@ -31,6 +31,8 @@
   pivot_root	"/tmp/" -> "/tmp/",
   umount	"/",
 
+  owner @{PROC}/@{pid}/uid_map		r,	# conf_ugid()
+
   network netlink raw,				# nl_sock_init_do(), netlink.c
 
   network inet stream,				# tcp.c
-- 
2.39.2

[PATCH 4/5] apparmor: Allow pasta to remount /proc, access entries under its own copy

[Stefano Brivio]

Since commit b0e450aa8500 ("pasta: Detach mount namespace, (re)mount
procfs before spawning command"), we need to explicitly permit mount
of /proc, and access to entries under /proc/PID/net (after remount,
that's what AppArmor sees as path).

Fixes: b0e450aa8500 ("pasta: Detach mount namespace, (re)mount procfs before spawning command")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/apparmor/abstractions/pasta | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/contrib/apparmor/abstractions/pasta b/contrib/apparmor/abstractions/pasta
index 9cba25a..05c5d46 100644
--- a/contrib/apparmor/abstractions/pasta
+++ b/contrib/apparmor/abstractions/pasta
@@ -15,11 +15,18 @@
 
   include <abstractions/passt>
 
+  mount		""	-> "/proc/",
+
   @{PROC}/net/tcp			r,	# procfs_scan_listen(), util.c
   @{PROC}/net/tcp6			r,
   @{PROC}/net/udp			r,
   @{PROC}/net/udp6			r,
 
+  @{PROC}/@{pid}/net/tcp		r,	# procfs_scan_listen(), util.c
+  @{PROC}/@{pid}/net/tcp6		r,
+  @{PROC}/@{pid}/net/udp		r,
+  @{PROC}/@{pid}/net/udp6		r,
+
   @{run}/user/@{uid}/netns/*		r,	# pasta_open_ns(), pasta.c
 
   @{PROC}/[0-9]*/ns/net			r,	# pasta_wait_for_ns(),
-- 
@@ -15,11 +15,18 @@
 
   include <abstractions/passt>
 
+  mount		""	-> "/proc/",
+
   @{PROC}/net/tcp			r,	# procfs_scan_listen(), util.c
   @{PROC}/net/tcp6			r,
   @{PROC}/net/udp			r,
   @{PROC}/net/udp6			r,
 
+  @{PROC}/@{pid}/net/tcp		r,	# procfs_scan_listen(), util.c
+  @{PROC}/@{pid}/net/tcp6		r,
+  @{PROC}/@{pid}/net/udp		r,
+  @{PROC}/@{pid}/net/udp6		r,
+
   @{run}/user/@{uid}/netns/*		r,	# pasta_open_ns(), pasta.c
 
   @{PROC}/[0-9]*/ns/net			r,	# pasta_wait_for_ns(),
-- 
2.39.2

[PATCH 5/5] apparmor: Add pasta's own profile

[Stefano Brivio]

If pasta and pasta.avx2 are hard links to passt and passt.avx2,
AppArmor will attach their own profiles on execution, and we can
restrict passt's profile to what it actually needs. Note that pasta
needs to access all the resources that passt needs, so the pasta
abstraction still includes passt's one.

I plan to push the adaptation required for the Debian package in
commit 5bb812e79143 ("debian/rules: Override pasta symbolic links
with hard links"), on Salsa. If other distributions need to support
AppArmor profiles they can follow a similar approach.

The profile itself will be installed, there, via dh_apparmor, in a
separate commit, b52557fedcb1 ("debian/rules: Install new pasta
profile using dh_apparmor").

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/apparmor/abstractions/pasta |  2 ++
 contrib/apparmor/usr.bin.passt      | 12 ++----------
 contrib/apparmor/usr.bin.pasta      | 27 +++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 10 deletions(-)
 create mode 100644 contrib/apparmor/usr.bin.pasta

diff --git a/contrib/apparmor/abstractions/pasta b/contrib/apparmor/abstractions/pasta
index 05c5d46..a890391 100644
--- a/contrib/apparmor/abstractions/pasta
+++ b/contrib/apparmor/abstractions/pasta
@@ -40,3 +40,5 @@
 
   owner @{PROC}/sys/net/ipv4/ping_group_range w, # pasta_spawn_cmd(), pasta.c
   /{usr/,}bin/**			Ux,
+
+  /usr/bin/pasta.avx2			ix,	# arch_avx2_exec(), arch.c
diff --git a/contrib/apparmor/usr.bin.passt b/contrib/apparmor/usr.bin.passt
index 652051d..564f82f 100644
--- a/contrib/apparmor/usr.bin.passt
+++ b/contrib/apparmor/usr.bin.passt
@@ -6,7 +6,7 @@
 # PASTA - Pack A Subtle Tap Abstraction
 #  for network namespace/tap device mode
 #
-# contrib/apparmor/usr.bin.passt - AppArmor profile for passt(1) and pasta(1)
+# contrib/apparmor/usr.bin.passt - AppArmor profile for passt(1)
 #
 # Copyright (c) 2022 Red Hat GmbH
 # Author: Stefano Brivio <sbrivio@redhat.com>
@@ -15,13 +15,7 @@ abi <abi/3.0>,
 
 include <tunables/global>
 
-profile passt /usr/bin/passt{,.avx2} flags=(attach_disconnected) {
-  ### TODO: AppArmor doesn't give us the chance to attach a separate profile
-  ### depending on the executable symlink. That's possible with SELinux. Two
-  ### alternatives: implement that in AppArmor, or consider aa_change_hat(2).
-  ### With this, rules for passt(1) could be restricted significantly. Note that
-  ### the attach_disconnected flag is not needed for passt(1).
-
+profile passt /usr/bin/passt{,.avx2} {
   include <abstractions/passt>
 
   # Alternatively: include <abstractions/user-tmp>
@@ -30,6 +24,4 @@ profile passt /usr/bin/passt{,.avx2} flags=(attach_disconnected) {
 						# logfile_init()
 
   owner @{HOME}/**			w,	# pcap(), write_pidfile()
-
-  include <abstractions/pasta>
 }
diff --git a/contrib/apparmor/usr.bin.pasta b/contrib/apparmor/usr.bin.pasta
new file mode 100644
index 0000000..e5ee4df
--- /dev/null
+++ b/contrib/apparmor/usr.bin.pasta
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# PASST - Plug A Simple Socket Transport
+#  for qemu/UNIX domain socket mode
+#
+# PASTA - Pack A Subtle Tap Abstraction
+#  for network namespace/tap device mode
+#
+# contrib/apparmor/usr.bin.pasta - AppArmor profile for pasta(1)
+#
+# Copyright (c) 2022 Red Hat GmbH
+# Author: Stefano Brivio <sbrivio@redhat.com>
+
+abi <abi/3.0>,
+
+include <tunables/global>
+
+profile pasta /usr/bin/pasta{,.avx2} flags=(attach_disconnected) {
+  include <abstractions/pasta>
+
+  # Alternatively: include <abstractions/user-tmp>
+  owner /tmp/**				w,	# tap_sock_unix_init(), pcap(),
+						# write_pidfile(),
+						# logfile_init()
+
+  owner @{HOME}/**			w,	# pcap(), write_pidfile()
+}
-- 
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# PASST - Plug A Simple Socket Transport
+#  for qemu/UNIX domain socket mode
+#
+# PASTA - Pack A Subtle Tap Abstraction
+#  for network namespace/tap device mode
+#
+# contrib/apparmor/usr.bin.pasta - AppArmor profile for pasta(1)
+#
+# Copyright (c) 2022 Red Hat GmbH
+# Author: Stefano Brivio <sbrivio@redhat.com>
+
+abi <abi/3.0>,
+
+include <tunables/global>
+
+profile pasta /usr/bin/pasta{,.avx2} flags=(attach_disconnected) {
+  include <abstractions/pasta>
+
+  # Alternatively: include <abstractions/user-tmp>
+  owner /tmp/**				w,	# tap_sock_unix_init(), pcap(),
+						# write_pidfile(),
+						# logfile_init()
+
+  owner @{HOME}/**			w,	# pcap(), write_pidfile()
+}
-- 
2.39.2