Restrict creating per-user encrypted directories

Creating a per-user encrypted directory such as /data/system_ce/0 and
the subdirectories in it too early has been a recurring bug.  Typically,
individual services in system_server are to blame; system_server has
permission to create these directories, and it's easy to write
"mkdirs()" instead of "mkdir()".  Such bugs are very bad, as they
prevent these directories from being encrypted, as encryption policies
can only be set on empty directories.  Due to recent changes, a factory
reset is now forced in such cases, which helps detect these bugs;
however, it would be much better to prevent them in the first place.

This CL locks down the ability to create these directories to just vold
and init, or to just vold when possible.  This is done by assigning new
types to the directories that contain these directories, and then only
allowing the needed domains to write to these parent directories.  This
is similar to what https://r.android.com/1117297 did for /data itself.

Three new types are used instead of just one, since these directories
had three different types already (system_data_file, media_rw_data_file,
vendor_data_file), and this allows the policy to be a bit more precise.

A significant limitation is that /data/user/0 is currently being created
by init during early boot.  Therefore, this CL doesn't help much for
/data/user/0, though it helps a lot for the other directories.  As the
next step, I'll try to eliminate the /data/user/0 quirk.  Anyway, this
CL is needed regardless of whether we're able to do that.

Test: Booted cuttlefish.  Ran 'sm partition disk:253,32 private', then
      created and deleted a user.  Used 'ls -lZ' to check the relevant
      SELinux labels on both internal and adoptable storage.  Also did
      similar tests on raven, with the addition of going through the
      setup wizard and using an app that creates media files.  No
      relevant SELinux denials seen during any of this.
Bug: 156305599
Change-Id: I1fbdd180f56dd2fe4703763936f5850cef8ab0ba
This commit is contained in:
Eric Biggers 2022-05-04 22:18:02 +00:00
parent cec541e9ab
commit 9a5992336e
16 changed files with 142 additions and 27 deletions

View file

@ -33,9 +33,12 @@ allow apexd apex_module_data_file:file { create_file_perms relabelfrom };
allow apexd apex_rollback_data_file:dir create_dir_perms;
allow apexd apex_rollback_data_file:file create_file_perms;
# Allow apexd to read directories under /data/misc_de in order to snapshot and
# restore apex data for all users.
allow apexd system_data_file:dir r_dir_perms;
# Allow apexd to read /data/misc_de and the directories under it, in order to
# snapshot and restore apex data for all users.
allow apexd {
system_userdir_file
system_data_file
}:dir r_dir_perms;
# allow apexd to create loop devices with /dev/loop-control
allow apexd loop_control_device:chr_file rw_file_perms;

View file

@ -1919,7 +1919,7 @@
(typeattributeset media_metrics_service_33_0 (media_metrics_service))
(typeattributeset media_projection_service_33_0 (media_projection_service))
(typeattributeset media_router_service_33_0 (media_router_service))
(typeattributeset media_rw_data_file_33_0 (media_rw_data_file))
(typeattributeset media_rw_data_file_33_0 (media_rw_data_file media_userdir_file))
(typeattributeset media_session_service_33_0 (media_session_service))
(typeattributeset media_variant_prop_33_0 (media_variant_prop))
(typeattributeset mediadrm_config_prop_33_0 (mediadrm_config_prop))
@ -2362,7 +2362,7 @@
(typeattributeset system_boot_reason_prop_33_0 (system_boot_reason_prop))
(typeattributeset system_bootstrap_lib_file_33_0 (system_bootstrap_lib_file))
(typeattributeset system_config_service_33_0 (system_config_service))
(typeattributeset system_data_file_33_0 (system_data_file))
(typeattributeset system_data_file_33_0 (system_data_file system_userdir_file))
(typeattributeset system_data_root_file_33_0 (system_data_root_file))
(typeattributeset system_dlkm_file_33_0 (system_dlkm_file))
(typeattributeset system_event_log_tags_file_33_0 (system_event_log_tags_file))
@ -2505,7 +2505,7 @@
(typeattributeset vendor_app_file_33_0 (vendor_app_file))
(typeattributeset vendor_cgroup_desc_file_33_0 (vendor_cgroup_desc_file))
(typeattributeset vendor_configs_file_33_0 (vendor_configs_file))
(typeattributeset vendor_data_file_33_0 (vendor_data_file))
(typeattributeset vendor_data_file_33_0 (vendor_data_file vendor_userdir_file))
(typeattributeset vendor_default_prop_33_0 (vendor_default_prop))
(typeattributeset vendor_file_33_0 (vendor_file))
(typeattributeset vendor_framework_file_33_0 (vendor_framework_file))

