From ab82125fc84b2bb154b1b68a11db543a5352d533 Mon Sep 17 00:00:00 2001 From: Jeff Vander Stoep Date: Tue, 29 May 2018 10:41:36 -0700 Subject: [PATCH] Improve tests protecting private app data In particular, add assertions limiting which processes may directly open files owned by apps. Reduce this to just apps, init, and installd. App data is protected by a combination of selinux permissions and Unix permissions, so limiting the open permission to just apps (which are not allowed to have CAP_DAC_OVERRIDE or CAP_DAC_READ_SEARCH) ensures that only installd and init have complete access an app's private directory. In addition to apps/init/installd, other processes currently granted open are mediaserver, uncrypt, and vold. Uncrypt's access appears to be deprecated (b/80299612). Uncrypt now uses /data/ota_package instead. b/80418809 and b/80300620 track removal for vold and mediaserver. Test: build/boot aosp_taimen-userdebug. Verify no "granted" audit messages in the logs. Bug: 80190017 Bug: 80300620 Bug: 80418809 Fixes: 80299612 Change-Id: I153bc7b62294b36ccd596254a5976dd887fed046 --- private/domain.te | 55 +++++++++++++++++++++++++++++++++++++++++++ public/domain.te | 14 ----------- public/init.te | 2 ++ public/mediaserver.te | 3 +++ public/uncrypt.te | 3 --- public/vold.te | 3 +++ 6 files changed, 63 insertions(+), 17 deletions(-) diff --git a/private/domain.te b/private/domain.te index 3a7ef4242..f7f5d66da 100644 --- a/private/domain.te +++ b/private/domain.te @@ -121,3 +121,58 @@ full_treble_only(` # Disallow direct access by other processes. neverallow { domain -init -system_server } dropbox_data_file:dir *; neverallow { domain -init -system_server } dropbox_data_file:file ~{ getattr read }; + +### +# Services should respect app sandboxes +neverallow { + domain + -appdomain + -installd # creation of sandbox +} app_data_file:dir_file_class_set { create unlink }; + +# Only the following processes should be directly accessing private app +# directories. +neverallow { + domain + -adbd + -appdomain + -dexoptanalyzer + -init + -installd + -mediaserver # b/80300620 + userdebug_or_eng(`-perfprofd') + -profman + -runas + -system_server + -vold +} app_data_file:dir *; + +# Only apps should be modifying app data. init and installd are exempted for +# restorecon and package install/uninstall. +neverallow { + domain + -appdomain + -init + -installd +} app_data_file:dir ~r_dir_perms; + +neverallow { + domain + -appdomain + -installd + -mediaserver # b/80300620 + userdebug_or_eng(`-perfprofd') + -vold # b/80418809 +} app_data_file:file_class_set open; + +neverallow { + domain + -appdomain + -installd # creation of sandbox +} app_data_file:dir_file_class_set { create unlink }; + +neverallow { + domain + -init + -installd +} app_data_file:dir_file_class_set { relabelfrom relabelto }; diff --git a/public/domain.te b/public/domain.te index f58b4567c..0f977294a 100644 --- a/public/domain.te +++ b/public/domain.te @@ -1182,20 +1182,6 @@ neverallow { priv_app } system_app_data_file:dir_file_class_set { create unlink open }; - -# Services should respect app sandboxes -neverallow { - domain - -appdomain - -installd # creation of sandbox -} app_data_file:dir_file_class_set { create unlink }; - -neverallow { - domain - -init - -installd -} app_data_file:dir_file_class_set { relabelfrom relabelto }; - # # Only these domains should transition to shell domain. This domain is # permissible for the "shell user". If you need a process to exec a shell diff --git a/public/init.te b/public/init.te index 24dfb1d13..2519311db 100644 --- a/public/init.te +++ b/public/init.te @@ -210,6 +210,8 @@ allow init { allow init cache_file:lnk_file r_file_perms; allow init { file_type -system_file -vendor_file_type -exec_type }:dir_file_class_set relabelto; +# does init really need to relabel app data? +userdebug_or_eng(`auditallow init app_data_file:dir_file_class_set relabelto;') allow init { sysfs debugfs debugfs_tracing debugfs_tracing_debug }:{ dir file lnk_file } { getattr relabelfrom }; allow init { sysfs_type debugfs_type }:{ dir file lnk_file } { relabelto getattr }; allow init dev_type:dir create_dir_perms; diff --git a/public/mediaserver.te b/public/mediaserver.te index 861d11d61..4032a7623 100644 --- a/public/mediaserver.te +++ b/public/mediaserver.te @@ -30,7 +30,10 @@ binder_service(mediaserver) allow mediaserver media_data_file:dir create_dir_perms; allow mediaserver media_data_file:file create_file_perms; +# TODO(b/80190017, b/80300620): remove direct access to private app data +userdebug_or_eng(`auditallow mediaserver app_data_file:dir search;') allow mediaserver app_data_file:dir search; +userdebug_or_eng(`auditallow mediaserver app_data_file:file open;') allow mediaserver app_data_file:file rw_file_perms; allow mediaserver sdcard_type:file write; allow mediaserver gpu_device:chr_file rw_file_perms; diff --git a/public/uncrypt.te b/public/uncrypt.te index 1e48b831d..367498028 100644 --- a/public/uncrypt.te +++ b/public/uncrypt.te @@ -4,9 +4,6 @@ type uncrypt_exec, exec_type, file_type; allow uncrypt self:global_capability_class_set dac_override; -# Read OTA zip file from /data/data/com.google.android.gsf/app_download -r_dir_file(uncrypt, app_data_file) - userdebug_or_eng(` # For debugging, allow /data/local/tmp access r_dir_file(uncrypt, shell_data_file) diff --git a/public/vold.te b/public/vold.te index fd27e35ca..06deefce3 100644 --- a/public/vold.te +++ b/public/vold.te @@ -81,7 +81,10 @@ allow vold tmpfs:dir create_dir_perms; allow vold tmpfs:dir mounton; allow vold self:global_capability_class_set { net_admin dac_override mknod sys_admin chown fowner fsetid }; allow vold self:netlink_kobject_uevent_socket create_socket_perms_no_ioctl; +# TODO(b/80418809): remove direct access to private app data +userdebug_or_eng(`auditallow vold app_data_file:dir search;') allow vold app_data_file:dir search; +userdebug_or_eng(`auditallow vold app_data_file:file rw_file_perms;') allow vold app_data_file:file rw_file_perms; allow vold loop_control_device:chr_file rw_file_perms; allow vold loop_device:blk_file { create setattr unlink rw_file_perms };