From: dominick.grift@gmail.com (Dominick Grift) Date: Wed, 08 Aug 2012 13:42:46 +0200 Subject: [refpolicy] [PATCH] Initial BIRD Internet Routing Daemon policy In-Reply-To: <5022443F.2040601@trentalancia.com> References: <1344415924-27382-1-git-send-email-dominick.grift@gmail.com> <5022443F.2040601@trentalancia.com> Message-ID: <1344426166.2306.31.camel@d30.localdomain> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com On Wed, 2012-08-08 at 12:49 +0200, Guido Trentalancia wrote: > 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 > > 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. You have a good point and i have been thinking abou this issue obviously. I decided to go this way because existing init daemons also only have the /etc/rc.d/init.d and not the /etc/init.d. Maybe a better solution is to just add: /etc/init.d /etc/rc.d/init.d to file_contexts.subs_dist > > +/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 @@ > > +## BIRD Internet Routing Daemon. > > + > > +######################################## > > +## > > +## 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. good point. However for the sake of uniformity ill just keep it like this. (this is the common summary for this interface) > > +## > > +## > > +## > > +## Domain allowed access. > > +## > > +## > > +## > > +## > > +## Role allowed access.. > > +## > > +## > > +## > > +# > > +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... This is better in my view. The patterns provide permissions that are not needed here. i.e. read_files_pattern(bird_t, bird_etc_t , bird_etc_t) allow bird_t to search bird_etc_t dirs but there aren't any. > > +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. > No, This combination make it possible to not allow bird_T to "write" bird_log_t files which is a real benefit over manage_file_perms. > 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. unlabeled is a default in systems. The boolean would be have to be on by default. On other systems this rules would harm since the packets would be labeled. and for the sake of uniformity i will leave it like this (other existing modules also dont have this conditional) > > +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. True and in general it is a good argument to not create unused interfaces. However this is a exception to that rules. We want end-users to be able to easily create a birdadm_r role. This interface facilitates that. If we would create a role for each init daemon by default we would end up with too many roles. So the middle road is to not create the role by default but to provide the interface so that endusers can easily create it if they so desire. > 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. This argument is not valid in light of my comment above. > Regards, > > Guido