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: [PATCH v2] Add knot module
To:     Alexander Miroshnichenko <alex@millerson.name>,
        selinux-refpolicy@vger.kernel.org
References: <20190702155823.GA27193@brutus.lan>
 <20190705120251.1121-1-alex@millerson.name>
From:   Chris PeBenito <pebenito@ieee.org>
Message-ID: <1e3cd1ef-1643-ee92-d6f3-55c45bdee331@ieee.org>
Date:   Mon, 8 Jul 2019 20:47:45 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <20190705120251.1121-1-alex@millerson.name>
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 7/5/19 8:02 AM, Alexander Miroshnichenko wrote:
> Add a SELinux Reference Policy module for the
> Knot authoritative-only DNS server.
> 
> Signed-off-by: Alexander Miroshnichenko <alex@millerson.name>
> ---
>   policy/modules/roles/sysadm.te  |   4 +
>   policy/modules/services/knot.fc |   9 ++
>   policy/modules/services/knot.if | 219 ++++++++++++++++++++++++++++++++
>   policy/modules/services/knot.te | 104 +++++++++++++++
>   policy/modules/system/init.te   |   4 +
>   5 files changed, 340 insertions(+)
>   create mode 100644 policy/modules/services/knot.fc
>   create mode 100644 policy/modules/services/knot.if
>   create mode 100644 policy/modules/services/knot.te

I think the rules are probably ok, but the interface names need work. 
They should all start with knot_*, for starters.  See below.


