Received-SPF: softfail (google.com: domain of transitioning selinux-refpolicy-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <e80b4363-6d18-3866-630a-961525fcfe47@linux.microsoft.com>
Date:   Thu, 17 Feb 2022 09:54:19 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH] rasdaemon V2
Content-Language: en-US
To:     Russell Coker <russell@coker.com.au>,
        selinux-refpolicy@vger.kernel.org
References: <Yg5fx37xkpo1WVX6@xev.coker.com.au>
From:   Daniel Burgener <dburgener@linux.microsoft.com>
In-Reply-To: <Yg5fx37xkpo1WVX6@xev.coker.com.au>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/17/2022 9:46 AM, Russell Coker wrote:
> Same as before but with the needed summary and removed the obsolete lockdown
> rule.
> 
> Should be ready for merging now.
> Signed-off-by: Russell Coker <russell@coker.com.au>

I thought this sounded familiar, so I searched my inbox, and it looks
like you submitted this same patch a year ago, and there was feedback
from Chris and Dominick that doesn't seem to be addressed yet.  I'll
try to capture it all inline below.

> 
> Index: refpolicy-2.20220217/policy/modules/kernel/filesystem.if
> ===================================================================
> --- refpolicy-2.20220217.orig/policy/modules/kernel/filesystem.if
> +++ refpolicy-2.20220217/policy/modules/kernel/filesystem.if
> @@ -5485,6 +5485,43 @@ interface(`fs_getattr_tracefs_files',`
>   
>   ########################################
>   ## <summary>
> +##	Read/write trace filesystem files
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`fs_write_tracefs_files',`
> +	gen_require(`
> +		type tracefs_t;
> +	')
> +
> +	allow $1 tracefs_t:dir list_dir_perms;
> +	allow $1 tracefs_t:file rw_file_perms;
> +')
> +
> +########################################
> +## <summary>
> +##	create trace filesystem directories
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`fs_create_tracefs_dirs',`
> +	gen_require(`
> +		type tracefs_t;
> +	')
> +
> +	allow $1 tracefs_t:dir { create rw_dir_perms };
> +')
> +
> +########################################
> +## <summary>
>   ##	Mount a XENFS filesystem.
>   ## </summary>
>   ## <param name="domain">
> Index: refpolicy-2.20220217/policy/modules/services/rasdaemon.fc
> ===================================================================
> --- /dev/null
> +++ refpolicy-2.20220217/policy/modules/services/rasdaemon.fc
> @@ -0,0 +1,3 @@
> +/usr/sbin/rasdaemon			--	gen_context(system_u:object_r:rasdaemon_exec_t,s0)
> +/var/lib/rasdaemon(/.*)?			gen_context(system_u:object_r:rasdaemon_var_t,s0)
> +
> Index: refpolicy-2.20220217/policy/modules/services/rasdaemon.if
> ===================================================================
> --- /dev/null
> +++ refpolicy-2.20220217/policy/modules/services/rasdaemon.if
> @@ -0,0 +1 @@
> +## <summary>RAS (Reliability, Availability and Serviceability) logging tool</summary>
> Index: refpolicy-2.20220217/policy/modules/services/rasdaemon.te
> ===================================================================
> --- /dev/null
> +++ refpolicy-2.20220217/policy/modules/services/rasdaemon.te
> @@ -0,0 +1,47 @@
> +policy_module(rasdaemon, 1.0.0)
> +
> +# rasdaemon is a RAS (Reliability, Availability and Serviceability) logging
> +# tool.  It currently records memory errors, using the EDAC tracing events.
> +# EDAC are drivers in the Linux kernel that handle detection of ECC errors
> +# from memory controllers for most chipsets on x86 and ARM architectures.
> +#
> +# https://git.infradead.org/users/mchehab/rasdaemon.git

This can get wrapped in xml <summary> and <desc> tags so it gets put in 
docs.

> +
> +########################################
> +#
> +# Declarations
> +#
> +
> +type rasdaemon_t;
> +type rasdaemon_exec_t;
> +init_daemon_domain(rasdaemon_t, rasdaemon_exec_t)
> +
> +type rasdaemon_var_t;
> +files_type(rasdaemon_var_t)
> +
> +########################################
> +#
> +# Local policy
> +#
> +
> +allow rasdaemon_t self:unix_dgram_socket create_socket_perms;

This is redundant, implied by logging_send_syslog_message()

> +
> +allow rasdaemon_t rasdaemon_var_t:dir manage_dir_perms;
> +allow rasdaemon_t rasdaemon_var_t:file manage_file_perms;
> +
> +kernel_read_debugfs(rasdaemon_t)
> +kernel_read_system_state(rasdaemon_t)
> +kernel_read_vm_overcommit_sysctl(rasdaemon_t)
> +kernel_search_fs_sysctls(rasdaemon_t)
> +
> +dev_list_sysfs(rasdaemon_t)
> +dev_read_urand(rasdaemon_t)
> +
> +files_read_etc_symlinks(rasdaemon_t)

This is redundant (implied by miscfiles_read_localization)

> +files_search_var_lib(rasdaemon_t)
> +fs_write_tracefs_files(rasdaemon_t)
> +fs_create_tracefs_dirs(rasdaemon_t)
> +
> +logging_send_syslog_msg(rasdaemon_t)
> +miscfiles_read_localization(rasdaemon_t)
> +