View file

@ -563,7 +563,8 @@
/data/local/tmp(/.*)? u:object_r:shell_data_file:s0
/data/local/tmp/ltp(/.*)? u:object_r:nativetest_data_file:s0
/data/local/traces(/.*)? u:object_r:trace_data_file:s0
/data/media(/.*)? u:object_r:media_rw_data_file:s0
/data/media u:object_r:media_userdir_file:s0
/data/media/.* u:object_r:media_rw_data_file:s0
/data/mediadrm(/.*)? u:object_r:media_data_file:s0
/data/nativetest(/.*)? u:object_r:nativetest_data_file:s0
/data/nativetest64(/.*)? u:object_r:nativetest_data_file:s0
@ -580,6 +581,12 @@
/data/rollback/\d+/[^/]+/.*\.apk u:object_r:apk_data_file:s0
/data/rollback/\d+/[^/]+/.*\.apex u:object_r:staging_data_file:s0
/data/fonts/files(/.*)? u:object_r:font_data_file:s0
/data/misc_ce u:object_r:system_userdir_file:s0
/data/misc_de u:object_r:system_userdir_file:s0
/data/system_ce u:object_r:system_userdir_file:s0
/data/system_de u:object_r:system_userdir_file:s0
/data/user u:object_r:system_userdir_file:s0
/data/user_de u:object_r:system_userdir_file:s0
# Misc data
/data/misc/adb(/.*)? u:object_r:adb_keys_file:s0
@ -665,8 +672,10 @@
/data/misc/profiles/ref(/.*)? u:object_r:user_profile_data_file:s0
/data/misc/profman(/.*)? u:object_r:profman_dump_data_file:s0
/data/vendor(/.*)? u:object_r:vendor_data_file:s0
/data/vendor_ce(/.*)? u:object_r:vendor_data_file:s0
/data/vendor_de(/.*)? u:object_r:vendor_data_file:s0
/data/vendor_ce u:object_r:vendor_userdir_file:s0
/data/vendor_ce/.* u:object_r:vendor_data_file:s0
/data/vendor_de u:object_r:vendor_userdir_file:s0
/data/vendor_de/.* u:object_r:vendor_data_file:s0
# storaged proto files
/data/misc_de/[0-9]+/storaged(/.*)? u:object_r:storaged_data_file:s0
@ -721,8 +730,17 @@
#############################
# Expanded data files
#
/mnt/expand(/.*)? u:object_r:mnt_expand_file:s0
/mnt/expand/[^/]+(/.*)? u:object_r:system_data_file:s0
/mnt/expand u:object_r:mnt_expand_file:s0
# CAREFUL: the two system_data_file patterns below can't be replaced with one
# pattern "/mnt/expand/[^/]+(/.*)?", since SELinux would prioritize that over
# "/mnt/expand/[^/]+/user". This is because when a path is matched by two
# patterns that contain regex meta-characters, SELinux just chooses the longer
# pattern (or the later pattern if the patterns are the same length), rather
# than the pattern containing fewer regex meta-characters. Splitting the
# pattern into "/mnt/expand/[^/]+" and "/mnt/expand/[^/]+/.*" works around this
# problem, except for 1-character filenames which we aren't using.
/mnt/expand/[^/]+ u:object_r:system_data_file:s0
/mnt/expand/[^/]+/.* u:object_r:system_data_file:s0
/mnt/expand/[^/]+/app(/.*)? u:object_r:apk_data_file:s0
/mnt/expand/[^/]+/app/[^/]+/oat(/.*)? u:object_r:dalvikcache_data_file:s0
# /mnt/expand/..../app/[randomStringA]/[packageName]-[randomStringB]/base.apk layout
@ -730,8 +748,13 @@
/mnt/expand/[^/]+/app/vmdl[^/]+\.tmp(/.*)? u:object_r:apk_tmp_file:s0
/mnt/expand/[^/]+/app/vmdl[^/]+\.tmp/oat(/.*)? u:object_r:dalvikcache_data_file:s0
/mnt/expand/[^/]+/local/tmp(/.*)? u:object_r:shell_data_file:s0
/mnt/expand/[^/]+/media(/.*)? u:object_r:media_rw_data_file:s0
/mnt/expand/[^/]+/media u:object_r:media_userdir_file:s0
/mnt/expand/[^/]+/media/.* u:object_r:media_rw_data_file:s0
/mnt/expand/[^/]+/misc/vold(/.*)? u:object_r:vold_data_file:s0
/mnt/expand/[^/]+/misc_ce u:object_r:system_userdir_file:s0
/mnt/expand/[^/]+/misc_de u:object_r:system_userdir_file:s0
/mnt/expand/[^/]+/user u:object_r:system_userdir_file:s0
/mnt/expand/[^/]+/user_de u:object_r:system_userdir_file:s0
# coredump directory for userdebug/eng devices
/cores(/.*)? u:object_r:coredump_file:s0

