Merge profiles: force read permission to their attachment path

Unconfined delegates access to open file descriptors. Therefore when running a confined binary from unconfined, it will work even when the attachment path is not read-allowed.

However, as soon as these confined binaries are run from another confined process, this delegation is not permitted anymore and the program breaks.

This has been the cause of several bugs such as https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2107455 or https://github.com/canonical/snapd/pull/15181 .

This MR makes sure every confining AppArmor profiles explicitly allow (at least) read access to their attachment path.

This Merge request:
 - Introduce `test_profile.sh`, a helper script that ensures confining AppArmor profiles explicitly allow (at least) read access to their attachment path.
 - Modifies a lot of profiles so that all profiles have r/mr access to their attachment path
 - Extends `make check` to automatically ensure all AppArmor profile grant explicit read access to their attachment path, preventing future omissions.
 - Modifies apparmor_parser to show attachment in --debug output

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1637
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>

parser/parser.h

@@ -410,6 +410,7 @@ extern int process_policydb(Profile *prof);
 
 extern int process_policy_ents(Profile *prof);
 extern void filter_slashes(char *path);
+extern const char *local_name(const char *name);
 
 /* parser_variable.c */
 int expand_entry_variables(char **name);

parser/parser_regex.c

@@ -433,7 +433,7 @@ out:
 	return ptype;
 }
 
