Skip to contentSkip to conversation

locallogin: fix the sulogin submodule (emergency shell!)

PATCH Superseded Guido Trentalancia 4 participants 13 messages Apr 21, 2017 – Apr 26, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 locallogin: fix the sulogin submodule (emergency shell!)

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.

---
 policy/modules/system/locallogin.te |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
policy/modules/system/locallogin.te +14 -11
@@ -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)
#1 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

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.

---
 policy/modules/system/locallogin.te |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
policy/modules/system/locallogin.te +14 -10
@@ -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)
#2 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

Quoted 70 lines · Guido Trentalancia via refpolicy

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.

---
 policy/modules/system/locallogin.te |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
policy/modules/system/locallogin.te +8 -9
@@ -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)
-', `

policy/modules/system/locallogin.te userdom_use_user_ptys(sulogin_t) · line 270

Quoted 10 lines
policy/modules/system/locallogin.te +6 -1
-	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.

Signature · 1 line
Chris PeBenito
#3 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

In reply to Guido Trentalancia
Quoted 25 lines · Guido Trentalancia via refpolicy

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.

---
 policy/modules/system/locallogin.te |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

policy/modules/system/locallogin.te optional_policy(` · line 218

Quoted 5 lines
policy/modules/system/locallogin.te +1 -1
@@ -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

Quoted 54 lines
policy/modules/system/locallogin.te +13 -9
+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

Signature · 10 lines
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 
#4 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

Hello.

Quoted 32 lines · Dominick Grift via refpolicy
Quoted 15 lines · Guido Trentalancia via refpolicy

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.

Quoted 8 lines

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

---
 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

Quoted 1 line

+++ b/policy/modules/system/locallogin.te 2017-04-26

00:44:23.028943625 +0200

Quoted 4 lines
policy/modules/system/locallogin.te +0 -0
@@ -215,7 +215,8 @@ optional_policy(`
 # Sulogin local policy
 #
 
Quoted 5 lines

Review optional_policy(` · line 218

Quoted 2 lines
(quoted diff) +1 -1
-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...

Quoted 46 lines
Quoted 46 lines
policy/modules/system/locallogin.te +12 -9
+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
Quoted 8 lines
Quoted 8 lines
policy/modules/system/locallogin.te +1 -0
+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

#5 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

Quoted 35 lines · Guido Trentalancia via refpolicy

Hello.

Quoted 32 lines · Dominick Grift via refpolicy
Quoted 15 lines · Guido Trentalancia via refpolicy

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.

Quoted 8 lines

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

---
 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

Quoted 1 line

+++ b/policy/modules/system/locallogin.te 2017-04-26

00:44:23.028943625 +0200

Quoted 4 lines
policy/modules/system/locallogin.te +0 -0
@@ -215,7 +215,8 @@ optional_policy(`
 # Sulogin local policy
 #
 
Quoted 7 lines
Quoted 5 lines

Review optional_policy(` · line 218

Quoted 2 lines
(quoted diff) +1 -1
-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)

Quoted 65 lines

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

Quoted 46 lines
Quoted 46 lines
policy/modules/system/locallogin.te +12 -9
+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
Quoted 8 lines
Quoted 8 lines
policy/modules/system/locallogin.te +1 -0
+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

Signature · 10 lines
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 
#6 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

In reply to Guido Trentalancia
Quoted 10 lines · Guido Trentalancia via refpolicy
Quoted 10 lines

policy/modules/system/locallogin.te optional_policy(` · line 219

Quoted 7 lines
policy/modules/system/locallogin.te +1 -1
@@ -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 };

Quoted 7 lines
Quoted 1 line

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.

Signature · 2 lines
My Main Blog         http://etbe.coker.com.au/
My Documents Blog    http://doc.coker.com.au/
#7 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

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

Quoted 40 lines · Russell Coker
Quoted 5 lines · Guido Trentalancia via refpolicy
Quoted 5 lines
Quoted 5 lines
policy/modules/system/locallogin.te +0 -0
@@ -215,7 +215,8 @@ optional_policy(`
 
 # Sulogin local policy
 #
 
Quoted 8 lines
Quoted 6 lines

Review optional_policy(` · line 219

Quoted 2 lines
(quoted diff) +1 -1
-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

Quoted 4 lines

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.

#8 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

Quoted 5 lines · Guido Trentalancia via refpolicy

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.

Quoted 3 lines

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.

Quoted 3 lines

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.

Signature · 2 lines
My Main Blog         http://etbe.coker.com.au/
My Documents Blog    http://doc.coker.com.au/
#9 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

Quoted 11 lines · Russell Coker via refpolicy
Quoted 5 lines · Guido Trentalancia via refpolicy

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

Quoted 26 lines
Quoted 4 lines

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.

Quoted 3 lines

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.

Signature · 6 lines
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
Signature · 10 lines
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 
#10 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

This patch has already been submitted.

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

Quoted 21 lines · Dominick Grift via refpolicy
Quoted 10 lines · Russell Coker via refpolicy
Quoted 7 lines · Guido Trentalancia via refpolicy

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...

Quoted 8 lines

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.

Quoted 33 lines

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

Quoted 28 lines
Quoted 6 lines

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.

Quoted 5 lines

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

#11 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

Quoted 60 lines · Guido Trentalancia via refpolicy

This patch has already been submitted.

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

Quoted 54 lines · Dominick Grift via refpolicy
Quoted 10 lines · Russell Coker via refpolicy
Quoted 7 lines · Guido Trentalancia via refpolicy

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...

Quoted 8 lines

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

Quoted 28 lines
Quoted 6 lines

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.

Quoted 5 lines

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.

Signature · 1 line
Chris PeBenito
#12 lore raw

PATCH v2 1/1 locallogin: fix the sulogin submodule (emergency shell!)

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

Quoted 74 lines · Chris PeBenito
Quoted 7 lines · Guido Trentalancia via refpolicy

This patch has already been submitted.

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

Quoted 1 line · Dominick Grift via refpolicy

On Thu, Apr 27, 2017 at 03:23:25AM +1000, Russell Coker via

refpolicy

Quoted 48 lines
Quoted 48 lines
Quoted 10 lines · wrote:
Quoted 7 lines · Guido Trentalancia via refpolicy

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...

Quoted 8 lines

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

Quoted 23 lines
Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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.