From 606d2fd6651027204727b5141c03e5e47ed1f6e4 Mon Sep 17 00:00:00 2001
From: William Roberts <william.c.roberts@intel.com>
Date: Thu, 19 Jan 2017 13:23:52 -0800
Subject: [PATCH] te_macros: introduce add_service() macro

Introduce the add_service() macro which wraps up add/find
permissions for the source domain with a neverallow preventing
others from adding it. Only a particular domain should
add a particular service.

Use the add_service() macro to automatically add a neverallow
that prevents other domains from adding the service.

mediadrmserver was adding services labeled mediaserver_service.
Drop the add permission as it should just need the find
permission.

Additionally, the macro adds the { add find } permission which
causes some existing neverallow's to assert. Adjust those
neverallow's so "self" can always find.

Test: compile and run on hikey and emulator. No new denials were
found, and all services, where applicable, seem to be running OK.

Change-Id: Ibbd2a5304edd5f8b877bc86852b0694732be993c
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
 private/storaged.te      |  2 +-
 public/audioserver.te    |  2 +-
 public/cameraserver.te   |  2 +-
 public/drmserver.te      |  2 +-
 public/dumpstate.te      |  9 +++------
 public/fingerprintd.te   |  2 +-
 public/gatekeeperd.te    |  4 +---
 public/healthd.te        |  2 +-
 public/inputflinger.te   |  2 +-
 public/installd.te       |  6 +++---
 public/keystore.te       |  2 +-
 public/mediacodec.te     |  2 +-
 public/mediadrmserver.te |  4 ++--
 public/mediaextractor.te |  2 +-
 public/mediametrics.te   |  2 +-
 public/mediaserver.te    |  2 +-
 public/netd.te           |  6 +++---
 public/nfc.te            |  2 +-
 public/radio.te          |  2 +-
 public/surfaceflinger.te |  5 +++--
 public/system_server.te  |  2 +-
 public/te_macros         | 10 ++++++++++
 public/update_engine.te  |  2 +-
 public/wificond.te       |  2 +-
 24 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/private/storaged.te b/private/storaged.te
