From c4055f0d04f731e44b7ca4b11197855a2d1158dd Mon Sep 17 00:00:00 2001
From: Alex Vakulenko <avakulenko@google.com>
Date: Mon, 1 May 2017 13:01:44 -0700
Subject: [PATCH] SELinux policies for PDX services

Specify per-service rules for PDX transport. Now being able to
grant permissions to individual services provided by processes,
not all services of a process.

Also tighter control over which permissions are required for
client and server for individual components of IPC (endpoints,
channels, etc).

Bug: 37646189
Change-Id: I78eb8ae8b6e08105666445a66bfcbd2f1d69d0ea
Merged-Id: I78eb8ae8b6e08105666445a66bfcbd2f1d69d0ea
---
 private/app.te            | 13 ++++---
 private/file_contexts     | 14 +++++++-
 private/surfaceflinger.te | 14 +++++---
 public/attributes         | 14 ++++++++
 public/bufferhubd.te      |  6 ++--
 public/file.te            | 17 ++++++++-
 public/mediacodec.te      |  2 +-
 public/performanced.te    |  2 +-
 public/sensord.te         |  7 ++--
 public/te_macros          | 75 +++++++++++++++++++++++++++++++--------
 10 files changed, 130 insertions(+), 34 deletions(-)

diff --git a/private/app.te b/private/app.te
index 1cf86ff46..359c35411 100644
--- a/private/app.te
+++ b/private/app.te
@@ -261,11 +261,14 @@ allow appdomain proc_meminfo:file r_file_perms;
 # For app fuse.
 allow appdomain app_fuse_file:file { getattr read append write };
 
-use_pdx({ appdomain -isolated_app -ephemeral_app }, surfaceflinger)
-use_pdx({ appdomain -isolated_app -ephemeral_app }, sensord)
-use_pdx({ appdomain -isolated_app -ephemeral_app }, performanced)
-# TODO: apps do not directly open the IPC socket for bufferhubd.
-use_pdx({ appdomain -isolated_app -ephemeral_app }, bufferhubd)
+pdx_client({ appdomain -isolated_app -ephemeral_app }, display_client)
+pdx_client({ appdomain -isolated_app -ephemeral_app }, display_manager)
+pdx_client({ appdomain -isolated_app -ephemeral_app }, display_vsync)
+pdx_client({ appdomain -isolated_app -ephemeral_app }, sensors_client)
+pdx_client({ appdomain -isolated_app -ephemeral_app }, pose_client)
+pdx_client({ appdomain -isolated_app -ephemeral_app }, performance_client)
+# Apps do not directly open the IPC socket for bufferhubd.
+pdx_use({ appdomain -isolated_app -ephemeral_app }, bufferhub_client)
 
 ###
 ### CTS-specific rules
diff --git a/private/file_contexts b/private/file_contexts
index adae7dcfd..6b6498424 100644
--- a/private/file_contexts
+++ b/private/file_contexts
@@ -129,7 +129,19 @@
 /dev/socket/mdnsd	u:object_r:mdnsd_socket:s0
 /dev/socket/mtpd	u:object_r:mtpd_socket:s0
 /dev/socket/netd	u:object_r:netd_socket:s0
-/dev/socket/pdx(/.*)?	u:object_r:pdx_socket:s0
+/dev/socket/pdx/system/buffer_hub	u:object_r:pdx_bufferhub_dir:s0
+/dev/socket/pdx/system/buffer_hub/client	u:object_r:pdx_bufferhub_client_endpoint_socket:s0
+/dev/socket/pdx/system/performance	u:object_r:pdx_performance_dir:s0
+/dev/socket/pdx/system/performance/client	u:object_r:pdx_performance_client_endpoint_socket:s0
+/dev/socket/pdx/system/vr/sensors	u:object_r:pdx_sensors_dir:s0
+/dev/socket/pdx/system/vr/sensors/client	u:object_r:pdx_sensors_client_endpoint_socket:s0
+/dev/socket/pdx/system/vr/pose	u:object_r:pdx_pose_dir:s0
+/dev/socket/pdx/system/vr/pose/client	u:object_r:pdx_pose_client_endpoint_socket:s0
+/dev/socket/pdx/system/vr/display	u:object_r:pdx_display_dir:s0
+/dev/socket/pdx/system/vr/display/client	u:object_r:pdx_display_client_endpoint_socket:s0
+/dev/socket/pdx/system/vr/display/manager	u:object_r:pdx_display_manager_endpoint_socket:s0
+/dev/socket/pdx/system/vr/display/screenshot	u:object_r:pdx_display_screenshot_endpoint_socket:s0
+/dev/socket/pdx/system/vr/display/vsync	u:object_r:pdx_display_vsync_endpoint_socket:s0
 /dev/socket/property_service	u:object_r:property_socket:s0
 /dev/socket/racoon	u:object_r:racoon_socket:s0
 /dev/socket/rild	u:object_r:rild_socket:s0
