Received-SPF: pass (google.com: best guess record for domain of selinux-refpolicy-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: Chrome/Chromium patches
To:     Jason Zaman <jason@perfinion.com>,
        Russell Coker <russell@coker.com.au>
Cc:     "selinux-refpolicy@vger.kernel.org" 
        <selinux-refpolicy@vger.kernel.org>
References: <4241549.o4G3l8VaYd@xev>
 <20200216100021.GA25357@baraddur.perfinion.com>
From:   Chris PeBenito <pebenito@ieee.org>
Message-ID: <56ad4024-71c7-30cf-eba4-997a585d07b3@ieee.org>
Date:   Sun, 16 Feb 2020 10:54:27 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.3.1
MIME-Version: 1.0
In-Reply-To: <20200216100021.GA25357@baraddur.perfinion.com>
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 2/16/20 5:00 AM, Jason Zaman wrote:
> On Wed, Feb 12, 2020 at 01:30:39PM +1100, Russell Coker wrote:
>> The attached patch against the git refpolicy from 3 days ago makes Chrome work
>> with Pulseaudio, DRI, Flash, and lots of little things.
> 
> Flash is dead. My chrome even pops up a banner every time i start it
> saying flash is disabled by default and will be completely removed in a
> few months. Not sure adding the flash is worth it.

Agreed.  If there are any flash-specific rules, please remove them.


>> Adds tunables chromium_exec_plugins and xserver_allow_dri.
>>
>> I think this is ready to merge with the names of the tunables and interfaces
>> being the only possible changes needed.
>>
>> -- 
>> My Main Blog         http://etbe.coker.com.au/
>> My Documents Blog    http://doc.coker.com.au/
> 
>> Chromium and DRI policy
>>
>> Index: refpolicy-2.20200209/policy/modules/apps/chromium.te
>> ===================================================================
>> --- refpolicy-2.20200209.orig/policy/modules/apps/chromium.te
>> +++ refpolicy-2.20200209/policy/modules/apps/chromium.te
>> @@ -39,6 +39,13 @@ gen_tunable(chromium_bind_tcp_unreserved
>>   ## </desc>
>>   gen_tunable(chromium_rw_usb_dev, false)
>>   
>> +## <desc>
>> +## <p>
>> +## Allow chromium to execute it's config (for plugins like Flash)
>> +## </p>
>> +## </desc>
>> +gen_tunable(chromium_exec_plugins, false)
>> +
>>   type chromium_t;
>>   domain_dyntrans_type(chromium_t)
>>   
>> @@ -63,6 +70,9 @@ type chromium_tmpfs_t;
>>   userdom_user_tmpfs_file(chromium_tmpfs_t)
>>   optional_policy(`
>>   	pulseaudio_tmpfs_content(chromium_tmpfs_t)
>> +	pulseaudio_rw_tmpfs_files(chromium_t)
>> +	pulseaudio_stream_connect(chromium_t)
>> +	pulseaudio_use_fds(chromium_t)
>>   ')
>>   
>>   type chromium_xdg_config_t;
>> @@ -77,7 +87,9 @@ xdg_cache_content(chromium_xdg_cache_t)
>>   #
>>   
>>   # execmem for load in plugins
>> -allow chromium_t self:process { execmem getsched getcap setcap setrlimit setsched sigkill signal };
>> +allow chromium_t self:process { execmem getsched getcap setcap setrlimit setsched sigkill signal signull };
>> +allow chromium_t self:dir { write add_name };
>> +allow chromium_t self:file create;
>>   allow chromium_t self:fifo_file rw_fifo_file_perms;
>>   allow chromium_t self:sem create_sem_perms;
>>   allow chromium_t self:netlink_kobject_uevent_socket client_stream_socket_perms;
>> @@ -96,6 +108,7 @@ allow chromium_t chromium_renderer_t:uni
>>   
>>   allow chromium_t chromium_sandbox_t:unix_dgram_socket { getattr read write };
>>   allow chromium_t chromium_sandbox_t:unix_stream_socket { getattr read write };
>> +allow chromium_t chromium_sandbox_t:file read_file_perms;
>>   
>>   allow chromium_t chromium_naclhelper_t:process { share };
>>   
>> @@ -108,6 +121,9 @@ manage_sock_files_pattern(chromium_t, ch
>>   manage_fifo_files_pattern(chromium_t, chromium_tmp_t, chromium_tmp_t)
>>   files_tmp_filetrans(chromium_t, chromium_tmp_t, { file dir sock_file })
>>   
>> +# for /run/user/$UID
>> +userdom_user_runtime_filetrans(chromium_t, chromium_tmp_t, { file sock_file })
>> +
>>   manage_files_pattern(chromium_t, chromium_tmpfs_t, chromium_tmpfs_t)
>>   allow chromium_t chromium_tmpfs_t:file map;
>>   fs_tmpfs_filetrans(chromium_t, chromium_tmpfs_t, file)
>> @@ -128,7 +144,11 @@ dyntrans_pattern(chromium_t, chromium_re
>>   domtrans_pattern(chromium_t, chromium_sandbox_exec_t, chromium_sandbox_t)
>>   domtrans_pattern(chromium_t, chromium_naclhelper_exec_t, chromium_naclhelper_t)
>>   
>> +# for self:file create
>> +kernel_associate_proc(chromium_t)

Where under /proc/self does this work?

>>   kernel_list_proc(chromium_t)
>> +kernel_read_kernel_sysctls(chromium_t)
>>   kernel_read_net_sysctls(chromium_t)
>>   
>>   corecmd_exec_bin(chromium_t)
>> @@ -145,6 +165,9 @@ dev_read_sound(chromium_t)
>>   dev_write_sound(chromium_t)
>>   dev_read_urand(chromium_t)
>>   dev_read_rand(chromium_t)
>> +tunable_policy(`xserver_allow_dri', `
>> +	dev_rw_dri(chromium_t)
>> +')
>>   dev_rw_xserver_misc(chromium_t)
>>   dev_map_xserver_misc(chromium_t)
>>   
>> @@ -178,14 +201,15 @@ userdom_use_user_terminals(chromium_t)
>>   userdom_manage_user_certs(chromium_t)
>>   userdom_user_home_dir_filetrans_user_cert(chromium_t, dir, ".pki")
>>   
>> -xdg_create_cache_dirs(chromium_t)
>> -xdg_create_config_dirs(chromium_t)
>> -xdg_create_data_dirs(chromium_t)
>> +xdg_manage_cache(chromium_t)
>> +xdg_manage_config(chromium_t)
>> +xdg_manage_data(chromium_t)
> 
> NAK. Chrome should only be able to create the base ~/.config/cache dirs,
> not manage other things inside them.
> The template userdom_user_content_access_template is for this exact
> thing. By default chrome will have no access then the template generates
> booleans the user can easily toggle to enable access. That stuff was
> done specifically to confine things like browsers.
> 
>>   xdg_manage_downloads(chromium_t)
>> -xdg_read_config_files(chromium_t)
>> -xdg_read_data_files(chromium_t)
>>   
>>   xserver_user_x_domain_template(chromium, chromium_t, chromium_tmpfs_t)
>> +xserver_stream_connect_xdm(chromium_t)
>> +
>> +xserver_manage_mesa_shader_cache(chromium_t)
>>   
>>   tunable_policy(`chromium_bind_tcp_unreserved_ports',`
>>   	corenet_tcp_bind_generic_node(chromium_t)
>> @@ -198,6 +222,11 @@ tunable_policy(`chromium_rw_usb_dev',`
>>   	udev_read_db(chromium_t)
>>   ')
>>   
>> +tunable_policy(`chromium_exec_plugins',`
>> +	# sometimes .config/google-chrome/PepperFlash/32.0.0.142/libpepflashplayer.so gets chromium_tmp_t
>> +	can_exec(chromium_t, { chromium_xdg_config_t chromium_tmp_t })
>> +')
>> +
> 
> Not really sure theres much point to adding this just to remove it in 6
> months.
> 
> 
>>   tunable_policy(`chromium_read_system_info',`
>>   	kernel_read_kernel_sysctls(chromium_t)
>>   	# Memory optimizations & optimizations based on OS/version
>> @@ -229,6 +258,10 @@ optional_policy(`
>>   ')
>>   
>>   optional_policy(`
>> +	networkmanager_dbus_chat(chromium_t)
>> +')
>> +
>> +optional_policy(`
>>   	dbus_all_session_bus_client(chromium_t)
>>   	dbus_system_bus_client(chromium_t)
>>   
>> @@ -241,8 +274,13 @@ optional_policy(`
>>   	')
>>   
>>   	optional_policy(`
>> +		devicekit_dbus_chat_disk(chromium_t)
>>   		devicekit_dbus_chat_power(chromium_t)
>>   	')
>> +
>> +	optional_policy(`
>> +		systemd_dbus_chat_hostnamed(chromium_t)
>> +	')
>>   ')
>>   
>>   optional_policy(`
>> @@ -252,6 +290,10 @@ optional_policy(`
>>   	dpkg_read_db(chromium_t)
>>   ')
>>   
>> +optional_policy(`
>> +	ssh_dontaudit_agent_tmp(chromium_t)
>> +')
>> +
>>   ifdef(`use_alsa',`
>>   	optional_policy(`
>>   		alsa_domain(chromium_t, chromium_tmpfs_t)
>> @@ -259,6 +301,7 @@ ifdef(`use_alsa',`
>>   
>>   	optional_policy(`
>>   		pulseaudio_domtrans(chromium_t)
>> +		pulseaudio_read_home(chromium_t)
>>   	')
>>   ')
>>   
>> @@ -299,6 +342,9 @@ userdom_use_user_terminals(chromium_rend
>>   
>>   xdg_read_config_files(chromium_renderer_t)
>>   
>> +# should we have a tunable for this?
>> +xdg_read_pictures(chromium_t)
> 
> I personally don't really want my browser to be able to access my
> photos. A boolean would be good yeah. Maybe name it similar to the ones
> userdom_user_content_access_template makes?
> 
>> +
>>   xserver_user_x_domain_template(chromium_renderer, chromium_renderer_t, chromium_tmpfs_t)
>>   
>>   tunable_policy(`chromium_read_system_info',`
>> @@ -360,3 +406,6 @@ tunable_policy(`chromium_read_system_inf
>>   
>>   dev_read_sysfs(chromium_naclhelper_t)
>>   dev_read_urand(chromium_naclhelper_t)
>> +kernel_list_proc(chromium_naclhelper_t)
>> +
>> +miscfiles_read_localization(chromium_naclhelper_t)
>> Index: refpolicy-2.20200209/policy/modules/kernel/kernel.if
>> ===================================================================
>> --- refpolicy-2.20200209.orig/policy/modules/kernel/kernel.if
>> +++ refpolicy-2.20200209/policy/modules/kernel/kernel.if
>> @@ -2424,6 +2424,24 @@ interface(`kernel_rw_all_sysctls',`
>>   
>>   ########################################
>>   ## <summary>
>> +##	Associate a file to proc_t (/proc)
>> +## </summary>
>> +## <param name="domain">
>> +##	<summary>
>> +##	Domain allowed access.
>> +##	</summary>
>> +## </param>
>> +## <rolecap/>
>> +#
>> +interface(`kernel_associate_proc',`
>> +	gen_require(`
>> +		type proc_t;
>> +	')
>> +	allow $1 proc_t:filesystem associate;
>> +')
>> +
>> +########################################
>> +## <summary>
>>   ##	Send a kill signal to unlabeled processes.
>>   ## </summary>
>>   ## <param name="domain">
>> Index: refpolicy-2.20200209/policy/modules/services/xserver.te
>> ===================================================================
>> --- refpolicy-2.20200209.orig/policy/modules/services/xserver.te
>> +++ refpolicy-2.20200209/policy/modules/services/xserver.te
>> @@ -55,6 +55,13 @@ gen_tunable(xserver_gnome_xdm, false)
>>   ## </desc>
>>   gen_tunable(xserver_object_manager, false)
>>   
>> +## <desc>
>> +## <p>
>> +## Allow DRI access
>> +## </p>
>> +## </desc>
>> +gen_tunable(xserver_allow_dri, false)
>> +
>>   attribute x_domain;
>>   
>>   # X Events
>> Index: refpolicy-2.20200209/policy/modules/services/xserver.if
>> ===================================================================
>> --- refpolicy-2.20200209.orig/policy/modules/services/xserver.if
>> +++ refpolicy-2.20200209/policy/modules/services/xserver.if
>> @@ -48,8 +48,9 @@ interface(`xserver_restricted_role',`
>>   	files_search_tmp($2)
>>   
>>   	# Communicate via System V shared memory.
>> +	allow $2 xserver_t:fd use;
>>   	allow $2 xserver_t:shm r_shm_perms;
>> -	allow $2 xserver_tmpfs_t:file read_file_perms;
>> +	allow $2 xserver_tmpfs_t:file { map read_file_perms };
>>   
>>   	# allow ps to show iceauth
>>   	ps_process_pattern($2, iceauth_t)
>> @@ -75,10 +76,6 @@ interface(`xserver_restricted_role',`
>>   	allow $2 xdm_tmp_t:sock_file { read write };
>>   	dontaudit $2 xdm_t:tcp_socket { read write };
>>   
>> -	# Client read xserver shm
>> -	allow $2 xserver_t:fd use;
>> -	allow $2 xserver_tmpfs_t:file read_file_perms;
>> -
>>   	# Read /tmp/.X0-lock
>>   	allow $2 xserver_tmp_t:file { getattr read };
>>   
>> @@ -91,6 +88,9 @@ interface(`xserver_restricted_role',`
>>   	# open office is looking for the following
>>   	dev_getattr_agp_dev($2)
>>   	dev_dontaudit_rw_dri($2)
>> +	tunable_policy(`xserver_allow_dri',`
>> +		dev_rw_dri($2)
>> +	')
> 
> This whole dri thing might need to be looked at separately from this
> patch. It apparently depends on the graphics driver so I used to think
> it should be a boolean. But other policies just add rw_dri without a
> boolean so not really sure. Personally on my machine I just add
> dev_rw_dri(x_domain) and be done with it.

This is a good point.  If we can get some clarity that would be useful, 
since many things are muddy with X.


>>   	# GNOME checks for usb and other devices:
>>   	dev_rw_usbfs($2)
>>   
>> @@ -1670,6 +1670,26 @@ interface(`xserver_rw_mesa_shader_cache'
>>   
>>   	rw_dirs_pattern($1, mesa_shader_cache_t, mesa_shader_cache_t)
>>   	rw_files_pattern($1, mesa_shader_cache_t, mesa_shader_cache_t)
>> +	xdg_search_cache_dirs($1)
>> +')
>> +
>> +########################################
>> +## <summary>
>> +##	Manage the mesa shader cache.
>> +## </summary>
>> +## <param name="domain">
>> +##	<summary>
>> +##	Domain allowed access.
>> +##	</summary>
>> +## </param>
>> +#
>> +interface(`xserver_manage_mesa_shader_cache',`
>> +	gen_require(`
>> +		type mesa_shader_cache_t;
>> +	')
>> +
>> +	manage_dirs_pattern($1, mesa_shader_cache_t, mesa_shader_cache_t)
>> +	manage_files_pattern($1, mesa_shader_cache_t, mesa_shader_cache_t)
>>   	allow $1 mesa_shader_cache_t:file map;
>>   
>>   	xdg_search_cache_dirs($1)
>> Index: refpolicy-2.20200209/policy/modules/apps/chromium.if
>> ===================================================================
>> --- refpolicy-2.20200209.orig/policy/modules/apps/chromium.if
>> +++ refpolicy-2.20200209/policy/modules/apps/chromium.if
>> @@ -38,7 +38,15 @@ interface(`chromium_role',`
>>   
>>   	allow $2 chromium_t:process signal_perms;
>>   	allow $2 chromium_renderer_t:process signal_perms;
>> +	allow $2 chromium_sandbox_t:process signal_perms;
>>   	allow $2 chromium_naclhelper_t:process signal_perms;
>> +	allow chromium_t $2:process { signull signal };
>> +	allow $2 chromium_t:file manage_file_perms;
>> +
>> +	allow $2 chromium_t:unix_stream_socket connectto;
>> +
>> +	# for /tmp/.ICE-unix/* sockets
>> +	allow chromium_t $2:unix_stream_socket connectto;
>>   
>>   	allow chromium_sandbox_t $2:fd use;
>>   	allow chromium_naclhelper_t $2:fd use;
>> @@ -109,6 +117,7 @@ interface(`chromium_domtrans',`
>>   	gen_require(`
>>   		type chromium_t;
>>   		type chromium_exec_t;
>> +		class dbus send_msg;
>>   	')
>>   
>>   	corecmd_search_bin($1)
>> Index: refpolicy-2.20200209/policy/modules/services/ssh.if
>> ===================================================================
>> --- refpolicy-2.20200209.orig/policy/modules/services/ssh.if
>> +++ refpolicy-2.20200209/policy/modules/services/ssh.if
>> @@ -772,3 +772,21 @@ interface(`ssh_delete_tmp',`
>>   	files_search_tmp($1)
>>   	delete_files_pattern($1, sshd_tmp_t, sshd_tmp_t)
>>   ')
>> +
>> +#######################################
>> +## <summary>
>> +##	dontaudit access to ssh agent tmp dirs
>> +## </summary>
>> +## <param name="domain">
>> +##	<summary>
>> +##	Domain not to audit.
>> +##	</summary>
>> +## </param>
>> +#
>> +interface(`ssh_dontaudit_agent_tmp',`
>> +	gen_require(`
>> +		type ssh_agent_tmp_t;
>> +	')
>> +
>> +	dontaudit $1 ssh_agent_tmp_t:dir list_dir_perms;
>> +')
> 


-- 
Chris PeBenito