> diff --git a/policy/modules/roles/sysadm.te b/policy/modules/roles/sysadm.te
> index 8f891c83865f..e3079ad65d17 100644
> --- a/policy/modules/roles/sysadm.te
> +++ b/policy/modules/roles/sysadm.te
> @@ -550,6 +550,10 @@ optional_policy(`
>   	keystone_admin(sysadm_t, sysadm_r)
>   ')
>   
> +optional_policy(`
> +	knotc_role(sysadm_r, sysadm_t)
> +')
> +
>   optional_policy(`
>   	kismet_admin(sysadm_t, sysadm_r)
>   ')
> diff --git a/policy/modules/services/knot.fc b/policy/modules/services/knot.fc
> new file mode 100644
> index 000000000000..02a1c2022661
> --- /dev/null
> +++ b/policy/modules/services/knot.fc
> @@ -0,0 +1,9 @@
> +/etc/knot(/.*)?		gen_context(system_u:object_r:knot_conf_t,s0)
> +
> +/usr/sbin/knotd		--	gen_context(system_u:object_r:knotd_exec_t,s0)
> +
> +/usr/sbin/knotc		--      gen_context(system_u:object_r:knotc_exec_t,s0)
> +
> +/var/lib/knot(/.*)?	gen_context(system_u:object_r:knot_var_lib_t,s0)
> +
> +/run/knot(/.*)?		gen_context(system_u:object_r:knot_runtime_t,s0)
> diff --git a/policy/modules/services/knot.if b/policy/modules/services/knot.if
> new file mode 100644
> index 000000000000..fef08da46a79
> --- /dev/null
> +++ b/policy/modules/services/knot.if
> @@ -0,0 +1,219 @@
> +## <summary>high-performance authoritative-only DNS server.</summary>
> +
> +########################################
> +## <summary>
> +##	Execute knotd_exec_t in the knotd domain.
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +##	Domain allowed to transition.
> +## </summary>
> +## </param>
> +#
> +interface(`knotd_domtrans',`
> +	gen_require(`
> +		type knotd_t, knotd_exec_t;
> +	')
> +
> +	corecmd_search_bin($1)
> +	domtrans_pattern($1, knotd_exec_t, knotd_t)
> +')

This doesn't seem needed, since a service is usually started by init. 
If it is needed, then it should be something like knot_domtrans(), and 
then the latter knotc_domtrans should be something like 
knot_domtrans_client().


> +########################################
> +## <summary>
> +##      Manage Knot runtime.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_manage_runtime',`
> +	gen_require(`
> +		type knot_runtime_t;
> +	')
> +
> +	manage_dirs_pattern($1, knot_runtime_t, knot_runtime_t)
> +	manage_files_pattern($1, knot_runtime_t, knot_runtime_t)
> +	manage_lnk_files_pattern($1, knot_runtime_t, knot_runtime_t)
> +	manage_sock_files_pattern($1, knot_runtime_t, knot_runtime_t)
> +	files_search_pids($1)

While there are a few interfaces that have this, I don't want this to be 
the standard.  This should either be split into 4 different interfaces 
or put the rules directly in knot.te.

> +')
> +
> +########################################
> +## <summary>
> +##      Manage knot var lib.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_manage_var_lib',`
> +	gen_require(`
> +		type knot_var_lib_t;
> +	')
> +
> +	manage_dirs_pattern($1, knot_var_lib_t, knot_var_lib_t)
> +	manage_files_pattern($1, knot_var_lib_t, knot_var_lib_t)
> +	manage_lnk_files_pattern($1, knot_var_lib_t, knot_var_lib_t)
> +	files_search_var_lib($1)
> +')

Same thing as above.


> +########################################
> +## <summary>
> +##      Mmap knot var lib files.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_mmap_var_lib_files',`
"map" not "mmap"


> +	gen_require(`
> +		type knot_var_lib_t;
> +	')
> +
> +	allow $1 knot_var_lib_t:file map;
> +')
> +
> +########################################
> +## <summary>
> +##      Read, mmap knot config files.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_mmap_config_file',`

In this case, mmap_read, not just mmap.  Or split map perm to another 
interface.


> +	gen_require(`
> +		type knot_conf_t;
> +	')
> +
> +	mmap_read_files_pattern($1, knot_conf_t, knot_conf_t)
> +	files_search_etc($1)
> +')
> +
> +########################################
> +## <summary>
> +##      Manage knot tmp.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_manage_tmp',`
> +	gen_require(`
> +		type knot_tmp_t;
> +	')
> +
> +	allow $1 knot_tmp_t:file manage_file_perms;
> +	allow $1 knot_tmp_t:dir manage_dir_perms;

Needs 2 interfaces.

> +')
> +
> +########################################
> +## <summary>
> +##      Mmap knot tmp.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_mmap_tmp_files',`

Similar comment to above.

> +	gen_require(`
> +		type knot_tmp_t;
> +	')
> +
> +	allow $1 knot_tmp_t:file map;
> +')
> +
> +########################################
> +## <summary>
> +##      Create knot tmp files, directories in
> +##      temporary directory.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +## <param name="file_type">
> +##      <summary>
> +##      The type of the object to be created
> +##      </summary>
> +## </param>
> +## <param name="object_class">
> +##      <summary>
> +##      The object class.
> +##      </summary>
> +## </param>
> +## <param name="name" optional="true">
> +##      <summary>
> +##      The name of the object being created.
> +##      </summary>
> +## </param>
> +#
> +interface(`knot_tmp_filetrans',`
> +	gen_require(`
> +		type knot_tmp_t;
> +	')
> +
> +	files_tmp_filetrans($1, knot_tmp_t, { file dir })
> +')
> +
> +########################################
> +## <summary>
> +##      Execute knotc in the knotc domain.
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +##      Domain allowed to transition.
> +## </summary>
> +## </param>
> +#
> +interface(`knotc_domtrans',`
> +	gen_require(`
> +		type knotc_t, knotc_exec_t;
> +	')
> +
> +	corecmd_search_bin($1)
> +	domtrans_pattern($1, knotc_exec_t, knotc_t)
> +')
> +
> +########################################
> +## <summary>
> +##      Role access for knotc
> +## </summary>
> +## <param name="role">
> +##      <summary>
> +##      Role allowed access
> +##      </summary>
> +## </param>
> +## <param name="domain">
> +##      <summary>
> +##      User domain for the role
> +##      </summary>
> +## </param>
> +#
> +interface(`knotc_role',`
> +	gen_require(`
> +		type knotc_t;
> +		attribute_role knotc_roles;
> +	')
> +
> +	roleattribute $1 knotc_roles;
> +
> +	knotc_domtrans($2)
> +
> +	ps_process_pattern($2, knotc_t)
> +	allow $2 knotc_t:process { signull signal sigkill };
> +')
> diff --git a/policy/modules/services/knot.te b/policy/modules/services/knot.te
> new file mode 100644
> index 000000000000..780535759cf5
> --- /dev/null
> +++ b/policy/modules/services/knot.te
> @@ -0,0 +1,104 @@
> +policy_module(knot, 1.0.0)
> +
> +########################################
> +#
> +# Declarations
> +#
> +
> +type knotd_t;
> +type knotd_exec_t;
> +init_daemon_domain(knotd_t, knotd_exec_t)
> +
> +type knotc_t;
> +type knotc_exec_t;
> +application_domain(knotc_t, knotc_exec_t)
> +init_daemon_domain(knotc_t, knotc_exec_t)
> +role knotc_roles types knotc_t;
> +
> +attribute_role knotc_roles;
> +
> +type knot_conf_t;
> +files_config_file(knot_conf_t)
> +
> +type knot_runtime_t;
> +files_pid_file(knot_runtime_t)
> +
> +type knot_var_lib_t;
> +files_type(knot_var_lib_t)
> +
> +type knot_tmp_t;
> +files_tmp_file(knot_tmp_t)
> +
> +########################################
> +#
> +# knotd local policy
> +#
> +allow knotd_t self:capability { dac_override dac_read_search setgid setpcap setuid };
> +allow knotd_t self:process { signal_perms getcap getsched setsched };
> +allow knotd_t self:tcp_socket create_stream_socket_perms;
> +allow knotd_t self:udp_socket create_socket_perms;
> +allow knotd_t self:unix_stream_socket create_stream_socket_perms;
> +
> +corenet_tcp_bind_generic_node(knotd_t)
> +corenet_udp_bind_generic_node(knotd_t)
> +
> +corenet_sendrecv_dns_server_packets(knotd_t)
> +corenet_tcp_bind_dns_port(knotd_t)
> +corenet_udp_bind_dns_port(knotd_t)
> +# Slave replication
> +corenet_tcp_connect_dns_port(knotd_t)
> +
> +kernel_read_kernel_sysctls(knotd_t)
> +
> +knot_mmap_config_file(knotd_t)
> +
> +knot_manage_runtime(knotd_t)
> +files_pid_filetrans(knotd_t, knot_runtime_t, dir)
> +
> +knot_manage_var_lib(knotd_t)
> +knot_mmap_var_lib_files(knotd_t)
> +files_var_lib_filetrans(knotd_t, knot_var_lib_t, dir)
> +
> +knot_manage_tmp(knotd_t)
> +knot_mmap_tmp_files(knotd_t)
> +knot_tmp_filetrans(knotd_t)
> +
> +files_map_etc_files(knotd_t)
> +
> +fs_getattr_xattr_fs(knotd_t)
> +
> +fs_getattr_tmpfs(knotd_t)
> +
> +auth_use_nsswitch(knotd_t)
> +
> +logging_send_syslog_msg(knotd_t)
> +
> +miscfiles_read_localization(knotd_t)
> +
> +########################################
> +#
> +# knotc local policy
> +#
> +allow knotc_t self:capability { dac_override dac_read_search };
> +allow knotc_t self:process signal;
> +
> +stream_connect_pattern(knotc_t, knot_runtime_t, knot_runtime_t, knotd_t)
> +
> +knot_mmap_config_file(knotc_t)
> +
> +knot_manage_tmp(knotc_t)
> +knot_mmap_tmp_files(knotc_t)
> +knot_tmp_filetrans(knotc_t)
> +
> +knot_manage_var_lib(knotc_t)
> +knot_mmap_var_lib_files(knotc_t)
> +
> +files_read_etc_files(knotc_t)
> +
> +fs_getattr_tmpfs(knotc_t)
> +
> +domain_use_interactive_fds(knotc_t)
> +
> +miscfiles_read_localization(knotc_t)
> +
> +userdom_use_user_ptys(knotc_t)
> diff --git a/policy/modules/system/init.te b/policy/modules/system/init.te
> index f4d27bff3ea2..d38a0a8549d3 100644
> --- a/policy/modules/system/init.te
> +++ b/policy/modules/system/init.te
> @@ -1158,6 +1158,10 @@ optional_policy(`
>   	kerberos_use(initrc_t)
>   ')
>   
> +optional_policy(`
> +        knot_mmap_config_file(initrc_t)
> +')
> +
>   optional_policy(`
>   	ldap_read_config(initrc_t)
>   	ldap_list_db(initrc_t)
> 


-- 
Chris PeBenito