[refpolicy] [PATCH] locallogin: fix the sulogin submodule (emergency shell!)

[refpolicy] [PATCH] locallogin: fix the sulogin submodule (emergency shell!)

[Guido Trentalancia]

This patch fixes the policy for sulogin. It is very important
because without this patch, sulogin cannot work properly and
it should be considered that it is used as an emergency shell
when there are serious consistency errors in the system, so it
constitutes the only way to recover the system in such
circumstances.

Nowadays, sulogin never uses PAM (at least not the official one
from util-linux), so obsolete, confusing and buggy policy has
been removed.

Extensive testing carried out while creating this patch indicates
that there aren't other permissions needed to successfully run
sulogin.

Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
---
 policy/modules/system/locallogin.te |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

--- a/policy/modules/system/locallogin.te	2017-02-04 19:30:17.000000000 +0100
+++ b/policy/modules/system/locallogin.te	2017-04-21 21:01:04.693531271 +0200
@@ -210,8 +210,8 @@ optional_policy(`
 # Sulogin local policy
 #
 
-allow sulogin_t self:capability dac_override;
-allow sulogin_t self:process ~{ ptrace setcurrent setexec setfscreate setrlimit execmem execstack execheap };
+allow sulogin_t self:capability { dac_override sys_admin sys_tty_config };
+allow sulogin_t self:process ~{ ptrace setcurrent setfscreate setrlimit execmem execstack execheap };
 allow sulogin_t self:fd use;
 allow sulogin_t self:fifo_file rw_fifo_file_perms;
 allow sulogin_t self:unix_dgram_socket create_socket_perms;
@@ -224,6 +224,9 @@ allow sulogin_t self:msgq create_msgq_pe
 allow sulogin_t self:msg { send receive };
 
 kernel_read_system_state(sulogin_t)
+kernel_read_crypto_sysctls(sulogin_t)
+kernel_stream_connect(sulogin_t)
+kernel_use_fds(sulogin_t)
 # because file systems are not mounted:
 kernel_dontaudit_search_unlabeled(sulogin_t)
 
@@ -234,10 +237,13 @@ files_read_etc_files(sulogin_t)
 
 auth_read_shadow(sulogin_t)
 
+init_getpgid(sulogin_t)
 init_getpgid_script(sulogin_t)
 
 logging_send_syslog_msg(sulogin_t)
 
+miscfiles_read_localization(sulogin_t)
+
 seutil_read_config(sulogin_t)
 seutil_read_default_contexts(sulogin_t)
 
@@ -248,15 +254,12 @@ userdom_use_user_ptys(sulogin_t)
 
 sysadm_shell_domtrans(sulogin_t)
 
-# suse and debian do not use pam with sulogin...
-ifdef(`distro_suse', `define(`sulogin_no_pam')')
-ifdef(`distro_debian', `define(`sulogin_no_pam')')
-
-ifdef(`sulogin_no_pam', `
-	allow sulogin_t self:capability sys_tty_config;
-	init_getpgid(sulogin_t)
-', `
-	allow sulogin_t self:process setexec;
+term_use_console(sulogin_t)
+term_use_unallocated_ttys(sulogin_t)
+
+# by default, sulogin does not use pam...
+# sulogin_pam might need to be defined otherwise
+ifdef(`sulogin_pam', `
 	selinux_get_fs_mount(sulogin_t)
 	selinux_validate_context(sulogin_t)
 	selinux_compute_access_vector(sulogin_t)

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Guido Trentalancia]

This patch fixes the policy for sulogin. It is very important
because without this patch, sulogin cannot work properly and
it should be considered that it is used as an emergency shell
when there are serious consistency errors in the system, so it
constitutes the only way to recover the system in such
circumstances.

Nowadays, sulogin never uses PAM (at least not the official one
from util-linux), so obsolete, confusing and buggy policy has
been removed.

Extensive testing carried out while creating this patch indicates
that there aren't other permissions needed to successfully run
sulogin.

This second version should apply cleanly to the latest git tree.

Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
---
 policy/modules/system/locallogin.te |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

--- a/policy/modules/system/locallogin.te	2017-04-13 22:04:35.111202539 +0200
+++ b/policy/modules/system/locallogin.te	2017-04-26 00:44:23.028943625 +0200
@@ -215,7 +215,8 @@ optional_policy(`
 # Sulogin local policy
 #
 
-allow sulogin_t self:capability dac_override;
+allow sulogin_t self:capability { dac_override sys_admin sys_tty_config };
+allow sulogin_t self:process setexec;
 allow sulogin_t self:fd use;
 allow sulogin_t self:fifo_file rw_fifo_file_perms;
 allow sulogin_t self:unix_dgram_socket create_socket_perms;
@@ -228,6 +229,9 @@ allow sulogin_t self:msgq create_msgq_pe
 allow sulogin_t self:msg { send receive };
 
 kernel_read_system_state(sulogin_t)
+kernel_read_crypto_sysctls(sulogin_t)
+kernel_stream_connect(sulogin_t)
+kernel_use_fds(sulogin_t)
 # because file systems are not mounted:
 kernel_dontaudit_search_unlabeled(sulogin_t)
 
@@ -238,10 +242,13 @@ files_read_etc_files(sulogin_t)
 
 auth_read_shadow(sulogin_t)
 
+init_getpgid(sulogin_t)
 init_getpgid_script(sulogin_t)
 
 logging_send_syslog_msg(sulogin_t)
 
+miscfiles_read_localization(sulogin_t)
+
 seutil_read_config(sulogin_t)
 seutil_read_default_contexts(sulogin_t)
 
@@ -252,15 +259,12 @@ userdom_use_user_ptys(sulogin_t)
 
 sysadm_shell_domtrans(sulogin_t)
 
-# suse and debian do not use pam with sulogin...
-ifdef(`distro_suse', `define(`sulogin_no_pam')')
-ifdef(`distro_debian', `define(`sulogin_no_pam')')
-
-ifdef(`sulogin_no_pam', `
-	allow sulogin_t self:capability sys_tty_config;
-	init_getpgid(sulogin_t)
-', `
-	allow sulogin_t self:process setexec;
+term_use_console(sulogin_t)
+term_use_unallocated_ttys(sulogin_t)
+
+# by default, sulogin does not use pam...
+# sulogin_pam might need to be defined otherwise
+ifdef(`sulogin_pam', `
 	selinux_get_fs_mount(sulogin_t)
 	selinux_validate_context(sulogin_t)
 	selinux_compute_access_vector(sulogin_t)

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Chris PeBenito]

On 04/25/2017 06:50 PM, Guido Trentalancia via refpolicy wrote:
> This patch fixes the policy for sulogin. It is very important
> because without this patch, sulogin cannot work properly and
> it should be considered that it is used as an emergency shell
> when there are serious consistency errors in the system, so it
> constitutes the only way to recover the system in such
> circumstances.
>
> Nowadays, sulogin never uses PAM (at least not the official one
> from util-linux), so obsolete, confusing and buggy policy has
> been removed.
>
> Extensive testing carried out while creating this patch indicates
> that there aren't other permissions needed to successfully run
> sulogin.
>
> This second version should apply cleanly to the latest git tree.
>
> Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
> ---
>  policy/modules/system/locallogin.te |   24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> --- a/policy/modules/system/locallogin.te	2017-04-13 22:04:35.111202539 +0200
> +++ b/policy/modules/system/locallogin.te	2017-04-26 00:44:23.028943625 +0200
> @@ -215,7 +215,8 @@ optional_policy(`
>  # Sulogin local policy
>  #
>
> -allow sulogin_t self:capability dac_override;
> +allow sulogin_t self:capability { dac_override sys_admin sys_tty_config };
> +allow sulogin_t self:process setexec;
>  allow sulogin_t self:fd use;
>  allow sulogin_t self:fifo_file rw_fifo_file_perms;
>  allow sulogin_t self:unix_dgram_socket create_socket_perms;
> @@ -228,6 +229,9 @@ allow sulogin_t self:msgq create_msgq_pe
>  allow sulogin_t self:msg { send receive };
>
>  kernel_read_system_state(sulogin_t)
> +kernel_read_crypto_sysctls(sulogin_t)
> +kernel_stream_connect(sulogin_t)
> +kernel_use_fds(sulogin_t)
>  # because file systems are not mounted:
>  kernel_dontaudit_search_unlabeled(sulogin_t)
>
> @@ -238,10 +242,13 @@ files_read_etc_files(sulogin_t)
>
>  auth_read_shadow(sulogin_t)
>
> +init_getpgid(sulogin_t)
>  init_getpgid_script(sulogin_t)
>
>  logging_send_syslog_msg(sulogin_t)
>
> +miscfiles_read_localization(sulogin_t)
> +
>  seutil_read_config(sulogin_t)
>  seutil_read_default_contexts(sulogin_t)
>
> @@ -252,15 +259,12 @@ userdom_use_user_ptys(sulogin_t)
>
>  sysadm_shell_domtrans(sulogin_t)
>
> -# suse and debian do not use pam with sulogin...
> -ifdef(`distro_suse', `define(`sulogin_no_pam')')
> -ifdef(`distro_debian', `define(`sulogin_no_pam')')
> -
> -ifdef(`sulogin_no_pam', `
> -	allow sulogin_t self:capability sys_tty_config;
> -	init_getpgid(sulogin_t)
> -', `
> -	allow sulogin_t self:process setexec;
> +term_use_console(sulogin_t)
> +term_use_unallocated_ttys(sulogin_t)
> +
> +# by default, sulogin does not use pam...
> +# sulogin_pam might need to be defined otherwise
> +ifdef(`sulogin_pam', `
>  	selinux_get_fs_mount(sulogin_t)
>  	selinux_validate_context(sulogin_t)
>  	selinux_compute_access_vector(sulogin_t)

Merged, though I moved the terminal lines.


-- 
Chris PeBenito

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Dominick Grift]

On Wed, Apr 26, 2017 at 12:50:02AM +0200, Guido Trentalancia via refpolicy wrote:
> This patch fixes the policy for sulogin. It is very important
> because without this patch, sulogin cannot work properly and
> it should be considered that it is used as an emergency shell
> when there are serious consistency errors in the system, so it
> constitutes the only way to recover the system in such
> circumstances.
> 
> Nowadays, sulogin never uses PAM (at least not the official one
> from util-linux), so obsolete, confusing and buggy policy has
> been removed.
> 
> Extensive testing carried out while creating this patch indicates
> that there aren't other permissions needed to successfully run
> sulogin.
> 
> This second version should apply cleanly to the latest git tree.
> 
> Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
> ---
>  policy/modules/system/locallogin.te |   24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> --- a/policy/modules/system/locallogin.te	2017-04-13 22:04:35.111202539 +0200
> +++ b/policy/modules/system/locallogin.te	2017-04-26 00:44:23.028943625 +0200
> @@ -215,7 +215,8 @@ optional_policy(`
>  # Sulogin local policy
>  #
>  
> -allow sulogin_t self:capability dac_override;
> +allow sulogin_t self:capability { dac_override sys_admin sys_tty_config };

I suspect that cap_sys_admin can be safely dontaudited

> +allow sulogin_t self:process setexec;
>  allow sulogin_t self:fd use;
>  allow sulogin_t self:fifo_file rw_fifo_file_perms;
>  allow sulogin_t self:unix_dgram_socket create_socket_perms;
> @@ -228,6 +229,9 @@ allow sulogin_t self:msgq create_msgq_pe
>  allow sulogin_t self:msg { send receive };
>  
>  kernel_read_system_state(sulogin_t)
> +kernel_read_crypto_sysctls(sulogin_t)
> +kernel_stream_connect(sulogin_t)
> +kernel_use_fds(sulogin_t)
>  # because file systems are not mounted:
>  kernel_dontaudit_search_unlabeled(sulogin_t)
>  
> @@ -238,10 +242,13 @@ files_read_etc_files(sulogin_t)
>  
>  auth_read_shadow(sulogin_t)
>  
> +init_getpgid(sulogin_t)
>  init_getpgid_script(sulogin_t)
>  
>  logging_send_syslog_msg(sulogin_t)
>  
> +miscfiles_read_localization(sulogin_t)
> +
>  seutil_read_config(sulogin_t)
>  seutil_read_default_contexts(sulogin_t)
>  
> @@ -252,15 +259,12 @@ userdom_use_user_ptys(sulogin_t)
>  
>  sysadm_shell_domtrans(sulogin_t)
>  
> -# suse and debian do not use pam with sulogin...
> -ifdef(`distro_suse', `define(`sulogin_no_pam')')
> -ifdef(`distro_debian', `define(`sulogin_no_pam')')
> -
> -ifdef(`sulogin_no_pam', `
> -	allow sulogin_t self:capability sys_tty_config;
> -	init_getpgid(sulogin_t)
> -', `
> -	allow sulogin_t self:process setexec;
> +term_use_console(sulogin_t)
> +term_use_unallocated_ttys(sulogin_t)
> +
> +# by default, sulogin does not use pam...
> +# sulogin_pam might need to be defined otherwise
> +ifdef(`sulogin_pam', `
>  	selinux_get_fs_mount(sulogin_t)
>  	selinux_validate_context(sulogin_t)
>  	selinux_compute_access_vector(sulogin_t)
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170426/ddfb4df2/attachment.bin 

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Guido Trentalancia]

Hello.

On the 26th of April 2017 15:05:44 CEST, Dominick Grift via refpolicy <refpolicy@oss.tresys.com> wrote:
>On Wed, Apr 26, 2017 at 12:50:02AM +0200, Guido Trentalancia via
>refpolicy wrote:
>> This patch fixes the policy for sulogin. It is very important
>> because without this patch, sulogin cannot work properly and
>> it should be considered that it is used as an emergency shell
>> when there are serious consistency errors in the system, so it
>> constitutes the only way to recover the system in such
>> circumstances.
>> 
>> Nowadays, sulogin never uses PAM (at least not the official one
>> from util-linux), so obsolete, confusing and buggy policy has
>> been removed.
>> 
>> Extensive testing carried out while creating this patch indicates
>> that there aren't other permissions needed to successfully run
>> sulogin.
>> 
>> This second version should apply cleanly to the latest git tree.
>> 
>> Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
>> ---
>>  policy/modules/system/locallogin.te |   24 ++++++++++++++----------
>>  1 file changed, 14 insertions(+), 10 deletions(-)
>> 
>> --- a/policy/modules/system/locallogin.te	2017-04-13
>22:04:35.111202539 +0200
>> +++ b/policy/modules/system/locallogin.te	2017-04-26
>00:44:23.028943625 +0200
>> @@ -215,7 +215,8 @@ optional_policy(`
>>  # Sulogin local policy
>>  #
>>  
>> -allow sulogin_t self:capability dac_override;
>> +allow sulogin_t self:capability { dac_override sys_admin
>sys_tty_config };
>
>I suspect that cap_sys_admin can be safely dontaudited

Yes, I thought the same, but then considering it is a sysadmin shell, I did not even check.

Also, remember we probably still have sys_admin for getty which runs unprivileged shells... 

>> +allow sulogin_t self:process setexec;
>>  allow sulogin_t self:fd use;
>>  allow sulogin_t self:fifo_file rw_fifo_file_perms;
>>  allow sulogin_t self:unix_dgram_socket create_socket_perms;
>> @@ -228,6 +229,9 @@ allow sulogin_t self:msgq create_msgq_pe
>>  allow sulogin_t self:msg { send receive };
>>  
>>  kernel_read_system_state(sulogin_t)
>> +kernel_read_crypto_sysctls(sulogin_t)
>> +kernel_stream_connect(sulogin_t)
>> +kernel_use_fds(sulogin_t)
>>  # because file systems are not mounted:
>>  kernel_dontaudit_search_unlabeled(sulogin_t)
>>  
>> @@ -238,10 +242,13 @@ files_read_etc_files(sulogin_t)
>>  
>>  auth_read_shadow(sulogin_t)
>>  
>> +init_getpgid(sulogin_t)
>>  init_getpgid_script(sulogin_t)
>>  
>>  logging_send_syslog_msg(sulogin_t)
>>  
>> +miscfiles_read_localization(sulogin_t)
>> +
>>  seutil_read_config(sulogin_t)
>>  seutil_read_default_contexts(sulogin_t)
>>  
>> @@ -252,15 +259,12 @@ userdom_use_user_ptys(sulogin_t)
>>  
>>  sysadm_shell_domtrans(sulogin_t)
>>  
>> -# suse and debian do not use pam with sulogin...
>> -ifdef(`distro_suse', `define(`sulogin_no_pam')')
>> -ifdef(`distro_debian', `define(`sulogin_no_pam')')
>> -
>> -ifdef(`sulogin_no_pam', `
>> -	allow sulogin_t self:capability sys_tty_config;
>> -	init_getpgid(sulogin_t)
>> -', `
>> -	allow sulogin_t self:process setexec;
>> +term_use_console(sulogin_t)
>> +term_use_unallocated_ttys(sulogin_t)
>> +
>> +# by default, sulogin does not use pam...
>> +# sulogin_pam might need to be defined otherwise
>> +ifdef(`sulogin_pam', `
>>  	selinux_get_fs_mount(sulogin_t)
>>  	selinux_validate_context(sulogin_t)
>>  	selinux_compute_access_vector(sulogin_t)
>> _______________________________________________
>> refpolicy mailing list
>> refpolicy at oss.tresys.com
>> http://oss.tresys.com/mailman/listinfo/refpolicy

Regards, 

Guido 

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Dominick Grift]

On Wed, Apr 26, 2017 at 05:42:31PM +0200, Guido Trentalancia via refpolicy wrote:
> Hello.
> 
> On the 26th of April 2017 15:05:44 CEST, Dominick Grift via refpolicy <refpolicy@oss.tresys.com> wrote:
> >On Wed, Apr 26, 2017 at 12:50:02AM +0200, Guido Trentalancia via
> >refpolicy wrote:
> >> This patch fixes the policy for sulogin. It is very important
> >> because without this patch, sulogin cannot work properly and
> >> it should be considered that it is used as an emergency shell
> >> when there are serious consistency errors in the system, so it
> >> constitutes the only way to recover the system in such
> >> circumstances.
> >> 
> >> Nowadays, sulogin never uses PAM (at least not the official one
> >> from util-linux), so obsolete, confusing and buggy policy has
> >> been removed.
> >> 
> >> Extensive testing carried out while creating this patch indicates
> >> that there aren't other permissions needed to successfully run
> >> sulogin.
> >> 
> >> This second version should apply cleanly to the latest git tree.
> >> 
> >> Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
> >> ---
> >>  policy/modules/system/locallogin.te |   24 ++++++++++++++----------
> >>  1 file changed, 14 insertions(+), 10 deletions(-)
> >> 
> >> --- a/policy/modules/system/locallogin.te	2017-04-13
> >22:04:35.111202539 +0200
> >> +++ b/policy/modules/system/locallogin.te	2017-04-26
> >00:44:23.028943625 +0200
> >> @@ -215,7 +215,8 @@ optional_policy(`
> >>  # Sulogin local policy
> >>  #
> >>  
> >> -allow sulogin_t self:capability dac_override;
> >> +allow sulogin_t self:capability { dac_override sys_admin
> >sys_tty_config };
> >
> >I suspect that cap_sys_admin can be safely dontaudited
> 
> Yes, I thought the same, but then considering it is a sysadmin shell, I did not even check.

The cap_sys_admin for getty can also be safely dontaudited AFAIK

Also the dac_override for sulogin can also be dontaudited AFAIK (allow dac_read_search instead)

> 
> Also, remember we probably still have sys_admin for getty which runs unprivileged shells... 
> 
> >> +allow sulogin_t self:process setexec;
> >>  allow sulogin_t self:fd use;
> >>  allow sulogin_t self:fifo_file rw_fifo_file_perms;
> >>  allow sulogin_t self:unix_dgram_socket create_socket_perms;
> >> @@ -228,6 +229,9 @@ allow sulogin_t self:msgq create_msgq_pe
> >>  allow sulogin_t self:msg { send receive };
> >>  
> >>  kernel_read_system_state(sulogin_t)
> >> +kernel_read_crypto_sysctls(sulogin_t)
> >> +kernel_stream_connect(sulogin_t)
> >> +kernel_use_fds(sulogin_t)
> >>  # because file systems are not mounted:
> >>  kernel_dontaudit_search_unlabeled(sulogin_t)
> >>  
> >> @@ -238,10 +242,13 @@ files_read_etc_files(sulogin_t)
> >>  
> >>  auth_read_shadow(sulogin_t)
> >>  
> >> +init_getpgid(sulogin_t)
> >>  init_getpgid_script(sulogin_t)
> >>  
> >>  logging_send_syslog_msg(sulogin_t)
> >>  
> >> +miscfiles_read_localization(sulogin_t)
> >> +
> >>  seutil_read_config(sulogin_t)
> >>  seutil_read_default_contexts(sulogin_t)
> >>  
> >> @@ -252,15 +259,12 @@ userdom_use_user_ptys(sulogin_t)
> >>  
> >>  sysadm_shell_domtrans(sulogin_t)
> >>  
> >> -# suse and debian do not use pam with sulogin...
> >> -ifdef(`distro_suse', `define(`sulogin_no_pam')')
> >> -ifdef(`distro_debian', `define(`sulogin_no_pam')')
> >> -
> >> -ifdef(`sulogin_no_pam', `
> >> -	allow sulogin_t self:capability sys_tty_config;
> >> -	init_getpgid(sulogin_t)
> >> -', `
> >> -	allow sulogin_t self:process setexec;
> >> +term_use_console(sulogin_t)
> >> +term_use_unallocated_ttys(sulogin_t)
> >> +
> >> +# by default, sulogin does not use pam...
> >> +# sulogin_pam might need to be defined otherwise
> >> +ifdef(`sulogin_pam', `
> >>  	selinux_get_fs_mount(sulogin_t)
> >>  	selinux_validate_context(sulogin_t)
> >>  	selinux_compute_access_vector(sulogin_t)
> >> _______________________________________________
> >> refpolicy mailing list
> >> refpolicy at oss.tresys.com
> >> http://oss.tresys.com/mailman/listinfo/refpolicy
> 
> Regards, 
> 
> Guido 
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170426/d343130e/attachment.bin 

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Russell Coker]

On Thu, 27 Apr 2017 01:42:31 AM Guido Trentalancia via refpolicy wrote:
> >> @@ -215,7 +215,8 @@ optional_policy(`
> >> 
> >>  # Sulogin local policy
> >>  #
> >> 
> >> -allow sulogin_t self:capability dac_override;
> >> +allow sulogin_t self:capability { dac_override sys_admin
> >
> >sys_tty_config };
> >
> >I suspect that cap_sys_admin can be safely dontaudited
> 
> Yes, I thought the same, but then considering it is a sysadmin shell, I did
> not even check.
> 
> Also, remember we probably still have sys_admin for getty which runs
> unprivileged shells...

http://oss.tresys.com/pipermail/refpolicy/2016-March/007901.html

Above is the list discussion from last time this came up.  If you can get 
sulogin to operate correctly without sys_admin then the next thing to do would 
be to try and get getty to do the same.  As you note getty runs unprivileged 
shells, but also it tends to be run from les secure devices such as serial 
consoles, modems, etc that sulogin will never be run from.

I'm a little surprised at your "considering it is a sysadmin shell" argument 
given that the reason you started working on sulogin policy is that you 
believed that I was giving it excess permissions.  Previously you didn't 
accept my argument that sulogin is permitted to run "bash -c setsebool" etc.

-- 
My Main Blog         http://etbe.coker.com.au/
My Documents Blog    http://doc.coker.com.au/

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Guido Trentalancia]

Hello Russell. 

Unfortunately, your sulogin patch didn't work, so it was not just a matter of unneeded permissions!

You can check by yourself that it was missing critical permissions while granting unneeded ones...

Also, I have already stressed out several times that getty should probably run without the sys_admin capability. They didn't want to change it, I am not going to tell that again. 

Feel free to submit your sys_admin capability patch for getty, sulogin or both. Consider, I have not tested other variations for sulogin, I consider the change of minor importance compared to this patch.

I hope this helps. 

Regards, 

Guido 

On the 26th of April 2017 18:20:27 CEST, Russell Coker <russell@coker.com.au> wrote:
>On Thu, 27 Apr 2017 01:42:31 AM Guido Trentalancia via refpolicy wrote:
>> >> @@ -215,7 +215,8 @@ optional_policy(`
>> >> 
>> >>  # Sulogin local policy
>> >>  #
>> >> 
>> >> -allow sulogin_t self:capability dac_override;
>> >> +allow sulogin_t self:capability { dac_override sys_admin
>> >
>> >sys_tty_config };
>> >
>> >I suspect that cap_sys_admin can be safely dontaudited
>> 
>> Yes, I thought the same, but then considering it is a sysadmin shell,
>I did
>> not even check.
>> 
>> Also, remember we probably still have sys_admin for getty which runs
>> unprivileged shells...
>
>http://oss.tresys.com/pipermail/refpolicy/2016-March/007901.html
>
>Above is the list discussion from last time this came up.  If you can
>get 
>sulogin to operate correctly without sys_admin then the next thing to
>do would 
>be to try and get getty to do the same.  As you note getty runs
>unprivileged 
>shells, but also it tends to be run from les secure devices such as
>serial 
>consoles, modems, etc that sulogin will never be run from.
>
>I'm a little surprised at your "considering it is a sysadmin shell"
>argument 
>given that the reason you started working on sulogin policy is that you
>
>believed that I was giving it excess permissions.  Previously you
>didn't 
>accept my argument that sulogin is permitted to run "bash -c setsebool"
>etc.

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Russell Coker]

On Thu, 27 Apr 2017 02:32:05 AM Guido Trentalancia via refpolicy wrote:
> Unfortunately, your sulogin patch didn't work, so it was not just a matter
> of unneeded permissions!
> 
> You can check by yourself that it was missing critical permissions while
> granting unneeded ones...

It worked for me last time I tested it on Debian.  Maybe other distributions 
need different permissions.  Maybe the Debian sulogin changed to require more 
permissions since the last time I tested it.  But I don't submit policy based 
on what I imagine programs might do, it's based on what I observe them doing.

> Also, I have already stressed out several times that getty should probably
> run without the sys_admin capability. They didn't want to change it, I am
> not going to tell that again. 

As the previous discussion that I linked to showed there was a situation where 
a character could be lost if that permission wasn't granted.  I expect that 
getty could be changed to address that issue.  But I also recall that there 
was another issue which I couldn't get the details of in 10 minutes of 
Googling.

> Feel free to submit your sys_admin capability patch for getty, sulogin or
> both. Consider, I have not tested other variations for sulogin, I consider
> the change of minor importance compared to this patch.

As I have stated several times sulogin has a sole purpose of running a shell 
with ultimate privileges and therefore I think that restricting it's access is 
futile.

-- 
My Main Blog         http://etbe.coker.com.au/
My Documents Blog    http://doc.coker.com.au/

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Dominick Grift]

On Thu, Apr 27, 2017 at 03:23:25AM +1000, Russell Coker via refpolicy wrote:
> On Thu, 27 Apr 2017 02:32:05 AM Guido Trentalancia via refpolicy wrote:
> > Unfortunately, your sulogin patch didn't work, so it was not just a matter
> > of unneeded permissions!
> > 
> > You can check by yourself that it was missing critical permissions while
> > granting unneeded ones...
> 
> It worked for me last time I tested it on Debian.  Maybe other distributions 
> need different permissions.  Maybe the Debian sulogin changed to require more 
> permissions since the last time I tested it.  But I don't submit policy based 
> on what I imagine programs might do, it's based on what I observe them doing.

I can confirm that cap_dac_override and cap_sys_admin arent needed for sulogin in debian stretch

https://www.youtube.com/watch?v=NBj2W7yiu_c

> 
> > Also, I have already stressed out several times that getty should probably
> > run without the sys_admin capability. They didn't want to change it, I am
> > not going to tell that again. 
> 
> As the previous discussion that I linked to showed there was a situation where 
> a character could be lost if that permission wasn't granted.  I expect that 
> getty could be changed to address that issue.  But I also recall that there 
> was another issue which I couldn't get the details of in 10 minutes of 
> Googling.
> 
> > Feel free to submit your sys_admin capability patch for getty, sulogin or
> > both. Consider, I have not tested other variations for sulogin, I consider
> > the change of minor importance compared to this patch.
> 
> As I have stated several times sulogin has a sole purpose of running a shell 
> with ultimate privileges and therefore I think that restricting it's access is 
> futile.
> 
> -- 
> My Main Blog         http://etbe.coker.com.au/
> My Documents Blog    http://doc.coker.com.au/
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170426/d3eca139/attachment.bin 

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Guido Trentalancia]

This patch has already been submitted.

Feel free to submit a patch to improve things as you say...

On Wed, 26/04/2017 at 19.58 +0200, Dominick Grift via
refpolicy wrote:
> On Thu, Apr 27, 2017 at 03:23:25AM +1000, Russell Coker via refpolicy
> wrote:
> > On Thu, 27 Apr 2017 02:32:05 AM Guido Trentalancia via refpolicy
> > wrote:
> > > Unfortunately, your sulogin patch didn't work, so it was not just
> > > a matter
> > > of unneeded permissions!
> > > 
> > > You can check by yourself that it was missing critical
> > > permissions while
> > > granting unneeded ones...
> > 
> > It worked for me last time I tested it on Debian.??Maybe other
> > distributions?
> > need different permissions.??Maybe the Debian sulogin changed to
> > require more?
> > permissions since the last time I tested it.??But I don't submit
> > policy based?
> > on what I imagine programs might do, it's based on what I observe
> > them doing.
> 
> I can confirm that cap_dac_override and cap_sys_admin arent needed
> for sulogin in debian stretch
> 
> https://www.youtube.com/watch?v=NBj2W7yiu_c
> 
> > 
> > > Also, I have already stressed out several times that getty should
> > > probably
> > > run without the sys_admin capability. They didn't want to change
> > > it, I am
> > > not going to tell that again.?
> > 
> > As the previous discussion that I linked to showed there was a
> > situation where?
> > a character could be lost if that permission wasn't granted.??I
> > expect that?
> > getty could be changed to address that issue.??But I also recall
> > that there?
> > was another issue which I couldn't get the details of in 10 minutes
> > of?
> > Googling.
> > 
> > > Feel free to submit your sys_admin capability patch for getty,
> > > sulogin or
> > > both. Consider, I have not tested other variations for sulogin, I
> > > consider
> > > the change of minor importance compared to this patch.
> > 
> > As I have stated several times sulogin has a sole purpose of
> > running a shell?
> > with ultimate privileges and therefore I think that restricting
> > it's access is?
> > futile.

Guido

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Chris PeBenito]

On 04/26/2017 02:00 PM, Guido Trentalancia via refpolicy wrote:
> This patch has already been submitted.
>
> Feel free to submit a patch to improve things as you say...
>
> On Wed, 26/04/2017 at 19.58 +0200, Dominick Grift via
> refpolicy wrote:
>> On Thu, Apr 27, 2017 at 03:23:25AM +1000, Russell Coker via refpolicy
>> wrote:
>>> On Thu, 27 Apr 2017 02:32:05 AM Guido Trentalancia via refpolicy
>>> wrote:
>>>> Unfortunately, your sulogin patch didn't work, so it was not just
>>>> a matter
>>>> of unneeded permissions!
>>>>
>>>> You can check by yourself that it was missing critical
>>>> permissions while
>>>> granting unneeded ones...
>>>
>>> It worked for me last time I tested it on Debian.  Maybe other
>>> distributions
>>> need different permissions.  Maybe the Debian sulogin changed to
>>> require more
>>> permissions since the last time I tested it.  But I don't submit
>>> policy based
>>> on what I imagine programs might do, it's based on what I observe
>>> them doing.
>>
>> I can confirm that cap_dac_override and cap_sys_admin arent needed
>> for sulogin in debian stretch
>>
>> https://www.youtube.com/watch?v=NBj2W7yiu_c
>>
>>>
>>>> Also, I have already stressed out several times that getty should
>>>> probably
>>>> run without the sys_admin capability. They didn't want to change
>>>> it, I am
>>>> not going to tell that again.
>>>
>>> As the previous discussion that I linked to showed there was a
>>> situation where
>>> a character could be lost if that permission wasn't granted.  I
>>> expect that
>>> getty could be changed to address that issue.  But I also recall
>>> that there
>>> was another issue which I couldn't get the details of in 10 minutes
>>> of
>>> Googling.
>>>
>>>> Feel free to submit your sys_admin capability patch for getty,
>>>> sulogin or
>>>> both. Consider, I have not tested other variations for sulogin, I
>>>> consider
>>>> the change of minor importance compared to this patch.
>>>
>>> As I have stated several times sulogin has a sole purpose of
>>> running a shell
>>> with ultimate privileges and therefore I think that restricting
>>> it's access is
>>> futile.

Ideally, I'd like least privilege across the board; however, I also tend 
to agree with Russell in this case.  I'd rather spend more time on 
domains where risk is higher.  If sulogin is somehow compromised, the 
system is in serious trouble, but the risk of that is pretty low.

That being said, I'm still happy to take further patches on this, if you 
can agree on the changes (I can't test all distros to make sure it 
works).  I'd prefer to avoid adding and removing the same permissions 
repeatedly.

-- 
Chris PeBenito

[refpolicy] [PATCH v2] locallogin: fix the sulogin submodule (emergency shell!)

[Guido Trentalancia]

Hello Christopher. 

You can possibly remove dac_override, provided that you grant dac_read_search.

You should not remove sys_admin.

However, there is very limited benefit in making such change, I would prefer to leave things as they are.

You can only get some substantial gain by dontauditing sys_admin in the getty module, without loosing functionality, as already explained several times. 

Regards, 

Guido 

On the 26st of April 2017 23:04:35 CEST, Chris PeBenito <pebenito@ieee.org> wrote:
>On 04/26/2017 02:00 PM, Guido Trentalancia via refpolicy wrote:
>> This patch has already been submitted.
>>
>> Feel free to submit a patch to improve things as you say...
>>
>> On Wed, 26/04/2017 at 19.58 +0200, Dominick Grift via
>> refpolicy wrote:
>>> On Thu, Apr 27, 2017 at 03:23:25AM +1000, Russell Coker via
>refpolicy
>>> wrote:
>>>> On Thu, 27 Apr 2017 02:32:05 AM Guido Trentalancia via refpolicy
>>>> wrote:
>>>>> Unfortunately, your sulogin patch didn't work, so it was not just
>>>>> a matter
>>>>> of unneeded permissions!
>>>>>
>>>>> You can check by yourself that it was missing critical
>>>>> permissions while
>>>>> granting unneeded ones...
>>>>
>>>> It worked for me last time I tested it on Debian.  Maybe other
>>>> distributions
>>>> need different permissions.  Maybe the Debian sulogin changed to
>>>> require more
>>>> permissions since the last time I tested it.  But I don't submit
>>>> policy based
>>>> on what I imagine programs might do, it's based on what I observe
>>>> them doing.
>>>
>>> I can confirm that cap_dac_override and cap_sys_admin arent needed
>>> for sulogin in debian stretch
>>>
>>> https://www.youtube.com/watch?v=NBj2W7yiu_c
>>>
>>>>
>>>>> Also, I have already stressed out several times that getty should
>>>>> probably
>>>>> run without the sys_admin capability. They didn't want to change
>>>>> it, I am
>>>>> not going to tell that again.
>>>>
>>>> As the previous discussion that I linked to showed there was a
>>>> situation where
>>>> a character could be lost if that permission wasn't granted.  I
>>>> expect that
>>>> getty could be changed to address that issue.  But I also recall
>>>> that there
>>>> was another issue which I couldn't get the details of in 10 minutes
>>>> of
>>>> Googling.
>>>>
>>>>> Feel free to submit your sys_admin capability patch for getty,
>>>>> sulogin or
>>>>> both. Consider, I have not tested other variations for sulogin, I
>>>>> consider
>>>>> the change of minor importance compared to this patch.
>>>>
>>>> As I have stated several times sulogin has a sole purpose of
>>>> running a shell
>>>> with ultimate privileges and therefore I think that restricting
>>>> it's access is
>>>> futile.
>
>Ideally, I'd like least privilege across the board; however, I also
>tend 
>to agree with Russell in this case.  I'd rather spend more time on 
>domains where risk is higher.  If sulogin is somehow compromised, the 
>system is in serious trouble, but the risk of that is pretty low.
>
>That being said, I'm still happy to take further patches on this, if
>you 
>can agree on the changes (I can't test all distros to make sure it 
>works).  I'd prefer to avoid adding and removing the same permissions 
>repeatedly.