Subject: Re: [PATCH cron 2/2] user_crontab_t etc
To:     Russell Coker <russell@coker.com.au>,
        selinux-refpolicy@vger.kernel.org
References: <20190107031005.GA13945@aaa.coker.com.au>
From:   Chris PeBenito <pebenito@ieee.org>
Message-ID: <b56674cc-183d-630e-defe-bf93237485a3@ieee.org>
Date:   Mon, 7 Jan 2019 18:47:27 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.3.0
MIME-Version: 1.0
In-Reply-To: <20190107031005.GA13945@aaa.coker.com.au>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk

On 1/6/19 10:10 PM, Russell Coker wrote:
> This patch adds a $1_crontab_t domain and makes it a compile option for

What is the goal for reintroducing a crontab domain per-user-domain?


> having a $1_cronjob_t domain.
> 
> I anticipate that even if this patch is accepted later on there will be some
> changes required.  Please review this not for inclusion immediately but for
> changes necessary.  However the previous patch is good to go if you like the
> concept.

I'm not keen on this.  The current policy is intended to make it easy to 
decide if you want to use a *_cronjob_t domain or simply transition to 
the user's domain by tweaking the default_contexts.


> Index: refpolicy-2.20180701/policy/modules/services/cron.if
> ===================================================================
> --- refpolicy-2.20180701.orig/policy/modules/services/cron.if
> +++ refpolicy-2.20180701/policy/modules/services/cron.if
> @@ -21,23 +21,33 @@ template(`cron_common_crontab_template',
>   	# Declarations
>   	#
>   
> -	type $1_t, crontab_domain;
> -	userdom_user_application_domain($1_t, crontab_exec_t)
> +	type $1_crontab_t, crontab_domain;
> +	userdom_user_application_domain($1_crontab_t, crontab_exec_t)
>   
> -	type $1_tmp_t;
> -	userdom_user_tmp_file($1_tmp_t)
> +	type $1_crontab_tmp_t;
> +	userdom_user_tmp_file($1_crontab_tmp_t)
> +
> +	type $1_cron_spool_t, cron_spool_type;
>   
>   	##############################
>   	#
>   	# Local policy
>   	#
>   
> -	manage_dirs_pattern($1_t, $1_tmp_t, $1_tmp_t)
> -	manage_files_pattern($1_t, $1_tmp_t, $1_tmp_t)
> -	files_tmp_filetrans($1_t, $1_tmp_t, { dir file })
> +	manage_dirs_pattern($1_crontab_t, $1_crontab_tmp_t, $1_crontab_tmp_t)
> +	manage_files_pattern($1_crontab_t, $1_crontab_tmp_t, $1_crontab_tmp_t)
> +	files_tmp_filetrans($1_crontab_t, $1_crontab_tmp_t, { dir file })
> +
> +	auth_domtrans_chk_passwd($1_crontab_t)
> +	auth_use_nsswitch($1_crontab_t)
> +	allow $1_crontab_t self:capability fsetid;
> +
> +	files_type($1_cron_spool_t)
> +	ubac_constrained($1_cron_spool_t)
> +	mta_system_content($1_cron_spool_t)
>   
> -	auth_domtrans_chk_passwd($1_t)
> -	auth_use_nsswitch($1_t)
> +	manage_files_pattern($1_crontab_t, { cron_spool_t user_cron_spool_t }, $1_cron_spool_t)
> +	filetrans_pattern($1_crontab_t, cron_spool_t, $1_cron_spool_t, file)
>   ')
>   
>   ########################################
> @@ -58,9 +68,11 @@ template(`cron_common_crontab_template',
>   #
>   interface(`cron_role',`
>   	gen_require(`
> +ifdef(`cronjob_domain', `
>   		type cronjob_t;
> +')
>   		type crontab_exec_t, crond_t;
> -		type crontab_t, user_cron_spool_t;
> +		type $2_crontab_t, $2_cron_spool_t;
>   		bool cron_userdomain_transition;
>   	')
>   
> @@ -69,60 +81,51 @@ interface(`cron_role',`
>   	# Declarations
>   	#
>   
> +ifdef(`cronjob_domain', `
>   	role $1 types { cronjob_t };
> -	role $1 types { crontab_t };
> +')
> +	role $1 types { $2_crontab_t };
>   
>   	##############################
>   	#
>   	# Local policy
>   	#
>   
> -	domtrans_pattern($2_t, crontab_exec_t, crontab_t)
> +	domtrans_pattern($2_t, crontab_exec_t, $2_crontab_t)
>   
>   	dontaudit crond_t $2_t:process { noatsecure siginh rlimitinh };
>   	allow $2_t crond_t:process sigchld;
>   
> -	allow $2_t user_cron_spool_t:file { getattr read write ioctl };
> +	allow $2_t $2_cron_spool_t:file { getattr read write ioctl };
>   
> -	allow $2_t crontab_t:process { ptrace signal_perms };
> -	ps_process_pattern($2_t, crontab_t)
> +	allow $2_t $2_crontab_t:process { ptrace signal_perms };
> +	ps_process_pattern($2_t, $2_crontab_t)
>   
> -	corecmd_exec_bin(crontab_t)
> -	corecmd_exec_shell(crontab_t)
> +	corecmd_exec_bin($2_crontab_t)
> +	corecmd_exec_shell($2_crontab_t)
>   
> +ifndef(`cronjob_domain', `
>   	tunable_policy(`cron_userdomain_transition',`
> +')
>   		allow crond_t $2_t:process transition;
>   		allow crond_t $2_t:fd use;
>   		allow crond_t $2_t:key manage_key_perms;
>   
> -		allow $2_t user_cron_spool_t:file entrypoint;
> +		allow $2_t $2_cron_spool_t:file entrypoint;
>   
>   		allow $2_t crond_t:fifo_file rw_fifo_file_perms;
> -
> -		allow $2_t cronjob_t:process { ptrace signal_perms };
> -		ps_process_pattern($2_t, cronjob_t)
> +ifndef(`cronjob_domain', `
>   	',`
>   		dontaudit crond_t $2_t:process transition;
>   		dontaudit crond_t $2_t:fd use;
>   		dontaudit crond_t $2_t:key manage_key_perms;
>   
> -		dontaudit $2_t user_cron_spool_t:file entrypoint;
> +		dontaudit $2_t $2_cron_spool_t:file entrypoint;
>   
>   		dontaudit $2_t crond_t:fifo_file rw_fifo_file_perms;
> -
> -		dontaudit $2_t cronjob_t:process { ptrace signal_perms };
> -	')
> -
> -	optional_policy(`
> -		gen_require(`
> -			class dbus send_msg;
> -		')
> -
> -		dbus_stub(cronjob_t)
> -
> -		allow cronjob_t $2_t:dbus send_msg;
>   	')
>   ')
> +')
>   
>   ########################################
>   ## <summary>
> @@ -139,6 +142,7 @@ interface(`cron_role',`
>   ##	</summary>
>   ## </param>
>   #
> +ifdef(`cronjob_domain', `
>   interface(`cron_unconfined_role',`
>   	gen_require(`
>   		type unconfined_cronjob_t, crontab_t, crontab_exec_t;
> @@ -204,6 +208,7 @@ interface(`cron_unconfined_role',`
>   		allow unconfined_cronjob_t $2:dbus send_msg;
>   	')
>   ')
> +')
>   
>   ########################################
>   ## <summary>
> Index: refpolicy-2.20180701/policy/modules/services/cron.te
> ===================================================================
> --- refpolicy-2.20180701.orig/policy/modules/services/cron.te
> +++ refpolicy-2.20180701/policy/modules/services/cron.te
> @@ -25,7 +25,9 @@ gen_tunable(cron_can_relabel, false)
>   ##	the generic cronjob domain.
>   ##	</p>
>   ## </desc>
> -gen_tunable(cron_userdomain_transition, false)
> +ifndef(`cronjob_domain', `
> +gen_tunable(cron_userdomain_transition, true)
> +')
>   
>   ## <desc>
>   ##	<p>
> @@ -83,15 +85,16 @@ files_pid_file(crond_var_run_t)
>   type crontab_exec_t;
>   application_executable_file(crontab_exec_t)
>   
> -cron_common_crontab_template(admin_crontab)
> -typealias admin_crontab_t alias sysadm_crontab_t;
> -typealias admin_crontab_tmp_t alias sysadm_crontab_tmp_t;
> -
> -cron_common_crontab_template(crontab)
> -typealias crontab_t alias { user_crontab_t staff_crontab_t };
> -typealias crontab_t alias { auditadm_crontab_t secadm_crontab_t };
> -typealias crontab_tmp_t alias { user_crontab_tmp_t staff_crontab_tmp_t };
> -typealias crontab_tmp_t alias { auditadm_crontab_tmp_t secadm_crontab_tmp_t };
> +cron_common_crontab_template(sysadm)
> +typealias sysadm_crontab_t alias admin_crontab_t;
> +typealias sysadm_crontab_tmp_t alias admin_crontab_tmp_t;
> +
> +cron_common_crontab_template(user)
> +cron_common_crontab_template(staff)
> +cron_common_crontab_template(unconfined)
> +typealias user_crontab_t alias { crontab_t };
> +typealias sysadm_crontab_t alias { auditadm_crontab_t secadm_crontab_t };
> +typealias sysadm_crontab_tmp_t alias { auditadm_crontab_tmp_t secadm_crontab_tmp_t };
>   
>   type system_cron_spool_t, cron_spool_type;
>   files_type(system_cron_spool_t)
> @@ -113,11 +116,7 @@ files_type(system_cronjob_var_lib_t)
>   type system_cronjob_var_run_t;
>   files_pid_file(system_cronjob_var_run_t)
>   
> -type user_cron_spool_t, cron_spool_type;
> -typealias user_cron_spool_t alias { staff_cron_spool_t sysadm_cron_spool_t unconfined_cron_spool_t };
> -typealias user_cron_spool_t alias { auditadm_cron_spool_t secadm_cron_spool_t };
> -files_type(user_cron_spool_t)
> -ubac_constrained(user_cron_spool_t)
> +typealias sysadm_cron_spool_t alias { auditadm_cron_spool_t secadm_cron_spool_t };
>   
>   type user_cron_spool_log_t;
>   logging_log_file(user_cron_spool_log_t)
> @@ -145,9 +144,6 @@ allow crontab_domain self:capability { c
>   allow crontab_domain self:process { getcap setsched signal_perms };
>   allow crontab_domain self:fifo_file rw_fifo_file_perms;
>   
> -manage_files_pattern(crontab_domain, { cron_spool_t user_cron_spool_t }, user_cron_spool_t)
> -filetrans_pattern(crontab_domain, cron_spool_t, user_cron_spool_t, file)
> -
>   allow crontab_domain cron_spool_t:dir setattr_dir_perms;
>   
>   allow crontab_domain crond_t:process signal;
> @@ -216,8 +212,8 @@ tunable_policy(`fcron_crond',`
>   # Daemon local policy
>   #
>   
> -allow crond_t self:capability { chown dac_override dac_read_search fowner setgid setuid sys_nice };
> -dontaudit crond_t self:capability { sys_resource sys_tty_config };
> +allow crond_t self:capability { chown dac_override dac_read_search fowner setgid setuid sys_nice sys_resource };
> +dontaudit crond_t self:capability { sys_tty_config };
>   
>   allow crond_t self:process { transition signal_perms getsched setsched getsession getpgid setpgid getcap setcap share getattr setexec setfscreate noatsecure siginh setrlimit rlimitinh dyntransition setkeycreate setsockcreate getrlimit };
>   allow crond_t self:fd use;
> @@ -231,6 +227,7 @@ allow crond_t self:msg { send receive };
>   allow crond_t self:key { search write link };
>   dontaudit crond_t self:netlink_audit_socket nlmsg_tty_audit;
>   
> +allow crond_t cron_spool_type:file read_file_perms;
>   allow crond_t cron_log_t:file { append_file_perms create_file_perms setattr_file_perms };
>   logging_log_filetrans(crond_t, cron_log_t, file)
>   
> 


-- 
Chris PeBenito