Merge pull request #1375 from infinito-nexus/fix/apparmor/readonly-proc-sys

fix(apparmor): leave a read-only /proc/sys alone in teardown and restore

scripts/system/apparmor/common.sh

@@ -70,9 +70,12 @@ apparmor_lift_unconfined_restriction() {
 		return 0
 	fi
 
+	if ! printf '0\n' 2>/dev/null >"${sysctl}"; then
+		apparmor_log "[apparmor] ${sysctl} is not writable here; the unprivileged-unconfined restriction stays ${current}"
+		return 0
+	fi
 	mkdir -p "$(dirname "${state}")"
 	printf '%s\n' "${current}" >"${state}"
-	printf '0\n' >"${sysctl}"
 	apparmor_log "[apparmor] lifted unprivileged-unconfined restriction (was ${current}); containers keep plain docker-default"
 }
 
@@ -87,7 +90,10 @@ apparmor_reinstate_unconfined_restriction() {
 		return 0
 	fi
 
-	tr -d '[:space:]' <"${state}" >"${sysctl}"
+	if ! tr -d '[:space:]' <"${state}" 2>/dev/null >"${sysctl}"; then
+		apparmor_log "[apparmor] ${sysctl} is not writable here; ${state} stays for a later restore"
+		return 0
+	fi
 	rm -f "${state}"
 	apparmor_log "[apparmor] reinstated unprivileged-unconfined restriction"
 }

tests/unit/python/scripts/system/apparmor/test_common.py

@@ -15,6 +15,7 @@ from pathlib import Path
 from utils.cache.files import PROJECT_ROOT
 
 COMMON = PROJECT_ROOT / "scripts" / "system" / "apparmor" / "common.sh"
+READ_ONLY_SYSCTL = Path("/proc/sys/kernel/ostype")
 
 PROFILES = """wg-quick (enforce)
 wg-quick//ip (enforce)
@@ -51,7 +52,7 @@ def _call(function: str, sysctl: Path, state: Path) -> subprocess.CompletedProce
         [
             "bash",
             "-c",
-            f'source "{COMMON}"; {function} "$1" "$2"',
+            f'set -euo pipefail; source "{COMMON}"; {function} "$1" "$2"',
             "-",
             str(sysctl),
             str(state),
@@ -114,6 +115,26 @@ class TestUnconfinedRestriction(unittest.TestCase):
         self.assertEqual(result.returncode, 0, result.stderr)
         self.assertEqual(_value(self.sysctl), "0")
 
+    @unittest.skipUnless(READ_ONLY_SYSCTL.is_file(), "needs procfs")
+    def test_a_read_only_sysctl_leaves_the_teardown_running(self) -> None:
+        result = _call(
+            "apparmor_lift_unconfined_restriction", READ_ONLY_SYSCTL, self.state
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn("not writable", result.stdout)
+        self.assertFalse(self.state.exists())
+
+    @unittest.skipUnless(READ_ONLY_SYSCTL.is_file(), "needs procfs")
+    def test_a_read_only_sysctl_keeps_the_record_for_a_later_restore(self) -> None:
+        self.state.parent.mkdir(parents=True)
+        self.state.write_text("1\n")
+        result = _call(
+            "apparmor_reinstate_unconfined_restriction", READ_ONLY_SYSCTL, self.state
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn("not writable", result.stdout)
+        self.assertEqual(_value(self.state), "1")
+
 
 if __name__ == "__main__":
     unittest.main()