On Tue, May 07, 2019 at 05:42:01PM +0300, Alexander Miroshnichenko wrote:
> Signed-off-by: Alexander Miroshnichenko <alex@millerson.name>

Some observations below.

> ---
>  policy/modules/services/nginx.fc |  64 ++++++++++++
>  policy/modules/services/nginx.if | 104 +++++++++++++++++++
>  policy/modules/services/nginx.te | 169 +++++++++++++++++++++++++++++++
>  3 files changed, 337 insertions(+)
>  create mode 100644 policy/modules/services/nginx.fc
>  create mode 100644 policy/modules/services/nginx.if
>  create mode 100644 policy/modules/services/nginx.te
> 
> diff --git a/policy/modules/services/nginx.fc b/policy/modules/services/nginx.fc
> new file mode 100644
> index 000000000000..62f12620ea88
> --- /dev/null
> +++ b/policy/modules/services/nginx.fc
> @@ -0,0 +1,64 @@
> +###############################################################################
> +# SELinux module for the NGINX Web Server
> +#
> +# Project Contact Information:
> +#   Stuart Cianos
> +#   Email: scianos@alphavida.com
> +#
> +###############################################################################
> +# (C) Copyright 2009 by Stuart Cianos, d/b/a AlphaVida. All Rights Reserved.
> +#
> +#
> +# Stuart Cianos licenses this file to You under the GNU General Public License,
> +# Version 3.0 (the "License"); you may not use this file except in compliance
> +# with the License.  You may obtain a copy of the License at
> +#
> +#     http://www.gnu.org/licenses/gpl.txt
> +#
> +# or in the COPYING file included in the original archive.
> +#
> +# Disclaimer of Warranty.
> +#
> +# THERE IS NO WARRANTY FOR THE PROGRAM, TO THE EXTENT PERMITTED BY
> +# APPLICABLE LAW.  EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT
> +# HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS" WITHOUT WARRANTY
> +# OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, BUT NOT LIMITED TO,
> +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> +# PURPOSE.  THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE PROGRAM
> +# IS WITH YOU.  SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME THE COST OF
> +# ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
> +#
> +# Limitation of Liability.
> +#
> +# IN NO EVENT UNLESS REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING
> +# WILL ANY COPYRIGHT HOLDER, OR ANY OTHER PARTY WHO MODIFIES AND/OR CONVEYS
> +# THE PROGRAM AS PERMITTED ABOVE, BE LIABLE TO YOU FOR DAMAGES, INCLUDING ANY
> +# GENERAL, SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE
> +# USE OR INABILITY TO USE THE PROGRAM (INCLUDING BUT NOT LIMITED TO LOSS OF
> +# DATA OR DATA BEING RENDERED INACCURATE OR LOSSES SUSTAINED BY YOU OR THIRD
> +# PARTIES OR A FAILURE OF THE PROGRAM TO OPERATE WITH ANY OTHER PROGRAMS),
> +# EVEN IF SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE POSSIBILITY OF
> +# SUCH DAMAGES.
> +###############################################################################
> +# nginx executable will have:
> +# label: system_u:object_r:nginx_exec_t
> +# MLS sensitivity: s0
> +# MCS categories: <none>
> +
> +#
> +# /etc
> +#
> +/etc/nginx(/.*)?				gen_context(system_u:object_r:nginx_conf_t,s0)
> +/etc/ssl/nginx(/.*)?				gen_context(system_u:object_r:nginx_conf_t,s0)

/etc/ssl is for certificates, should probably use a "cert_type" here

> +
> +#
> +# /usr
> +#
> +/usr/sbin/nginx				--	gen_context(system_u:object_r:nginx_exec_t,s0)
> +
> +#
> +# /var
> +#
> +/var/lib/nginx/tmp(/.*)?			gen_context(system_u:object_r:nginx_tmp_t,s0)

I would just label /var/lib/nginx(/.*)?" type nginx_var_lib_t