diff --git a/private/surfaceflinger.te b/private/surfaceflinger.te
index f143580dd..f1ad667b8 100644
--- a/private/surfaceflinger.te
+++ b/private/surfaceflinger.te
@@ -90,11 +90,15 @@ allow surfaceflinger system_server:fd use;
 allow surfaceflinger ion_device:chr_file r_file_perms;
 
 # pdx IPC
-pdx_server(surfaceflinger)
-
-use_pdx(surfaceflinger, bufferhubd)
-use_pdx(surfaceflinger, performanced)
-use_pdx(surfaceflinger, sensord)
+pdx_server(surfaceflinger, display_client)
+pdx_server(surfaceflinger, display_manager)
+pdx_server(surfaceflinger, display_screenshot)
+pdx_server(surfaceflinger, display_vsync)
+
+pdx_client(surfaceflinger, bufferhub_client)
+pdx_client(surfaceflinger, performance_client)
+pdx_client(surfaceflinger, sensors_client)
+pdx_client(surfaceflinger, pose_client)
 
 ###
 ### Neverallow rules
diff --git a/public/attributes b/public/attributes
index 00035abba..d729a7b63 100644
--- a/public/attributes
+++ b/public/attributes
@@ -122,6 +122,20 @@ attribute coredomain;
 # TODO(b/35870313): Remove this once there are no violations
 attribute binder_in_vendor_violators;
 
+# PDX services
+attribute pdx_endpoint_dir_type;
+attribute pdx_endpoint_socket_type;
+attribute pdx_channel_socket_type;
+
+pdx_service_attributes(display_client)
+pdx_service_attributes(display_manager)
+pdx_service_attributes(display_screenshot)
+pdx_service_attributes(display_vsync)
+pdx_service_attributes(performance_client)
+pdx_service_attributes(sensors_client)
+pdx_service_attributes(pose_client);
+pdx_service_attributes(bufferhub_client)
+
 # All HAL servers
 attribute halserverdomain;
 # All HAL clients
diff --git a/public/bufferhubd.te b/public/bufferhubd.te
index 7d5be49fe..274c2716b 100644
--- a/public/bufferhubd.te
+++ b/public/bufferhubd.te
@@ -4,8 +4,8 @@ type bufferhubd_exec, exec_type, file_type;
 
 hal_client_domain(bufferhubd, hal_graphics_allocator)
 
-pdx_server(bufferhubd)
-use_pdx(bufferhubd, performanced)
+pdx_server(bufferhubd, bufferhub_client)
+pdx_client(bufferhubd, performance_client)
 
 # Access the GPU.
 allow bufferhubd gpu_device:chr_file rw_file_perms;
@@ -16,5 +16,5 @@ allow bufferhubd ion_device:chr_file r_file_perms;
 # Receive sync fence FDs from mediacodec. Note that mediacodec never directly
 # connects to bufferhubd via PDX. Instead, a VR app acts as a bridge between
 # those two: it talks to mediacodec via Binder and talks to bufferhubd via PDX.
-# Thus, there is no need to use use_pdx macro.
+# Thus, there is no need to use pdx_client macro.
 allow bufferhubd mediacodec:fd use;
