Thank you for the feedback. An updated patch for inclusion into the
sepolicy tree, is below.
My replies to your comments are below the patch.
From: Bryan Hinton <bryan@xxxxxxxxxxxxxxx>
Date: Fri, 2 Mar 2012 15:45:06 -0600
Subject: [PATCH] Updated policy for SCH-i515. These changes placed
into the public domain.
Change-Id: Ie8776fd6ed6e84c40f545091358d2508e31bd02b
---
domain.te | 2 ++
file.te | 1 +
file_contexts | 12 +++++++++++-
rild.te | 1 +
4 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/domain.te b/domain.te
index 55c9ecd..cd7f938 100644
--- a/domain.te
+++ b/domain.te
@@ -86,6 +86,8 @@ allow domain sysfs:file rw_file_perms;
}
allow domain sysfs_writable:file rw_file_perms;
+allow domain sysfs_nfc_power_writable:file rw_file_perms;
+
# Read access to pseudo filesystems.
r_dir_file(domain, proc)
r_dir_file(domain, sysfs)
diff --git a/file.te b/file.te
index 11c3ef6..a530f2c 100644
--- a/file.te
+++ b/file.te
@@ -43,6 +43,7 @@ type systemkeys_data_file, file_type, data_file_type;
type wifi_data_file, file_type, data_file_type;
type radio_data_file, file_type, data_file_type;
type nfc_data_file, file_type, data_file_type;
+type sysfs_nfc_power_writable, fs_type, sysfs_type, mlstrustedobject;
# /data/data subdirectories - app sandboxes
type app_data_file, file_type, data_file_type;
# Default type for anything under /cache
diff --git a/file_contexts b/file_contexts
index 92c6bb0..b249004 100644
--- a/file_contexts
+++ b/file_contexts
@@ -19,6 +19,8 @@
/dev/block/loop[0-9]* u:object_r:loop_device:s0
/dev/block/ram[0-9]* u:object_r:ram_device:s0
/dev/block/mtdblock5 u:object_r:radio_device:s0
+/dev/cdma_.* u:object_r:radio_device:s0
+/dev/lte_.* u:object_r:radio_device:s0
/dev/cam u:object_r:camera_device:s0
/dev/console u:object_r:console_device:s0
/dev/cpuctl(/.*)? u:object_r:cpuctl_device:s0
@@ -35,6 +37,7 @@
/dev/mtd/mtd5ro u:object_r:radio_device:s0
/dev/mtp_usb u:object_r:mtp_device:s0
/dev/pn544 u:object_r:nfc_device:s0
+/dev/ttyO3 u:object_r:nfc_device:s0
/dev/ptmx u:object_r:ptmx_device:s0
/dev/pvrsrvkm u:object_r:powervr_device:s0
/dev/qemu_.* u:object_r:qemu_device:s0
@@ -66,7 +69,8 @@
/dev/socket/zygote u:object_r:zygote_socket:s0
/dev/spdif_out.* u:object_r:audio_device:s0
/dev/tegra.* u:object_r:video_device:s0
-/dev/tty[0-9]* u:object_r:tty_device:s0
+/dev/tty[0-2]* u:object_r:tty_device:s0
+/dev/tty[4-9]* u:object_r:tty_device:s0
/dev/ttyS[0-9]* u:object_r:serial_device:s0
/dev/uinput u:object_r:input_device:s0
/dev/urandom u:object_r:urandom_device:s0
@@ -116,10 +120,15 @@
/data/misc/wifi(/.*)? u:object_r:wifi_data_file:s0
# App sandboxes
/data/data/.* u:object_r:app_data_file:s0
+/data/data/com.android.providers.telephony/databases(/.*)?
u:object_r:radio_data_file:s0
+/data/data/com.android.providers.telephony/(optable\.db)?
u:object_r:radio_data_file:s0
+
#############################
# efs files
#
/efs(/.*)? u:object_r:efs_file:s0
+/data/radio/nv_data.bin.* u:object_r:radio_data_file:s0
+/factory/nv_data.bin.* u:object_r:radio_data_file:s0
#############################
# Cache files
#
@@ -128,3 +137,4 @@
# sysfs files
#
/sys/qemu_trace/process_name -- u:object_r:sysfs_writable:s0
+/sys/devices/platform/nfc-power/nfc_power --
u:object_r:sysfs_nfc_power_writable:s0
diff --git a/rild.te b/rild.te
index 2857892..9201d43 100644
--- a/rild.te
+++ b/rild.te
@@ -7,6 +7,7 @@ net_domain(rild)
allow rild kernel:system module_request;
unix_socket_connect(rild, property, init)
unix_socket_connect(rild, qemud, qemud)
+allow rild self:netlink_route_socket { setopt };
allow rild self:capability { setuid net_admin net_raw };
allow rild alarm_device:chr_file rw_file_perms;
allow rild cgroup:dir create_dir_perms;
--
On Fri, Mar 2, 2012 at 1:31 PM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> On Fri, 2012-03-02 at 11:51 -0600, Bryan Hinton wrote:
>> Here is the latest policy that I am using.
>
> Thanks for posting this. A few comments below.
>
>> diff --git a/device.te b/device.te
>> index 6424db6..08437a5 100644
>> --- a/device.te
>> +++ b/device.te
>> @@ -23,6 +23,7 @@ type log_device, dev_type, mlstrustedobject;
>> type mtd_device, dev_type;
>> type mtp_device, dev_type, mlstrustedobject;
>> type nfc_device, dev_type;
>> +type nfc_ctrl_device, dev_type;
>
> Types are intended to be used as equivalence classes, so if the same set
> of domains are going to be allowed to access nfc_ctrl_device with the
> same permissions as for nfc_device, then just reuse nfc_device rather
> than introducing a new type.
>>>Understood. I removed the nfc_ctrl_device type and reused nfc_device for /dev/ttyO3.
>
>> diff --git a/domain.te b/domain.te
>> index 55c9ecd..88fb261 100644
>> --- a/domain.te
>> +++ b/domain.te
>> @@ -85,6 +85,7 @@ if (in_qemu) {
>> allow domain sysfs:file rw_file_perms;
>> }
>> allow domain sysfs_writable:file rw_file_perms;
>> +allow domain sysfs_nfc_power_writable:file rw_file_perms;
>
> Likewise here, if you truly need to allow all domains rw access to this
> type, why not just reuse sysfs_writable? I was wondering though whether
> we truly should be allowing all domains such access.
>>>I'm also thinking that all domains should not have access.
>>>This domain is only used for the label on this file /sys/devices/platform/nfc-power/nfc_power as follows:
>>>u:object_r:sysfs_nfc_power_writable:s0
>>>I think this could use some improvement in terms of increased granularity. ...I'm thinking about how to go about it.
>
>> diff --git a/file.te b/file.te
>> index 11c3ef6..ec7a02e 100644
>> --- a/file.te
>> +++ b/file.te
>> @@ -8,6 +8,7 @@ type selinuxfs, fs_type;
>> type cgroup, fs_type, mlstrustedobject;
>> type sysfs, fs_type, mlstrustedobject;
>> type sysfs_writable, fs_type, sysfs_type, mlstrustedobject;
>> +type sysfs_nfc_power_writable, fs_type, sysfs_type, mlstrustedobject;
>
>>> Per above item.
>
>> @@ -43,6 +44,11 @@ type systemkeys_data_file, file_type, data_file_type;
>> type wifi_data_file, file_type, data_file_type;
>> type radio_data_file, file_type, data_file_type;
>> type nfc_data_file, file_type, data_file_type;
>> +
>> +type radio_nv_data_file, file_type, data_file_type;
>> +type efs_radio_nv_data_file, file_type, data_file_type;
>> +type radio_telephony_data_file, file_type, data_file_type;
>
> Do we need the distinction between radio_nv_data_file and
> efs_radio_nv_rdata_file? Where is that distinction used in the policy
> allow rules?
>>>I removed both and reused radio_data_file.
>
>> diff --git a/file_contexts b/file_contexts
>> index 92c6bb0..59bac40 100644
>> --- a/file_contexts
>> +++ b/file_contexts
>> @@ -19,6 +19,16 @@
>> /dev/block/loop[0-9]* u:object_r:loop_device:s0
>> /dev/block/ram[0-9]* u:object_r:ram_device:s0
>> /dev/block/mtdblock5 u:object_r:radio_device:s0
>> +# rild needs access to the cdma and lte device nodes
>> +/dev/cdma_ipc0 u:object_r:radio_device:s0
>> +/dev/cdma_rmnet5 u:object_r:radio_device:s0
>> +/dev/lte_ipc0 u:object_r:radio_device:s0
>> +/dev/lte_rmnet4 u:object_r:radio_device:s0
>> +/dev/lte_boot0 u:object_r:radio_device:s0
>> +/dev/lte_spi u:object_r:radio_device:s0
>> +/dev/ttyGS1 u:object_r:radio_device:s0
>> +/dev/lte_rfs0 u:object_r:radio_device:s0
>
> You can shorten this specification by using pathname regexes, e.g.
> /dev/cdma_.* u:object_r:radio_device:s0
> /dev/lte_.* u:object_r:radio_device:s0
> /dev/ttyGS[0-9] u:object_r:radio_device:s0
>>>Done.
>
>> @@ -68,6 +78,7 @@
>> /dev/tegra.* u:object_r:video_device:s0
>> /dev/tty[0-9]* u:object_r:tty_device:s0
>> /dev/ttyS[0-9]* u:object_r:serial_device:s0
>> +/dev/ttyO3 u:object_r:nfc_ctrl_device:s0
>> /dev/uinput u:object_r:input_device:s0
>> /dev/urandom u:object_r:urandom_device:s0
>> /dev/vcs[0-9a-z]* u:object_r:vcs_device:s0
>> @@ -116,10 +127,24 @@
>> /data/misc/wifi(/.*)? u:object_r:wifi_data_file:s0
>> # App sandboxes
>> /data/data/.* u:object_r:app_data_file:s0
>> +
>> +# rild needs access to the databases that the android telephony
>> provider manages
>> +/data/data/com.android.providers.telephony/databases(/.*)?
>> u:object_r:radio_telephony_data_file:s0
>> +/data/data/com.android.providers.telephony/optable.db
>> u:object_r:radio_telephony_data_file:s0
>> +/data/data/com.android.providers.telephony/databases/telephony.db
>> u:object_r:radio_telephony_data_file:s0
>> +/data/data/com.android.providers.telephony/databases/telephony.db-journal
>> u:object_r:radio_telephony_data_file:s0
>
> The first entry (with the (/.*)? suffix) should match the latter two
> entries already, making them unnecessary.
>>>Fixed per above item.
>
>> +# rild needs acess to radio image and associated md5 sum on userdata.img
>> +/data/radio/nv_data.bin u:object_r:radio_nv_data_file:s0
>> +/data/radio/nv_data.bin.md5 u:object_r:radio_nv_data_file:s0
>
> /data/radio/nv_data.bin.* would work here.
>
>> +
>> #############################
>> # efs files
>> #
>> /efs(/.*)? u:object_r:efs_file:s0
>> +# rild needs access to radio image and associated md5 sum on
>> /efs(/factory) ext4 image
>> +/factory/nv_data.bin u:object_r:efs_radio_nv_data_file:s0
>> +/factory/nv_data.bin.md5 u:object_r:efs_radio_nv_data_file:s0
>
> Likewise. Do we need the efs vs non-efs distinction?
>>>I removed both and reused radio_data_file.
>
>> diff --git a/seapp_contexts b/seapp_contexts
>> index c301792..52bbfa2 100644
>> --- a/seapp_contexts
>> +++ b/seapp_contexts
>> @@ -32,6 +32,9 @@ isSystemServer=true domain=system
>> user=system domain=system_app type=system_data_file
>> user=nfc domain=nfc type=nfc_data_file
>> user=radio domain=radio type=radio_data_file
>> +user=radio domain=radio type=radio_telephony_data_file
>> +user=radio domain=radio type=radio_nv_data_file
>> +user=radio domain=radio type=efs_radio_nv_data_file
>> user=app_* domain=untrusted_app type=app_data_file levelFromUid=true
>> user=app_* seinfo=systemApp domain=trusted_app levelFromUid=true
>> user=app_* seinfo=systemApp name=com.android.browser
>> domain=browser_app levelFromUid=true
>
> You can only have one type= specification per user=, and this is only
> used by installd to label the /data/data/<packagename> directory and
> files when they are created. So the additional entries here should be
> no-ops effectively. We should likely add a validator program for
> seapp_contexts to check for these kinds of errors.
>>>Removed additional types per above comment, and reused radio_data_file.
Bryan Hinton
>
> --
> Stephen Smalley
> National Security Agency
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