> +/var/log/nginx(/.*)?				gen_context(system_u:object_r:nginx_log_t,s0)
> +/var/tmp/nginx(/.*)?				gen_context(system_u:object_r:nginx_tmp_t,s0)
> diff --git a/policy/modules/services/nginx.if b/policy/modules/services/nginx.if
> new file mode 100644
> index 000000000000..ebef6e759e3f
> --- /dev/null
> +++ b/policy/modules/services/nginx.if
> @@ -0,0 +1,104 @@
> +###############################################################################
> +# SELinux module for the NGINX Web Server
> +#
> +# Project Contact Information:
> +#   Stuart Cianos
> +#   Email: scianos@alphavida.com
> +#
> +###############################################################################
> +# (C) Copyright 2009 by Stuart Cianos, d/b/a AlphaVida. All Rights Reserved.
> +#
> +#
> +# Stuart Cianos licenses this file to You under the GNU General Public License,
> +# Version 3.0 (the "License"); you may not use this file except in compliance
> +# with the License.  You may obtain a copy of the License at
> +#
> +#     http://www.gnu.org/licenses/gpl.txt
> +#
> +# or in the COPYING file included in the original archive.
> +#
> +# Disclaimer of Warranty.
> +#
> +# THERE IS NO WARRANTY FOR THE PROGRAM, TO THE EXTENT PERMITTED BY
> +# APPLICABLE LAW.  EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT
> +# HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS" WITHOUT WARRANTY
> +# OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, BUT NOT LIMITED TO,
> +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> +# PURPOSE.  THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE PROGRAM
> +# IS WITH YOU.  SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME THE COST OF
> +# ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
> +#
> +# Limitation of Liability.
> +#
> +# IN NO EVENT UNLESS REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING
> +# WILL ANY COPYRIGHT HOLDER, OR ANY OTHER PARTY WHO MODIFIES AND/OR CONVEYS
> +# THE PROGRAM AS PERMITTED ABOVE, BE LIABLE TO YOU FOR DAMAGES, INCLUDING ANY
> +# GENERAL, SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE
> +# USE OR INABILITY TO USE THE PROGRAM (INCLUDING BUT NOT LIMITED TO LOSS OF
> +# DATA OR DATA BEING RENDERED INACCURATE OR LOSSES SUSTAINED BY YOU OR THIRD
> +# PARTIES OR A FAILURE OF THE PROGRAM TO OPERATE WITH ANY OTHER PROGRAMS),
> +# EVEN IF SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE POSSIBILITY OF
> +# SUCH DAMAGES.
> +###############################################################################
> +## <summary>policy for nginx</summary>

Documentation (summary) is lacking.

