From: guido@trentalancia.com (Guido Trentalancia) Date: Wed, 08 Aug 2012 16:54:14 +0200 Subject: [refpolicy] [PATCH] Initial BIRD Internet Routing Daemon policy In-Reply-To: <1344426166.2306.31.camel@d30.localdomain> References: <1344415924-27382-1-git-send-email-dominick.grift@gmail.com> <5022443F.2040601@trentalancia.com> <1344426166.2306.31.camel@d30.localdomain> Message-ID: <50227D96.8060200@trentalancia.com> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com On 08/08/2012 13:42, Dominick Grift wrote: > > > 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 Yes, that should be a better solution. >>> +/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. >>> + [cut] >>> +######################################## >>> +# >>> +# 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. What often happens is that distributions create internal directories within /etc (as in /etc/bird), that's why using the pattern is a bit more future-proof. >>> +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. You're right, it's the other way around. manage is lots more permissions than create+append+setattr. I think I got confused by the fact that most other modules simply use manage. >> 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) Perhaps a tunable policy can be introduced internally within all interfaces that are using unlabelled connections so that there is only one boolean defaulting to on... >>> +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. Ok. >> Regards, >> >> Guido