2012-08-06 15:19:38

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v2]: mcelog module initial rewrite

Hello Dominick.

>On Mon, 2012-08-06 at 14:45 +0200, Guido Trentalancia wrote:

[cut]

>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09 18:29:23.578610955 +0200
>> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc 2012-08-06 13:34:45.568049105 +0200
>> @@ -1 +1,16 @@
>> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +/etc/mcelog/.*.local -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +
>
>I dont have a good argument for using bin_t but i dont have a good
>argument for using mcelog_exec_t either , therefore i suggest keeping it
>bin_t

If we keep bin_t, then we need to use corecmd_exec_bin() or whatever that is called, which means it can execute any script and in particular any binary.

So, the good reason is restricting the type of files that mcelog can execute. In my opinion policy should always been designed that way, when the application needs to execute internal (or user-defined) scripts or binaries (as opposed to system-wide executables in /bin, /sbin, /usr/bin or /usr/sbin).

Unless you give me a good reason, I won't change that.

>> +ifdef(`distro_redhat',`
>> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +')
>> +
>> +/etc/rc.d/init.d/mcelog -- gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
>> +
>
>Weird, i still dont see the periods escaped above.

Unfortunately, at the moment I do not have another mail client at hand, so I suppose, it needs manual editing.

>> /usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +
>> +/var/log/mcelog -- gen_context(system_u:object_r:mcelog_log_t,s0)
>> +/var/run/mcelog.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> +/var/run/mcelog-client -s gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.if refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.if
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.if 2011-09-09 18:29:23.578610955 +0200
>> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.if 2012-08-06 15:37:21.714522005 +0200
>> @@ -18,3 +18,78 @@ interface(`mcelog_domtrans',`
>> domtrans_pattern($1, mcelog_exec_t, mcelog_t)
>> ')
>>
>> +########################################
>> +## <summary>
>> +## Read the mcelog configuration files.
>
>I would probably use "Read mcelog configuration files."

Ok. I will create a third version (v3) with further changes as necessary.

>> +## </summary>
>> +## <param name="domain">
>> +## <summary>
>> +## Domain allowed access.
>> +## </summary>
>> +## </param>
>> +## <rolecap/>
>> +#
>> +interface(`mcelog_read_config',`
>
>I would probably call the interface "mcelog_read_config_files" or
>"mcelog_read_etc_files"

Yes, reasonable.

>> + gen_require(`
>> + type mcelog_etc_t;
>> + ')
>> +
>> + files_search_etc($1)
>> + read_files_pattern($1, mcelog_etc_t, mcelog_etc_t)
>> + allow $1 mcelog_etc_t:dir list_dir_perms;
>> +')
>> +
>> +########################################
>> +## <summary>
>> +## Create an mcelog unix stream socket.
>> +## </summary>
>> +## <param name="domain">
>> +## <summary>
>> +## Domain allowed access.
>> +## </summary>
>> +## </param>
>> +#
>> +interface(`mcelog_stream_socket_create',`
>> + gen_require(`
>> + type mcelog_t;
>> + ')
>> +
>> + allow $1 mcelog_t:unix_stream_socket create_socket_perms;
>> +')
>
>Not needed.

You suggested (initial review):

allow mcelog_t self:unix_stream_socket create_socket_perms;

I have just turned that into an interface...

So, I don't get the point now.

>> +########################################
>> +## <summary>
>> +## Read from an mcelog unix stream socket.
>> +## </summary>
>> +## <param name="domain">
>> +## <summary>
>> +## Domain allowed access.
>> +## </summary>
>> +## </param>
>> +#
>> +interface(`mcelog_stream_socket_read',`
>> + gen_require(`
>> + type mcelog_t, mcelog_var_run_t;
>> + ')
>> +
>> + allow $1 mcelog_var_run_t:unix_stream_socket { read };
>> +')
>
>not needed.

I think it gets audited as denied otherwise (possibly the client mode). If time allows, I'll double-check.

>> +########################################
>> +## <summary>
>> +## Connect to mcelog over an unix stream socket.
>> +## </summary>
>> +## <param name="domain">
>> +## <summary>
>> +## Domain allowed access.
>> +## </summary>
>> +## </param>
>> +#
>> +interface(`mcelog_stream_socket_connect',`
>> + gen_require(`
>> + type mcelog_t, mcelog_var_run_t;
>> + ')
>> +
>> + files_search_pids($1)
>> + stream_connect_pattern($1, mcelog_var_run_t, mcelog_var_run_t, mcelog_t);
>> +')
>
>Not needed.

In client-mode it needs to connect to the socket.

Has the feature been removed in subsequent versions ? I can't find it anymore on kernel.org...

>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09 18:29:23.578610955 +0200
>> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te 2012-08-06 16:01:45.087746478 +0200
>> @@ -1,4 +1,4 @@
>> -policy_module(mcelog, 1.1.0)
>> +policy_module(mcelog, 1.1.1)
>>
>> ########################################
>> #
>> @@ -7,8 +7,20 @@ policy_module(mcelog, 1.1.0)
>>
>> type mcelog_t;
>> type mcelog_exec_t;
>> -application_domain(mcelog_t, mcelog_exec_t)
>> -cron_system_entry(mcelog_t, mcelog_exec_t)
>> +corecmd_executable_file(mcelog_exec_t);
>
>I still dont think corecmd_executable_file is needed. Not sure though.

The triggers are scripts. Their execution is triggered by CPU error events.

If such scripts are labelled as bin_t, all is needed is corecmd_exec_bin() or whatever it is called.

Otherwise, if they are labelled differently for increased security as explained above, it should need both corecmd_executable_file() and can_exec() on the private executable type.

>> +init_daemon_domain(mcelog_t, mcelog_exec_t)
>> +
>> +type mcelog_initrc_exec_t;
>> +init_script_file(mcelog_initrc_exec_t)
>> +
>> +type mcelog_etc_t;
>> +files_config_file(mcelog_etc_t)
>> +
>> +type mcelog_log_t;
>> +logging_log_file(mcelog_log_t)
>> +
>> +type mcelog_var_run_t;
>> +files_pid_file(mcelog_var_run_t)
>>
>> ########################################
>> #
>> @@ -17,16 +29,48 @@ cron_system_entry(mcelog_t, mcelog_exec_
>>
>> allow mcelog_t self:capability sys_admin;
>
>add:
>
>allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
>
>>
>> +can_exec(mcelog_t, mcelog_exec_t)
>> +
>
>If we keep using bin_t then this isnt needed

To be honest I would rather prefer not using bin_t. Perhaps, it needs to be able to transition from the private exec type ?

>> kernel_read_system_state(mcelog_t)
>>
>> dev_read_raw_memory(mcelog_t)
>> dev_read_kmsg(mcelog_t)
>>
>> +dev_rw_sysfs(mcelog_t)
>> +
>> +# optional support for running it as a cron job
>> +optional_policy(`
>> + cron_system_entry(mcelog_t, mcelog_exec_t)
>> +')
>> +
>> +manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })
>> +
>> +# needed in daemon mode
>> files_read_etc_files(mcelog_t)
>>
>> -# for /dev/mem access
>> -mls_file_read_all_levels(mcelog_t)
>> +locallogin_use_fds(mcelog_t)
>> +
>> +# create/append a logfile in a private log directory
>> +create_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +append_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +setattr_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +logging_log_filetrans(mcelog_t, mcelog_log_t, file)
>>
>> +# use syslog functionality (optional, configurable)
>> logging_send_syslog_msg(mcelog_t)
>>
>> +# to read the standard configuration file
>> +mcelog_read_config(mcelog_t)
>
>use read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>
>Interfaces are not for internal use

As far as I know other modules are using interfaces internally. I will double-check and if neccessary remove them.

>> +
>> +mcelog_stream_socket_create(mcelog_t)
>> +mcelog_stream_socket_read(mcelog_t)
>> +mcelog_stream_socket_connect(mcelog_t)
>> +
>
>none of the above are needed
>
>> miscfiles_read_localization(mcelog_t)
>> +
>> +# for /dev/mem access
>> +mls_file_read_all_levels(mcelog_t)
>> +
>> +term_use_all_ttys(mcelog_t)

term_use_all_ttys() is needed for interactive use. Do you know anything more restrictive than that ?

>> diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
>> --- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-05 04:52:17.194005067 +0200
>> +++ refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc 2012-08-05 17:49:05.594838788 +0200
>> @@ -72,12 +72,6 @@ ifdef(`distro_redhat',`
>> /etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)
>>
>> /etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
>> -/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
>> -/etc/mcelog/.*.local -- gen_context(system_u:object_r:bin_t,s0)
>> -
>> -ifdef(`distro_redhat',`
>> -/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
>> -')
>>
>> /etc/mgetty+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)

Regards,

Guido


2012-08-06 15:30:46

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v2]: mcelog module initial rewrite

On Mon, 2012-08-06 at 17:19 +0200, Guido Trentalancia wrote:
> If we keep bin_t, then we need to use corecmd_exec_bin() or whatever that is called, which means it can execute any script and in particular any binary.
>
> So, the good reason is restricting the type of files that mcelog can execute. In my opinion policy should always been designed that way, when the application needs to execute internal (or user-defined) scripts or binaries (as opposed to system-wide executables in /bin, /sbin, /usr/bin or /usr/sbin).
>
> Unless you give me a good reason, I won't change that.

The only good reason i can come up with right now is that if you change
this the maintainer might not accept the patch.

> Ok. I will create a third version (v3) with further changes as necessary.

This interface is not needed at all.

> >
> >Not needed.
>
> You suggested (initial review):
>
> allow mcelog_t self:unix_stream_socket create_socket_perms;
>
> I have just turned that into an interface...
>
> So, I don't get the point now.

You should not create an interface for that. Just add it to the
mcelog.te file (but use create_stream_socket_perms instead, my mistake)

> I think it gets audited as denied otherwise (possibly the client mode). If time allows, I'll double-check.

If you add: allow mcelog_t self:unix_stream_socket
create_stream_socket_perms; to mcelog.te this will be allowed i think

> In client-mode it needs to connect to the socket.
>
> Has the feature been removed in subsequent versions ? I can't find it anymore on kernel.org...

That's not my point. This is already allowed.

mcelog_t already has full access to mcelog_var_run_t sockets and if you
add allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
to mcelog.te then mcelog_t will also be allowed to connect via unix
stream socket.

> Otherwise, if they are labelled differently for increased security as explained above, it should need both corecmd_executable_file() and can_exec() on the private executable type.

There is not much increased security in my view.

> >
>
> To be honest I would rather prefer not using bin_t. Perhaps, it needs to be able to transition from the private exec type ?

Assumption is bad. Until proven otherwise bin_t seems fine.

>
> As far as I know other modules are using interfaces internally. I will double-check and if neccessary remove them.

some modules are using local templates, i guess this is a exception to
the rule. Calling internal interfaces should not be done.

> term_use_all_ttys() is needed for interactive use. Do you know anything more restrictive than that ?

I would need to see the avc denial to make a judgement

2012-08-06 18:43:31

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v3]: mcelog module initial rewrite

On 06/08/2012 17:30, Dominick Grift wrote:
> On Mon, 2012-08-06 at 17:19 +0200, Guido Trentalancia wrote:
>> If we keep bin_t, then we need to use corecmd_exec_bin() or whatever that is called, which means it can execute any script and in particular any binary.
>>
>> So, the good reason is restricting the type of files that mcelog can execute. In my opinion policy should always been designed that way, when the application needs to execute internal (or user-defined) scripts or binaries (as opposed to system-wide executables in /bin, /sbin, /usr/bin or /usr/sbin).
>>
>> Unless you give me a good reason, I won't change that.

A good reason is that it won't work properly as it was and this is
because the triggers are scripts which in turn execute generic bin_t.

So, I have changed the executable file contexts as generic bin_t.

> The only good reason i can come up with right now is that if you change
> this the maintainer might not accept the patch.
>
>> Ok. I will create a third version (v3) with further changes as necessary.
>
> This interface is not needed at all.

I have then removed all internal interfaces and also added other
stylistic corrections as you suggested.

However, I have not removed term_use_all_ttys completely because
otherwise user might think the application is broken, while it is not
(consider --help and a foreground mode are all referenced in the manual
page).
Instead I have created tunable policy to disable use of ttys in a
paranoid setting (default is applcation is allowed to use ttys).

Consider, it wasn't working at all in current policy, that's why I have
rushed a little bit more with an initial version.

Here is the latest version (v3):

Rewrite of mcelog module:
- version increment
- fix and extend file contexts (private types)
- support daemon mode and init scripting (+ deprecated and untested cron
mode)
- support triggers for all distributions, while leaving
compatibility with their alternate location in Fedora (and
current policy)
- initial support for client mode (untested)
- support for sysfs (rw)
- includes several revisions from Dominick Grift

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/mcelog.fc | 15 +++++++
policy/modules/contrib/mcelog.te | 67
+++++++++++++++++++++++++++++++---
policy/modules/kernel/corecommands.fc | 6 ---
3 files changed, 77 insertions(+), 11 deletions(-)

diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
--- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
18:29:23.578610955 +0200
+++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
2012-08-06 21:11:19.617661468 +0200
@@ -1 +1,16 @@
+/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
+/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
+/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
+
+ifdef(`distro_redhat',`
+/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
+/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
+')
+
+/etc/rc\.d/init\.d/mcelog --
gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
+
/usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
+
+/var/log/mcelog -- gen_context(system_u:object_r:mcelog_log_t,s0)
+/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
+/var/run/mcelog-client -s
gen_context(system_u:object_r:mcelog_var_run_t,s0)
diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
--- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
18:29:23.578610955 +0200
+++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
2012-08-06 22:18:27.551975687 +0200
@@ -1,14 +1,34 @@
-policy_module(mcelog, 1.1.0)
+policy_module(mcelog, 1.1.1)

########################################
#
# Declarations
#

+## <desc>
+## <p>
+## Allow mcelog to use all the ttys.
+## Required in foreground mode and to
+## print out usage and version information.
+## </p>
+## </desc>
+gen_tunable(mcelog_foreground, true)
+
type mcelog_t;
type mcelog_exec_t;
-application_domain(mcelog_t, mcelog_exec_t)
-cron_system_entry(mcelog_t, mcelog_exec_t)
+init_daemon_domain(mcelog_t, mcelog_exec_t)
+
+type mcelog_initrc_exec_t;
+init_script_file(mcelog_initrc_exec_t)
+
+type mcelog_etc_t;
+files_config_file(mcelog_etc_t)
+
+type mcelog_log_t;
+logging_log_file(mcelog_log_t)
+
+type mcelog_var_run_t;
+files_pid_file(mcelog_var_run_t)

########################################
#
@@ -17,16 +37,53 @@ cron_system_entry(mcelog_t, mcelog_exec_

allow mcelog_t self:capability sys_admin;

+allow mcelog_t mcelog_etc_t:dir list_dir_perms;
+
+allow mcelog_t mcelog_t:unix_stream_socket create_socket_perms;
+
kernel_read_system_state(mcelog_t)

+corecmd_exec_bin(mcelog_t)
+
dev_read_raw_memory(mcelog_t)
dev_read_kmsg(mcelog_t)
+dev_rw_sysfs(mcelog_t)
+
+manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
+manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
+files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })

files_read_etc_files(mcelog_t)
+files_search_etc(mcelog_t)
+files_search_pids(mcelog_t)
+read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
+
+locallogin_use_fds(mcelog_t)
+
+# manage a logfile in a generic or private log directory
+manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
+manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
+logging_log_filetrans(mcelog_t, mcelog_log_t, file)
+
+# use syslog functionality (optional, configurable)
+logging_send_syslog_msg(mcelog_t)
+
+miscfiles_read_localization(mcelog_t)

# for /dev/mem access
mls_file_read_all_levels(mcelog_t)

-logging_send_syslog_msg(mcelog_t)
+stream_connect_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t,
mcelog_t)

-miscfiles_read_localization(mcelog_t)
+term_dontaudit_use_all_ptys(mcelog_t)
+term_dontaudit_use_all_ttys(mcelog_t)
+
+tunable_policy(`mcelog_foreground',`
+term_use_all_ttys(mcelog_t)
+term_use_all_ptys(mcelog_t)
+')
+
+# optional support for running it as a cron job
+optional_policy(`
+ cron_system_entry(mcelog_t, mcelog_exec_t)
+')
Binary files refpolicy-04062012/policy/modules/contrib/.xen.te.swp and
refpolicy-04062012-mcelog-support/policy/modules/contrib/.xen.te.swp differ
diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
--- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-05
04:52:17.194005067 +0200
+++
refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
2012-08-05 17:49:05.594838788 +0200
@@ -72,12 +72,6 @@ ifdef(`distro_redhat',`
/etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)

/etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
-/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
-/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
-
-ifdef(`distro_redhat',`
-/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
-')

/etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)

2012-08-06 19:44:11

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v3]: mcelog module initial rewrite



On Mon, 2012-08-06 at 20:43 +0200, Guido Trentalancia wrote:

>
> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
> policy/modules/contrib/mcelog.fc | 15 +++++++
> policy/modules/contrib/mcelog.te | 67
> +++++++++++++++++++++++++++++++---
> policy/modules/kernel/corecommands.fc | 6 ---
> 3 files changed, 77 insertions(+), 11 deletions(-)
>
> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
> refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
> 18:29:23.578610955 +0200
> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
> 2012-08-06 21:11:19.617661468 +0200
> @@ -1 +1,16 @@
> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
> +/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
> +/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
> +
> +ifdef(`distro_redhat',`
> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
> +/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
The context specs with bin_t do not belong in this module. they should
be moved to corecommands.fc (i believe)

> +')
> +
> +/etc/rc\.d/init\.d/mcelog --
> gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
> +
> /usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
> +
> +/var/log/mcelog -- gen_context(system_u:object_r:mcelog_log_t,s0)

I would use "/var/log/mcelog.*" for logrotate support. logrotate
sometimes append datestamps to rotated logs and we still want them to
keep the right label

> +/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
> +/var/run/mcelog-client -s
> gen_context(system_u:object_r:mcelog_var_run_t,s0)
> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
> refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
> --- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
> 18:29:23.578610955 +0200
> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
> 2012-08-06 22:18:27.551975687 +0200
> @@ -1,14 +1,34 @@
> -policy_module(mcelog, 1.1.0)
> +policy_module(mcelog, 1.1.1)
>
> ########################################
> #
> # Declarations
> #
>
> +## <desc>
> +## <p>
> +## Allow mcelog to use all the ttys.
> +## Required in foreground mode and to
> +## print out usage and version information.
> +## </p>
> +## </desc>
> +gen_tunable(mcelog_foreground, true)

No need for a boolean for this imho

> type mcelog_t;
> type mcelog_exec_t;
> -application_domain(mcelog_t, mcelog_exec_t)
> -cron_system_entry(mcelog_t, mcelog_exec_t)
> +init_daemon_domain(mcelog_t, mcelog_exec_t)
> +
> +type mcelog_initrc_exec_t;
> +init_script_file(mcelog_initrc_exec_t)
> +
> +type mcelog_etc_t;
> +files_config_file(mcelog_etc_t)
> +
> +type mcelog_log_t;
> +logging_log_file(mcelog_log_t)
> +
> +type mcelog_var_run_t;
> +files_pid_file(mcelog_var_run_t)
>
> ########################################
> #
> @@ -17,16 +37,53 @@ cron_system_entry(mcelog_t, mcelog_exec_
>
> allow mcelog_t self:capability sys_admin;
>
> +allow mcelog_t mcelog_etc_t:dir list_dir_perms;
> +
> +allow mcelog_t mcelog_t:unix_stream_socket create_socket_perms;
This needs to go under the "allow mcelog_t self:capability sysadmin;"
See style guide.

also use create_stream_socket_perms instead of create_socket_perms

> kernel_read_system_state(mcelog_t)
>
> +corecmd_exec_bin(mcelog_t)
> +
> dev_read_raw_memory(mcelog_t)
> dev_read_kmsg(mcelog_t)
> +dev_rw_sysfs(mcelog_t)
> +
> +manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
> +manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
> +files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })

This needs to above "kernel_read_system_state(mcelog_t) See style guide

> files_read_etc_files(mcelog_t)
> +files_search_etc(mcelog_t)

No need for this. files_read_etc_files(mcelog_t) already allow this

> +files_search_pids(mcelog_t)
> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
> +
This needs to go above kernel_read_system_state(mcelog_t) See style
guide

> +locallogin_use_fds(mcelog_t)
> +
> +# manage a logfile in a generic or private log directory
> +manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
> +manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
> +logging_log_filetrans(mcelog_t, mcelog_log_t, file)

This needs to go above kernel_read_system_state(mcelog_t) See style
guide

> +# use syslog functionality (optional, configurable)
> +logging_send_syslog_msg(mcelog_t)
> +
> +miscfiles_read_localization(mcelog_t)
>
> # for /dev/mem access
> mls_file_read_all_levels(mcelog_t)
>
> -logging_send_syslog_msg(mcelog_t)
> +stream_connect_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t,
> mcelog_t)

This isnt needed

> -miscfiles_read_localization(mcelog_t)
> +term_dontaudit_use_all_ptys(mcelog_t)
> +term_dontaudit_use_all_ttys(mcelog_t)

not needed. use: userdom_use_user_terminals(mcelog_t)

> +tunable_policy(`mcelog_foreground',`
> +term_use_all_ttys(mcelog_t)
> +term_use_all_ptys(mcelog_t)
> +')

Not needed.

> +# optional support for running it as a cron job
> +optional_policy(`
> + cron_system_entry(mcelog_t, mcelog_exec_t)
> +')
> Binary files refpolicy-04062012/policy/modules/contrib/.xen.te.swp and
> refpolicy-04062012-mcelog-support/policy/modules/contrib/.xen.te.swp differ
> diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
> refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
> --- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-05
> 04:52:17.194005067 +0200
> +++
> refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
> 2012-08-05 17:49:05.594838788 +0200
> @@ -72,12 +72,6 @@ ifdef(`distro_redhat',`
> /etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)
>
> /etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
> -/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
> -/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
> -
> -ifdef(`distro_redhat',`
> -/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
> -')

dont remove this, this belongs here and not in mcelog.fc

> /etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)
>
>

2012-08-07 17:34:00

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite

Here is the latest (v4) version of the mcelog module:

Rewrite of mcelog module:
- version increment
- fix and extend file contexts (private types)
- support daemon mode and init scripting (+ deprecated and untested cron
mode)
- support optional triggers for all distributions, while leaving
compatibility with their alternate location in Fedora (and
current policy)
- initial configurable support for client/server mode (untested)
- support for sysfs (rw)
- includes several revisions from Dominick Grift

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/mcelog.fc | 12 +++
policy/modules/contrib/mcelog.te | 119
++++++++++++++++++++++++++++++++--
policy/modules/kernel/corecommands.fc | 8 ++
3 files changed, 133 insertions(+), 6 deletions(-)

diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
--- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
18:29:23.578610955 +0200
+++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
2012-08-07 21:10:43.247757154 +0200
@@ -1 +1,13 @@
+/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
+
+ifdef(`distro_redhat',`
+/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
+')
+
+/etc/rc\.d/init\.d/mcelog --
gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
+
/usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
+
+/var/log/mcelog.* -- gen_context(system_u:object_r:mcelog_log_t,s0)
+/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
+/var/run/mcelog-client -s
gen_context(system_u:object_r:mcelog_var_run_t,s0)
diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
--- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
18:29:23.578610955 +0200
+++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
2012-08-07 20:53:05.056511692 +0200
@@ -1,14 +1,70 @@
-policy_module(mcelog, 1.1.0)
+policy_module(mcelog, 1.1.1)

########################################
#
# Declarations
#

+## <desc>
+## <p>
+## Allow mcelog to run in client mode.
+## Required to run mcelog in client
+## mode.
+## </p>
+## </desc>
+gen_tunable(mcelog_client, false)
+
+## <desc>
+## <p>
+## Allow mcelog to execute scripts.
+## Required to execute optional triggers
+## and/or local scripts.
+## </p>
+## </desc>
+gen_tunable(mcelog_exec_scripts, true)
+
+## <desc>
+## <p>
+## Allow mcelog to use all the user ttys.
+## Required in foreground mode and to
+## print out usage and version information.
+## </p>
+## </desc>
+gen_tunable(mcelog_foreground, true)
+
+## <desc>
+## <p>
+## Allow mcelog to run a server.
+## Required to enable the optional configurable
+## Unix stream socket server functionality.
+## </p>
+## </desc>
+gen_tunable(mcelog_server, false)
+
+## <desc>
+## <p>
+## Allow mcelog to use syslog.
+## Required to use the configurable
+## syslog option.
+## </p>
+## </desc>
+gen_tunable(mcelog_syslog, true)
+
type mcelog_t;
type mcelog_exec_t;
-application_domain(mcelog_t, mcelog_exec_t)
-cron_system_entry(mcelog_t, mcelog_exec_t)
+init_daemon_domain(mcelog_t, mcelog_exec_t)
+
+type mcelog_initrc_exec_t;
+init_script_file(mcelog_initrc_exec_t)
+
+type mcelog_etc_t;
+files_config_file(mcelog_etc_t)
+
+type mcelog_log_t;
+logging_log_file(mcelog_log_t)
+
+type mcelog_var_run_t;
+files_pid_file(mcelog_var_run_t)

########################################
#
@@ -17,16 +73,69 @@ cron_system_entry(mcelog_t, mcelog_exec_

allow mcelog_t self:capability sys_admin;

+allow mcelog_t mcelog_etc_t:dir list_dir_perms;
+
+files_search_pids(mcelog_t)
+read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
+
+# manage a logfile in a generic or private log directory
+manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
+manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
+logging_log_filetrans(mcelog_t, mcelog_log_t, file)
+
+manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
+manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
+files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })
+
kernel_read_system_state(mcelog_t)

dev_read_raw_memory(mcelog_t)
dev_read_kmsg(mcelog_t)
+dev_rw_sysfs(mcelog_t)

files_read_etc_files(mcelog_t)
+files_search_pids(mcelog_t)
+read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)

-# for /dev/mem access
-mls_file_read_all_levels(mcelog_t)
+locallogin_use_fds(mcelog_t)

+# use syslog functionality (optional, configurable)
logging_send_syslog_msg(mcelog_t)

miscfiles_read_localization(mcelog_t)
+
+# for /dev/mem access
+mls_file_read_all_levels(mcelog_t)
+
+# needed in client-mode
+tunable_policy(`mcelog_client',`
+ stream_connect_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t,
mcelog_t)
+')
+
+# required for executing optional triggers and scripts
+tunable_policy(`mcelog_exec_scripts',`
+ allow mcelog_t self:fifo_file { read getattr write };
+ corecmd_exec_bin(mcelog_t)
+ corecmd_exec_shell(mcelog_t)
+')
+
+# required for optional foreground mode and
+# console output
+tunable_policy(`mcelog_foreground',`
+ userdom_use_user_terminals(mcelog_t)
+')
+
+# required for the optional server functionality
+tunable_policy(`mcelog_server',`
+ allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
+')
+
+# use syslog functionality (optional, configurable)
+tunable_policy(`mcelog_syslog',`
+ logging_send_syslog_msg(mcelog_t)
+')
+
+# optional support for running it as a cron job
+optional_policy(`
+ cron_system_entry(mcelog_t, mcelog_exec_t)
+')
diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
--- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-07
18:38:05.323569047 +0200
+++
refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
2012-08-07 15:54:20.796905090 +0200
@@ -72,8 +72,14 @@ ifdef(`distro_redhat',`
/etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)

/etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
-/etc/mcelog/cache-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
+
+/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
+/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
+
+ifdef(`distro_redhat',`
/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
+')
+
/etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)

/etc/netplug\.d(/.*)? gen_context(system_u:object_r:bin_t,s0)



On 06/08/2012 21:44, Dominick Grift wrote:
>
>
> On Mon, 2012-08-06 at 20:43 +0200, Guido Trentalancia wrote:
>
>>
>> Signed-off-by: Guido Trentalancia <[email protected]>
>> ---
>> policy/modules/contrib/mcelog.fc | 15 +++++++
>> policy/modules/contrib/mcelog.te | 67
>> +++++++++++++++++++++++++++++++---
>> policy/modules/kernel/corecommands.fc | 6 ---
>> 3 files changed, 77 insertions(+), 11 deletions(-)
>>
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
>> refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
>> 18:29:23.578610955 +0200
>> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
>> 2012-08-06 21:11:19.617661468 +0200
>> @@ -1 +1,16 @@
>> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
>> +/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
>> +
>> +ifdef(`distro_redhat',`
>> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
> The context specs with bin_t do not belong in this module. they should
> be moved to corecommands.fc (i believe)

Done.

>> +')
>> +
>> +/etc/rc\.d/init\.d/mcelog --
>> gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
>> +
>> /usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +
>> +/var/log/mcelog -- gen_context(system_u:object_r:mcelog_log_t,s0)
>
> I would use "/var/log/mcelog.*" for logrotate support. logrotate
> sometimes append datestamps to rotated logs and we still want them to
> keep the right label

Good idea !

>> +/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> +/var/run/mcelog-client -s
>> gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
>> refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
>> 18:29:23.578610955 +0200
>> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.te
>> 2012-08-06 22:18:27.551975687 +0200
>> @@ -1,14 +1,34 @@
>> -policy_module(mcelog, 1.1.0)
>> +policy_module(mcelog, 1.1.1)
>>
>> ########################################
>> #
>> # Declarations
>> #
>>
>> +## <desc>
>> +## <p>
>> +## Allow mcelog to use all the ttys.
>> +## Required in foreground mode and to
>> +## print out usage and version information.
>> +## </p>
>> +## </desc>
>> +gen_tunable(mcelog_foreground, true)
>
> No need for a boolean for this imho

I've decided to leave them and add another one for syslog functionality.
They all default to true (except from the client/server ones, see
below), so each user has maximum freedom of choice, while leaving a
default full-featured behaviour.

>> type mcelog_t;
>> type mcelog_exec_t;
>> -application_domain(mcelog_t, mcelog_exec_t)
>> -cron_system_entry(mcelog_t, mcelog_exec_t)
>> +init_daemon_domain(mcelog_t, mcelog_exec_t)
>> +
>> +type mcelog_initrc_exec_t;
>> +init_script_file(mcelog_initrc_exec_t)
>> +
>> +type mcelog_etc_t;
>> +files_config_file(mcelog_etc_t)
>> +
>> +type mcelog_log_t;
>> +logging_log_file(mcelog_log_t)
>> +
>> +type mcelog_var_run_t;
>> +files_pid_file(mcelog_var_run_t)
>>
>> ########################################
>> #
>> @@ -17,16 +37,53 @@ cron_system_entry(mcelog_t, mcelog_exec_
>>
>> allow mcelog_t self:capability sys_admin;
>>
>> +allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>> +
>> +allow mcelog_t mcelog_t:unix_stream_socket create_socket_perms;
> This needs to go under the "allow mcelog_t self:capability sysadmin;"
> See style guide.
>
> also use create_stream_socket_perms instead of create_socket_perms

Done.

>> kernel_read_system_state(mcelog_t)
>>
>> +corecmd_exec_bin(mcelog_t)
>> +
>> dev_read_raw_memory(mcelog_t)
>> dev_read_kmsg(mcelog_t)
>> +dev_rw_sysfs(mcelog_t)
>> +
>> +manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })
>
> This needs to above "kernel_read_system_state(mcelog_t) See style guide

Done.

>> files_read_etc_files(mcelog_t)
>> +files_search_etc(mcelog_t)
>
> No need for this. files_read_etc_files(mcelog_t) already allow this

Done.

>> +files_search_pids(mcelog_t)
>> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>> +
> This needs to go above kernel_read_system_state(mcelog_t) See style
> guide

Done.

>> +locallogin_use_fds(mcelog_t)
>> +
>> +# manage a logfile in a generic or private log directory
>> +manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +logging_log_filetrans(mcelog_t, mcelog_log_t, file)
>
> This needs to go above kernel_read_system_state(mcelog_t) See style
> guide

Done.

>> +# use syslog functionality (optional, configurable)
>> +logging_send_syslog_msg(mcelog_t)
>> +
>> +miscfiles_read_localization(mcelog_t)
>>
>> # for /dev/mem access
>> mls_file_read_all_levels(mcelog_t)
>>
>> -logging_send_syslog_msg(mcelog_t)
>> +stream_connect_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t,
>> mcelog_t)
>
> This isnt needed

It's needed for the (untested) client mode.

There is a boolean for that (and for the server mode, as one might want
to write another client for example).

>> -miscfiles_read_localization(mcelog_t)
>> +term_dontaudit_use_all_ptys(mcelog_t)
>> +term_dontaudit_use_all_ttys(mcelog_t)
>
> not needed. use: userdom_use_user_terminals(mcelog_t)

It works and it appears to be widely used.

However I am not entirely clear to me what would happen if the
userdomain module is explicitly turned off and whether it will keep
working in single-user mode...

>> +tunable_policy(`mcelog_foreground',`
>> +term_use_all_ttys(mcelog_t)
>> +term_use_all_ptys(mcelog_t)
>> +')
>
> Not needed.

See above.

>> +# optional support for running it as a cron job
>> +optional_policy(`
>> + cron_system_entry(mcelog_t, mcelog_exec_t)
>> +')
>> Binary files refpolicy-04062012/policy/modules/contrib/.xen.te.swp and
>> refpolicy-04062012-mcelog-support/policy/modules/contrib/.xen.te.swp differ
>> diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
>> refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
>> --- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-05
>> 04:52:17.194005067 +0200
>> +++
>> refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
>> 2012-08-05 17:49:05.594838788 +0200
>> @@ -72,12 +72,6 @@ ifdef(`distro_redhat',`
>> /etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)
>>
>> /etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
>> -/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
>> -/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
>> -
>> -ifdef(`distro_redhat',`
>> -/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
>> -')
>
> dont remove this, this belongs here and not in mcelog.fc

Right, after reverting the scripts to bin_t, I have now moved it back to
corecommands.

>> /etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)
>>
>>

Regards,

Guido

2012-08-07 17:43:24

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite


>
> It's needed for the (untested) client mode.
>
> There is a boolean for that (and for the server mode, as one might want
> to write another client for example).
>

Its already allowed... I will explain it one more time:

allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)

is what allows this already. Its already there and therefore the
stream_connect_pattern() is reduntant.

> >> -miscfiles_read_localization(mcelog_t)
> >> +term_dontaudit_use_all_ptys(mcelog_t)
> >> +term_dontaudit_use_all_ttys(mcelog_t)
> >
> > not needed. use: userdom_use_user_terminals(mcelog_t)
>
> It works and it appears to be widely used.
>
> However I am not entirely clear to me what would happen if the
> userdomain module is explicitly turned off and whether it will keep
> working in single-user mode...
>

No need to worry about that. The userdomain module is not optional.

> >> +tunable_policy(`mcelog_foreground',`
> >> +term_use_all_ttys(mcelog_t)
> >> +term_use_all_ptys(mcelog_t)
> >> +')
> >
> > Not needed.
>
> See above.

Although the policy improved i still have issues with various parts of
your policy.

However i won't review it anymore because i have made my points already
in previous reviews. No need for repeating myself.

2012-08-07 17:57:46

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite

On 07/08/2012 19:43, Dominick Grift wrote:
>
>>
>> It's needed for the (untested) client mode.
>>
>> There is a boolean for that (and for the server mode, as one might want
>> to write another client for example).
>>
>
> Its already allowed... I will explain it one more time:
>
> allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
> manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>
> is what allows this already. Its already there and therefore the
> stream_connect_pattern() is reduntant.

The above two do not have "connectto", which is needed by the client.

>>>> -miscfiles_read_localization(mcelog_t)
>>>> +term_dontaudit_use_all_ptys(mcelog_t)
>>>> +term_dontaudit_use_all_ttys(mcelog_t)
>>>
>>> not needed. use: userdom_use_user_terminals(mcelog_t)
>>
>> It works and it appears to be widely used.
>>
>> However I am not entirely clear to me what would happen if the
>> userdomain module is explicitly turned off and whether it will keep
>> working in single-user mode...
>>
>
> No need to worry about that. The userdomain module is not optional.

It's a bit strange, if I turn it off, it still gets compiled in...

>>>> +tunable_policy(`mcelog_foreground',`
>>>> +term_use_all_ttys(mcelog_t)
>>>> +term_use_all_ptys(mcelog_t)
>>>> +')
>>>
>>> Not needed.
>>
>> See above.
>
> Although the policy improved i still have issues with various parts of
> your policy.

Perfection does not exist. Time available is not infinite. And there
several (or sometimes even infinite) degrees of freedom when it comes to
implement things.

Since at the moment mcelog, is not supported or its support is broken
(whichever you prefer), I suggest this v4 version is applied and then if
you (or others) like to modify it, you can do so directly and indipendently.

> However i won't review it anymore because i have made my points already
> in previous reviews. No need for repeating myself.

Your suggestions have been introduced (except from "connectto", see above).

If it is for the booleans, then I think it's much better to have a
configurable module once the default values allow the most common way of
execution. You are unlikely to convince me here: I do not want a server
writing to a socket information about the cpu.

Regards,

Guido

2012-08-07 19:35:27

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite

Hello Dominick.

On 07/08/2012 19:43, Dominick Grift wrote:
>
>>
>> It's needed for the (untested) client mode.
>>
>> There is a boolean for that (and for the server mode, as one might want
>> to write another client for example).
>>
>
> Its already allowed... I will explain it one more time:
>
> allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
> manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>
> is what allows this already. Its already there and therefore the
> stream_connect_pattern() is reduntant.

I have triple-checked now, so at least you could double-check...

stream_connect_pattern() is needed for "connectto" (client-mode)

removal of manage_sock_files_pattern would prevent sock_file creation:
it's the physical entry in the filesystem, not the logical socket
created by create_stream_socket_perms !

> However i won't review it anymore because i have made my points already
> in previous reviews. No need for repeating myself.

As already explained, all your revision have been introduced as applicable.

My advice is to apply it as it is now and then you can submit further
patches as needed, which also seems much more efficient.

But I would strongly recommend you to also carry out some testing,
because otherwise, no matter how skilled you are, things similar to the
above might happen.

Regards,

Guido

2012-08-07 19:48:36

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite



On Tue, 2012-08-07 at 21:35 +0200, Guido Trentalancia wrote:
> Hello Dominick.
>
> On 07/08/2012 19:43, Dominick Grift wrote:
> >
> >>
> >> It's needed for the (untested) client mode.
> >>
> >> There is a boolean for that (and for the server mode, as one might want
> >> to write another client for example).
> >>
> >
> > Its already allowed... I will explain it one more time:
> >
> > allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
> > manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
> >
> > is what allows this already. Its already there and therefore the
> > stream_connect_pattern() is reduntant.
>
> I have triple-checked now, so at least you could double-check...
>

whoops you are right but in that case just do something like this
instead:

allow mcslog_t self:unix_stream_socket { create_socket_perms
connectto; }

or show me the avc denials related to mcelog_t operating on mcelog_t
unix stream sockets so that we can figure out the exact permissions.

but using stream_connect_pattern() is not the way to go here

> stream_connect_pattern() is needed for "connectto" (client-mode)
>
> removal of manage_sock_files_pattern would prevent sock_file creation:
> it's the physical entry in the filesystem, not the logical socket
> created by create_stream_socket_perms !

I am not saying that you should remove it. I am saying that you should
use the stream_connect_pattern()

> > However i won't review it anymore because i have made my points already
> > in previous reviews. No need for repeating myself.
>
> As already explained, all your revision have been introduced as applicable.
>
> My advice is to apply it as it is now and then you can submit further
> patches as needed, which also seems much more efficient.

It's not my call. I am just reviewing.

> But I would strongly recommend you to also carry out some testing,
> because otherwise, no matter how skilled you are, things similar to the
> above might happen.

It should be tested indeed

> Regards,
>
> Guido

2012-08-07 20:20:40

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite

On 07/08/2012 21:48, Dominick Grift wrote:
>
>
> On Tue, 2012-08-07 at 21:35 +0200, Guido Trentalancia wrote:
>> Hello Dominick.
>>
>> On 07/08/2012 19:43, Dominick Grift wrote:
>>>
>>>>
>>>> It's needed for the (untested) client mode.
>>>>
>>>> There is a boolean for that (and for the server mode, as one might want
>>>> to write another client for example).
>>>>
>>>
>>> Its already allowed... I will explain it one more time:
>>>
>>> allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
>>> manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>>>
>>> is what allows this already. Its already there and therefore the
>>> stream_connect_pattern() is reduntant.
>>
>> I have triple-checked now, so at least you could double-check...
>>
>
> whoops you are right but in that case just do something like this
> instead:
>
> allow mcslog_t self:unix_stream_socket { create_socket_perms
> connectto; }

Yes, I think it can be done, but I would need to check. But is this not
going to be considered overengineering ?

I don't like such term very much in this context anyway, as usually
there is always an advantage in terms of maintanability.

> or show me the avc denials related to mcelog_t operating on mcelog_t
> unix stream sockets so that we can figure out the exact permissions.
>
> but using stream_connect_pattern() is not the way to go here

Initially you had suggested that pattern, so I went for that.

>> stream_connect_pattern() is needed for "connectto" (client-mode)
>>
>> removal of manage_sock_files_pattern would prevent sock_file creation:
>> it's the physical entry in the filesystem, not the logical socket
>> created by create_stream_socket_perms !
>
> I am not saying that you should remove it. I am saying that you should
> use the stream_connect_pattern()

>>> However i won't review it anymore because i have made my points already
>>> in previous reviews. No need for repeating myself.
>>
>> As already explained, all your revision have been introduced as applicable.
>>
>> My advice is to apply it as it is now and then you can submit further
>> patches as needed, which also seems much more efficient.
>
> It's not my call. I am just reviewing.
>
>> But I would strongly recommend you to also carry out some testing,
>> because otherwise, no matter how skilled you are, things similar to the
>> above might happen.
>
> It should be tested indeed
>
>> Regards,
>>
>> Guido

2012-08-07 20:27:00

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite



On Tue, 2012-08-07 at 22:20 +0200, Guido Trentalancia wrote:

> >
> > allow mcslog_t self:unix_stream_socket { create_socket_perms
> > connectto; }
>
> Yes, I think it can be done, but I would need to check. But is this not
> going to be considered overengineering ?

No that is the way to go

> I don't like such term very much in this context anyway, as usually
> there is always an advantage in terms of maintanability.
>
> > or show me the avc denials related to mcelog_t operating on mcelog_t
> > unix stream sockets so that we can figure out the exact permissions.
> >
> > but using stream_connect_pattern() is not the way to go here
>
> Initially you had suggested that pattern, so I went for that.

Right but at that time i didnt see the big picture (context of the
usage)

2012-08-07 22:04:45

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

Here is the latest (v6) version:

Rewrite of mcelog module:
- version increment
- fix and extend file contexts (private types)
- support daemon mode and init scripting (+ deprecated and untested cron
mode)
- support optional triggers for all distributions, while leaving
compatibility with their alternate location in Fedora (and
current policy)
- initial configurable support for client/server mode (untested)
- support for sysfs (rw)
- includes several revisions from Dominick Grift
- removed duplicate syslog interface over previous version 4
- reduced stream_connect_pattern to permissions from version 5

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/mcelog.fc | 12 +++
policy/modules/contrib/mcelog.te | 118
++++++++++++++++++++++++++++++++--
policy/modules/kernel/corecommands.fc | 8 ++
3 files changed, 131 insertions(+), 7 deletions(-)

diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
--- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
18:29:23.578610955 +0200
+++
refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
2012-08-07 21:10:43.247757154 +0200
@@ -1 +1,13 @@
+/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
+
+ifdef(`distro_redhat',`
+/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
+')
+
+/etc/rc\.d/init\.d/mcelog --
gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
+
/usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
+
+/var/log/mcelog.* -- gen_context(system_u:object_r:mcelog_log_t,s0)
+/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
+/var/run/mcelog-client -s
gen_context(system_u:object_r:mcelog_var_run_t,s0)
diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
--- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
18:29:23.578610955 +0200
+++
refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
2012-08-08 01:11:06.572330170 +0200
@@ -1,14 +1,70 @@
-policy_module(mcelog, 1.1.0)
+policy_module(mcelog, 1.1.1)

########################################
#
# Declarations
#

+## <desc>
+## <p>
+## Allow mcelog to run in client mode.
+## Required to run mcelog in client
+## mode.
+## </p>
+## </desc>
+gen_tunable(mcelog_client, false)
+
+## <desc>
+## <p>
+## Allow mcelog to execute scripts.
+## Required to execute optional triggers
+## and/or local scripts.
+## </p>
+## </desc>
+gen_tunable(mcelog_exec_scripts, true)
+
+## <desc>
+## <p>
+## Allow mcelog to use all the user ttys.
+## Required in foreground mode and to
+## print out usage and version information.
+## </p>
+## </desc>
+gen_tunable(mcelog_foreground, true)
+
+## <desc>
+## <p>
+## Allow mcelog to run a server.
+## Required to enable the optional configurable
+## Unix stream socket server functionality.
+## </p>
+## </desc>
+gen_tunable(mcelog_server, false)
+
+## <desc>
+## <p>
+## Allow mcelog to use syslog.
+## Required to use the configurable
+## syslog option.
+## </p>
+## </desc>
+gen_tunable(mcelog_syslog, true)
+
type mcelog_t;
type mcelog_exec_t;
-application_domain(mcelog_t, mcelog_exec_t)
-cron_system_entry(mcelog_t, mcelog_exec_t)
+init_daemon_domain(mcelog_t, mcelog_exec_t)
+
+type mcelog_initrc_exec_t;
+init_script_file(mcelog_initrc_exec_t)
+
+type mcelog_etc_t;
+files_config_file(mcelog_etc_t)
+
+type mcelog_log_t;
+logging_log_file(mcelog_log_t)
+
+type mcelog_var_run_t;
+files_pid_file(mcelog_var_run_t)

########################################
#
@@ -16,17 +72,67 @@ cron_system_entry(mcelog_t, mcelog_exec_
#

allow mcelog_t self:capability sys_admin;
+allow mcelog_t self:unix_stream_socket connected_socket_perms;
+allow mcelog_t mcelog_etc_t:dir list_dir_perms;
+
+files_search_pids(mcelog_t)
+read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
+
+# manage a logfile in a generic or private log directory
+manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
+manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
+logging_log_filetrans(mcelog_t, mcelog_log_t, file)
+
+manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
+manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
+files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })

kernel_read_system_state(mcelog_t)

dev_read_raw_memory(mcelog_t)
dev_read_kmsg(mcelog_t)
+dev_rw_sysfs(mcelog_t)

files_read_etc_files(mcelog_t)
+files_search_pids(mcelog_t)
+read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
+
+locallogin_use_fds(mcelog_t)
+
+miscfiles_read_localization(mcelog_t)

# for /dev/mem access
mls_file_read_all_levels(mcelog_t)

-logging_send_syslog_msg(mcelog_t)
-
-miscfiles_read_localization(mcelog_t)
+# needed in client-mode
+tunable_policy(`mcelog_client',`
+ allow mcelog_t self:unix_stream_socket connectto;
+')
+
+# required for executing optional triggers and scripts
+tunable_policy(`mcelog_exec_scripts',`
+ allow mcelog_t self:fifo_file { read getattr write };
+ corecmd_exec_bin(mcelog_t)
+ corecmd_exec_shell(mcelog_t)
+')
+
+# required for optional foreground mode and
+# console output
+tunable_policy(`mcelog_foreground',`
+ userdom_use_user_terminals(mcelog_t)
+')
+
+# required for the optional server functionality
+tunable_policy(`mcelog_server',`
+ allow mcelog_t self:unix_stream_socket { listen accept };
+')
+
+# use syslog functionality (optional, configurable)
+tunable_policy(`mcelog_syslog',`
+ logging_send_syslog_msg(mcelog_t)
+')
+
+# optional support for running it as a cron job
+optional_policy(`
+ cron_system_entry(mcelog_t, mcelog_exec_t)
+')
diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
refpolicy-04062012-mcelog-support-v6/policy/modules/kernel/corecommands.fc
--- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-07
18:38:05.323569047 +0200
+++
refpolicy-04062012-mcelog-support-v6/policy/modules/kernel/corecommands.fc
2012-08-07 15:54:20.796905090 +0200
@@ -72,8 +72,14 @@ ifdef(`distro_redhat',`
/etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)

/etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
-/etc/mcelog/cache-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
+
+/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
+/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
+
+ifdef(`distro_redhat',`
/etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
+')
+
/etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)

/etc/netplug\.d(/.*)? gen_context(system_u:object_r:bin_t,s0)


On 07/08/2012 22:27, Dominick Grift wrote:
>
>
> On Tue, 2012-08-07 at 22:20 +0200, Guido Trentalancia wrote:
>
>>>
>>> allow mcslog_t self:unix_stream_socket { create_socket_perms
>>> connectto; }
>>
>> Yes, I think it can be done, but I would need to check. But is this not
>> going to be considered overengineering ?
>
> No that is the way to go

Since apparently it hasn't been applied yet, I have attached a new
version (v6) above. It reduces the stream_connect_pattern so that it
works better with the client/server tunable policy.

>> I don't like such term very much in this context anyway, as usually
>> there is always an advantage in terms of maintanability.
>>
>>> or show me the avc denials related to mcelog_t operating on mcelog_t
>>> unix stream sockets so that we can figure out the exact permissions.
>>>
>>> but using stream_connect_pattern() is not the way to go here
>>
>> Initially you had suggested that pattern, so I went for that.
>
> Right but at that time i didnt see the big picture (context of the
> usage)

Ok, I hope now it's fine for everyone. There is a lot more to be done,
so it's probably pointless focusing eccesively on this at this point.

Regards,

Guido

2012-08-08 13:02:00

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

On 08/07/12 18:04, Guido Trentalancia wrote:
> Here is the latest (v6) version:
>
> Rewrite of mcelog module:
> - version increment
> - fix and extend file contexts (private types)
> - support daemon mode and init scripting (+ deprecated and untested cron
> mode)
> - support optional triggers for all distributions, while leaving
> compatibility with their alternate location in Fedora (and
> current policy)
> - initial configurable support for client/server mode (untested)
> - support for sysfs (rw)
> - includes several revisions from Dominick Grift
> - removed duplicate syslog interface over previous version 4
> - reduced stream_connect_pattern to permissions from version 5

Merged. In the future, please do not increment the module version.


> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
> policy/modules/contrib/mcelog.fc | 12 +++
> policy/modules/contrib/mcelog.te | 118
> ++++++++++++++++++++++++++++++++--
> policy/modules/kernel/corecommands.fc | 8 ++
> 3 files changed, 131 insertions(+), 7 deletions(-)
>
> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
> 18:29:23.578610955 +0200
> +++
> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
> 2012-08-07 21:10:43.247757154 +0200
> @@ -1 +1,13 @@
> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
> +
> +ifdef(`distro_redhat',`
> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
> +')
> +
> +/etc/rc\.d/init\.d/mcelog --
> gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
> +
> /usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
> +
> +/var/log/mcelog.* -- gen_context(system_u:object_r:mcelog_log_t,s0)
> +/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
> +/var/run/mcelog-client -s
> gen_context(system_u:object_r:mcelog_var_run_t,s0)
> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
> --- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
> 18:29:23.578610955 +0200
> +++
> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
> 2012-08-08 01:11:06.572330170 +0200
> @@ -1,14 +1,70 @@
> -policy_module(mcelog, 1.1.0)
> +policy_module(mcelog, 1.1.1)
>
> ########################################
> #
> # Declarations
> #
>
> +## <desc>
> +## <p>
> +## Allow mcelog to run in client mode.
> +## Required to run mcelog in client
> +## mode.
> +## </p>
> +## </desc>
> +gen_tunable(mcelog_client, false)
> +
> +## <desc>
> +## <p>
> +## Allow mcelog to execute scripts.
> +## Required to execute optional triggers
> +## and/or local scripts.
> +## </p>
> +## </desc>
> +gen_tunable(mcelog_exec_scripts, true)
> +
> +## <desc>
> +## <p>
> +## Allow mcelog to use all the user ttys.
> +## Required in foreground mode and to
> +## print out usage and version information.
> +## </p>
> +## </desc>
> +gen_tunable(mcelog_foreground, true)
> +
> +## <desc>
> +## <p>
> +## Allow mcelog to run a server.
> +## Required to enable the optional configurable
> +## Unix stream socket server functionality.
> +## </p>
> +## </desc>
> +gen_tunable(mcelog_server, false)
> +
> +## <desc>
> +## <p>
> +## Allow mcelog to use syslog.
> +## Required to use the configurable
> +## syslog option.
> +## </p>
> +## </desc>
> +gen_tunable(mcelog_syslog, true)
> +
> type mcelog_t;
> type mcelog_exec_t;
> -application_domain(mcelog_t, mcelog_exec_t)
> -cron_system_entry(mcelog_t, mcelog_exec_t)
> +init_daemon_domain(mcelog_t, mcelog_exec_t)
> +
> +type mcelog_initrc_exec_t;
> +init_script_file(mcelog_initrc_exec_t)
> +
> +type mcelog_etc_t;
> +files_config_file(mcelog_etc_t)
> +
> +type mcelog_log_t;
> +logging_log_file(mcelog_log_t)
> +
> +type mcelog_var_run_t;
> +files_pid_file(mcelog_var_run_t)
>
> ########################################
> #
> @@ -16,17 +72,67 @@ cron_system_entry(mcelog_t, mcelog_exec_
> #
>
> allow mcelog_t self:capability sys_admin;
> +allow mcelog_t self:unix_stream_socket connected_socket_perms;
> +allow mcelog_t mcelog_etc_t:dir list_dir_perms;
> +
> +files_search_pids(mcelog_t)
> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
> +
> +# manage a logfile in a generic or private log directory
> +manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
> +manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
> +logging_log_filetrans(mcelog_t, mcelog_log_t, file)
> +
> +manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
> +manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
> +files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })
>
> kernel_read_system_state(mcelog_t)
>
> dev_read_raw_memory(mcelog_t)
> dev_read_kmsg(mcelog_t)
> +dev_rw_sysfs(mcelog_t)
>
> files_read_etc_files(mcelog_t)
> +files_search_pids(mcelog_t)
> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
> +
> +locallogin_use_fds(mcelog_t)
> +
> +miscfiles_read_localization(mcelog_t)
>
> # for /dev/mem access
> mls_file_read_all_levels(mcelog_t)
>
> -logging_send_syslog_msg(mcelog_t)
> -
> -miscfiles_read_localization(mcelog_t)
> +# needed in client-mode
> +tunable_policy(`mcelog_client',`
> + allow mcelog_t self:unix_stream_socket connectto;
> +')
> +
> +# required for executing optional triggers and scripts
> +tunable_policy(`mcelog_exec_scripts',`
> + allow mcelog_t self:fifo_file { read getattr write };
> + corecmd_exec_bin(mcelog_t)
> + corecmd_exec_shell(mcelog_t)
> +')
> +
> +# required for optional foreground mode and
> +# console output
> +tunable_policy(`mcelog_foreground',`
> + userdom_use_user_terminals(mcelog_t)
> +')
> +
> +# required for the optional server functionality
> +tunable_policy(`mcelog_server',`
> + allow mcelog_t self:unix_stream_socket { listen accept };
> +')
> +
> +# use syslog functionality (optional, configurable)
> +tunable_policy(`mcelog_syslog',`
> + logging_send_syslog_msg(mcelog_t)
> +')
> +
> +# optional support for running it as a cron job
> +optional_policy(`
> + cron_system_entry(mcelog_t, mcelog_exec_t)
> +')
> diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
> refpolicy-04062012-mcelog-support-v6/policy/modules/kernel/corecommands.fc
> --- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-07
> 18:38:05.323569047 +0200
> +++
> refpolicy-04062012-mcelog-support-v6/policy/modules/kernel/corecommands.fc
> 2012-08-07 15:54:20.796905090 +0200
> @@ -72,8 +72,14 @@ ifdef(`distro_redhat',`
> /etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)
>
> /etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
> -/etc/mcelog/cache-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
> +
> +/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
> +/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
> +
> +ifdef(`distro_redhat',`
> /etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
> +')
> +
> /etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)
>
> /etc/netplug\.d(/.*)? gen_context(system_u:object_r:bin_t,s0)

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2012-08-08 14:34:16

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

Hello Christopher.

On 08/08/2012 15:02, Christopher J. PeBenito wrote:
> On 08/07/12 18:04, Guido Trentalancia wrote:
>> Here is the latest (v6) version:
>>
>> Rewrite of mcelog module:
>> - version increment
>> - fix and extend file contexts (private types)
>> - support daemon mode and init scripting (+ deprecated and untested cron
>> mode)
>> - support optional triggers for all distributions, while leaving
>> compatibility with their alternate location in Fedora (and
>> current policy)
>> - initial configurable support for client/server mode (untested)
>> - support for sysfs (rw)
>> - includes several revisions from Dominick Grift
>> - removed duplicate syslog interface over previous version 4
>> - reduced stream_connect_pattern to permissions from version 5
>
> Merged. In the future, please do not increment the module version.

Good, so at least it would run now. At the end, it includes thermal
errors detection, so no one now can be blamed if a CPU burns up ;)

>> Signed-off-by: Guido Trentalancia <[email protected]>
>> ---
>> policy/modules/contrib/mcelog.fc | 12 +++
>> policy/modules/contrib/mcelog.te | 118
>> ++++++++++++++++++++++++++++++++--
>> policy/modules/kernel/corecommands.fc | 8 ++
>> 3 files changed, 131 insertions(+), 7 deletions(-)
>>
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
>> 18:29:23.578610955 +0200
>> +++
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
>> 2012-08-07 21:10:43.247757154 +0200
>> @@ -1 +1,13 @@
>> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +
>> +ifdef(`distro_redhat',`
>> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +')
>> +
>> +/etc/rc\.d/init\.d/mcelog --
>> gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)

Dominick has also noted that Debian (and Gentoo) are actually using a
different path for the init scripts. I don't know how to move on with
this really, as the rest of the current reference policy only has
support for such alternative location in the hadoop module.

I have already pointed out that without an updated official list of
files from each distribution is not very easy to honour all these subtle
differences (that unfortunately will prevent the reference policy from
being plug-and-play).

>> +
>> /usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +
>> +/var/log/mcelog.* -- gen_context(system_u:object_r:mcelog_log_t,s0)
>> +/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> +/var/run/mcelog-client -s
>> gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
>> 18:29:23.578610955 +0200
>> +++
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
>> 2012-08-08 01:11:06.572330170 +0200
>> @@ -1,14 +1,70 @@
>> -policy_module(mcelog, 1.1.0)
>> +policy_module(mcelog, 1.1.1)

[cut]

>> @@ -16,17 +72,67 @@ cron_system_entry(mcelog_t, mcelog_exec_
>> #
>>
>> allow mcelog_t self:capability sys_admin;
>> +allow mcelog_t self:unix_stream_socket connected_socket_perms;
>> +allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>> +
>> +files_search_pids(mcelog_t)
>> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>> +
>> +# manage a logfile in a generic or private log directory
>> +manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +logging_log_filetrans(mcelog_t, mcelog_log_t, file)
>> +
>> +manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })
>>
>> kernel_read_system_state(mcelog_t)
>>
>> dev_read_raw_memory(mcelog_t)
>> dev_read_kmsg(mcelog_t)
>> +dev_rw_sysfs(mcelog_t)
>>
>> files_read_etc_files(mcelog_t)
>> +files_search_pids(mcelog_t)
>> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)

I just realized the line above is duplicate and the above block of three
lines should go before the kernel interfaces according to the style
guidelines (no functional difference though).

Thanks very much once again to Dominick for reviewing...

Regards,

Guido

2012-08-08 14:41:05

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

On 08/08/12 10:34, Guido Trentalancia wrote:
> On 08/08/2012 15:02, Christopher J. PeBenito wrote:
>> On 08/07/12 18:04, Guido Trentalancia wrote:

>>> @@ -1 +1,13 @@
>>> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
>>> +
>>> +ifdef(`distro_redhat',`
>>> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
>>> +')
>>> +
>>> +/etc/rc\.d/init\.d/mcelog --
>>> gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
>
> Dominick has also noted that Debian (and Gentoo) are actually using a different path for the init scripts. I don't know how to move on with this really, as the rest of the current reference policy only has support for such alternative location in the hadoop module.

We can try the file context substitution feature that Dominick suggested.

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2012-08-08 19:33:13

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

Christopher,

you can also add the following, if you like (I see from latest git that
the duplicate etc_t file pattern interface have already been removed):

Reorder one file pattern interface in the recently updated mcelog.

Signed-off-by: Guido Trentalancia <[email protected]>
---

--- refpolicy/policy/modules/contrib/mcelog.te 2012-08-08
21:22:01.160888610 +0200
+++ refpolicy-08082012/policy/modules/contrib/mcelog.te 2012-08-08
21:22:19.204057838 +0200
@@ -75,6 +75,7 @@ allow mcelog_t self:capability sys_admin
allow mcelog_t self:unix_stream_socket connected_socket_perms;
allow mcelog_t mcelog_etc_t:dir list_dir_perms;

+files_read_etc_files(mcelog_t)
read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)

# manage a logfile in a generic or private log directory
@@ -92,8 +93,6 @@ dev_read_raw_memory(mcelog_t)
dev_read_kmsg(mcelog_t)
dev_rw_sysfs(mcelog_t)

-files_read_etc_files(mcelog_t)
-
# for /dev/mem access
mls_file_read_all_levels(mcelog_t)



On 08/08/2012 15:02, Christopher J. PeBenito wrote:
> On 08/07/12 18:04, Guido Trentalancia wrote:
>> Here is the latest (v6) version:
>>
>> Rewrite of mcelog module:
>> - version increment
>> - fix and extend file contexts (private types)
>> - support daemon mode and init scripting (+ deprecated and untested cron
>> mode)
>> - support optional triggers for all distributions, while leaving
>> compatibility with their alternate location in Fedora (and
>> current policy)
>> - initial configurable support for client/server mode (untested)
>> - support for sysfs (rw)
>> - includes several revisions from Dominick Grift
>> - removed duplicate syslog interface over previous version 4
>> - reduced stream_connect_pattern to permissions from version 5
>
> Merged. In the future, please do not increment the module version.
>
>
>> Signed-off-by: Guido Trentalancia <[email protected]>
>> ---
>> policy/modules/contrib/mcelog.fc | 12 +++
>> policy/modules/contrib/mcelog.te | 118
>> ++++++++++++++++++++++++++++++++--
>> policy/modules/kernel/corecommands.fc | 8 ++
>> 3 files changed, 131 insertions(+), 7 deletions(-)
>>
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09
>> 18:29:23.578610955 +0200
>> +++
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.fc
>> 2012-08-07 21:10:43.247757154 +0200
>> @@ -1 +1,13 @@
>> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +
>> +ifdef(`distro_redhat',`
>> +/etc/mcelog/triggers -d gen_context(system_u:object_r:mcelog_etc_t,s0)
>> +')
>> +
>> +/etc/rc\.d/init\.d/mcelog --
>> gen_context(system_u:object_r:mcelog_initrc_exec_t,s0)
>> +
>> /usr/sbin/mcelog -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> +
>> +/var/log/mcelog.* -- gen_context(system_u:object_r:mcelog_log_t,s0)
>> +/var/run/mcelog\.pid -- gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> +/var/run/mcelog-client -s
>> gen_context(system_u:object_r:mcelog_var_run_t,s0)
>> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.te
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
>> --- refpolicy-04062012/policy/modules/contrib/mcelog.te 2011-09-09
>> 18:29:23.578610955 +0200
>> +++
>> refpolicy-04062012-mcelog-support-v6/policy/modules/contrib/mcelog.te
>> 2012-08-08 01:11:06.572330170 +0200
>> @@ -1,14 +1,70 @@
>> -policy_module(mcelog, 1.1.0)
>> +policy_module(mcelog, 1.1.1)
>>
>> ########################################
>> #
>> # Declarations
>> #
>>
>> +## <desc>
>> +## <p>
>> +## Allow mcelog to run in client mode.
>> +## Required to run mcelog in client
>> +## mode.
>> +## </p>
>> +## </desc>
>> +gen_tunable(mcelog_client, false)
>> +
>> +## <desc>
>> +## <p>
>> +## Allow mcelog to execute scripts.
>> +## Required to execute optional triggers
>> +## and/or local scripts.
>> +## </p>
>> +## </desc>
>> +gen_tunable(mcelog_exec_scripts, true)
>> +
>> +## <desc>
>> +## <p>
>> +## Allow mcelog to use all the user ttys.
>> +## Required in foreground mode and to
>> +## print out usage and version information.
>> +## </p>
>> +## </desc>
>> +gen_tunable(mcelog_foreground, true)
>> +
>> +## <desc>
>> +## <p>
>> +## Allow mcelog to run a server.
>> +## Required to enable the optional configurable
>> +## Unix stream socket server functionality.
>> +## </p>
>> +## </desc>
>> +gen_tunable(mcelog_server, false)
>> +
>> +## <desc>
>> +## <p>
>> +## Allow mcelog to use syslog.
>> +## Required to use the configurable
>> +## syslog option.
>> +## </p>
>> +## </desc>
>> +gen_tunable(mcelog_syslog, true)
>> +
>> type mcelog_t;
>> type mcelog_exec_t;
>> -application_domain(mcelog_t, mcelog_exec_t)
>> -cron_system_entry(mcelog_t, mcelog_exec_t)
>> +init_daemon_domain(mcelog_t, mcelog_exec_t)
>> +
>> +type mcelog_initrc_exec_t;
>> +init_script_file(mcelog_initrc_exec_t)
>> +
>> +type mcelog_etc_t;
>> +files_config_file(mcelog_etc_t)
>> +
>> +type mcelog_log_t;
>> +logging_log_file(mcelog_log_t)
>> +
>> +type mcelog_var_run_t;
>> +files_pid_file(mcelog_var_run_t)
>>
>> ########################################
>> #
>> @@ -16,17 +72,67 @@ cron_system_entry(mcelog_t, mcelog_exec_
>> #
>>
>> allow mcelog_t self:capability sys_admin;
>> +allow mcelog_t self:unix_stream_socket connected_socket_perms;
>> +allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>> +
>> +files_search_pids(mcelog_t)
>> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>> +
>> +# manage a logfile in a generic or private log directory
>> +manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +manage_files_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
>> +logging_log_filetrans(mcelog_t, mcelog_log_t, file)
>> +
>> +manage_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>> +files_pid_filetrans(mcelog_t, mcelog_var_run_t, { file sock_file })
>>
>> kernel_read_system_state(mcelog_t)
>>
>> dev_read_raw_memory(mcelog_t)
>> dev_read_kmsg(mcelog_t)
>> +dev_rw_sysfs(mcelog_t)
>>
>> files_read_etc_files(mcelog_t)
>> +files_search_pids(mcelog_t)
>> +read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>> +
>> +locallogin_use_fds(mcelog_t)
>> +
>> +miscfiles_read_localization(mcelog_t)
>>
>> # for /dev/mem access
>> mls_file_read_all_levels(mcelog_t)
>>
>> -logging_send_syslog_msg(mcelog_t)
>> -
>> -miscfiles_read_localization(mcelog_t)
>> +# needed in client-mode
>> +tunable_policy(`mcelog_client',`
>> + allow mcelog_t self:unix_stream_socket connectto;
>> +')
>> +
>> +# required for executing optional triggers and scripts
>> +tunable_policy(`mcelog_exec_scripts',`
>> + allow mcelog_t self:fifo_file { read getattr write };
>> + corecmd_exec_bin(mcelog_t)
>> + corecmd_exec_shell(mcelog_t)
>> +')
>> +
>> +# required for optional foreground mode and
>> +# console output
>> +tunable_policy(`mcelog_foreground',`
>> + userdom_use_user_terminals(mcelog_t)
>> +')
>> +
>> +# required for the optional server functionality
>> +tunable_policy(`mcelog_server',`
>> + allow mcelog_t self:unix_stream_socket { listen accept };
>> +')
>> +
>> +# use syslog functionality (optional, configurable)
>> +tunable_policy(`mcelog_syslog',`
>> + logging_send_syslog_msg(mcelog_t)
>> +')
>> +
>> +# optional support for running it as a cron job
>> +optional_policy(`
>> + cron_system_entry(mcelog_t, mcelog_exec_t)
>> +')
>> diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc
>> refpolicy-04062012-mcelog-support-v6/policy/modules/kernel/corecommands.fc
>> --- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-07
>> 18:38:05.323569047 +0200
>> +++
>> refpolicy-04062012-mcelog-support-v6/policy/modules/kernel/corecommands.fc
>> 2012-08-07 15:54:20.796905090 +0200
>> @@ -72,8 +72,14 @@ ifdef(`distro_redhat',`
>> /etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)
>>
>> /etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
>> -/etc/mcelog/cache-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
>> +
>> +/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
>> +/etc/mcelog/.*\.local -- gen_context(system_u:object_r:bin_t,s0)
>> +
>> +ifdef(`distro_redhat',`
>> /etc/mcelog/triggers(/.*)? gen_context(system_u:object_r:bin_t,s0)
>> +')
>> +
>> /etc/mgetty\+sendfax/new_fax -- gen_context(system_u:object_r:bin_t,s0)
>>
>> /etc/netplug\.d(/.*)? gen_context(system_u:object_r:bin_t,s0)
>

2012-08-09 16:34:06

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

On 08/08/12 15:33, Guido Trentalancia wrote:
> Christopher,
>
> you can also add the following, if you like (I see from latest git that the duplicate etc_t file pattern interface have already been removed):
>
> Reorder one file pattern interface in the recently updated mcelog.
>
> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
>
> --- refpolicy/policy/modules/contrib/mcelog.te 2012-08-08 21:22:01.160888610 +0200
> +++ refpolicy-08082012/policy/modules/contrib/mcelog.te 2012-08-08 21:22:19.204057838 +0200
> @@ -75,6 +75,7 @@ allow mcelog_t self:capability sys_admin
> allow mcelog_t self:unix_stream_socket connected_socket_perms;
> allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>
> +files_read_etc_files(mcelog_t)
> read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>
> # manage a logfile in a generic or private log directory
> @@ -92,8 +93,6 @@ dev_read_raw_memory(mcelog_t)
> dev_read_kmsg(mcelog_t)
> dev_rw_sysfs(mcelog_t)
>
> -files_read_etc_files(mcelog_t)
> -
> # for /dev/mem access
> mls_file_read_all_levels(mcelog_t)

This isn't necessary. The reading etc files stands on its own where it is. If we want to be really thorough, you could add files_search_etc() by the read_files_pattern, but I think its fine as is.

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2012-08-09 21:54:09

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

Hello Christopher.

On 09/08/2012 18:34, Christopher J. PeBenito wrote:
> On 08/08/12 15:33, Guido Trentalancia wrote:
>> Christopher,
>>
>> you can also add the following, if you like (I see from latest git that the duplicate etc_t file pattern interface have already been removed):
>>
>> Reorder one file pattern interface in the recently updated mcelog.
>>
>> Signed-off-by: Guido Trentalancia <[email protected]>
>> ---
>>
>> --- refpolicy/policy/modules/contrib/mcelog.te 2012-08-08 21:22:01.160888610 +0200
>> +++ refpolicy-08082012/policy/modules/contrib/mcelog.te 2012-08-08 21:22:19.204057838 +0200
>> @@ -75,6 +75,7 @@ allow mcelog_t self:capability sys_admin
>> allow mcelog_t self:unix_stream_socket connected_socket_perms;
>> allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>>
>> +files_read_etc_files(mcelog_t)
>> read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>>
>> # manage a logfile in a generic or private log directory
>> @@ -92,8 +93,6 @@ dev_read_raw_memory(mcelog_t)
>> dev_read_kmsg(mcelog_t)
>> dev_rw_sysfs(mcelog_t)
>>
>> -files_read_etc_files(mcelog_t)
>> -
>> # for /dev/mem access
>> mls_file_read_all_levels(mcelog_t)
>
> This isn't necessary. The reading etc files stands on its own where it is. If we want to be really thorough, you could add files_search_etc() by the read_files_pattern, but I think its fine as is.

Yes, why not ? I have also added a comment to be 100% transparent to the
user, if anybodys want to further restrict it under particular
circumstances:

mcelog module:
- allow files_search_etc() as it might be needed for non-standard
configuration subdirectories;
- add a comment for the files_read_etc_files() interface, so that
it can be later restricted further if needed.

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/contrib/mcelog.te | 2 ++
1 file changed, 2 insertions(+)

diff -pruN refpolicy-08092012/policy/modules/contrib/mcelog.te
refpolicy-08092012-mcelog-allow_files_search_etc_t/policy/modules/contrib/mcelog.te
--- refpolicy-08092012/policy/modules/contrib/mcelog.te 2012-08-08
21:22:01.160888610 +0200
+++
refpolicy-08092012-mcelog-allow_files_search_etc_t/policy/modules/contrib/mcelog.te
2012-08-09 23:33:25.321471690 +0200
@@ -76,6 +76,7 @@ allow mcelog_t self:unix_stream_socket c
allow mcelog_t mcelog_etc_t:dir list_dir_perms;

read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
+files_search_etc(mcelog_t)

# manage a logfile in a generic or private log directory
manage_dirs_pattern(mcelog_t, mcelog_log_t, mcelog_log_t)
@@ -92,6 +93,7 @@ dev_read_raw_memory(mcelog_t)
dev_read_kmsg(mcelog_t)
dev_rw_sysfs(mcelog_t)

+# needed in daemon mode only
files_read_etc_files(mcelog_t)

# for /dev/mem access

2012-08-10 14:47:35

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

On 08/09/12 17:54, Guido Trentalancia wrote:
> On 09/08/2012 18:34, Christopher J. PeBenito wrote:
>> On 08/08/12 15:33, Guido Trentalancia wrote:
>>> --- refpolicy/policy/modules/contrib/mcelog.te 2012-08-08 21:22:01.160888610 +0200
>>> +++ refpolicy-08082012/policy/modules/contrib/mcelog.te 2012-08-08 21:22:19.204057838 +0200
>>> @@ -75,6 +75,7 @@ allow mcelog_t self:capability sys_admin
>>> allow mcelog_t self:unix_stream_socket connected_socket_perms;
>>> allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>>>
>>> +files_read_etc_files(mcelog_t)
>>> read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>>>
>>> # manage a logfile in a generic or private log directory
>>> @@ -92,8 +93,6 @@ dev_read_raw_memory(mcelog_t)
>>> dev_read_kmsg(mcelog_t)
>>> dev_rw_sysfs(mcelog_t)
>>>
>>> -files_read_etc_files(mcelog_t)
>>> -
>>> # for /dev/mem access
>>> mls_file_read_all_levels(mcelog_t)
>>
>> This isn't necessary. The reading etc files stands on its own where it is. If we want to be really thorough, you could add files_search_etc() by the read_files_pattern, but I think its fine as is.
>
> Yes, why not ? I have also added a comment to be 100% transparent to the
> user, if anybodys want to further restrict it under particular
> circumstances:

Reading etc_t files is not a required access for reading mcelog_etc_t files. If mcelog does not read etc_t files, then moving the line makes sense, if its also changed to files_search_etc().


--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2012-08-10 19:27:54

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

Hello Christopher.

On 10/08/2012 16:47, Christopher J. PeBenito wrote:
> On 08/09/12 17:54, Guido Trentalancia wrote:
>> On 09/08/2012 18:34, Christopher J. PeBenito wrote:
>>> On 08/08/12 15:33, Guido Trentalancia wrote:
>>>> --- refpolicy/policy/modules/contrib/mcelog.te 2012-08-08 21:22:01.160888610 +0200
>>>> +++ refpolicy-08082012/policy/modules/contrib/mcelog.te 2012-08-08 21:22:19.204057838 +0200
>>>> @@ -75,6 +75,7 @@ allow mcelog_t self:capability sys_admin
>>>> allow mcelog_t self:unix_stream_socket connected_socket_perms;
>>>> allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>>>>
>>>> +files_read_etc_files(mcelog_t)
>>>> read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>>>>
>>>> # manage a logfile in a generic or private log directory
>>>> @@ -92,8 +93,6 @@ dev_read_raw_memory(mcelog_t)
>>>> dev_read_kmsg(mcelog_t)
>>>> dev_rw_sysfs(mcelog_t)
>>>>
>>>> -files_read_etc_files(mcelog_t)
>>>> -
>>>> # for /dev/mem access
>>>> mls_file_read_all_levels(mcelog_t)
>>>
>>> This isn't necessary. The reading etc files stands on its own where it is. If we want to be really thorough, you could add files_search_etc() by the read_files_pattern, but I think its fine as is.
>>
>> Yes, why not ? I have also added a comment to be 100% transparent to the
>> user, if anybodys want to further restrict it under particular
>> circumstances:
>
> Reading etc_t files is not a required access for reading mcelog_etc_t files. If mcelog does not read etc_t files, then moving the line makes sense, if its also changed to files_search_etc().

I think it might need to read the passwd database in daemon mode only
for uid/gid. Perhaps there is a more specific interface for doing so...

Regards,

Guido

2012-08-14 12:23:36

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH v6]: mcelog module initial rewrite

On 08/10/12 15:27, Guido Trentalancia wrote:
> On 10/08/2012 16:47, Christopher J. PeBenito wrote:
>> On 08/09/12 17:54, Guido Trentalancia wrote:
>>> On 09/08/2012 18:34, Christopher J. PeBenito wrote:
>>>> On 08/08/12 15:33, Guido Trentalancia wrote:
>>>>> --- refpolicy/policy/modules/contrib/mcelog.te 2012-08-08 21:22:01.160888610 +0200
>>>>> +++ refpolicy-08082012/policy/modules/contrib/mcelog.te 2012-08-08 21:22:19.204057838 +0200
>>>>> @@ -75,6 +75,7 @@ allow mcelog_t self:capability sys_admin
>>>>> allow mcelog_t self:unix_stream_socket connected_socket_perms;
>>>>> allow mcelog_t mcelog_etc_t:dir list_dir_perms;
>>>>>
>>>>> +files_read_etc_files(mcelog_t)
>>>>> read_files_pattern(mcelog_t, mcelog_etc_t, mcelog_etc_t)
>>>>>
>>>>> # manage a logfile in a generic or private log directory
>>>>> @@ -92,8 +93,6 @@ dev_read_raw_memory(mcelog_t)
>>>>> dev_read_kmsg(mcelog_t)
>>>>> dev_rw_sysfs(mcelog_t)
>>>>>
>>>>> -files_read_etc_files(mcelog_t)
>>>>> -
>>>>> # for /dev/mem access
>>>>> mls_file_read_all_levels(mcelog_t)
>>>>
>>>> This isn't necessary. The reading etc files stands on its own where it is. If we want to be really thorough, you could add files_search_etc() by the read_files_pattern, but I think its fine as is.
>>>
>>> Yes, why not ? I have also added a comment to be 100% transparent to the
>>> user, if anybodys want to further restrict it under particular
>>> circumstances:
>>
>> Reading etc_t files is not a required access for reading mcelog_etc_t files. If mcelog does not read etc_t files, then moving the line makes sense, if its also changed to files_search_etc().
>
> I think it might need to read the passwd database in daemon mode only for uid/gid. Perhaps there is a more specific interface for doing so...

There isn't, since /etc/passwd is etc_t.

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com