From: aranea@aixah.de (Luis Ressel)
Date: Mon, 10 Aug 2015 15:42:34 +0200
Subject: [refpolicy] [PATCH 1/2] Policy for gpg's dirmngr
In-Reply-To: <20150810072526.GA3707@x250>
References: <1439154658-18322-1-git-send-email-aranea@aixah.de>
	<20150810072526.GA3707@x250>
Message-ID: <20150810154234.7e0c7aa3@gentp.lnet>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On Mon, 10 Aug 2015 09:25:27 +0200
Dominick Grift <dac.override@gmail.com> wrote:

> On Sun, Aug 09, 2015 at 11:10:57PM +0200, Luis Ressel wrote:
> > GnuPG 2.1 uses a separate dirmngr process for retrieving keys from a
> > keyserver.
> 
> I have a policy for dirmngr as well. Comparing mine with yours. Below
> are some commants.
> 
> > ---
> >  gpg.fc |  1 +
> >  gpg.if | 16 +++++++++-------
> >  gpg.te | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 60 insertions(+), 7 deletions(-)
> > 
> > diff --git a/gpg.fc b/gpg.fc
> > index 888cd2c..d492dc5 100644
> > --- a/gpg.fc
> > +++ b/gpg.fc
> > @@ -1,6 +1,7 @@
> >  HOME_DIR/\.gnupg(/.+)?
> > gen_context(system_u:object_r:gpg_secret_t,s0)
> > HOME_DIR/\.gnupg/log-socket	-s
> > gen_context(system_u:object_r:gpg_agent_tmp_t,s0)
> > +/usr/bin/dirmngr        --
> > gen_context(system_u:object_r:gpg_dirmngr_exec_t,s0) /usr/bin/gpg(2)?	--
> > gen_context(system_u:object_r:gpg_exec_t,s0) /usr/bin/gpgsm	--
> > gen_context(system_u:object_r:gpg_exec_t,s0) /usr/bin/gpg-agent	--
> > gen_context(system_u:object_r:gpg_agent_exec_t,s0) diff --git
> > a/gpg.if b/gpg.if index b299418..13149ca 100644 --- a/gpg.if
> > +++ b/gpg.if
> > @@ -17,31 +17,33 @@
> >  #
> >  interface(`gpg_role',`
> >  	gen_require(`
> > -		attribute_role gpg_roles, gpg_agent_roles,
> > gpg_helper_roles, gpg_pinentry_roles;
> > +		attribute_role gpg_roles, gpg_agent_roles,
> > gpg_dirmngr_roles, gpg_helper_roles, gpg_pinentry_roles; type
> > gpg_t, gpg_exec_t, gpg_agent_t; type gpg_agent_exec_t,
> > gpg_agent_tmp_t, gpg_helper_t; type gpg_pinentry_t,
> > gpg_pinentry_tmp_t, gpg_secret_t;
> > +		type gpg_dirmngr_t, gpg_dirmngr_tmp_t;
> >  	')
> >  
> >  	roleattribute $1 gpg_roles;
> >  	roleattribute $1 gpg_agent_roles;
> > +	roleattribute $1 gpg_dirmngr_roles;
> >  	roleattribute $1 gpg_helper_roles;
> >  	roleattribute $1 gpg_pinentry_roles;
> >  
> >  	domtrans_pattern($2, gpg_exec_t, gpg_t)
> >  	domtrans_pattern($2, gpg_agent_exec_t, gpg_agent_t)
> >  
> > -	allow $2 { gpg_t gpg_agent_t gpg_helper_t
> > gpg_pinentry_t }:process { ptrace signal_perms };
> > -	ps_process_pattern($2, { gpg_t gpg_agent_t gpg_helper_t
> > gpg_pinentry_t })
> > +	allow $2 { gpg_t gpg_agent_t gpg_dirmngr_t gpg_helper_t
> > gpg_pinentry_t }:process { ptrace signal_perms };
> > +	ps_process_pattern($2, { gpg_t gpg_agent_t gpg_dirmngr_t
> > gpg_helper_t gpg_pinentry_t }) 
> >  	allow gpg_pinentry_t $2:process signull;
> >  	allow gpg_helper_t $2:fd use;
> > -	allow { gpg_t gpg_agent_t gpg_helper_t gpg_pinentry_t }
> > $2:fifo_file { read write };
> > +	allow { gpg_t gpg_agent_t gpg_dirmngr_t gpg_helper_t
> > gpg_pinentry_t } $2:fifo_file { read write };
> 
> I do not believe the above is required. dirmngr can be run directly
> by the caller so it should already have permissions to rw inherited
> fifo files of the caller as per the domtrans_pattern()
> 

In fact, the fifo permissions don't seem to be neccessary.

> You would need to allow the user domain to run dirmngr with a domain
> transition first though, but it makes sense because you can dir
> dirmngr as a server (see man dirmngr)
> 

I missed this. I'll add a domtrans.

> >  
> > -	allow $2 { gpg_agent_tmp_t gpg_secret_t }:dir
> > { manage_dir_perms relabel_dir_perms };
> > -	allow $2 { gpg_agent_tmp_t gpg_secret_t }:file
> > { manage_file_perms relabel_file_perms };
> > +	allow $2 { gpg_agent_tmp_t gpg_dirmngr_tmp_t
> > gpg_secret_t }:dir { manage_dir_perms relabel_dir_perms };
> > +	allow $2 { gpg_agent_tmp_t gpg_dirmngr_tmp_t
> > gpg_secret_t }:file { manage_file_perms relabel_file_perms }; allow
> > $2 gpg_secret_t:lnk_file { manage_lnk_file_perms
> > relabel_lnk_file_perms };
> > -	allow $2 { gpg_agent_tmp_t gpg_pinentry_tmp_t
> > gpg_secret_t }:sock_file { manage_sock_file_perms
> > relabel_sock_file_perms };
> > +	allow $2 { gpg_agent_tmp_t gpg_dirmngr_tmp_t
> > gpg_pinentry_tmp_t gpg_secret_t }:sock_file
> > { manage_sock_file_perms relabel_sock_file_perms };
> > filetrans_pattern($2, gpg_secret_t, gpg_agent_tmp_t, sock_file,
> > "log-socket") userdom_user_home_dir_filetrans($2, gpg_secret_t,
> > dir, ".gnupg") diff --git a/gpg.te b/gpg.te index f878352..a40ac69
> > 100644 --- a/gpg.te
> > +++ b/gpg.te
> > @@ -19,6 +19,8 @@ roleattribute system_r gpg_roles;
> >  
> >  attribute_role gpg_agent_roles;
> >  
> > +attribute_role gpg_dirmngr_roles;
> > +
> >  attribute_role gpg_helper_roles;
> >  roleattribute system_r gpg_helper_roles;
> >  
> > @@ -72,6 +74,18 @@ optional_policy(`
> >  	pulseaudio_tmpfs_content(gpg_pinentry_tmpfs_t)
> >  ')
> >  
> > +type gpg_dirmngr_t;
> > +type gpg_dirmngr_exec_t;
> > +typealias gpg_dirmngr_t alias { user_gpg_dirmngr_t
> > staff_gpg_dirmngr_t sysadm_gpg_dirmngr_t }; +typealias
> > gpg_dirmngr_t alias { auditadm_gpg_dirmngr_t
> > secadm_gpg_dirmngr_t };
> > +userdom_user_application_domain(gpg_dirmngr_t, gpg_dirmngr_exec_t)
> > +role gpg_dirmngr_roles types gpg_dirmngr_t; + +type
> > gpg_dirmngr_tmp_t; +typealias gpg_dirmngr_tmp_t alias
> > { user_gpg_dirmngr_tmp_t staff_gpg_dirmngr_tmp_t
> > sysadm_gpg_dirmngr_tmp_t }; +typealias gpg_dirmngr_tmp_t alias
> > { auditadm_gpg_dirmngr_tmp_t secadm_gpg_dirmngr_tmp_t };
> > +userdom_user_tmp_file(gpg_dirmngr_tmp_t) +
> 
> It does not seem to actually maintain temporary files. Instead it
> maintains content in ~/.gnupg. So classifying that type
> userdom_user_tmp_file is inaccurate in my view
> 

Yes, I only needed gpg_dirmngr_tmp_t for the socket file, it's not used
for anything in /tmp. I'll change the declaration. Should I also change
the name to something other than _tmp_t?

> >  ########################################
> >  #
> >  # Local policy
> > @@ -94,8 +108,10 @@ manage_lnk_files_pattern(gpg_t, gpg_secret_t,
> > gpg_secret_t) userdom_user_home_dir_filetrans(gpg_t, gpg_secret_t,
> > dir) 
> >  gpg_stream_connect_agent(gpg_t)
> > +stream_connect_pattern(gpg_t, gpg_dirmngr_tmp_t,
> > gpg_dirmngr_tmp_t, gpg_dirmngr_t) 
> >  domtrans_pattern(gpg_t, gpg_agent_exec_t, gpg_agent_t)
> > +domtrans_pattern(gpg_t, gpg_dirmngr_exec_t, gpg_dirmngr_t)
> 
> Users should probably also be allowed to run dirmngr with a domain
> transition
> 
> >  domtrans_pattern(gpg_t, gpg_helper_exec_t, gpg_helper_t)
> >  
> >  kernel_read_sysctl(gpg_t)
> > @@ -344,3 +360,37 @@ optional_policy(`
> >  optional_policy(`
> >  	xserver_user_x_domain_template(gpg_pinentry,
> > gpg_pinentry_t, gpg_pinentry_tmpfs_t) ')
> > +
> > +##############################
> > +#
> > +# Dirmngr local policy
> > +#
> > +
> > +manage_dirs_pattern(gpg_dirmngr_t, gpg_secret_t, gpg_secret_t)
> > +manage_sock_files_pattern(gpg_dirmngr_t, gpg_secret_t,
> > gpg_secret_t) +manage_files_pattern(gpg_dirmngr_t, gpg_secret_t,
> > gpg_secret_t) +manage_lnk_files_pattern(gpg_dirmngr_t,
> > gpg_secret_t, gpg_secret_t)
> 
> I do not believe that dirmngr needs to be able to maintain gpg
> secrets. (I am pretty sure about that)
> 

It does not need access to the secret files, just the config files
and .gnupg/{dirmngr-cache,crls}.d/, which are currently labeled
gpg_secret_t (also, the .gnupg/ directory itself has this type). I'll
improve this.

> > +
> > +manage_dirs_pattern(gpg_dirmngr_t, gpg_dirmngr_tmp_t,
> > gpg_dirmngr_tmp_t) +manage_files_pattern(gpg_dirmngr_t,
> > gpg_dirmngr_tmp_t, gpg_dirmngr_tmp_t)
> > +manage_sock_files_pattern(gpg_dirmngr_t, gpg_dirmngr_tmp_t,
> > gpg_dirmngr_tmp_t) +files_tmp_filetrans(gpg_dirmngr_t,
> > gpg_dirmngr_tmp_t, { file sock_file dir })
> 
> I do not believe dirmngr maintains anything in /tmp or /var/tmp,
> everything is maintained in ~/.gnupg
> 
> > +
> > +filetrans_pattern(gpg_dirmngr_t, gpg_secret_t, gpg_dirmngr_tmp_t,
> > sock_file, "S.dirmngr")
> 
> I believe it also maintains ~/.gnupg/crls\.d(/.*)?
> 
> > +
> > +userdom_use_user_terminals(gpg_dirmngr_t)
> > +userdom_search_user_home_dirs(gpg_dirmngr_t)
> > +
> > +dev_read_rand(gpg_dirmngr_t)
> > +dev_read_urand(gpg_dirmngr_t)
> 
> I was not able to confirm the above two instead thoug it wants to
> read crypto sysctls here
> 

On my system, dirmngr fails to start without those.

avc:  denied  { read } for  pid=2126 comm=636F6E6E2066643D30
name="random" dev="devtmpfs" ino=1032
scontext=staff_u:staff_r:gpg_dirmngr_t
tcontext=system_u:object_r:random_device_t tclass=chr_file permissive=0

> > +
> > +auth_use_nsswitch(gpg_dirmngr_t)
> > +
> > +corenet_all_recvfrom_unlabeled(gpg_dirmngr_t)
> > +corenet_all_recvfrom_netlabel(gpg_dirmngr_t)
> > +corenet_tcp_sendrecv_generic_if(gpg_dirmngr_t)
> > +corenet_tcp_sendrecv_generic_node(gpg_dirmngr_t)
> > +
> > +corenet_sendrecv_all_client_packets(gpg_dirmngr_t)
> > +corenet_tcp_connect_all_ports(gpg_dirmngr_t)
> 
> I think that this may be a bit too much. I suppose it needs to be
> able to hkp and http ports instead?

The network permissions are in fact a bit wide, it only needs access to
hkp, http and ldap. However, the same could be said about gpg_t and
gpg_agent_t (I copied the permissions from their policies).

-- 
Luis Ressel