> +
> +########################################
> +## <summary>
> +##	Execute a domain transition to run nginx.
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +##	Domain allowed to transition.
> +## </summary>
> +## </param>
> +#
> +interface(`nginx_domtrans',`
> +	gen_require(`
> +		type nginx_t, nginx_exec_t;
> +	')
> +	allow nginx_t $1:fd use;
> +	allow nginx_t $1:fifo_file rw_file_perms;
> +	allow nginx_t $1:process sigchld;
> +
> +	domain_auto_transition_pattern($1, nginx_exec_t, nginx_t)
> +')

Use domtrans() here

> +
> +########################################
> +## <summary>
> +##   Administer the nginx domain
> +## </summary>
> +## <param name="domain">
> +##	<summary>
> +##	Domain allowed access.
> +##	</summary>
> +## </param>
> +## <param name="role">
> +##	<summary>
> +##	The role to be allowed to manage the nginx domain.
> +##	</summary>
> +## </param>
> +## <rolecap/>
> +#
> +interface(`nginx_admin',`
> +	gen_require(`
> +		type nginx_t, nginx_conf_t, nginx_log_t, nginx_var_lib_t, nginx_var_run_t;
> +		type nginx_exec_t;
> +	')

you reference nginx_var_lib_t and nginx_var_run_t here but there are no references to those types in the .fc file

> +
> +	allow $1 nginx_t:process { ptrace signal_perms };
> +	ps_process_pattern($1, nginx_t)
> +
> +	files_list_etc($1)
> +	admin_pattern($1, nginx_conf_t)
> +
> +	can_exec($1, nginx_exec_t)
> +
> +	files_list_var_lib($1)
> +	admin_pattern($1, nginx_var_lib_t)
> +
> +	logging_list_logs($1)
> +	admin_pattern($1, nginx_log_t)
> +
> +	files_list_pids($1)
> +	admin_pattern($1, nginx_var_run_t)
> +')
> diff --git a/policy/modules/services/nginx.te b/policy/modules/services/nginx.te
> new file mode 100644
> index 000000000000..be59babb8596
> --- /dev/null
> +++ b/policy/modules/services/nginx.te
> @@ -0,0 +1,169 @@
> +# SELinux module for the NGINX Web Server
> +policy_module(nginx,1.0.10)
> +
> +########################################
> +#
> +# Declarations
> +#
> +
> +## <desc>
> +## <p>
> +## Allow nginx to serve HTTP content (act as an http server)
> +## </p>
> +## </desc>
> +gen_tunable(nginx_enable_http_server, false)

Not sure if it worth it to make this conditional

> +
> +## <desc>
> +## <p>
> +## Allow nginx to act as an imap proxy server)
> +## </p>
> +## </desc>
> +gen_tunable(nginx_enable_imap_server, false)
> +
> +## <desc>
> +## <p>
> +## Allow nginx to act as a pop3 server)
> +## </p>
> +## </desc>
> +gen_tunable(nginx_enable_pop3_server, false)
> +
> +## <desc>
> +## <p>
> +## Allow nginx to act as an smtp server)
> +## </p>
> +## </desc>
> +gen_tunable(nginx_enable_smtp_server, false)
> +
> +## <desc>
> +## <p>
> +## Allow nginx to connect to remote HTTP servers
> +## </p>
> +## </desc>
> +gen_tunable(nginx_can_network_connect_http, false)
> +
> +## <desc>
> +## <p>
> +## Allow nginx to connect to remote servers (regardless of protocol)
> +## </p>
> +## </desc>
> +gen_tunable(nginx_can_network_connect, false)
> +
> +type nginx_t;
> +type nginx_exec_t;
> +init_daemon_domain(nginx_t, nginx_exec_t)
> +
> +# conf files
> +type nginx_conf_t;
> +files_type(nginx_conf_t)

use files_config_file()

> +
> +# log files
> +type nginx_log_t;
> +logging_log_file(nginx_log_t)
> +
> +# tmp files
> +type nginx_tmp_t;
> +files_tmp_file(nginx_tmp_t)
> +
> +# var/lib files
> +type nginx_var_lib_t;
> +files_type(nginx_var_lib_t)
> +
> +# pid files
> +type nginx_var_run_t;
> +files_pid_file(nginx_var_run_t)
> +
> +########################################
> +#
> +# nginx local policy
> +#
> +
> +allow nginx_t self:fifo_file { read write };

use rw_fifo_file_perms

> +allow nginx_t self:unix_stream_socket create_stream_socket_perms;

I see below that it maintains a socket in /var/lib/.* what is that for.
Can entities connect to nginx with a unix domain socket?

> +allow nginx_t self:tcp_socket { listen accept };
> +allow nginx_t self:capability { setuid net_bind_service setgid chown };
> +
> +# conf files
> +list_dirs_pattern(nginx_t, nginx_conf_t, nginx_conf_t)
> +read_files_pattern(nginx_t, nginx_conf_t, nginx_conf_t)
> +
> +# log files
> +manage_files_pattern(nginx_t, nginx_log_t, nginx_log_t)

you may be able to ommit the "write" permission here.

> +logging_log_filetrans(nginx_t, nginx_log_t, { file dir })

the dir here does not make sense, as nginx_t is not allowed to create it anyway. Either allow nginx to manage nginx_log_t dirs or remove it

> +
> +
> +# pid file
> +manage_dirs_pattern(nginx_t, nginx_var_run_t, nginx_var_run_t)
> +manage_files_pattern(nginx_t, nginx_var_run_t, nginx_var_run_t)
> +files_pid_filetrans(nginx_t, nginx_var_run_t, file)

Here its the opposite. you allos it to create dirs but theres no file transition rule. In addition .fc spec is missing.

> +
> +# tmp files
> +manage_files_pattern(nginx_t, nginx_tmp_t, nginx_tmp_t)
> +manage_dirs_pattern(nginx_t, nginx_tmp_t, nginx_tmp_t)
> +files_tmp_filetrans(nginx_t, nginx_tmp_t, dir)
> +
> +# var/lib files
> +create_files_pattern(nginx_t, nginx_var_lib_t, nginx_var_lib_t)
> +create_sock_files_pattern(nginx_t, nginx_var_lib_t, nginx_var_lib_t)
> +files_var_lib_filetrans(nginx_t,nginx_var_lib_t, { file dir sock_file })

fc spec is missing. Its not allowed to create nginx_var_lib_t dirs.

> +
> +
> +kernel_read_kernel_sysctls(nginx_t)
> +corenet_tcp_bind_generic_node(nginx_t)
> +corenet_tcp_sendrecv_generic_if(nginx_t)
> +corenet_tcp_sendrecv_generic_node(nginx_t)
> +
> +dev_read_rand(nginx_t)
> +dev_read_urand(nginx_t)
> +
> +domain_use_interactive_fds(nginx_t)

Probaby not needed

> +
> +files_read_etc_files(nginx_t)
> +
> +
> +miscfiles_read_localization(nginx_t)
> +sysnet_dns_name_resolve(nginx_t)
> +
> +
> +tunable_policy(`nginx_enable_http_server',`
> +	corenet_tcp_bind_http_port(nginx_t)
> +	apache_read_all_content(nginx_t)
> +	apache_manage_all_rw_content(nginx_t)
> +')
> +
> +# We enable both binding and connecting, since nginx acts here as a reverse proxy
> +tunable_policy(`nginx_enable_imap_server',`
> +	corenet_tcp_bind_pop_port(nginx_t)
> +	corenet_tcp_connect_pop_port(nginx_t)
> +')
> +
> +tunable_policy(`nginx_enable_pop3_server',`
> +	corenet_tcp_bind_pop_port(nginx_t)
> +	corenet_tcp_connect_pop_port(nginx_t)
> +')
> +
> +tunable_policy(`nginx_enable_smtp_server',`
> +	corenet_tcp_bind_smtp_port(nginx_t)
> +	corenet_tcp_connect_smtp_port(nginx_t)
> +')
> +
> +tunable_policy(`nginx_can_network_connect_http',`
> +	corenet_tcp_connect_http_port(nginx_t)
> +')
> +
> +tunable_policy(`nginx_can_network_connect',`
> +	corenet_tcp_connect_all_ports(nginx_t)
> +')
> +
> +optional_policy(`
> +	phpfpm_stream_connect(nginx_t)
> +')
> +
> +ifdef(`distro_gentoo',`
> +
> +	# needs to be able to signal its children
> +	allow nginx_t self:process { signal sigchld };
> +
> +	optional_policy(`
> +		uwsgi_stream_connect(nginx_t)
> +	')
> +')
> -- 
> 2.21.0
> 

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift