On Thursday, 21 January 2021 1:36:46 AM AEDT Dominick Grift wrote:
> > Index: refpolicy-2.20210120/policy/modules/kernel/corecommands.if
> > ===================================================================
> > --- refpolicy-2.20210120.orig/policy/modules/kernel/corecommands.if
> > +++ refpolicy-2.20210120/policy/modules/kernel/corecommands.if
> > @@ -662,6 +662,7 @@ interface(`corecmd_read_all_executables'
> >
> > corecmd_search_bin($1)
> > read_files_pattern($1, exec_type, exec_type)
> >
> > + allow $1 exec_type:file map;
>
> create a corecmd_map_read_all_executables() instead. This macro name is
> "read_all_executables" if you extend it with this rule then you
> effectively do several things:
OK, I'll do that in another patch.
> > +interface(`files_mounton_kernel_symbol_table',`
> > + gen_require(`
> > + type boot_t, system_map_t;
> > + ')
> > +
> > + allow $1 boot_t:dir list_dir_perms;
> > + allow $1 system_map_t:file mounton;
>
> mount != listing boot_t dirs (i know its semi-related but you might want
> to mount on symbox table and not list boot_t and this will shut the door
> on that)
>
> instead you should probably imply getattr here:
>
> allow $1 system_map_t:file { getattr mounton };
>
> Would be even better to declare "mounton_file_perms" on a lower level
> and use that
>
> define(`mounton_file_perms',`{ getattr mounton }')
OK, that will be in the next version.
> > +## Mount on the selinuxfs filesystem.
> > +## </summary>
> > +## <param name="domain">
> > +## <summary>
> > +## Domain allowed access.
> > +## </summary>
> > +## </param>
> > +#
> > +interface(`selinux_mounton_fs',`
> > + gen_require(`
> > + type security_t;
> > + ')
> > +
> > + allow $1 security_t:dir mounton;
>
> getattr should probably be implied here
>
> a mounton_dir_perms would be even better:
>
> define(`mounton_dir_perms',`{ getattr mounton }')
OK.
> > Index: refpolicy-2.20210120/policy/modules/kernel/terminal.te
> > ===================================================================
> > --- refpolicy-2.20210120.orig/policy/modules/kernel/terminal.te
> > +++ refpolicy-2.20210120/policy/modules/kernel/terminal.te
> > @@ -31,6 +31,9 @@ fs_associate_tmpfs(devpts_t)
> >
> > fs_xattr_type(devpts_t)
> > fs_use_trans devpts gen_context(system_u:object_r:devpts_t,s0);
> >
> > +# for systemd-nspawn
> > +allow console_device_t devpts_t:filesystem associate;
>
> I am a fairly big user of systemd_nspawn and i have never ever
> encountered this. only pty devices should ever associate with devpts_t
> filesystems AFAIK
OK, I'll remove that and investigate other solutions.
> > +## Allow a domain to be transitioned to from init_t with nnp_transition
> > +## </summary>
> > +## <param name="domain">
> > +## <summary>
> > +## Domain to transition
> > +## </summary>
> > +## </param>
> > +#
> > +interface(`init_nnp_domain',`
> > + gen_require(`
> > + type init_t;
> > + ')
> > +
> > + allow init_t $1:process2 nnp_transition;
> > +')
>
> This is redundant. In systems with systemd (ifdef init_systemd) this access
> is already allowed.
OK, I'll remove it.
> > +######################################
> > +## <summary>
> > +## restart systemd units, for /run/systemd/transient/*
> > +## </summary>
> > +## <param name="domain">
> > +## <summary>
> > +## Domain allowed access.
> > +## </summary>
> > +## </param>
> > +#
> > +interface(`init_restart_units',`
> > + gen_require(`
> > + type init_var_run_t;
> > + ')
> > +
> > + allow $1 init_var_run_t:service { start status stop };
> > +')
>
> i would probably create a private type for "runtime units"
> but also in another patch you create another "restart_units" interface
> and that has different permissions (probably best to associate
> consistent permissions with interface names)
>
> not where "restart_units" means something different somewhere else
I'll move this to another patch.
> > Index: refpolicy-2.20210120/policy/modules/system/init.te
> > ===================================================================
> > --- refpolicy-2.20210120.orig/policy/modules/system/init.te
> > +++ refpolicy-2.20210120/policy/modules/system/init.te
> > @@ -239,7 +239,8 @@ ifdef(`init_systemd',`
> >
> > allow init_t self:unix_stream_socket { create_stream_socket_perms
> > connectto }; allow init_t self:netlink_audit_socket { nlmsg_relay
> > create_socket_perms }; allow init_t self:netlink_selinux_socket
> > create_socket_perms;
> >
> > - allow init_t self:system { status reboot halt reload };
> > + # why does kernel 4.9 make it need start and stop while 4.19 does not?
> > + allow init_t self:system { start stop status reboot halt reload
> >
> > };
>
> I would remove the above change. might have been a bug in 4.9, no need
> to support bugs besides kernel 4.9 is old.
OK, I've removed that.
> > @@ -1002,6 +1003,7 @@ ifdef(`enabled_mls',`
> >
> > ifdef(`init_systemd',`
> >
> > allow initrc_t init_t:system { start status reboot halt reload };
> >
> > + allow init_t initrc_t:process2 nnp_transition;
>
> this is dedundant. Should already be allowed
OK.
> > Index: refpolicy-2.20210120/policy/modules/system/locallogin.te
> > ===================================================================
> > --- refpolicy-2.20210120.orig/policy/modules/system/locallogin.te
> > +++ refpolicy-2.20210120/policy/modules/system/locallogin.te
> > @@ -125,7 +125,8 @@ auth_manage_pam_runtime_files(local_logi
> >
> > auth_manage_pam_console_data(local_login_t)
> > auth_domtrans_pam_console(local_login_t)
> >
> > -init_dontaudit_use_fds(local_login_t)
> > +# if local_login_t can not inherit fd from init it takes ages to login
> > +init_use_fds(local_login_t)
>
> Yes i think youre right but i think this applies to all processes forked
> by systemd. I believe that addressing rules associated with systemd
> forked processes should probably be addressed on a lower level instead
>
> for example:
>
> init_domain is obviously systemd forked in a systemd system (init_domain
> is allowed to use init fd via domtrans_pattern(init_t, $1, $2) in
> init_domain().
>
> Howver local_login is not a direct fork of systemd (its not an
> init_daemon) and instead its a indirect forked process of systemd (it
> gets executed by a init domain but not by init itself)
>
> I would create a type attribute "systemd_forked_type" and then associate
> the forked related rules to that and then use that
>
> i think these (or somthing like it):
>
> allow $1 systemd_forked_type:fd use;
> allow $1 systemd_forked_type:unix_stream_socket rw_socket_perms;
>
> These these can be removed:
I'll move this to another patch and another discussion.
> https://github.com/SELinuxProject/refpolicy/blob/ea6002ddf9c09a307dccc4bf662
> ff7efa2395572/policy/modules/system/init.if#L186
> https://github.com/SELinuxProject/refpolicy/blob/master/policy/modules/syst
> em/init.if#L149 etc
>
> otherwise you end up with very decentralized policy which is hard to
> maintain.
> > +######################################
> > +## <summary>
> > +## Allow lvm_t to use a semaphore
> > +## </summary>
> > +## <param name="domain">
> > +## <summary>
> > +## Domain that created the semaphore
> > +## </summary>
> > +## </param>
> > +#
> > +interface(`lvm_use_sem',`
> > + gen_require(`
> > + type lvm_t;
> > + ')
> > +
> > + allow lvm_t $1:sem all_sem_perms;
>
> Thats not allowed like this generally
OK, I'll do it differently.
> > @@ -99,6 +99,7 @@ fs_getattr_xattr_fs(kmod_t)
> >
> > fs_dontaudit_use_tmpfs_chr_dev(kmod_t)
> > fs_search_tracefs(kmod_t)
> >
> > +init_nnp_domain(kmod_t)
>
> shouldnt be needed : kmod is a init_system_domain which is a
> init_domain, and systemd can already nnp transition to all init_domain
> if ifdef init_systemd is set
OK, I'll test that out.
> > +term_use_unallocated_ttys(systemd_passwd_agent_t)
> >
> > auth_use_nsswitch(systemd_passwd_agent_t)
> >
> > @@ -1100,6 +1133,8 @@ logging_send_syslog_msg(systemd_pstore_t
> >
> > allow systemd_rfkill_t self:netlink_kobject_uevent_socket { bind create
> > getattr read setopt };>
> > +allow systemd_rfkill_t self:netlink_kobject_uevent_socket
> > client_stream_socket_perms;
> thats not a stream socket, do this instead:
>
> - allow systemd_rfkill_t self:netlink_kobject_uevent_socket { bind create
> getattr read setopt }; + allow systemd_rfkill_t
> self:netlink_kobject_uevent_socket create_socket_perms;
OK.
> > @@ -1264,6 +1299,8 @@ allow systemd_tmpfiles_t systemd_journal
> >
> > allow systemd_tmpfiles_t systemd_tmpfiles_conf_t:dir list_dir_perms;
> > allow systemd_tmpfiles_t systemd_tmpfiles_conf_type:file read_file_perms;
> >
> > +allow systemd_tmpfiles_t systemd_nspawn_runtime_t:fifo_file unlink;
>
> questionable
Why?
> > Index: refpolicy-2.20210120/policy/modules/system/unconfined.te
> > ===================================================================
> > --- refpolicy-2.20210120.orig/policy/modules/system/unconfined.te
> > +++ refpolicy-2.20210120/policy/modules/system/unconfined.te
> > @@ -83,6 +83,10 @@ optional_policy(`
> >
> > ')
> >
> > optional_policy(`
> >
> > + certbot_run(unconfined_t, unconfined_r)
>
> unconfined should be unconfined.
certbot needs execmem, we generally don't want to give that to unconfined, so
running certbot in a different domain seems better.
> > optional_policy(`
> >
> > lvm_run(unconfined_t, unconfined_r)
> >
> > + lvm_use_sem(unconfined_t)
>
> that lvm_use_sem should probably just be part of lvm_run()
>
> ie "allow $1 lvm_t:semd rw_sem_perms;"
OK, I'll do that.
> But in my personal view unconfined_t shouldnt run lvm with a domain
> transition in the first place (defeats the purpose of the unconfined domain)
I think the problem is the type transition rules. Run lvm etc as unconfined_t
and then lvm run from init etc won't be able to access it's temporary files
etc.
> > Index: refpolicy-2.20210120/policy/modules/system/userdomain.if
> > ===================================================================
> > --- refpolicy-2.20210120.orig/policy/modules/system/userdomain.if
> > +++ refpolicy-2.20210120/policy/modules/system/userdomain.if
> > @@ -2167,6 +2167,8 @@ interface(`userdom_read_user_home_conten
> >
> > ')
> >
> > read_files_pattern($1, { user_home_dir_t user_home_t }, user_home_t)
> >
> > + allow $1 user_home_t:file map;
>
> read != map
> and file != lnk_file
>
> by generalizing interfaces you shut doors for fine grained access control
OK, I'll remove that.
--
My Main Blog http://etbe.coker.com.au/
My Documents Blog http://doc.coker.com.au/
