[refpolicy] [PATCH 1/2]: cpucontrol module updates (stricter policy for CPU microcode updates)

Guido Trentalancia guido at trentalancia.com
Sat Sep 8 05:27:53 CDT 2012


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 <guido at trentalancia.com>
>> ---
>>    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
>>    #
>>
>> +## <desc>
>> +## <p>
>> +## Allow cpucontrol to upload new microcode
>> +## to the CPU.
>> +## </p>
>> +## </desc>
>> +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
  #

+## <desc>
+## <p>
+## Allow cpucontrol to upload new microcode
+## to the CPU.
+## </p>
+## </desc>
+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',`
  ########################################
  ## <summary>
  ##	Get the attributes of the CPU
-##	microcode and id interfaces.
+##	id interfaces.
  ## </summary>
  ## <param name="domain">
  ##	<summary>
@@ -1682,8 +1682,27 @@ interface(`dev_getattr_cpu_dev',`

  ########################################
  ## <summary>
+##	Get the attributes of the CPU
+##	microcode interface.
+## </summary>
+## <param name="domain">
+##	<summary>
+##	Domain allowed access.
+##	</summary>
+## </param>
+#
+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)
+')
+
+########################################
+## <summary>
  ##	Set the attributes of the CPU
-##	microcode and id interfaces.
+##	id interfaces.
  ## </summary>
  ## <param name="domain">
  ##	<summary>
@@ -1701,6 +1720,25 @@ interface(`dev_setattr_cpu_dev',`

  ########################################
  ## <summary>
+##	Set the attributes of the CPU
+##	id interfaces.
+## </summary>
+## <param name="domain">
+##	<summary>
+##	Domain allowed access.
+##	</summary>
+## </param>
+#
+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)
+')
+
+########################################
+## <summary>
  ##	Read the CPU identity.
  ## </summary>
  ## <param name="domain">
@@ -1719,21 +1757,31 @@ interface(`dev_read_cpuid',`

  ########################################
  ## <summary>
-##	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.
  ## </summary>
+## <desc>
+##      <p>
+##	Write the CPU microcode device.
+##	This interface can be used to upload the
+##	CPU microcode.
+##      </p>
+##      <p>
+##	There might be license modifications.
+##	</p>
+## </desc>
  ## <param name="domain">
  ##	<summary>
  ##	Domain allowed access.
  ##	</summary>
  ## </param>
  #
-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




More information about the refpolicy mailing list