diff --git a/public/file.te b/public/file.te
index 2abfe7062..8a48dfe30 100644
--- a/public/file.te
+++ b/public/file.te
@@ -236,7 +236,6 @@ type mdnsd_socket, file_type, mlstrustedobject;
 type misc_logd_file, file_type;
 type mtpd_socket, file_type;
 type netd_socket, file_type;
-type pdx_socket, file_type, mlstrustedobject;
 type property_socket, file_type, mlstrustedobject;
 type racoon_socket, file_type;
 type rild_socket, file_type;
@@ -256,6 +255,22 @@ type sap_uim_socket, file_type;
 # UART (for GPS) control proc file
 type gps_control, file_type;
 
+# PDX endpoint types
+type pdx_display_dir, pdx_endpoint_dir_type, file_type;
+type pdx_performance_dir, pdx_endpoint_dir_type, file_type;
+type pdx_sensors_dir, pdx_endpoint_dir_type, file_type;
+type pdx_pose_dir, pdx_endpoint_dir_type, file_type;
+type pdx_bufferhub_dir, pdx_endpoint_dir_type, file_type;
+
+pdx_service_socket_types(display_client, pdx_display_dir)
+pdx_service_socket_types(display_manager, pdx_display_dir)
+pdx_service_socket_types(display_screenshot, pdx_display_dir)
+pdx_service_socket_types(display_vsync, pdx_display_dir)
+pdx_service_socket_types(performance_client, pdx_performance_dir)
+pdx_service_socket_types(sensors_client, pdx_sensors_dir)
+pdx_service_socket_types(pose_client, pdx_pose_dir)
+pdx_service_socket_types(bufferhub_client, pdx_bufferhub_dir)
+
 # property_contexts file
 type property_contexts, file_type;
 
diff --git a/public/mediacodec.te b/public/mediacodec.te
index 469c8bab5..ff3795ab3 100644
--- a/public/mediacodec.te
+++ b/public/mediacodec.te
@@ -30,7 +30,7 @@ hal_client_domain(mediacodec, hal_allocator)
 # Recieve gralloc buffer FDs from bufferhubd. Note that mediacodec never
 # directly connects to bufferhubd via PDX. Instead, a VR app acts as a bridge
 # between those two: it talks to mediacodec via Binder and talks to bufferhubd
-# via PDX. Thus, there is no need to use use_pdx macro.
+# via PDX. Thus, there is no need to use pdx_client macro.
 allow mediacodec bufferhubd:fd use;
 
 ###
diff --git a/public/performanced.te b/public/performanced.te
index 8f9d16b05..7f2e13ff7 100644
--- a/public/performanced.te
+++ b/public/performanced.te
@@ -2,7 +2,7 @@
 type performanced, domain, mlstrustedsubject;
 type performanced_exec, exec_type, file_type;
 
-pdx_server(performanced)
+pdx_server(performanced, performance_client)
 
 # TODO: use file caps to obtain sys_nice instead of setuid / setgid.
 allow performanced self:capability { setuid setgid sys_nice };
diff --git a/public/sensord.te b/public/sensord.te
index 3211f8103..c9749cb10 100644
--- a/public/sensord.te
+++ b/public/sensord.te
@@ -5,9 +5,10 @@ type sensord_exec, exec_type, file_type;
 hal_client_domain(sensord, hal_graphics_allocator)
 allow sensord hal_graphics_allocator:fd use;
 
-pdx_server(sensord)
-use_pdx(sensord, bufferhubd)
-use_pdx(sensord, performanced)
+pdx_server(sensord, sensors_client)
+pdx_server(sensord, pose_client)
+pdx_client(sensord, bufferhub_client)
+pdx_client(sensord, performance_client)
 
 # Access /dev/ion
 allow sensord ion_device:chr_file r_file_perms;
diff --git a/public/te_macros b/public/te_macros
index 5b7879693..125ca81ee 100644
--- a/public/te_macros
+++ b/public/te_macros
@@ -85,26 +85,73 @@ allow $1 tmpfs:dir { getattr search };
 # rules from underlying transport (e.g. UDS-based implementation).
 
 #####################################