View file

@ -12,6 +12,7 @@ r_dir_file(mediaprovider_app, mnt_pass_through_file)
allow mediaprovider_app fuse_device:chr_file { read write ioctl getattr };
# Allow MediaProvider to read/write media_rw_data_file files and dirs
allow mediaprovider_app media_userdir_file:dir r_dir_perms;
allow mediaprovider_app media_rw_data_file:file create_file_perms;
allow mediaprovider_app media_rw_data_file:dir create_dir_perms;

View file

@ -110,6 +110,9 @@ neverallow perfetto {
data_file_type
-system_data_file
-system_data_root_file
-media_userdir_file
-system_userdir_file
-vendor_userdir_file
# TODO(b/72998741) Remove exemption. Further restricted in a subsequent
# neverallow. Currently only getattr and search are allowed.
-vendor_data_file

View file

@ -486,6 +486,10 @@ allow system_server keychain_data_file:dir create_dir_perms;
allow system_server keychain_data_file:file create_file_perms;
allow system_server keychain_data_file:lnk_file create_file_perms;
# Read the user parent directories like /data/user. Don't allow write access,
# as vold and init are responsible for creating and deleting the subdirectories.
allow system_server system_userdir_file:dir r_dir_perms;
# Manage /data/app.
allow system_server apk_data_file:dir create_dir_perms;
allow system_server apk_data_file:{ file lnk_file } { create_file_perms link };

View file

@ -95,6 +95,9 @@ neverallow traced {
-perfetto_traces_bugreport_data_file
-system_data_file
-system_data_root_file
-media_userdir_file
-system_userdir_file
-vendor_userdir_file
# TODO(b/72998741) Remove vendor_data_file exemption. Further restricted in a
# subsequent neverallow. Currently only getattr and search are allowed.
-vendor_data_file

View file

@ -126,6 +126,9 @@ neverallow traced_probes {
-dalvikcache_data_file
-system_data_file
-system_data_root_file
-media_userdir_file
-system_userdir_file
-vendor_userdir_file
-system_app_data_file
-backup_data_file
-bootstat_data_file

View file

@ -66,3 +66,45 @@ neverallow {
-apexd
-gsid
} vold_service:service_manager find;
# Allow vold to create and delete per-user directories like /data/user/$userId.
allow vold {
media_userdir_file
system_userdir_file
vendor_userdir_file
}:dir {
add_name
remove_name
write
};
# Only vold should create (and delete) per-user directories like
# /data/user/$userId. This is very important, as these directories need to be
# encrypted with per-user keys, which only vold can do. Encryption can only be
# set up on empty directories, so creation and encryption must happen together.
#
# Exception: init creates /data/user/0 and /data/media/obb, so that needs to be
# allowed for now. (/data/media/obb isn't actually a per-user directory, but
# it's located in /data/media so it constrains the sepolicy for that directory.)
neverallow {
domain
-vold
} {
vendor_userdir_file
}:dir {
add_name
remove_name
write
};
neverallow {
domain
-vold
-init
} {
system_userdir_file
media_userdir_file
}:dir {
add_name
remove_name
write
};

View file

@ -66,13 +66,15 @@ allow zygote apex_art_data_file:file { r_file_perms execute };
# them for app data isolation. Also traverse these directories (via
# /data_mirror) to find the allowlisted per-app directories to bind-mount in.
allow zygote {
# /data/data, /data/user{,_de}, /mnt/expand/$volume/user{,_de}
# /data/user{,_de}, /mnt/expand/$volume/user{,_de}
system_userdir_file
# /data/data
system_data_file
# /data/misc/profiles/cur
user_profile_root_file
# /data/misc/profiles/ref
user_profile_data_file
# /storage/emulated/$uid/Android/{data,obb}
# /storage/emulated/$userId/Android/{data,obb}
media_rw_data_file
}:dir { mounton search };
@ -100,6 +102,7 @@ allow zygote tmpfs:lnk_file create;
# standard labels. Note: it seems that not all dirs are actually relabeled yet,
# but it works anyway since all domains can search tmpfs:dir.
allow zygote tmpfs:{ dir lnk_file } relabelfrom;
allow zygote system_userdir_file:dir relabelto;
allow zygote system_data_file:{ dir lnk_file } relabelto;
# Read if sdcardfs is supported

View file

@ -242,16 +242,30 @@ r_dir_file(domain, sysfs_usb);
allow domain sysfs_transparent_hugepage:dir search;
allow domain sysfs_transparent_hugepage:file r_file_perms;
# files under /data.
# Allow search access, and sometimes getattr access, to various directories
# under /data. We are fairly lenient in allowing search access to top-level
# dirs that commonly need to be traversed to get access to the "real" files, as
# this greatly simplifies the policy and doesn't open up much attack surface.
not_full_treble(`
allow domain system_data_file:dir getattr;
')
allow { coredomain appdomain } system_data_file:dir getattr;
# /data has the label system_data_root_file. Vendor components need the search
# permission on system_data_root_file for path traversal to /data/vendor.
# Anything that accesses anything in /data needs search access to /data itself.
# This includes vendor components, as they need to access /data/vendor.
allow domain system_data_root_file:dir { search getattr } ;
# system_data_file is the default type for directories in /data. Anything
# accessing data files with a more specific type often has to traverse a
# system_data_file directory such as /data/misc to get there.
allow domain system_data_file:dir search;
# Anything that accesses files in /data/user (and /data/user_de, etc.) needs
# search access to these directories themselves. getattr access is sometimes
# needed too.
allow { coredomain appdomain } system_userdir_file:dir { search getattr };
# Anything that accesses files in /data/media needs search access to /data/media
# itself.
allow { coredomain appdomain } media_userdir_file:dir search;
# TODO restrict this to non-coredomain
allow domain vendor_userdir_file:dir { getattr search };
allow domain vendor_data_file:dir { getattr search };
# required by the dynamic linker
@ -853,6 +867,7 @@ full_treble_only(`
core_data_file_type
-system_data_file # default label for files on /data. Covered below...
-system_data_root_file
-vendor_userdir_file
-vendor_data_file
-zoneinfo_data_file
with_native_coverage(`-method_trace_data_file')
@ -865,6 +880,7 @@ full_treble_only(`
-unencrypted_data_file
-system_data_file
-system_data_root_file
-vendor_userdir_file
-vendor_data_file
-zoneinfo_data_file
with_native_coverage(`-method_trace_data_file')

View file

@ -299,13 +299,20 @@ type coredump_file, file_type;
type system_data_root_file, file_type, data_file_type, core_data_file_type;
# Default type for anything under /data.
type system_data_file, file_type, data_file_type, core_data_file_type;
# Default type for directories containing per-user encrypted directories, such
# as /data/user and /data/user_de.
type system_userdir_file, file_type, data_file_type, core_data_file_type;
# Type for /data/system/packages.list.
# TODO(b/129332765): Narrow down permissions to this.
# Find out users of system_data_file that should be granted only this.
type packages_list_file, file_type, data_file_type, core_data_file_type;
type game_mode_intervention_list_file, file_type, data_file_type, core_data_file_type;
# Default type for anything under /data/vendor{_ce,_de}.
# Default type for anything inside /data/vendor_{ce,de}.
type vendor_data_file, file_type, data_file_type;
# Type for /data/vendor_{ce,de} themselves. This has core_data_file_type
# because these directories themselves are platform-managed; only the files
# *inside* them are vendor data. (Somewhat similar to system_data_root_file.)
type vendor_userdir_file, file_type, data_file_type, core_data_file_type;
# Unencrypted data
type unencrypted_data_file, file_type, data_file_type, core_data_file_type;
# installd-create files in /data/misc/installd such as layout_version
@ -427,6 +434,7 @@ type keychain_data_file, file_type, data_file_type, core_data_file_type;
type keystore_data_file, file_type, data_file_type, core_data_file_type;
type media_data_file, file_type, data_file_type, core_data_file_type;
type media_rw_data_file, file_type, data_file_type, core_data_file_type, mlstrustedobject;
type media_userdir_file, file_type, data_file_type, core_data_file_type;
type misc_user_data_file, file_type, data_file_type, core_data_file_type;
type net_data_file, file_type, data_file_type, core_data_file_type;
type network_watchlist_data_file, file_type, data_file_type, core_data_file_type;

View file

@ -224,6 +224,7 @@ allow init {
-system_dlkm_file_type
-system_file_type
-vendor_file_type
-vendor_userdir_file
-vold_data_file
}:dir { write add_name remove_name rmdir relabelfrom };

View file

@ -42,13 +42,12 @@ allow installd seapp_contexts_file:file r_file_perms;
allow installd asec_image_file:dir search;
allow installd asec_image_file:file getattr;
# Create /data/user and /data/user/0 if necessary.
# Also required to initially create /data/data subdirectories
# Required to initially create subdirectories of /data/user/$userId
# and lib symlinks before the setfilecon call. May want to
# move symlink creation after setfilecon in installd.
allow installd system_data_file:dir create_dir_perms;
# Also, allow read for lnk_file so that we can process /data/user/0 links when
# optimizing application code.
# Also, allow read for lnk_file so that we can process symlinks within
# /data/user/$userId when optimizing application code.
allow installd system_data_file:lnk_file { create getattr read setattr unlink };
# Manage lower filesystem via pass_through mounts
@ -62,6 +61,7 @@ allow installd system_data_file:dir relabelfrom;
allow installd media_rw_data_file:dir relabelto;
# Delete /data/media files through sdcardfs, instead of going behind its back
allow installd media_userdir_file:dir r_dir_perms;
allow installd tmpfs:dir r_dir_perms;
allow installd storage_file:dir search;
allow installd { sdcard_type fuse }:dir { search open read write remove_name getattr rmdir };
@ -71,6 +71,7 @@ allow installd { sdcard_type fuse }:file { getattr unlink };
allow installd mirror_data_file:dir { create_dir_perms mounton };
# Upgrade /data/misc/keychain for multi-user if necessary.
allow installd system_userdir_file:dir r_dir_perms;
allow installd misc_user_data_file:dir create_dir_perms;
allow installd misc_user_data_file:file create_file_perms;
allow installd keychain_data_file:dir create_dir_perms;

View file

@ -22,11 +22,11 @@ neverallow { domain -init } toolbox:process transition;
neverallow * toolbox:process dyntransition;
neverallow toolbox { file_type fs_type -toolbox_exec}:file entrypoint;
# rm -rf directories in /data
# rm -rf /data/per_boot
allow toolbox system_data_root_file:dir { remove_name write };
allow toolbox system_data_file:dir { rmdir rw_dir_perms };
allow toolbox system_data_file:file { getattr unlink };
# chattr +F /data/media in init
allow toolbox media_rw_data_file:dir { r_dir_perms setattr };
allowxperm toolbox media_rw_data_file:dir ioctl { FS_IOC_SETFLAGS FS_IOC_GETFLAGS };
allow toolbox media_userdir_file:dir { r_dir_perms setattr };
allowxperm toolbox media_userdir_file:dir ioctl { FS_IOC_SETFLAGS FS_IOC_GETFLAGS };

View file

@ -99,8 +99,8 @@ allow vold media_rw_data_file:file create_file_perms;
# Allow mounting (lower filesystem) on parts of media for performance
allow vold media_rw_data_file:dir mounton;
# Allow setting extended attributes (for project quota IDs) on files and dirs
# and to enable project ID inheritance through FS_IOC_SETFLAGS
# Allow setting project quota IDs and enabling project ID inheritance on
# /data/media/$userId/* and /mnt/expand/$volume/media/$userId/*
allowxperm vold media_rw_data_file:{ dir file } ioctl {
FS_IOC_FSGETXATTR
FS_IOC_FSSETXATTR
@ -124,6 +124,10 @@ allow vold mnt_pass_through_file:lnk_file create_file_perms;
allow vold mnt_expand_file:dir { create_dir_perms mounton };
allow vold apk_data_file:dir { create getattr setattr };
allow vold shell_data_file:dir { create getattr setattr };
allow vold system_userdir_file:dir { create getattr setattr };
allow vold media_userdir_file:dir { create getattr setattr open read ioctl };
# Needed to set the casefold flag on /mnt/expand/$volume/media
allowxperm vold media_userdir_file:dir ioctl { FS_IOC_GETFLAGS FS_IOC_SETFLAGS };
# Allow to mount incremental file system on /data/incremental and create files
allow vold apk_data_file:dir { mounton rw_dir_perms };