From: guido@trentalancia.com (Guido Trentalancia)
Date: Wed, 08 Aug 2012 12:49:35 +0200
Subject: [refpolicy] [PATCH] Initial BIRD Internet Routing Daemon policy
In-Reply-To: <1344415924-27382-1-git-send-email-dominick.grift@gmail.com>
References: <1344415924-27382-1-git-send-email-dominick.grift@gmail.com>
Message-ID: <5022443F.2040601@trentalancia.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

Hello Dominick.

A very quick and possibly incomplete review, if it helps...

On 08/08/2012 10:52, Dominick Grift wrote:
>
> Signed-off-by: Dominick Grift <dominick.grift@gmail.com>
> diff --git a/bird.fc b/bird.fc
> new file mode 100644
> index 0000000..7b63b8e
> --- /dev/null
> +++ b/bird.fc
> @@ -0,0 +1,11 @@
> +/etc/bird\.conf	--	gen_context(system_u:object_r:bird_etc_t,s0)
> +
> +/etc/default/bird	--	gen_context(system_u:object_r:bird_etc_t,s0)
> +
> +/etc/rc\.d/init\.d/bird	--	gen_context(system_u:object_r:bird_initrc_exec_t,s0)

You might want to support init script locations for other distributions 
here, as in the oident module that you proposed to modify yesterday (I 
am going to modify the mcelog too for this purpose).

Debian (but also Gentoo and many others) are currently using /etc/init\.d.

The rest is unlikely to change, if it does, it's their business to 
modify the contexts, I think.

> +/usr/sbin/bird	--	gen_context(system_u:object_r:bird_exec_t,s0)
> +
> +/var/log/bird\.log.*	--	gen_context(system_u:object_r:bird_log_t,s0)
> +
> +/var/run/bird\.ctl	-s	gen_context(system_u:object_r:bird_var_run_t,s0)
> diff --git a/bird.if b/bird.if
> new file mode 100644
> index 0000000..fae3f36
> --- /dev/null
> +++ b/bird.if
> @@ -0,0 +1,42 @@
> +## <summary>BIRD Internet Routing Daemon.</summary>
> +
> +########################################
> +## <summary>
> +##	All of the rules required to administrate
> +##	an bird environment.

I think "Rules to administrate a BIRD environment" would read a bit 
better, but clearly it's not critical.

> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +## <param name="role">
> +##	<summary>
> +##	Role allowed access..
> +##	</summary>
> +## </param>
> +## <rolecap/>
> +#
> +interface(`bird_admin',`
> +	gen_require(`
> +		type bird_t, bird_etc_t, bird_log_t;
> +		type bird_var_run_t, bird_initrc_exec_t;
> +	')
> +
> +	allow $1 bird_t:process { ptrace signal_perms };
> +	ps_process_pattern($1, bird_t)
> +
> +	init_labeled_script_domtrans($1, bird_initrc_exec_t)
> +	domain_system_change_exemption($1)
> +	role_transition $2 bird_initrc_exec_t system_r;
> +	allow $2 system_r;
> +
> +	files_list_etc($1)
> +	admin_pattern($1, bird_etc_t)
> +
> +	logging_list_logs($1)
> +	admin_pattern($1, bird_log_t)
> +
> +	files_list_pids($1)
> +	admin_pattern($1, bird_var_run_t)
> +')

Not checked (see below).

> diff --git a/bird.te b/bird.te
> new file mode 100644
> index 0000000..9afd52b
> --- /dev/null
> +++ b/bird.te
> @@ -0,0 +1,57 @@
> +policy_module(bird, 1.0.0)
> +
> +########################################
> +#
> +# Declarations
> +#
> +
> +type bird_t;
> +type bird_exec_t;
> +init_daemon_domain(bird_t, bird_exec_t)
> +
> +type bird_initrc_exec_t;
> +init_script_file(bird_initrc_exec_t)
> +
> +type bird_etc_t;
> +files_config_file(bird_etc_t)
> +
> +type bird_log_t;
> +logging_log_file(bird_log_t)
> +
> +type bird_var_run_t;
> +files_pid_file(bird_var_run_t)
> +
> +########################################
> +#
> +# Local policy
> +#
> +
> +allow bird_t self:capability { net_admin net_bind_service };
> +allow bird_t self:netlink_route_socket create_netlink_socket_perms;
> +allow bird_t self:tcp_socket create_stream_socket_perms;
> +
> +allow bird_t bird_etc_t:file read_file_perms;

Use of patterns for reading, writing or managing standard files is more 
common throughout the whole reference policy. It's not critical but 
think about it, it might improve readability and coherence across the 
work as a whole...

> +allow bird_t bird_log_t:file { create_file_perms append_file_perms setattr_file_perms };

I think create+append+setattr grants more permissions than the 
manage_file_perms (or preferably corrispondent pattern), however I have 
not checked.

See above for using patterns versus individual or grouped permissions.

> +logging_log_filetrans(bird_t, bird_log_t, file)
> +
> +allow bird_t bird_var_run_t:sock_file manage_sock_file_perms;

See above for using patterns versus individual or grouped permissions.

> +files_pid_filetrans(bird_t, bird_var_run_t, sock_file)
> +
> +corenet_all_recvfrom_unlabeled(bird_t)

Unlabelled receive could probably made tunable as not everybody might 
like that.

> +corenet_all_recvfrom_netlabel(bird_t)
> +corenet_tcp_sendrecv_generic_if(bird_t)
> +corenet_tcp_bind_generic_node(bird_t)
> +corenet_tcp_sendrecv_generic_node(bird_t)
> +corenet_tcp_sendrecv_bgp_port(bird_t)
> +corenet_sendrecv_bgp_client_packets(bird_t)
> +corenet_tcp_connect_bgp_port(bird_t)
> +corenet_sendrecv_bgp_server_packets(bird_t)
> +corenet_tcp_bind_bgp_port(bird_t)
> +
> +# /etc/iproute2/rt_realms
> +files_read_etc_files(bird_t)
> +
> +logging_send_syslog_msg(bird_t)
> +
> +miscfiles_read_localization(bird_t)

You have not used the bird_admin() interface, since it's not a simple 
and generic interface (such as for managing files), you should really 
remove it in my opinion.

The most important reason is that permissions needed by BIRD might 
change with subsequent releases of the daemon by the time somebody will 
actually use the bird_admin() interface: if they shrink, it's a security 
matter (interface is loose) and you don't be doing much of a favour to 
anyone.

Regards,

Guido