Hi, I've now used this for some time and have a couple of things to add. On 4/14/20 4:54 PM, Chris PeBenito wrote: > On 4/10/20 4:07 AM, Dominick Grift wrote: >> Russell Coker <russell@xxxxxxxxxxxx> writes: >> >>> Signed-off-by: Russell Coker <russell@xxxxxxxxxxxx> >>> >>> I think this addresses all the issues Chris raised. > > > I don't have any comments beyond Dominick's. > >>> Index: refpolicy-2.20200410/policy/modules/services/certbot.fc >>> =================================================================== >>> --- /dev/null >>> +++ refpolicy-2.20200410/policy/modules/services/certbot.fc >>> @@ -0,0 +1,4 @@ >>> +/usr/bin/certbot -- >>> gen_context(system_u:object_r:certbot_exec_t,s0) >>> +/usr/bin/letsencrypt -- >>> gen_context(system_u:object_r:certbot_exec_t,s0) >>> +/var/log/letsencrypt(/.*)? >>> gen_context(system_u:object_r:certbot_log_t,s0) >>> +/var/lib/letsencrypt(/.*)? >>> gen_context(system_u:object_r:certbot_lib_t,s0) On my debian system certbot puts private certificates into /etc/letsencrypt hence I labeled it certbot_lib_t too but that probably isn't right. >>> Index: refpolicy-2.20200410/policy/modules/services/certbot.if >>> =================================================================== >>> --- /dev/null >>> +++ refpolicy-2.20200410/policy/modules/services/certbot.if >>> @@ -0,0 +1,46 @@ >>> +## <summary>SSL certificate requesting tool certbot AKA >>> letsencrypt.</summary> >>> + >>> +######################################## >>> +## <summary> >>> +## Execute certbot/letsencrypt in the certbot >>> +## domain. >>> +## </summary> >>> +## <param name="domain"> >>> +## <summary> >>> +## Domain allowed to transition. >>> +## </summary> >>> +## </param> >>> +# >>> +interface(`certbot_domtrans',` >>> + gen_require(` >>> + type certbot_t, certbot_exec_t; >>> + ') >>> + >>> + domtrans_pattern($1, certbot_exec_t, certbot_t) >>> +') >>> + >>> +######################################## >>> +## <summary> >>> +## Execute certbot/letsencrypt in the certbot >>> +## domain, and allow the specified role >>> +## the firstboot domain. >>> +## </summary> >>> +## <param name="role"> >>> +## <summary> >>> +## Role allowed access. >>> +## </summary> >>> +## </param> >>> +## <param name="domain"> >>> +## <summary> >>> +## Domain allowed to transition. >>> +## </summary> >>> +## </param> >>> +# >>> +interface(`certbot_run',` >>> + gen_require(` >>> + type certbot_t; >>> + ') >>> + >>> + certbot_domtrans($2) >>> + role $1 types certbot_t; >> >> might want to call this: certbot_run(sysadm_r, sysadm_t) >> Swapping the argument order here would be better in line with the rest of refpolicy. Calling this for sysadm would also be a good idea since it is usually used for obtaining the first certificate. >>> +') >>> Index: refpolicy-2.20200410/policy/modules/services/certbot.te >>> =================================================================== >>> --- /dev/null >>> +++ refpolicy-2.20200410/policy/modules/services/certbot.te >>> @@ -0,0 +1,99 @@ >>> +policy_module(certbot, 1.0.0) >>> + >>> +######################################## >>> +# >>> +# Declarations >>> +# >>> + >>> +type certbot_t; >>> +type certbot_exec_t; >>> +init_daemon_domain(certbot_t, certbot_exec_t) >>> + >>> +type certbot_log_t; >>> +logging_log_file(certbot_log_t) >>> + >>> +type certbot_runtime_t alias certbot_var_run_t; >>> +files_pid_file(certbot_runtime_t) >>> + >>> +type certbot_tmp_t; >>> +files_tmp_file(certbot_tmp_t) >>> + >>> +type certbot_tmpfs_t; >>> +files_tmpfs_file(certbot_tmpfs_t) >>> + >>> +type certbot_lib_t alias certbot_var_lib_t; >>> +files_type(certbot_lib_t) >> >> I would have used certbot_state_t here so that "lib" can be used for >> private library types >> >>> + >>> +######################################## >>> +# >>> +# Local policy >>> +# >>> + >>> +allow certbot_t self:fifo_file { getattr ioctl read write }; >>> +allow certbot_t self:capability { chown dac_override sys_resource }; >>> +allow certbot_t self:udp_socket all_udp_socket_perms; >>> +allow certbot_t self:tcp_socket all_tcp_socket_perms; >>> +allow certbot_t self:netlink_route_socket create_netlink_socket_perms; >>> + >>> +files_search_var_lib(certbot_t) >>> +manage_dirs_pattern(certbot_t, certbot_lib_t, certbot_lib_t) >>> +manage_files_pattern(certbot_t, certbot_lib_t, certbot_lib_t) >>> + >>> +manage_files_pattern(certbot_t, certbot_tmp_t, certbot_tmp_t) >>> +files_tmp_filetrans(certbot_t, certbot_tmp_t, { file }) >>> + >>> +manage_files_pattern(certbot_t, certbot_tmpfs_t, certbot_tmpfs_t) >>> +fs_tmpfs_filetrans(certbot_t, certbot_tmpfs_t, { file }) >>> + >>> +# this is for certbot to have write-exec memory, I know it is bad >>> +# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=913544 >>> +# the Debian bug report has background about python-acme and >>> python3-openssl >>> +allow certbot_t self:process execmem; >>> +allow certbot_t certbot_tmp_t:file { map execute }; >>> +allow certbot_t certbot_tmpfs_t:file { map execute }; >>> +allow certbot_t certbot_runtime_t:file { map execute }; >>> + >>> +logging_search_logs(certbot_t) >>> +allow certbot_t certbot_log_t:dir manage_dir_perms; >>> +allow certbot_t certbot_log_t:file manage_file_perms; >>> + I've found that this is missing a filetrans since certbot might try to create its own log directory. Removing 'remove_name reparent rmdir' from the permissions granted on certbot_log_t is also possible since imho domains normally shouldn't delete their own but it might be better to keep ':dir manage_dir_perms' +manage_files_pattern(certbot_t, certbot_log_t, certbot_log_t) +allow certbot_t certbot_log_t:dir { create_dir_perms search_dir_perms add_entry_dir_perms }; +logging_log_filetrans(certbot_t, certbot_log_t, dir) >>> +manage_files_pattern(certbot_t, certbot_runtime_t, certbot_runtime_t) >>> +files_pid_filetrans(certbot_t, certbot_runtime_t, file) >>> + >>> +kernel_search_fs_sysctls(certbot_t) >>> + >>> +corecmd_exec_bin(certbot_t) >>> +corecmd_list_bin(certbot_t) >>> +corecmd_mmap_bin_files(certbot_t) >> >> redundant: exec implies mmap >> >>> + >>> +corenet_tcp_bind_generic_node(certbot_t) >>> +corenet_tcp_connect_http_port(certbot_t) >>> + >>> +# bind to http port for standalone mode >>> +corenet_tcp_bind_http_port(certbot_t) >>> + >>> +domain_use_interactive_fds(certbot_t) >>> +files_read_etc_files(certbot_t) >>> + >>> +libs_exec_ldconfig(certbot_t) >>> +# for /usr/lib/gcc/x86_64-linux-gnu/8/collect2 >>> +libs_exec_lib_files(certbot_t) >>> + >>> +miscfiles_read_localization(certbot_t) >>> + >>> +miscfiles_read_generic_certs(certbot_t) >>> +miscfiles_manage_generic_tls_privkey_dirs(certbot_t) >>> +miscfiles_manage_generic_tls_privkey_files(certbot_t) >>> +miscfiles_manage_generic_tls_privkey_lnk_files(certbot_t) >>> + >>> +sysnet_read_config(certbot_t) >>> + >>> +userdom_dontaudit_search_user_home_dirs(certbot_t) >>> +userdom_use_user_ptys(certbot_t) >>> + >>> +optional_policy(` >>> + # for writing to webroot >>> + apache_manage_sys_content(certbot_t) >>> + >>> + apache_search_config(certbot_t) >>> +') >>> Index: refpolicy-2.20200410/policy/modules/system/miscfiles.if >>> =================================================================== >>> --- refpolicy-2.20200410.orig/policy/modules/system/miscfiles.if >>> +++ refpolicy-2.20200410/policy/modules/system/miscfiles.if >>> @@ -254,6 +254,26 @@ interface(`miscfiles_manage_generic_tls_ >>> ######################################## >>> ## <summary> >>> +## Manage generic SSL/TLS private >>> +## keys. >>> +## </summary> >>> +## <param name="domain"> >>> +## <summary> >>> +## Domain allowed access. >>> +## </summary> >>> +## </param> >>> +## <rolecap/> >>> +# >>> +interface(`miscfiles_manage_generic_tls_privkey_lnk_files',` >>> + gen_require(` >>> + type tls_privkey_t; >>> + ') >>> + >>> + manage_lnk_files_pattern($1, tls_privkey_t, tls_privkey_t) >>> +') >>> + >>> +######################################## >>> +## <summary> >>> ## Read fonts. >>> ## </summary> >>> ## <param name="domain"> >> > > Thanks, bauen1