-static const char *local_name(const char *name)
+const char *local_name(const char *name)
 {
 	const char *t;
 

parser/profile.h

@@ -459,7 +459,12 @@ public:
 			printf("Name:\t\t%s\n", name);
 		else
 			printf("Name:\t\t<NULL>\n");
-
+		if (attachment)
+			printf("Attachment:\t%s\n", attachment);
+		else {
+			const char *local = local_name(name);
+			printf("Attachment:\t%s\n", local[0] == '/' ? local : "<NULL>");
+		}
 		if (parent)
 			printf("Local To:\t%s\n", parent->name);
 		else

parser/tst/test_profile.sh

@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# Check if the current profile allow reading its attachment
+check_entry() {
+	local prof_name="$1"
+	local -n lines_ref="$2"
+	local attachment="$3"
+	local found=0
+
+	for line in "${lines_ref[@]}"; do
+		if [[ $line == Perms:*r*:*"($attachment)"* ]]; then
+			found=1
+			break
+		fi
+	done
+
+	if [[ $found -eq 0 ]]; then
+		echo -e "\e[0;31mProfile $prof_name: ERROR: no Perms rule for '$attachment'.\e[0m"
+		exit 1
+	fi
+
+	[[ -n "${VERBOSE:-}" ]] && echo -e "\e[0;32mProfile $prof_name: OK '$attachment' found\e[0m" || true
+}
+
+# Handle the end of a profile block: either skip it or check for the entry.
+finish_profile() {
+	local name="$1"
+	local prof_file="$2"
+	local skip="$3"
+	local attachment="$4"
+	local arr_name="$5"
+
+	if [[ -n $name ]]; then
+		if [[ $skip != 0 ]]; then
+			[[ -n "${VERBOSE:-}" ]] && echo "Profile '$name' skipped: $skip" || true
+		else
+			check_entry "$prof_file ($name)" "$arr_name" "$attachment"
+		fi
+	fi
+}
+
+process_profile() {
+	local prof_file="$1"
+	shift
+	local dump curr_name="" attachment="" skip_profile=0 in_entries=0
+	local block_lines=()
+
+	if ! dump=$(../parser/apparmor_parser $@ -d "$prof_file" 2>&1); then
+		echo "\e[0;31mERROR: Failed to parse '$prof_file': $dump\e[0m" >&2
+		exit 1
+	fi
+
+	IFS=$'\n' read -r -d '' -a lines < <(printf '%s\n' "$dump" && printf '\0')
+
+	for line in "${lines[@]}"; do
+		if [[ $line =~ ^[[:space:]]*Name:[[:space:]]*([^[:space:]]+) ]]; then
+			finish_profile "$curr_name" "$prof_file" "$skip_profile" "$attachment" block_lines
+			curr_name="${BASH_REMATCH[1]}"
+			attachment="" skip_profile=0 in_entries=0 block_lines=()
+		elif [[ $line =~ ^[[:space:]]*Mode:[[:space:]]*unconfined ]]; then
+			skip_profile="unconfined"
+		elif [[ $line =~ ^Perms:.*r.*:.*:.*\(/(\{?,?\*\*,*\}?)\) ]]; then
+			skip_profile="All files available"
+		elif [[ $line =~ ^[[:space:]]*Attachment:[[:space:]]*(.+) ]]; then
+			attachment="${BASH_REMATCH[1]}"
+			[[ $attachment == "<NULL>" ]] && skip_profile="no attachment"
+		elif [[ $line == ---\ Entries\ --- ]]; then
+			in_entries=1
+		elif [[ $in_entries -ne 0 ]]; then
+			block_lines+=("$line")
+		fi
+	done
+
+	# Last profile
+	finish_profile "$curr_name" "$prof_file" "$skip_profile" "$attachment" block_lines
+}
+
+if (( $# < 1 )); then
+	echo "Usage: $0 <profile-file> [parser_extra_args]"
+	exit 1
+fi
+
+process_profile $@

profiles/apparmor.d/Xorg

@@ -58,6 +58,8 @@ profile Xorg /usr/lib/xorg/Xorg flags=(attach_disconnected, complain) {
   /{,usr/}bin/{bash,dash,sh} ix,
   /usr/bin/xkbcomp           ix,
 
+  /usr/lib/xorg/Xorg      mr,
+
   @{PROC}/cmdline         r,
   @{PROC}/@{pid}/cmdline  r,
   @{PROC}/ioports         r,

profiles/apparmor.d/alsamixer

@@ -10,6 +10,8 @@ profile alsamixer /{usr,}/bin/alsamixer {
 
     include <abstractions/dbus-session-strict>
 
+    /{usr,}/bin/alsamixer mr,
+
     @{sys}/devices/virtual/dmi/id/sys_vendor r,
 
     @{PROC}/@{pid}/task/@{tid}/comm rw,

profiles/apparmor.d/babeld

@@ -17,6 +17,7 @@ profile babeld /usr/lib/frr/babeld flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
 
+  /usr/lib/frr/babeld mr,
   @{run}/frr/babel-state w,
 
   # Site-specific additions and overrides. See local/README for details.

profiles/apparmor.d/bfdd

@@ -20,6 +20,8 @@ profile bfdd /usr/lib/frr/bfdd flags=(attach_disconnected) {
   capability net_raw,
   capability sys_admin,
 
+
+  /usr/lib/frr/bfdd mr,
   @{run}/netns/* r,
 
   @{run}/frr/bfdd.sock w,

profiles/apparmor.d/bgpd

@@ -21,6 +21,8 @@ profile bgpd /usr/lib/frr/bgpd flags=(attach_disconnected) {
   capability net_raw,
   capability sys_admin,
 
+  /usr/lib/frr/bgpd mr,
+
   @{run}/netns/* r,
 
   owner @{PROC}/@{pid}/task/@{tid}/comm rw,

profiles/apparmor.d/bin.ping

@@ -22,7 +22,7 @@ profile ping /{usr/,}bin/{,iputils-}ping {
   network inet raw,
   network inet6 raw,
 
-  /{,usr/}bin/{,iputils-}ping mixr,
+  /{usr/,}bin/{,iputils-}ping mixr,
   /etc/modules.conf r,
   @{PROC}/sys/net/ipv6/conf/all/disable_ipv6 r,
 

profiles/apparmor.d/eigrpd

@@ -19,6 +19,8 @@ profile eigrpd /usr/lib/frr/eigrpd flags=(attach_disconnected) {
 
   capability net_raw,
 
+  /usr/lib/frr/eigrpd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/eigrpd>
 }

profiles/apparmor.d/fabricd

@@ -17,6 +17,8 @@ profile fabricd /usr/lib/frr/fabricd flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
 
+  /usr/lib/frr/fabricd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/fabricd>
 }

profiles/apparmor.d/isisd

@@ -20,6 +20,8 @@ profile isisd /usr/lib/frr/isisd flags=(attach_disconnected) {
 
   capability net_raw,
 
+  /usr/lib/frr/isisd mr,
+
   /var/lib/frr/ r, 
   /var/lib/frr/isisd.json{,.sav} rw,
 

profiles/apparmor.d/nhrpd

@@ -20,6 +20,7 @@ profile nhrpd /usr/lib/frr/nhrpd flags=(attach_disconnected) {
   capability net_raw,
   capability net_admin,
 
+  /usr/lib/frr/nhrpd mr,
   /usr/bin/dash ix,
   @{PROC}/sys/net/ipv4/conf/*/send_redirects w,
 

profiles/apparmor.d/ospf6d

@@ -21,6 +21,8 @@ profile ospf6d /usr/lib/frr/ospf6d flags=(attach_disconnected) {
   capability net_raw,
   capability sys_admin,
 
+  /usr/lib/frr/ospf6d mr,
+
   @{run}/netns/* r,
 
   @{run}/frr/ospf6d-gr.json w,

profiles/apparmor.d/ospfd

@@ -21,6 +21,8 @@ profile ospfd /usr/lib/frr/ospfd flags=(attach_disconnected) {
   capability net_raw,
   capability sys_admin,
 
+  /usr/lib/frr/ospfd mr,
+
   @{run}/netns/* r,
 
   @{run}/frr/ospfd-gr.json w,

profiles/apparmor.d/pathd

@@ -17,6 +17,8 @@ profile pathd /usr/lib/frr/pathd flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
 
+  /usr/lib/frr/pathd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/pathd>
 }

profiles/apparmor.d/pbrd

@@ -17,6 +17,8 @@ profile pbrd /usr/lib/frr/pbrd flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
 
+  /usr/lib/frr/pbrd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/pbrd>
 }

profiles/apparmor.d/pim6d

@@ -20,6 +20,8 @@ profile pim6d /usr/lib/frr/pim6d flags=(attach_disconnected) {
   capability net_raw,
   capability net_admin,
 
+  /usr/lib/frr/pim6d mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/pim6d>
 }

profiles/apparmor.d/pimd

@@ -20,6 +20,8 @@ profile pimd /usr/lib/frr/pimd flags=(attach_disconnected) {
   capability net_raw,
   capability net_admin,
 
+  /usr/lib/frr/pimd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/pimd>
 }

profiles/apparmor.d/ripd

@@ -18,6 +18,8 @@ profile ripd /usr/lib/frr/ripd flags=(attach_disconnected) {
   include <abstractions/frr>
   include <abstractions/frr-snmp>
 
+  /usr/lib/frr/ripd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/ripd>
 }

profiles/apparmor.d/ripngd

@@ -17,6 +17,8 @@ profile ripngd /usr/lib/frr/ripngd flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
 
+  /usr/lib/frr/ripngd mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/ripngd>
 }

profiles/apparmor.d/staticd

@@ -17,6 +17,8 @@ profile staticd /usr/lib/frr/staticd flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
  
+  /usr/lib/frr/staticd mr,
+
   /etc/frr/zebra.conf r,
 
   owner @{PROC}/@{pid}/task/@{tid}/comm rw,

profiles/apparmor.d/tnftp

@@ -29,6 +29,8 @@ profile tnftp /usr/bin/tnftp {
   network inet stream,
   network inet6 stream,
 
+  /usr/bin/tnftp mr,
+
   # required for the pager (less, more) to work
   file Cx /usr/bin/dash,
 

profiles/apparmor.d/transmission

@@ -17,6 +17,8 @@ profile transmission-daemon /usr/bin/transmission-daemon flags=(complain,attach_
   network inet  stream,
   network inet6 stream,
 
+  /usr/bin/transmission-daemon mr,
+
   owner @{PROC}/@{pid}/mounts r,
   @{PROC}/sys/kernel/random/uuid r,
 
@@ -42,6 +44,8 @@ profile transmission-cli /usr/bin/transmission-cli flags=(complain) {
   include <abstractions/transmission-common>
   include <abstractions/consoles>
 
+  /usr/bin/transmission-cli mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/transmission>
   include if exists <local/transmission-cli>
@@ -53,6 +57,8 @@ profile transmission-gtk /usr/bin/transmission-gtk flags=(complain,attach_discon
   include <abstractions/dconf>
   include <abstractions/gnome>
 
+  /usr/bin/transmission-gtk mr,
+
   owner @{run}/user/*/dconf/user w,
 
   # Site-specific additions and overrides. See local/README for details.
@@ -70,6 +76,8 @@ profile transmission-qt /usr/bin/transmission-qt flags=(complain) {
   include <abstractions/qt5>
   include <abstractions/qt5-settings-write>
 
+  /usr/bin/transmission-qt mr,
+
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/transmission>
   include if exists <local/transmission-qt>

profiles/apparmor.d/vrrpd

@@ -17,6 +17,7 @@ profile vrrpd /usr/lib/frr/vrrpd flags=(attach_disconnected) {
   include <abstractions/base>
   include <abstractions/frr>
 
+  /usr/lib/frr/vrrpd mr,
   # Site-specific additions and overrides. See local/README for details.
   include if exists <local/vrrpd>
 }

profiles/apparmor.d/wpa_supplicant

@@ -52,6 +52,8 @@ profile wpa_supplicant /usr/sbin/wpa_supplicant {
         interface=org.freedesktop.DBus
         member={AddMatch,GetNameOwner,Hello,ReleaseName,RemoveMatch,RequestName,StartServiceByName},
 
+  /usr/sbin/wpa_supplicant mr,
+
   owner /dev/rfkill r,
   owner /etc/group r,
   owner /etc/nsswitch.conf r,

profiles/apparmor.d/zgrep

@@ -31,11 +31,10 @@ profile zgrep /usr/bin/{x,}zgrep {
   /usr/bin/rm ix,
   /usr/bin/sed Cx -> sed,
   /usr/bin/xz Cx -> helper,
-  /usr/bin/xzgrep r,
   /usr/bin/zgrep Cx -> helper,
   /usr/bin/zstd Cx -> helper,
   owner /tmp/zgrep* rw,
-  /usr/bin/zgrep r,
+  /usr/bin/{x,}zgrep r,
 
   deny /etc/nsswitch.conf r,
   deny /etc/passwd r,

profiles/apparmor.d/znc

@@ -13,6 +13,8 @@ profile znc /usr/bin/znc {
 
   network tcp,
 
+  /usr/bin/znc mr,
+
   @{system_share_dirs}/znc/** r,
 
   owner @{HOME}/.znc/ rw,

profiles/apparmor/profiles/extras/firefox

@@ -110,6 +110,8 @@ profile firefox @{MOZ_LIBDIR}/@{MOZ_APP_NAME}{,*[^s][^h]} {
        member=GetAll
        peer=(label=unconfined),
 
+  @{MOZ_LIBDIR}/@{MOZ_APP_NAME}{,*[^s][^h]} mr,
+
   # should maybe be in abstractions
   /etc/ r,
   /etc/mime.types r,

profiles/apparmor/profiles/extras/firefox.sh

@@ -11,6 +11,8 @@ profile firefox.sh /usr/lib/firefox/firefox.sh {
 
   deny capability sys_ptrace,
 
+  /usr/lib/firefox/firefox.sh mr,
+
   /{usr/,}bin/basename rix,
   /{usr/,}bin/bash rix,
   /{usr/,}bin/grep rix,

profiles/apparmor/profiles/extras/usr.bin.acroread

@@ -26,6 +26,8 @@ include <tunables/global>
 
   capability dac_override,
 
+  /usr/X11R6/bin/acroread mr,
+
   /{usr/,}bin/basename mixr,
   /{usr/,}bin/bash mix,
   /{usr/,}bin/cat mixr,

profiles/apparmor/profiles/extras/usr.bin.svnserve

@@ -19,6 +19,8 @@ include <tunables/global>
   # network service ;)
   capability net_bind_service,
 
+  /usr/bin/svnserve mr,
+
   /srv/svn/*/conf/* r,
   /srv/svn/*/format r,
   /srv/svn/*/db/ r,

profiles/apparmor/profiles/extras/usr.lib.RealPlayer10.realplay

@@ -41,6 +41,7 @@ include <tunables/global>
   @{HOME}/ r,
   @{HOME}/.realplayerrc rw,
 
+  /usr/lib/RealPlayer10/realplay mr,
   /usr/lib/RealPlayer10/** mr,
   /usr/lib/RealPlayer10/realplay.bin Pxr,
   /usr/lib/firefox/firefox.sh Pxr,

profiles/apparmor/profiles/extras/usr.lib.evolution-data-server.evolution-data-server-1.10

@@ -33,6 +33,7 @@ include <tunables/global>
   /usr/lib/GConf/**.so mr,
   /usr/lib/GConf/2/gconfd-2 Pxr,
   /usr/lib64/GConf/2/gconfd-2 Pxr,
+  /usr/lib/evolution-data-server/evolution-data-server-1.10 mr,
   /usr/lib/evolution-data-server/evolution-data-server-* rmix,
   /usr/lib/evolution-data-server*/extensions r,
   /usr/lib/evolution-data-server*/extensions/lib*.so r,

profiles/apparmor/profiles/extras/usr.sbin.in.fingerd

@@ -19,6 +19,8 @@ include <tunables/global>
   @{HOME}/.plan          r,
   @{HOME}/.project       r,
 
+  /usr/sbin/in.fingerd mr,
+
   /usr/bin/finger        mix,
   /var/log/lastlog       r,
   /{,var/}run/utmp       rk,

profiles/apparmor/profiles/extras/usr.sbin.oidentd

@@ -21,6 +21,8 @@ include <tunables/global>
   capability dac_override,
   capability dac_read_search,
 
+  /usr/sbin/oidentd mr,
+
   /etc/oidentd.conf        r,
   /etc/oidentd_masq.conf   r,
   @{PROC}/net/tcp          r,

utils/test/logprof/ping.bin.ping

@@ -28,7 +28,7 @@ profile ping /{usr/,}bin/{,iputils-}ping {
 
   /etc/modules.conf r,
   /proc/21622/cmdline r,
-  /{,usr/}bin/{,iputils-}ping mrix,
+  /{usr/,}bin/{,iputils-}ping mrix,
   @{PROC}/sys/net/ipv6/conf/all/disable_ipv6 r,
 
 }