apparmor-utils: fix aa-remove-unknown read check

let aaru = "aa-remove-unknown"; in

aaru tests whether /sys/kernel/security/apparmor/profiles can be opened.
Even though the file's permissions usually are 0444, open() still might
return `EPERM`, as this is a virtual filesystem.  Thus, using `test -r`
doesn't suffice for this check.

What aaru does to solve this is (approximately)

  if ! read … < /sys/kernel/security/apparmor/profiles; then
    echo "Meh";
  fi

In principal this works just fine.  When looking closer, it doesn't
(which is the root cause of #273164).  Careful readers will notice that
the actual access check (for `open()`) isn't actually related to the
`read` invocation, but the shell's input redirection, which works
totally fine:

If the file can't be opened, the shell will return an error and the test
fails.  `read` won't even be invoked.  The culprit is, the `read` shell
builtin might potentially jeopardize the *successful* test result
(`open()` succeeding): When no profiles are loaded, the file will be
empty and `read` will return 1 for `EOF`.

As the `if`'s command is only invoked after the actual test succeeded,
`true` is the command of choice here.

I would prefer fixing this upstream, but I refuse to register an account
there because GitLab.com wants me to validate an email address (sure), a
phone number (why?) and a valid payment method ([redacted]).

This fixes #273164 (»Apparmor service fails to start after nixos-rebuild
switch«).

2 files changed, 33 insertions, 1 deletions

diff --git a/pkgs/os-specific/linux/apparmor/0001-aa-remove-unknown_empty-ruleset.patch b/pkgs/os-specific/linux/apparmor/0001-aa-remove-unknown_empty-ruleset.patch
new file mode 100644
index 000000000000..ce2a0e7cc5b3
--- /dev/null
+++ b/pkgs/os-specific/linux/apparmor/0001-aa-remove-unknown_empty-ruleset.patch
@@ -0,0 +1,30 @@
+commit 166afaf144d6473464975438353257359dd51708
+Author: Andreas Wiese <andreas.wiese@kernkonzept.com>
+Date:   Thu Feb 1 11:35:02 2024 +0100
+
+    aa-remove-unknown: fix readability check
+    
+    This check is intended for ensuring that the profiles file can actually
+    be opened.  The *actual* check is performed by the shell, not the read
+    utility, which won't even be executed if the input redirection (and
+    hence the test) fails.
+    
+    If the test succeeds, though, using `read` here might actually
+    jeopardize the test result if there are no profiles loaded and the file
+    is empty.
+    
+    This commit fixes that case by simply using `true` instead of `read`.
+
+diff --git a/utils/aa-remove-unknown b/utils/aa-remove-unknown
+index 0e00d6a0..3351feef 100755
+--- a/utils/aa-remove-unknown
++++ b/utils/aa-remove-unknown
+@@ -63,7 +63,7 @@ fi
+ # We have to do this check because error checking awk's getline() below is
+ # tricky and, as is, results in an infinite loop when apparmorfs returns an
+ # error from open().
+-if ! IFS= read -r _ < "$PROFILES" ; then
++if ! true < "$PROFILES" ; then
+ 	echo "ERROR: Unable to read apparmorfs profiles file" 1>&2
+ 	exit 1
+ elif [ ! -w "$REMOVE" ] ; then
diff --git a/pkgs/os-specific/linux/apparmor/default.nix b/pkgs/os-specific/linux/apparmor/default.nix
index 99c1000f0e4c..205a8a4465d6 100644
--- a/pkgs/os-specific/linux/apparmor/default.nix
+++ b/pkgs/os-specific/linux/apparmor/default.nix
@@ -56,7 +56,9 @@ let
       --replace "/usr/include/linux/capability.h" "${linuxHeaders}/include/linux/capability.h"
   '';
 
-  patches = lib.optionals stdenv.hostPlatform.isMusl [
+  patches = [
+    ./0001-aa-remove-unknown_empty-ruleset.patch
+  ] ++ lib.optionals stdenv.hostPlatform.isMusl [
     (fetchpatch {
       url = "https://git.alpinelinux.org/aports/plain/testing/apparmor/0003-Added-missing-typedef-definitions-on-parser.patch?id=74b8427cc21f04e32030d047ae92caa618105b53";
       name = "0003-Added-missing-typedef-definitions-on-parser.patch";