-# pdx_server(domain)
-define(`pdx_server', `
-allow $1 pdx_socket:dir create_dir_perms;
-allow $1 pdx_socket:sock_file create_file_perms;
+# pdx_service_attributes(service)
+# Defines type attribute used to identify various service-related types.
+define(`pdx_service_attributes', `
+attribute pdx_$1_endpoint_dir_type;
+attribute pdx_$1_endpoint_socket_type;
+attribute pdx_$1_channel_socket_type;
+attribute pdx_$1_server_type;
 ')
 
 #####################################
-# use_pdx(clientdomain, serverdomain)
-define(`use_pdx', `
-# Open the socket.
-allow $1 pdx_socket:dir r_dir_perms;
-allow $1 pdx_socket:sock_file rw_file_perms;
-# Use the socket.
-allow $1 $2:unix_stream_socket { connectto read write shutdown };
-# Clients recieve an event fd from the server.
-allow $1 $2:fd use;
+# pdx_service_socket_types(service, endpoint_dir_t)
+# Define types for endpoint and channel sockets.
+define(`pdx_service_socket_types', `
+typeattribute $2 pdx_$1_endpoint_dir_type;
+type pdx_$1_endpoint_socket, pdx_$1_endpoint_socket_type, pdx_endpoint_socket_type, file_type, mlstrustedobject, mlstrustedsubject;
+type pdx_$1_channel_socket, pdx_$1_channel_socket_type, pdx_channel_socket_type;
+')
+
+#####################################
+# pdx_server(server_domain, service)
+define(`pdx_server', `
+# Mark the server domain as a PDX server.
+typeattribute $1 pdx_$2_server_type;
+# Allow the init process to create the initial endpoint socket.
+allow init pdx_$2_endpoint_socket_type:unix_stream_socket { create bind };
+# Allow the server domain to use the endpoint socket and accept connections on it.
+# Not using macro like "rw_socket_perms_no_ioctl" because it provides more rights
+# than we need (e.g. we don"t need "bind" or "connect").
+allow $1 pdx_$2_endpoint_socket_type:unix_stream_socket { read getattr write setattr lock append getopt setopt shutdown listen accept };
+# Allow the server domain to apply security context label to the channel socket pair (allow process to use setsockcreatecon_raw()).
+allow $1 self:process setsockcreate;
+# Allow the server domain to create a client channel socket.
+allow $1 pdx_$2_channel_socket_type:unix_stream_socket create_stream_socket_perms;
+# Prevent other processes from claiming to be a server for the same service.
+neverallow {domain -$1} pdx_$2_endpoint_socket_type:unix_stream_socket { listen accept };
+')
+
+#####################################
+# pdx_connect(client, service)
+define(`pdx_connect', `
+# Allow client to open the service endpoint file.
+allow $1 pdx_$2_endpoint_dir_type:dir r_dir_perms;
+allow $1 pdx_$2_endpoint_socket_type:sock_file rw_file_perms;
+# Allow the client to connect to endpoint socket.
+allow $1 pdx_$2_endpoint_socket_type:unix_stream_socket { connectto read write shutdown };
+')
+
+#####################################
+# pdx_use(client, service)
+define(`pdx_use', `
+# Allow the client to use the PDX channel socket.
+# Not using macro like "rw_socket_perms_no_ioctl" because it provides more rights
+# than we need (e.g. we don"t need "bind" or "connect").
+allow $1 pdx_$2_channel_socket_type:unix_stream_socket { read getattr write setattr lock append getopt setopt shutdown };
+# Client needs to use an channel event fd from the server.
+allow $1 pdx_$2_server_type:fd use;
 # Servers may receive sync fences, gralloc buffers, etc, from clients.
 # This could be tightened on a per-server basis, but keeping track of service
 # clients is error prone.
-allow $2 $1:fd use;
+allow pdx_$2_server_type $1:fd use;
+')
+
+#####################################
+# pdx_client(client, service)
+define(`pdx_client', `
+pdx_connect($1, $2)
+pdx_use($1, $2)
 ')
 
 #####################################
-- 
GitLab