Received-SPF: pass (google.com: domain of selinux-refpolicy-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <fe8cdcaa-cfcd-75e5-783a-3fc2a441c8fe@ieee.org>
Date:   Wed, 27 Oct 2021 09:09:07 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
From:   Chris PeBenito <pebenito@ieee.org>
Subject: Re: [PATCH] another systemd misc patch
To:     Russell Coker <russell@coker.com.au>,
        selinux-refpolicy@vger.kernel.org
References: <YWFphFQ8fZmbEdGL@xev.coker.com.au>
Content-Language: en-US
In-Reply-To: <YWFphFQ8fZmbEdGL@xev.coker.com.au>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/9/21 06:05, Russell Coker wrote:
> Here's the latest version of this patch with the previous issues addressed.
> 
> Signed-off-by: Russell Coker <russell@coker.com.au>
> 
> Index: refpolicy-2.20210908/policy/modules/system/systemd.if
> ===================================================================
> --- refpolicy-2.20210908.orig/policy/modules/system/systemd.if
> +++ refpolicy-2.20210908/policy/modules/system/systemd.if
> @@ -1911,3 +1971,45 @@ interface(`systemd_use_inherited_machine
>   	allow $1 systemd_machined_t:fd use;
>   	allow $1 systemd_machined_devpts_t:chr_file rw_inherited_term_perms;
>   ')
> +
> +########################################
> +## <summary>
> +##  run systemd-nspawn in systemd_nspawn_t domain
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +## <param name="role">
> +##      <summary>
> +##      The role  of the object to create.
> +##      </summary>
> +## </param>
> +#
> +interface(`systemd_run_nspawn', `
> +	gen_require(`
> +		type systemd_nspawn_t, systemd_nspawn_exec_t;
> +	')
> +
> +	role $2 types systemd_nspawn_t;
> +	domtrans_pattern($1, systemd_nspawn_exec_t, systemd_nspawn_t)
> +')

What is the use case?  I see it later in the patch run by sysadm_t, but I don't 
understand why sysadm would run it directly, instead of using the systemctl.


> Index: refpolicy-2.20210908/policy/modules/system/systemd.te
> ===================================================================
> --- refpolicy-2.20210908.orig/policy/modules/system/systemd.te
> +++ refpolicy-2.20210908/policy/modules/system/systemd.te
> @@ -427,6 +445,7 @@ logging_send_syslog_msg(systemd_coredump
>   
>   seutil_search_default_contexts(systemd_coredump_t)
>   
> +
>   #######################################
>   #
>   # Systemd generator local policy

Please remove the extra endline.


> @@ -436,26 +455,44 @@ allow systemd_generator_t self:fifo_file
>   allow systemd_generator_t self:capability dac_override;
>   allow systemd_generator_t self:process setfscreate;
>   
> +allow systemd_generator_t self:tcp_socket create;
> +allow systemd_generator_t self:udp_socket create;

Create sockets but do nothing with them? i.e. read/write/ioctl


> +allow systemd_generator_t self:netlink_route_socket { create read bind getattr write nlmsg_read };
> +
>   allow systemd_generator_t systemd_unit_t:file getattr;
>   
> +kernel_dontaudit_getattr_proc(systemd_generator_t)
> +kernel_read_kernel_sysctls(systemd_generator_t)
> +kernel_read_network_state(systemd_generator_t)
> +kernel_read_system_state(systemd_generator_t)
> +kernel_search_network_sysctl(systemd_generator_t)
> +kernel_use_fds(systemd_generator_t)
> +
> +corecmd_exec_bin(systemd_generator_t)
>   corecmd_exec_shell(systemd_generator_t)
> -corecmd_getattr_bin_files(systemd_generator_t)
>   
>   dev_read_sysfs(systemd_generator_t)
> +dev_read_urand(systemd_generator_t)
>   dev_write_kmsg(systemd_generator_t)
>   dev_write_sysfs_dirs(systemd_generator_t)
>   
> -files_read_etc_files(systemd_generator_t)
> +application_exec(systemd_generator_t)
> +domain_read_all_entry_files(systemd_generator_t)

These last two could use blank lines for separation.


[...]
> @@ -974,14 +1047,29 @@ allow systemd_nspawn_t systemd_nspawn_tm
>   # for /run/systemd/nspawn/incoming in chroot
>   allow systemd_nspawn_t systemd_nspawn_runtime_t:dir mounton;
>   
> +term_create_pty(systemd_nspawn_t, systemd_nspawn_devpts_t)
> +allow systemd_nspawn_t systemd_nspawn_devpts_t:chr_file manage_chr_file_perms;

Please move these up after the self block of rules.


> +kernel_getattr_core_if(systemd_nspawn_t)
> +kernel_getattr_proc(systemd_nspawn_t)
> +kernel_getattr_unlabeled_dirs(systemd_nspawn_t)
> +
>   kernel_mount_proc(systemd_nspawn_t)
>   kernel_mounton_sysctl_dirs(systemd_nspawn_t)
>   kernel_mounton_kernel_sysctl_files(systemd_nspawn_t)
>   kernel_mounton_message_if(systemd_nspawn_t)
>   kernel_mounton_proc(systemd_nspawn_t)
> +kernel_mounton_sysctl_files(systemd_nspawn_t)
> +kernel_mounton_unlabeled_dirs(systemd_nspawn_t)
> +
> +kernel_read_irq_sysctls(systemd_nspawn_t)
> +kernel_read_network_state(systemd_nspawn_t)
>   kernel_read_kernel_sysctls(systemd_nspawn_t)
> +kernel_read_sysctl(systemd_nspawn_t)
>   kernel_read_system_state(systemd_nspawn_t)
>   kernel_remount_proc(systemd_nspawn_t)
> +kernel_request_load_module(systemd_nspawn_t)
> +kernel_search_network_sysctl(systemd_nspawn_t)

Please remove the extra newlines.

>   corecmd_exec_shell(systemd_nspawn_t)
>   corecmd_search_bin(systemd_nspawn_t)
> @@ -998,6 +1086,7 @@ dev_read_sysfs(systemd_nspawn_t)
>   dev_read_rand(systemd_nspawn_t)
>   dev_read_urand(systemd_nspawn_t)
>   
> +files_getattr_default_dirs(systemd_nspawn_t)
>   files_getattr_tmp_dirs(systemd_nspawn_t)
>   files_manage_etc_files(systemd_nspawn_t)
>   files_manage_mnt_dirs(systemd_nspawn_t)
> @@ -1009,11 +1098,17 @@ files_setattr_runtime_dirs(systemd_nspaw
>   
>   fs_getattr_cgroup(systemd_nspawn_t)
>   fs_getattr_tmpfs(systemd_nspawn_t)
> +fs_getattr_xattr_fs(systemd_nspawn_t)
> +fs_manage_cgroup_dirs(systemd_nspawn_t)
> +fs_manage_cgroup_files(systemd_nspawn_t)
> +fs_manage_tmpfs_blk_files(systemd_nspawn_t)
>   fs_manage_tmpfs_chr_files(systemd_nspawn_t)
> +fs_mount_cgroup(systemd_nspawn_t)
>   fs_mount_tmpfs(systemd_nspawn_t)
> +fs_mounton_cgroup(systemd_nspawn_t)
> +fs_read_nsfs_files(systemd_nspawn_t)
>   fs_remount_tmpfs(systemd_nspawn_t)
>   fs_remount_xattr_fs(systemd_nspawn_t)
> -fs_read_cgroup_files(systemd_nspawn_t)
>   
>   term_getattr_generic_ptys(systemd_nspawn_t)
>   term_getattr_pty_fs(systemd_nspawn_t)
> @@ -1021,6 +1116,7 @@ term_mount_devpts(systemd_nspawn_t)
>   term_search_ptys(systemd_nspawn_t)
>   term_setattr_generic_ptys(systemd_nspawn_t)
>   term_use_ptmx(systemd_nspawn_t)
> +term_use_generic_ptys(systemd_nspawn_t)
>   
>   init_domtrans_script(systemd_nspawn_t)
>   init_getrlimit(systemd_nspawn_t)
> @@ -1031,8 +1127,12 @@ init_write_runtime_socket(systemd_nspawn
>   init_spec_domtrans_script(systemd_nspawn_t)
>   
>   miscfiles_manage_localization(systemd_nspawn_t)
> +mount_exec(systemd_nspawn_t)
> +
>   udev_read_runtime_files(systemd_nspawn_t)
>   
> +sysnet_exec_ifconfig(systemd_nspawn_t)
> +
>   # for writing inside chroot
>   sysnet_manage_config(systemd_nspawn_t)

With all the mountons, it seems to make sense to switch it to mount on 
init_mountpoint_type.  See init.te:262, which is what we have for systemd.


[...]
> @@ -1491,6 +1599,11 @@ tunable_policy(`systemd_tmpfilesd_factor
>   ')
>   
>   optional_policy(`
> +	colord_read_lib_files(systemd_tmpfiles_t)
> +	colord_relabel_lib(systemd_tmpfiles_t)
> +')

Instead of new interfaces and calling here, you should add 
systemd_tmpfilesd_managed(colord_var_lib_t) in colord.te.


> Index: refpolicy-2.20210908/policy/modules/services/ssh.te
> ===================================================================
> --- refpolicy-2.20210908.orig/policy/modules/services/ssh.te
> +++ refpolicy-2.20210908/policy/modules/services/ssh.te
> @@ -270,6 +270,7 @@ ifdef(`init_systemd',`
>   	auth_use_pam_systemd(sshd_t)
>   	init_dbus_chat(sshd_t)
>   	init_rw_stream_sockets(sshd_t)
> +	systemd_dgram_nspawn(sshd_t)
>   	systemd_write_inherited_logind_sessions_pipes(sshd_t)
>   ')

Is this sshd running inside a namespace started by nspawn?

-- 
Chris PeBenito