From 0eb0a16fbdfbb986d4209c86fe0261db35129c01 Mon Sep 17 00:00:00 2001
From: Nick Kralevich <nnk@google.com>
Date: Wed, 12 Dec 2018 09:06:05 -0800
Subject: [PATCH] bless app created renderscript files
When an app uses renderscript to compile a Script instance,
renderscript compiles and links the script using /system/bin/bcc and
/system/bin/ld.mc, then places the resulting shared library into the
application's code_cache directory. The application then dlopen()s the
resulting shared library.
Currently, this executable code is writable to the application. This
violates the W^X property (https://en.wikipedia.org/wiki/W%5EX), which
requires any executable code be immutable.
This change introduces a new label "rs_data_file". Files created by
/system/bin/bcc and /system/bin/ld.mc in the application's home
directory assume this label. This allows us to differentiate in
security policy between app created files, and files created by
renderscript on behalf of the application.
Apps are allowed to delete these files, but cannot create or write these
files. This is enforced through a neverallow compile time assertion.
Several exceptions are added to Treble neverallow assertions to support
this functionality. However, because renderscript was previously invoked
from an application context, this is not a Treble separation regression.
This change is needed to support blocking dlopen() for non-renderscript
/data/data files, which will be submitted in a followup change.
Bug: 112357170
Test: cts-tradefed run cts -m CtsRenderscriptTestCases
Change-Id: Ie38bbd94d26db8a418c2a049c24500a5463698a3
---
private/app.te | 12 ++++++-----
private/app_neverallows.te | 7 ++++++
private/coredomain.te | 4 ++++
private/domain.te | 7 ++++--
private/ephemeral_app.te | 10 ++++++++-
private/file.te | 2 ++
private/file_contexts | 2 ++
private/installd.te | 3 +++
private/rs.te | 42 ++++++++++++++++++++++++++++++++++++
private/untrusted_app_all.te | 10 ++++++++-
10 files changed, 90 insertions(+), 9 deletions(-)
create mode 100644 private/rs.te
diff --git a/private/app.te b/private/app.te
index b2c1be334..ffe6598d6 100644
--- a/private/app.te
+++ b/private/app.te
@@ -16,8 +16,10 @@ neverallow appdomain system_server:udp_socket {
# Transition to a non-app domain.
# Exception for the shell and su domains, can transition to runas, etc.
-# Exception for crash_dump.
-neverallow { appdomain -shell userdebug_or_eng(`-su') } { domain -appdomain -crash_dump }:process
- { transition };
-neverallow { appdomain -shell userdebug_or_eng(`-su') } { domain -appdomain }:process
- { dyntransition };
+# Exception for crash_dump to allow for app crash reporting.
+# Exception for renderscript binaries (/system/bin/bcc, /system/bin/ld.mc)
+# to allow renderscript to create privileged executable files.
+neverallow { appdomain -shell userdebug_or_eng(`-su') }
+ { domain -appdomain -crash_dump -rs }:process { transition };
+neverallow { appdomain -shell userdebug_or_eng(`-su') }
+ { domain -appdomain }:process { dyntransition };
diff --git a/private/app_neverallows.te b/private/app_neverallows.te
index 7e14dd42d..6ebbd436a 100644
--- a/private/app_neverallows.te
+++ b/private/app_neverallows.te
@@ -41,6 +41,12 @@ neverallow { all_untrusted_apps -mediaprovider } property_type:property_service
# but otherwise disallow untrusted apps from reading this property.
neverallow { all_untrusted_apps -untrusted_app_25 } net_dns_prop:file read;
+# Renderscript created files within an app home directory can be
+# dlopen()ed. To maintain the W^X property, these files
+# must never be writable to the app.
+neverallow all_untrusted_apps rs_data_file:file
+ { append create link relabelfrom relabelto rename setattr write };
+
# Block calling execve() on files in an apps home directory.
# This is a W^X violation (loading executable code from a writable
# home directory). For compatibility, allow for targetApi <= 28.
@@ -121,6 +127,7 @@ neverallow { all_untrusted_apps -mediaprovider } {
file_type
-app_data_file # The apps sandbox itself
-privapp_data_file
+ -rs_data_file # stored within the app sandbox directory
-media_rw_data_file # Internal storage. Known that apps can
# leave artfacts here after uninstall.
-user_profile_data_file # Access to profile files
diff --git a/private/coredomain.te b/private/coredomain.te
index 04f7a4626..741351531 100644
--- a/private/coredomain.te
+++ b/private/coredomain.te
@@ -28,6 +28,7 @@ full_treble_only(`
userdebug_or_eng(`-perfprofd')
userdebug_or_eng(`-heapprofd')
-postinstall_dexopt
+ -rs # spawned by appdomain, so carryover the exception above
-system_server
} vendor_app_file:dir { open read getattr search };
')
@@ -43,6 +44,7 @@ full_treble_only(`
userdebug_or_eng(`-perfprofd')
userdebug_or_eng(`-heapprofd')
-postinstall_dexopt
+ -rs # spawned by appdomain, so carryover the exception above
-system_server
-mediaserver
} vendor_app_file:file r_file_perms;
@@ -56,6 +58,7 @@ full_treble_only(`
-idmap
-init
-installd
+ -rs # spawned by appdomain, so carryover the exception above
-system_server
-webview_zygote
-zygote
@@ -70,6 +73,7 @@ full_treble_only(`
-idmap
-init
-installd
+ -rs # spawned by appdomain, so carryover the exception above
-system_server
-webview_zygote
-zygote
diff --git a/private/domain.te b/private/domain.te
index 7a41ab213..15179e23b 100644
--- a/private/domain.te
+++ b/private/domain.te
@@ -25,8 +25,8 @@ userdebug_or_eng(`can_profile_heap({
# Path resolution access in cgroups.
allow domain cgroup:dir search;
-allow { domain -appdomain } cgroup:dir w_dir_perms;
-allow { domain -appdomain } cgroup:file w_file_perms;
+allow { domain -appdomain -rs } cgroup:dir w_dir_perms;
+allow { domain -appdomain -rs } cgroup:file w_file_perms;
# For now, everyone can access core property files
# Device specific properties are not granted by default
@@ -105,6 +105,7 @@ neverallow {
-installd
userdebug_or_eng(`-perfprofd')
-profman
+ -rs # spawned by appdomain, so carryover the exception above
-runas
-system_server
} { privapp_data_file app_data_file }:dir *;
@@ -115,6 +116,7 @@ neverallow {
domain
-appdomain
-installd
+ -rs # spawned by appdomain, so carryover the exception above
} { privapp_data_file app_data_file }:dir ~r_dir_perms;
neverallow {
@@ -122,6 +124,7 @@ neverallow {
-appdomain
-installd
userdebug_or_eng(`-perfprofd')
+ -rs # spawned by appdomain, so carryover the exception above
} { privapp_data_file app_data_file }:file_class_set open;
neverallow {
diff --git a/private/ephemeral_app.te b/private/ephemeral_app.te
index f28d28f04..4935f3340 100644
--- a/private/ephemeral_app.te
+++ b/private/ephemeral_app.te
@@ -21,7 +21,15 @@ allow ephemeral_app { sdcard_type media_rw_data_file }:file {read write getattr
# Some apps ship with shared libraries and binaries that they write out
# to their sandbox directory and then execute.
-allow ephemeral_app { app_data_file privapp_data_file }:file {r_file_perms execute};
+allow ephemeral_app privapp_data_file:file { r_file_perms execute };
+allow ephemeral_app app_data_file:file { r_file_perms execute };
+
+# Allow the renderscript compiler to be run.
+domain_auto_trans(ephemeral_app, rs_exec, rs)
+
+# Allow loading and deleting renderscript created shared libraries
+# within an application home directory.
+allow ephemeral_app rs_data_file:file { r_file_perms execute unlink };
# services
allow ephemeral_app audioserver_service:service_manager find;
diff --git a/private/file.te b/private/file.te
index fd1c2eec5..884374397 100644
--- a/private/file.te
+++ b/private/file.te
@@ -13,3 +13,5 @@ type perfetto_traces_data_file, file_type, data_file_type, core_data_file_type;
# /sys/kernel/debug/kcov for coverage guided kernel fuzzing in userdebug builds.
type debugfs_kcov, fs_type, debugfs_type;
+# renderscript created files in /data/data directories
+type rs_data_file, file_type, data_file_type, core_data_file_type;
diff --git a/private/file_contexts b/private/file_contexts
index abef72b8f..fac31a087 100644
--- a/private/file_contexts
+++ b/private/file_contexts
@@ -176,6 +176,7 @@
/system(/.*)? u:object_r:system_file:s0
/system/lib(64)?(/.*)? u:object_r:system_lib_file:s0
/system/bin/atrace u:object_r:atrace_exec:s0
+/system/bin/bcc u:object_r:rs_exec:s0
/system/bin/blank_screen u:object_r:blank_screen_exec:s0
/system/bin/e2fsdroid u:object_r:e2fs_exec:s0
/system/bin/mke2fs u:object_r:e2fs_exec:s0
@@ -189,6 +190,7 @@
/system/bin/tune2fs -- u:object_r:fsck_exec:s0
/system/bin/toolbox -- u:object_r:toolbox_exec:s0
/system/bin/toybox -- u:object_r:toolbox_exec:s0
+/system/bin/ld\.mc u:object_r:rs_exec:s0
/system/bin/logcat -- u:object_r:logcat_exec:s0
/system/bin/logcatd -- u:object_r:logcat_exec:s0
/system/bin/sh -- u:object_r:shell_exec:s0
diff --git a/private/installd.te b/private/installd.te
index 055371631..fd3535c86 100644
--- a/private/installd.te
+++ b/private/installd.te
@@ -20,3 +20,6 @@ type_transition installd system_data_file:file install_data_file;
# For collecting bugreports.
allow installd dumpstate:fd use;
allow installd dumpstate:fifo_file r_file_perms;
+
+# Delete /system/bin/bcc generated artifacts
+allow installd rs_data_file:file unlink;
diff --git a/private/rs.te b/private/rs.te
new file mode 100644
index 000000000..94cf6b4e8
--- /dev/null
+++ b/private/rs.te
@@ -0,0 +1,42 @@
+type rs, domain, coredomain;
+type rs_exec, system_file_type, exec_type, file_type;
+
+# Any files which would have been created as app_data_file
+# will be created as rs_data_file instead.
+allow rs app_data_file:dir ra_dir_perms;
+allow rs rs_data_file:file create_file_perms;
+type_transition rs app_data_file:file rs_data_file;
+
+# Read files from the app home directory.
+allow rs app_data_file:file r_file_perms;
+allow rs app_data_file:dir r_dir_perms;
+
+# Cleanup rs_data_file files in the app home directory.
+allow rs app_data_file:dir remove_name;
+
+# Use vendor resources
+allow rs vendor_file:dir r_dir_perms;
+r_dir_file(rs, vendor_overlay_file)
+r_dir_file(rs, vendor_app_file)
+
+# Read contents of app apks
+r_dir_file(rs, apk_data_file)
+
+allow rs gpu_device:chr_file rw_file_perms;
+allow rs ion_device:chr_file r_file_perms;
+allow rs same_process_hal_file:file { r_file_perms execute };
+
+# File descriptors passed from app to renderscript
+allow rs untrusted_app_all:fd use;
+
+# TODO: Explain why these dontaudits are needed. Most likely
+# these are file descriptors leaking across an exec() boundary
+# due to a missing O_CLOEXEC / SOCK_CLOEXEC
+dontaudit rs untrusted_app_all:unix_stream_socket { read write };
+dontaudit rs untrusted_app_all:fifo_file { read write };
+
+# TODO: Explain why this is necessary. I think this is a zygote
+# created logging socket and system server parceled file descriptor
+# which is not using the O_CLOEXEC flag.
+dontaudit rs zygote:fd use;
+dontaudit rs system_server:fd use;
diff --git a/private/untrusted_app_all.te b/private/untrusted_app_all.te
index 72e03e12f..aebb7118a 100644
--- a/private/untrusted_app_all.te
+++ b/private/untrusted_app_all.te
@@ -22,7 +22,12 @@
# Some apps ship with shared libraries and binaries that they write out
# to their sandbox directory and then execute.
-allow untrusted_app_all { app_data_file privapp_data_file }:file { r_file_perms execute };
+allow untrusted_app_all privapp_data_file:file { r_file_perms execute };
+allow untrusted_app_all app_data_file:file { r_file_perms execute };
+
+# Allow loading and deleting renderscript created shared libraries
+# within an application home directory.
+allow untrusted_app_all rs_data_file:file { r_file_perms execute unlink };
# ASEC
allow untrusted_app_all asec_apk_file:file r_file_perms;
@@ -122,6 +127,9 @@ unix_socket_connect(untrusted_app_all, traced_producer, traced)
allow untrusted_app_all system_server:udp_socket {
connect getattr read recvfrom sendto write getopt setopt };
+# Allow the renderscript compiler to be run.
+domain_auto_trans(untrusted_app_all, rs_exec, rs)
+
# This is allowed for targetSdkVersion <= 25 but disallowed on newer versions.
dontaudit untrusted_app_all net_dns_prop:file read;
--
GitLab