index c6276a31c..1d87251fd 100644
--- a/private/storaged.te
+++ b/private/storaged.te
@@ -24,7 +24,7 @@ userdebug_or_eng(`
 ')
 
 # Binder permissions
-allow storaged storaged_service:service_manager add;
+add_service(storaged, storaged_service)
 
 binder_use(storaged)
 binder_call(storaged, system_server)
diff --git a/public/audioserver.te b/public/audioserver.te
index 676b04e32..bc0b989ff 100644
--- a/public/audioserver.te
+++ b/public/audioserver.te
@@ -30,7 +30,7 @@ userdebug_or_eng(`
 allow audioserver audio_device:dir r_dir_perms;
 allow audioserver audio_device:chr_file rw_file_perms;
 
-allow audioserver audioserver_service:service_manager { add find };
+add_service(audioserver, audioserver_service)
 allow audioserver appops_service:service_manager find;
 allow audioserver batterystats_service:service_manager find;
 allow audioserver permission_service:service_manager find;
diff --git a/public/cameraserver.te b/public/cameraserver.te
index 41359261e..13c289021 100644
--- a/public/cameraserver.te
+++ b/public/cameraserver.te
@@ -21,11 +21,11 @@ allow cameraserver camera_device:chr_file rw_file_perms;
 allow cameraserver ion_device:chr_file rw_file_perms;
 allow cameraserver hal_graphics_allocator:fd use;
 
+add_service(cameraserver, cameraserver_service)
 allow cameraserver appops_service:service_manager find;
 allow cameraserver audioserver_service:service_manager find;
 allow cameraserver batterystats_service:service_manager find;
 allow cameraserver cameraproxy_service:service_manager find;
-allow cameraserver cameraserver_service:service_manager add;
 allow cameraserver mediaserver_service:service_manager find;
 allow cameraserver processinfo_service:service_manager find;
 allow cameraserver scheduling_policy_service:service_manager find;
diff --git a/public/drmserver.te b/public/drmserver.te
index ab42696d2..453ce1213 100644
--- a/public/drmserver.te
+++ b/public/drmserver.te
@@ -50,7 +50,7 @@ allow drmserver radio_data_file:file { read getattr };
 allow drmserver oemfs:dir search;
 allow drmserver oemfs:file r_file_perms;
 
-allow drmserver drmserver_service:service_manager { add find };
+add_service(drmserver, drmserver_service)
 allow drmserver permission_service:service_manager find;
 
 selinux_check_access(drmserver)
diff --git a/public/dumpstate.te b/public/dumpstate.te
index a49521136..c120736e8 100644
--- a/public/dumpstate.te
+++ b/public/dumpstate.te
@@ -188,17 +188,14 @@ allow dumpstate proc_interrupts:file r_file_perms;
 allow dumpstate proc_zoneinfo:file r_file_perms;
 
 # Create a service for talking back to system_server
-allow dumpstate dumpstate_service:service_manager add;
+add_service(dumpstate, dumpstate_service)
 
 ###
 ### neverallow rules
 ###
 
-# only dumpstate can add the dumpstate service
-neverallow { domain -dumpstate } dumpstate_service:service_manager add;
-
-# only system_server and shell can find the dumpstate service
-neverallow { domain -system_server -shell } dumpstate_service:service_manager find;
+# only system_server, dumpstate and shell can find the dumpstate service
+neverallow { domain -system_server -shell -dumpstate } dumpstate_service:service_manager find;
 
 # Dumpstate should not be writing to any generically labeled sysfs files.
 # Create a specific label for the file type
diff --git a/public/fingerprintd.te b/public/fingerprintd.te
index b27f014cf..57cde1db0 100644
--- a/public/fingerprintd.te
+++ b/public/fingerprintd.te
@@ -7,7 +7,7 @@ binder_use(fingerprintd)
 allow fingerprintd system_file:dir r_dir_perms;
 
 # need to find KeyStore and add self
-allow fingerprintd fingerprintd_service:service_manager { add find };
+add_service(fingerprintd, fingerprintd_service)
 
 # allow HAL module to read dir contents
 allow fingerprintd fingerprintd_data_file:file { create_file_perms };
diff --git a/public/gatekeeperd.te b/public/gatekeeperd.te
index 88a2e00fc..e842cd26c 100644
--- a/public/gatekeeperd.te
+++ b/public/gatekeeperd.te
@@ -8,7 +8,7 @@ binder_service(gatekeeperd)
 binder_use(gatekeeperd)
 
 # need to find KeyStore and add self
-allow gatekeeperd gatekeeper_service:service_manager { add find };
+add_service(gatekeeperd, gatekeeper_service)
 
 # Scan through /system/lib64/hw looking for installed HALs
 allow gatekeeperd system_file:dir r_dir_perms;
@@ -32,5 +32,3 @@ allow gatekeeperd gatekeeper_data_file:file create_file_perms;
 allow gatekeeperd hardware_properties_service:service_manager find;
 
 r_dir_file(gatekeeperd, cgroup)
-
-neverallow { domain -gatekeeperd } gatekeeper_service:service_manager add;
diff --git a/public/healthd.te b/public/healthd.te
index fcc5afc40..2f26b9e28 100644
--- a/public/healthd.te
+++ b/public/healthd.te
@@ -57,7 +57,7 @@ allow healthd ashmem_device:chr_file execute;
 allow healthd self:process execmem;
 allow healthd proc_sysrq:file rw_file_perms;
 
-allow healthd batteryproperties_service:service_manager { add find };
+add_service(healthd, batteryproperties_service)
 
 # Healthd needs to tell init to continue the boot
 # process when running in charger mode.
diff --git a/public/inputflinger.te b/public/inputflinger.te
index 14cfdc73f..e5f12a0c1 100644
--- a/public/inputflinger.te
+++ b/public/inputflinger.te
@@ -9,7 +9,7 @@ binder_call(inputflinger, system_server)
 
 wakelock_use(inputflinger)
 
-allow inputflinger inputflinger_service:service_manager { add find };
+add_service(inputflinger, inputflinger_service)
 allow inputflinger input_device:dir r_dir_perms;
 allow inputflinger input_device:chr_file rw_file_perms;
 
diff --git a/public/installd.te b/public/installd.te
index bf83b9d82..08255a4c0 100644
--- a/public/installd.te
+++ b/public/installd.te
@@ -121,7 +121,7 @@ allow installd toolbox_exec:file rx_file_perms;
 
 # Allow installd to publish a binder service and make binder calls.
 binder_use(installd)
-allow installd installd_service:service_manager add;
+add_service(installd, installd_service)
 allow installd dumpstate:fifo_file  { getattr write };
 
 # Allow installd to call into the system server so it can check permissions.
@@ -136,7 +136,7 @@ allow installd labeledfs:filesystem { quotaget quotamod };
 ### Neverallow rules
 ###
 
-# only system_server and dumpstate may interact with installd over binder
-neverallow { domain -system_server -dumpstate } installd_service:service_manager find;
+# only system_server, installd and dumpstate may interact with installd over binder
+neverallow { domain -system_server -dumpstate -installd } installd_service:service_manager find;
 neverallow { domain -system_server -dumpstate } installd:binder call;
 neverallow installd { domain -system_server -servicemanager userdebug_or_eng(`-su') }:binder call;
diff --git a/public/keystore.te b/public/keystore.te
index 42150176a..457ff376b 100644
--- a/public/keystore.te
+++ b/public/keystore.te
@@ -12,7 +12,7 @@ allow keystore keystore_exec:file { getattr };
 allow keystore tee_device:chr_file rw_file_perms;
 allow keystore tee:unix_stream_socket connectto;
 
-allow keystore keystore_service:service_manager { add find };
+add_service(keystore, keystore_service)
 allow keystore sec_key_att_app_id_provider_service:service_manager find;
 
 # Check SELinux permissions.
diff --git a/public/mediacodec.te b/public/mediacodec.te
index 27b27e0d1..9f07d8564 100644
--- a/public/mediacodec.te
+++ b/public/mediacodec.te
@@ -9,7 +9,7 @@ binder_call(mediacodec, binderservicedomain)
 binder_call(mediacodec, appdomain)
 binder_service(mediacodec)
 
-allow mediacodec mediacodec_service:service_manager add;
+add_service(mediacodec, mediacodec_service)
 allow mediacodec mediametrics_service:service_manager find;
 allow mediacodec surfaceflinger_service:service_manager find;
 allow mediacodec gpu_device:chr_file rw_file_perms;
diff --git a/public/mediadrmserver.te b/public/mediadrmserver.te
index 781229b72..f93cf4545 100644
--- a/public/mediadrmserver.te
+++ b/public/mediadrmserver.te
@@ -10,8 +10,8 @@ binder_call(mediadrmserver, binderservicedomain)
 binder_call(mediadrmserver, appdomain)
 binder_service(mediadrmserver)
 
-allow mediadrmserver mediadrmserver_service:service_manager { add find };
-allow mediadrmserver mediaserver_service:service_manager { add find };
+add_service(mediadrmserver, mediadrmserver_service)
+allow mediadrmserver mediaserver_service:service_manager find;
 allow mediadrmserver mediametrics_service:service_manager find;
 allow mediadrmserver processinfo_service:service_manager find;
 allow mediadrmserver surfaceflinger_service:service_manager find;
diff --git a/public/mediaextractor.te b/public/mediaextractor.te
index 7187c220a..deecc00ba 100644
--- a/public/mediaextractor.te
+++ b/public/mediaextractor.te
@@ -9,7 +9,7 @@ binder_call(mediaextractor, binderservicedomain)
 binder_call(mediaextractor, appdomain)
 binder_service(mediaextractor)
 
-allow mediaextractor mediaextractor_service:service_manager add;
+add_service(mediaextractor, mediaextractor_service)
 allow mediaextractor mediametrics_service:service_manager find;
 
 allow mediaextractor system_server:fd use;
diff --git a/public/mediametrics.te b/public/mediametrics.te
index 9b4409be2..84d184bd9 100644
--- a/public/mediametrics.te
+++ b/public/mediametrics.te
@@ -7,7 +7,7 @@ binder_use(mediametrics)
 binder_call(mediametrics, binderservicedomain)
 binder_service(mediametrics)
 
-allow mediametrics mediametrics_service:service_manager add;
+add_service(mediametrics, mediametrics_service)
 
 allow mediametrics system_server:fd use;
 
diff --git a/public/mediaserver.te b/public/mediaserver.te
index 56654e509..16b801328 100644
--- a/public/mediaserver.te
+++ b/public/mediaserver.te
@@ -78,6 +78,7 @@ unix_socket_connect(mediaserver, bluetooth, bluetooth)
 # Connect to tee service.
 allow mediaserver tee:unix_stream_socket connectto;
 
+add_service(mediaserver, mediaserver_service)
 allow mediaserver activity_service:service_manager find;
 allow mediaserver appops_service:service_manager find;
 allow mediaserver audioserver_service:service_manager find;
@@ -86,7 +87,6 @@ allow mediaserver batterystats_service:service_manager find;
 allow mediaserver drmserver_service:service_manager find;
 allow mediaserver mediaextractor_service:service_manager find;
 allow mediaserver mediacodec_service:service_manager find;
-allow mediaserver mediaserver_service:service_manager { add find };
 allow mediaserver mediametrics_service:service_manager find;
 allow mediaserver media_session_service:service_manager find;
 allow mediaserver permission_service:service_manager find;
diff --git a/public/netd.te b/public/netd.te
index 45a19525c..df1820361 100644
--- a/public/netd.te
+++ b/public/netd.te
@@ -61,7 +61,7 @@ set_prop(netd, ctl_mdnsd_prop)
 
 # Allow netd to publish a binder service and make binder calls.
 binder_use(netd)
-allow netd netd_service:service_manager add;
+add_service(netd, netd_service)
 allow netd dumpstate:fifo_file  { getattr write };
 
 # Allow netd to call into the system server so it can check permissions.
@@ -92,7 +92,7 @@ neverallow netd system_file:dir_file_class_set write;
 # Write to files in /data/data or system files on /data
 neverallow netd { app_data_file system_data_file }:dir_file_class_set write;
 
-# only system_server and dumpstate may interact with netd over binder
-neverallow { domain -system_server -dumpstate } netd_service:service_manager find;
+# only system_server, dumpstate and netd  may interact with netd over binder
+neverallow { domain -system_server -dumpstate -netd } netd_service:service_manager find;
 neverallow { domain -system_server -dumpstate } netd:binder call;
 neverallow netd { domain -system_server -servicemanager userdebug_or_eng(`-su') }:binder call;
diff --git a/public/nfc.te b/public/nfc.te
index 9a8b47183..866180bdb 100644
--- a/public/nfc.te
+++ b/public/nfc.te
@@ -25,7 +25,7 @@ allow nfc mediametrics_service:service_manager find;
 allow nfc mediaextractor_service:service_manager find;
 allow nfc mediaserver_service:service_manager find;
 
-allow nfc nfc_service:service_manager { add find };
+add_service(nfc, nfc_service)
 allow nfc radio_service:service_manager find;
 allow nfc surfaceflinger_service:service_manager find;
 allow nfc app_api_service:service_manager find;
diff --git a/public/radio.te b/public/radio.te
index eb52f099f..953b59ca2 100644
--- a/public/radio.te
+++ b/public/radio.te
@@ -24,12 +24,12 @@ set_prop(radio, net_radio_prop)
 # ctl interface
 set_prop(radio, ctl_rildaemon_prop)
 
+add_service(radio, radio_service)
 allow radio audioserver_service:service_manager find;
 allow radio cameraserver_service:service_manager find;
 allow radio drmserver_service:service_manager find;
 allow radio mediaserver_service:service_manager find;
 allow radio nfc_service:service_manager find;
-allow radio radio_service:service_manager { add find };
 allow radio surfaceflinger_service:service_manager find;
 allow radio app_api_service:service_manager find;
 allow radio system_api_service:service_manager find;
diff --git a/public/surfaceflinger.te b/public/surfaceflinger.te
index 2b1faec10..68e86b107 100644
--- a/public/surfaceflinger.te
+++ b/public/surfaceflinger.te
@@ -57,11 +57,12 @@ allow surfaceflinger tee_device:chr_file rw_file_perms;
 
 
 # media.player service
+add_service(surfaceflinger, gpu_service)
+add_service(surfaceflinger, surfaceflinger_service)
+
 allow surfaceflinger mediaserver_service:service_manager find;
 allow surfaceflinger permission_service:service_manager find;
 allow surfaceflinger power_service:service_manager find;
-allow surfaceflinger gpu_service:service_manager { add find };
-allow surfaceflinger surfaceflinger_service:service_manager { add find };
 allow surfaceflinger window_service:service_manager find;
 
 # allow self to set SCHED_FIFO
diff --git a/public/system_server.te b/public/system_server.te
index 84854807e..1dfdafaf7 100644
--- a/public/system_server.te
+++ b/public/system_server.te
@@ -482,6 +482,7 @@ allow system_server pstorefs:file r_file_perms;
 allow system_server sysfs_zram:dir search;
 allow system_server sysfs_zram:file r_file_perms;
 
+add_service(system_server, system_server_service);
 allow system_server audioserver_service:service_manager find;
 allow system_server batteryproperties_service:service_manager find;
 allow system_server cameraserver_service:service_manager find;
@@ -500,7 +501,6 @@ allow system_server mediadrmserver_service:service_manager find;
 allow system_server netd_service:service_manager find;
 allow system_server nfc_service:service_manager find;
 allow system_server radio_service:service_manager find;
-allow system_server system_server_service:service_manager { add find };
 allow system_server surfaceflinger_service:service_manager find;
 allow system_server wificond_service:service_manager find;
 
diff --git a/public/te_macros b/public/te_macros
index d4e132430..0eba3ff3f 100644
--- a/public/te_macros
+++ b/public/te_macros
@@ -371,6 +371,16 @@ define(`use_drmservice', `
   allow drmserver $1:process getattr;
 ')
 
+###########################################
+# add_service(domain, service)
+# Ability for domain to add a service to service_manager
+# and find it. It also creates a neverallow preventing
+# others from adding it.
+define(`add_service', `
+  allow $1 $2:service_manager { add find };
+  neverallow { domain -$1 } $2:service_manager add;
+')
+
 ##########################################
 # print a message with a trailing newline
 # print(`args')
diff --git a/public/update_engine.te b/public/update_engine.te
index 2c6e585b6..3a3340719 100644
--- a/public/update_engine.te
+++ b/public/update_engine.te
@@ -25,7 +25,7 @@ dontaudit update_engine kernel:system module_request;
 
 # Register the service to perform Binder IPC.
 binder_use(update_engine)
-allow update_engine update_engine_service:service_manager { add };
+add_service(update_engine, update_engine_service)
 
 # Allow update_engine to call the callback function provided by priv_app.
 binder_call(update_engine, priv_app)
diff --git a/public/wificond.te b/public/wificond.te
index 0fcc3ae9b..dd22d26b1 100644
--- a/public/wificond.te
+++ b/public/wificond.te
@@ -5,7 +5,7 @@ type wificond_exec, exec_type, file_type;
 binder_use(wificond)
 binder_call(wificond, system_server)
 
-allow wificond wificond_service:service_manager { add find };
+add_service(wificond, wificond_service)
 
 # wificond writes firmware paths to this file.
 # wificond also changes the owership of this file on startup.
-- 
GitLab