From 30404a42b8c79507c7df43e7b68ac4bc84a91710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thi=C3=A9baud=20Weksteen?= Date: Wed, 21 Feb 2024 16:29:38 +1100 Subject: [PATCH] Grant lockdown integrity to all processes The default policy for the "lockdown" access vector on Android was introduced in commit bcfca1a6. While the "confidentiality" permission was granted to all processes, the "integrity" was marked as neverallowed. Upstream, the support for that access vector was removed from kernel 5.16 onwards. It was found that the "integrity" permission either does not apply to Android or duplicates other access control (e.g., capabilities sys_admin). Instead of simply removing the neverallow rule, the access is granted to all processes. This will prevent the proliferation of references to this access vector in vendors' policies and ultimately facilitate its removal. Test: presubmit Bug: 285443587 Bug: 269377822 Bug: 319390252 Change-Id: If2ad34fbbf2c0d29ac54ab5d1be430623f86f1f7 (cherry picked from commit 99a4cbcee760b684ea48e29d1c3010fcc100de95) Merged-In: If2ad34fbbf2c0d29ac54ab5d1be430623f86f1f7 --- .../api/33.0/private/untrusted_app_all.te | 3 --- prebuilts/api/33.0/public/domain.te | 18 ++++++++---------- private/untrusted_app_all.te | 3 --- public/domain.te | 18 ++++++++---------- 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/prebuilts/api/33.0/private/untrusted_app_all.te b/prebuilts/api/33.0/private/untrusted_app_all.te index 26077f376..3ee54b2c4 100644 --- a/prebuilts/api/33.0/private/untrusted_app_all.te +++ b/prebuilts/api/33.0/private/untrusted_app_all.te @@ -166,9 +166,6 @@ create_pty(untrusted_app_all) userdebug_or_eng(` allow untrusted_app_all debugfs_kcov:file rw_file_perms; allowxperm untrusted_app_all debugfs_kcov:file ioctl { KCOV_INIT_TRACE KCOV_ENABLE KCOV_DISABLE }; - # The use of debugfs kcov is considered a breach of the kernel integrity - # according to the heuristic of lockdown. - allow untrusted_app_all self:lockdown integrity; ') # Allow running a VM for test/demo purposes. Note that access the service is diff --git a/prebuilts/api/33.0/public/domain.te b/prebuilts/api/33.0/public/domain.te index 132202545..abf186a27 100644 --- a/prebuilts/api/33.0/public/domain.te +++ b/prebuilts/api/33.0/public/domain.te @@ -282,13 +282,14 @@ allow domain debugfs_tracing:dir search; allow domain debugfs_tracing_debug:dir search; allow domain debugfs_trace_marker:file w_file_perms; -# Linux lockdown mode offers coarse-grained definitions for access controls. -# The "confidentiality" level detects access to tracefs or the perf subsystem. -# This overlaps with more precise declarations in Android's policy. The -# debugfs_trace_marker above is an example in which all processes should have -# some access to tracefs. Therefore, allow all domains to access this level. -# The "integrity" level is however enforced. -allow domain self:lockdown confidentiality; +# Linux lockdown mode offered coarse-grained definitions for access controls. In +# previous versions of the policy, the integrity permission was neverallowed. +# It was found that this permission mainly duplicates pre-existing rules in +# the policy (see b/285443587). Additionally, some access were found to be +# required (b/269377822). The access vector was removed from kernel 5.16 +# onwards. Grant unconditional access, these rules should be removed from the +# policy once no kernel <5.16 are supported. +allow domain self:lockdown { confidentiality integrity }; # Filesystem access. allow domain fs_type:filesystem getattr; @@ -1351,6 +1352,3 @@ neverallow { } ashmem_device:chr_file open; neverallow { domain -traced_probes -init -vendor_init } debugfs_tracing_printk_formats:file *; - -# Linux lockdown "integrity" level is enforced for user builds. -neverallow { domain userdebug_or_eng(`-domain') } self:lockdown integrity; diff --git a/private/untrusted_app_all.te b/private/untrusted_app_all.te index 26077f376..3ee54b2c4 100644 --- a/private/untrusted_app_all.te +++ b/private/untrusted_app_all.te @@ -166,9 +166,6 @@ create_pty(untrusted_app_all) userdebug_or_eng(` allow untrusted_app_all debugfs_kcov:file rw_file_perms; allowxperm untrusted_app_all debugfs_kcov:file ioctl { KCOV_INIT_TRACE KCOV_ENABLE KCOV_DISABLE }; - # The use of debugfs kcov is considered a breach of the kernel integrity - # according to the heuristic of lockdown. - allow untrusted_app_all self:lockdown integrity; ') # Allow running a VM for test/demo purposes. Note that access the service is diff --git a/public/domain.te b/public/domain.te index 132202545..abf186a27 100644 --- a/public/domain.te +++ b/public/domain.te @@ -282,13 +282,14 @@ allow domain debugfs_tracing:dir search; allow domain debugfs_tracing_debug:dir search; allow domain debugfs_trace_marker:file w_file_perms; -# Linux lockdown mode offers coarse-grained definitions for access controls. -# The "confidentiality" level detects access to tracefs or the perf subsystem. -# This overlaps with more precise declarations in Android's policy. The -# debugfs_trace_marker above is an example in which all processes should have -# some access to tracefs. Therefore, allow all domains to access this level. -# The "integrity" level is however enforced. -allow domain self:lockdown confidentiality; +# Linux lockdown mode offered coarse-grained definitions for access controls. In +# previous versions of the policy, the integrity permission was neverallowed. +# It was found that this permission mainly duplicates pre-existing rules in +# the policy (see b/285443587). Additionally, some access were found to be +# required (b/269377822). The access vector was removed from kernel 5.16 +# onwards. Grant unconditional access, these rules should be removed from the +# policy once no kernel <5.16 are supported. +allow domain self:lockdown { confidentiality integrity }; # Filesystem access. allow domain fs_type:filesystem getattr; @@ -1351,6 +1352,3 @@ neverallow { } ashmem_device:chr_file open; neverallow { domain -traced_probes -init -vendor_init } debugfs_tracing_printk_formats:file *; - -# Linux lockdown "integrity" level is enforced for user builds. -neverallow { domain userdebug_or_eng(`-domain') } self:lockdown integrity;