From: guido@trentalancia.com (Guido Trentalancia) Date: Sat, 08 Sep 2012 12:27:53 +0200 Subject: [refpolicy] [PATCH 1/2]: cpucontrol module updates (stricter policy for CPU microcode updates) In-Reply-To: <503E136A.5070102@tresys.com> References: <5037897A.5050305@trentalancia.com> <503E136A.5070102@tresys.com> Message-ID: <504B1DA9.9010601@trentalancia.com> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com Hello. Here is a revised version (1/1) of the patch for the cpucontrol module. On 29/08/2012 15:04, Christopher J. PeBenito wrote: > On 08/24/12 10:02, Guido Trentalancia wrote: >> cpucontrol module updates: >> - introduce the file contexts according to the standard location >> of the most common application named microcode_ctl >> (http://www.urbanmyth.org/microcode); Dropped as risky. Can be tackled at a later time or by users/distributions individually. >> - add file contexts for CPUs from two different vendors, taking >> into consideration specific customization of the location >> for one distribution; Not taking anymore into consideration binary location customizations as too risky. Only adding generic support for the most likely location of the application provided by the other manufacturer. >> - add file contexts and declarations for the init script; >> - introduce the ability to update the CPU microcode as >> tunable policy, as apparently such operation might >> modify the original licensing conditions (under some or >> all circumstances: "personal, non-commercial use only"); >> - create a new device type specifically for the CPU microcode >> updating functionality and modify the cpucontrol module so >> that such distinct new type is used for the microcode >> updating operation, thus leaving an open door for further >> modifications that distinguish different CPU-related >> applications/utilities to create further isolation; >> - modify the interface definitions so that the CPU microcode >> update utility can only write and not read (unneeded) the >> corresponding above mentioned device type. >> >> Signed-off-by: Guido Trentalancia >> --- >> policy/modules/contrib/cpucontrol.fc | 18 +++++++++- >> policy/modules/contrib/cpucontrol.te | 16 ++++++++- >> policy/modules/kernel/devices.fc | 3 + >> policy/modules/kernel/devices.if | 62 >> +++++++++++++++++++++++++++++++---- >> policy/modules/kernel/devices.te | 8 +++- >> 5 files changed, 96 insertions(+), 11 deletions(-) >> >> diff -pruN refpolicy-08082012/policy/modules/contrib/cpucontrol.fc >> refpolicy-08082012-microcode/policy/modules/contrib/cpucontrol.fc >> --- refpolicy-08082012/policy/modules/contrib/cpucontrol.fc 2011-09-09 >> 18:29:23.563610858 +0200 >> +++ refpolicy-08082012-microcode/policy/modules/contrib/cpucontrol.fc >> 2012-08-09 01:13:59.615119168 +0200 >> @@ -1,7 +1,23 @@ >> +/etc/microcode\.dat -- gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/etc/microcode_amd\.bin -- >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/etc/firmware/microcode\.dat -- >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/etc/firmware/microcode_amd.bin -- >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> >> -/etc/firmware/.* -- gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/etc/rc\.d/init\.d/microcode -- >> gen_context(system_u:object_r:cpucontrol_initrc_exec_t,s0) >> >> +ifdef(`distro_redhat',` >> +/lib/firmware/intel-ucode(/.*)? >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/lib/firmware/amd-ucode(/.*)? >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/lib/firmware/microcode\.dat -- >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +/lib/firmware/microcode_amd\.bat -- >> gen_context(system_u:object_r:cpucontrol_conf_t,s0) >> +') > > I'm conflicted. I assume that the /etc/microcode and /etc/firmware locations are the default for this app, /lib/firmware seems more appropriate. I suspect this is not a redhat-specific location. > >> +/usr/sbin/microcode_ctl -- >> gen_context(system_u:object_r:cpucontrol_exec_t,s0) >> +/usr/local/sbin/microcode_ctl -- >> gen_context(system_u:object_r:cpucontrol_exec_t,s0) > > The /usr/local label is not necessary, as it will be handled by the file context path substitutions. > >> +ifdef(`distro_redhat',` >> /sbin/microcode_ctl -- gen_context(system_u:object_r:cpucontrol_exec_t,s0) >> +') > > I also don't think this change is really necessary. > >> /usr/sbin/cpufreqd -- gen_context(system_u:object_r:cpuspeed_exec_t,s0) >> /usr/sbin/cpuspeed -- gen_context(system_u:object_r:cpuspeed_exec_t,s0) >> diff -pruN refpolicy-08082012/policy/modules/contrib/cpucontrol.te >> refpolicy-08082012-microcode/policy/modules/contrib/cpucontrol.te >> --- refpolicy-08082012/policy/modules/contrib/cpucontrol.te 2011-09-09 >> 18:29:23.563610858 +0200 >> +++ refpolicy-08082012-microcode/policy/modules/contrib/cpucontrol.te >> 2012-08-09 00:23:07.079532236 +0200 >> @@ -5,10 +5,21 @@ policy_module(cpucontrol, 1.3.0) >> # Declarations >> # >> >> +## >> +##

>> +## Allow cpucontrol to upload new microcode >> +## to the CPU. >> +##

>> +## >> +gen_tunable(cpucontrol_can_upload_cpu_microcode, false) >> + >> type cpucontrol_t; >> type cpucontrol_exec_t; >> init_system_domain(cpucontrol_t, cpucontrol_exec_t) >> >> +type cpucontrol_initrc_exec_t; >> +init_script_file(cpucontrol_initrc_exec_t) >> + >> type cpucontrol_conf_t; >> files_type(cpucontrol_conf_t) >> >> @@ -37,7 +48,6 @@ kernel_read_proc_symlinks(cpucontrol_t) >> kernel_read_kernel_sysctls(cpucontrol_t) >> >> dev_read_sysfs(cpucontrol_t) >> -dev_rw_cpu_microcode(cpucontrol_t) >> >> fs_search_auto_mountpoints(cpucontrol_t) >> >> @@ -54,6 +64,10 @@ logging_send_syslog_msg(cpucontrol_t) >> >> userdom_dontaudit_use_unpriv_user_fds(cpucontrol_t) >> >> +tunable_policy(`cpucontrol_can_upload_cpu_microcode',` >> + dev_write_cpu_microcode(cpucontrol_t) >> +') > > I don't understand why this is conditional, especially since you removed the read permission on /dev/microcode. The point of the app is to load microcode. diff -pruN refpolicy-09082012-git-master/policy/modules/contrib/cpucontrol.fc refpolicy-09082012-microcode-v3/policy/modules/contrib/cpucontrol.fc --- refpolicy-09082012-git-master/policy/modules/contrib/cpucontrol.fc Thu Aug 23 19:22:59 2012 +++ refpolicy-09082012-microcode-v3/policy/modules/contrib/cpucontrol.fc Sat Sep 8 10:16:21 2012 @@ -1,7 +1,12 @@ - /etc/firmware/.* -- gen_context(system_u:object_r:cpucontrol_conf_t,s0) +/etc/rc\.d/init\.d/microcode -- gen_context(system_u:object_r:cpucontrol_conf_t,s0) + +# Intel(R) devices supporting application /sbin/microcode_ctl -- gen_context(system_u:object_r:cpucontrol_exec_t,s0) + +# AMD(tm) devices application +/sbin/ucodeadm -- gen_context(system_u:object_r:cpucontrol_exec_t,s0) /usr/sbin/cpufreqd -- gen_context(system_u:object_r:cpuspeed_exec_t,s0) /usr/sbin/cpuspeed -- gen_context(system_u:object_r:cpuspeed_exec_t,s0) diff -pruN refpolicy-09082012-git-master/policy/modules/contrib/cpucontrol.te refpolicy-09082012-microcode-v3/policy/modules/contrib/cpucontrol.te --- refpolicy-09082012-git-master/policy/modules/contrib/cpucontrol.te Thu Aug 23 19:22:59 2012 +++ refpolicy-09082012-microcode-v3/policy/modules/contrib/cpucontrol.te Sat Sep 8 10:10:55 2012 @@ -5,10 +5,21 @@ policy_module(cpucontrol, 1.3.0) # Declarations # +## +##

+## Allow cpucontrol to upload new microcode +## to the CPU. +##

+## +gen_tunable(cpucontrol_can_upload_cpu_microcode, false) + type cpucontrol_t; type cpucontrol_exec_t; init_system_domain(cpucontrol_t, cpucontrol_exec_t) +type cpucontrol_initrc_exec_t; +init_script_file(cpucontrol_initrc_exec_t) + type cpucontrol_conf_t; files_type(cpucontrol_conf_t) @@ -37,7 +48,6 @@ kernel_read_proc_symlinks(cpucontrol_t) kernel_read_kernel_sysctls(cpucontrol_t) dev_read_sysfs(cpucontrol_t) -dev_rw_cpu_microcode(cpucontrol_t) fs_search_auto_mountpoints(cpucontrol_t) @@ -53,6 +63,10 @@ init_use_script_ptys(cpucontrol_t) logging_send_syslog_msg(cpucontrol_t) userdom_dontaudit_use_unpriv_user_fds(cpucontrol_t) + +tunable_policy(`cpucontrol_can_upload_cpu_microcode',` + dev_write_cpu_microcode(cpucontrol_t) +') optional_policy(` nscd_socket_use(cpucontrol_t) diff -pruN refpolicy-09082012-git-master/policy/modules/kernel/devices.fc refpolicy-09082012-microcode-v3/policy/modules/kernel/devices.fc --- refpolicy-09082012-git-master/policy/modules/kernel/devices.fc Thu Sep 6 10:50:18 2012 +++ refpolicy-09082012-microcode-v3/policy/modules/kernel/devices.fc Sat Sep 8 10:10:55 2012 @@ -65,7 +65,7 @@ /dev/mergemem -c gen_context(system_u:object_r:memory_device_t,mls_systemhigh) /dev/mga_vid.* -c gen_context(system_u:object_r:xserver_misc_device_t,s0) /dev/mice -c gen_context(system_u:object_r:mouse_device_t,s0) -/dev/microcode -c gen_context(system_u:object_r:cpu_device_t,s0) +/dev/microcode -c gen_context(system_u:object_r:cpu_microcode_device_t,s0) /dev/midi.* -c gen_context(system_u:object_r:sound_device_t,s0) /dev/misc/dlm.* -c gen_context(system_u:object_r:dlm_control_device_t,s0) /dev/mixer.* -c gen_context(system_u:object_r:sound_device_t,s0) @@ -139,6 +139,7 @@ ifdef(`distro_suse', ` /dev/cpu_dma_latency -c gen_context(system_u:object_r:netcontrol_device_t,s0) /dev/cpu.* -c gen_context(system_u:object_r:cpu_device_t,s0) +/dev/cpu/microcode -c gen_context(system_u:object_r:cpu_microcode_device_t,s0) /dev/cpu/mtrr -c gen_context(system_u:object_r:mtrr_device_t,s0) /dev/biometric/sensor.* -c gen_context(system_u:object_r:event_device_t,s0) diff -pruN refpolicy-09082012-git-master/policy/modules/kernel/devices.if refpolicy-09082012-microcode-v3/policy/modules/kernel/devices.if --- refpolicy-09082012-git-master/policy/modules/kernel/devices.if Thu Aug 23 19:16:55 2012 +++ refpolicy-09082012-microcode-v3/policy/modules/kernel/devices.if Sat Sep 8 10:10:55 2012 @@ -1664,7 +1664,7 @@ interface(`dev_filetrans_cardmgr',` ######################################## ## ## Get the attributes of the CPU -## microcode and id interfaces. +## id interfaces. ## ## ## @@ -1682,8 +1682,27 @@ interface(`dev_getattr_cpu_dev',` ######################################## ## +## Get the attributes of the CPU +## microcode interface. +## +## +## +## Domain allowed access. +## +## +# +interface(`dev_getattr_cpu_microcode_dev',` + gen_require(` + type device_t, cpu_microcode_device_t; + ') + + getattr_chr_files_pattern($1, device_t, cpu_microcode_device_t) +') + +######################################## +## ## Set the attributes of the CPU -## microcode and id interfaces. +## id interfaces. ## ## ## @@ -1701,6 +1720,25 @@ interface(`dev_setattr_cpu_dev',` ######################################## ## +## Set the attributes of the CPU +## id interfaces. +## +## +## +## Domain allowed access. +## +## +# +interface(`dev_setattr_cpu_microcode_dev',` + gen_require(` + type device_t, cpu_microcode_device_t; + ') + + setattr_chr_files_pattern($1, device_t, cpu_microcode_device_t) +') + +######################################## +## ## Read the CPU identity. ## ## @@ -1719,21 +1757,31 @@ interface(`dev_read_cpuid',` ######################################## ## -## Read and write the the CPU microcode device. This -## is required to load CPU microcode. +## Write the CPU microcode device. This +## is required to upload the CPU microcode. ## +## +##

+## Write the CPU microcode device. +## This interface can be used to upload the +## CPU microcode. +##

+##

+## There might be license modifications. +##

+## ## ## ## Domain allowed access. ## ## # -interface(`dev_rw_cpu_microcode',` +interface(`dev_write_cpu_microcode',` gen_require(` - type device_t, cpu_device_t; + type device_t, cpu_microcode_device_t; ') - rw_chr_files_pattern($1, device_t, cpu_device_t) + write_chr_files_pattern($1, device_t, cpu_microcode_device_t) ') ######################################## diff -pruN refpolicy-09082012-git-master/policy/modules/kernel/devices.te refpolicy-09082012-microcode-v3/policy/modules/kernel/devices.te --- refpolicy-09082012-git-master/policy/modules/kernel/devices.te Thu Sep 6 10:50:18 2012 +++ refpolicy-09082012-microcode-v3/policy/modules/kernel/devices.te Sat Sep 8 10:10:55 2012 @@ -51,10 +51,16 @@ type clock_device_t; dev_node(clock_device_t) # -# cpu control devices /dev/cpu/0/* +# cpu control devices /dev/cpu/?/* # type cpu_device_t; dev_node(cpu_device_t) + +# +# cpu microcode device /dev/cpu/microcode +# +type cpu_microcode_device_t; +dev_node(cpu_microcode_device_t) # # Type for /dev/crash