sddm issue and patch not for inclusion

sddm issue and patch not for inclusion

[Russell Coker]

In Debian/Unstable (which will soon be frozen and become the next stable
release) the sddm X login program (the one that's generally recommended
and specifically known to generally work well with SE Linux) uses PAM to
start a session for the "greeter" (the program that asks for a password
before a new session is started).

With the policy currently in Debian that means the sddm user matches
"__default__" and gets unconfined_u:unconfined_r:unconfined_t, not what
is desirable for a program that takes input from unauthenticated users.

role xdm_r;
role xdm_r types xdm_t;
allow system_r xdm_r;
allow xdm_t xdm_tmpfs_t:file execmod;
corecmd_bin_entry_type(xdm_t)

To get this working as a test I put the above in a local policy file,
edited /etc/selinux/default/contexts/default_contexts to add a suitable
context to the system_r:xdm_t:s0 line, and run the following 2 commands:
semanage user -a -r s0 -L s0 -R xdm_r -P user xdm
semanage login -a -s xdm -r s0 sddm

I mention the above for the benefit of people who do web searches for such
things and get the list archives.

Below is the policy I'm using which will be in the next release of Debian
if no-one else has a better idea.  NB a "better idea" doesn't mean running
the greeter as unconfined_t IMHO.  Also while we can debate about whether
modifying sddm to not use PAM for the greeter session is a good idea, such
a change would potentially affect people who don't use SE Linux so I won't
even waste the time of the sddm maintainer by discussing that possibility
with them before the release.  After the release we can discuss such
things, but now we need to get things working well in the next few days in
a manner that will make users happy for the next 2 years.

Index: refpolicy-2.20210126/policy/modules/services/xserver.te
===================================================================
--- refpolicy-2.20210126.orig/policy/modules/services/xserver.te
+++ refpolicy-2.20210126/policy/modules/services/xserver.te
@@ -18,6 +18,7 @@ gen_require(`
 	class x_resource all_x_resource_perms;
 	class x_event all_x_event_perms;
 	class x_synthetic_event all_x_synthetic_event_perms;
+	role xdm_r;
 ')
 
 ########################################
@@ -152,6 +153,10 @@ init_daemon_domain(xdm_t, xdm_exec_t)
 xserver_object_types_template(xdm)
 xserver_common_x_domain_template(xdm, xdm_t)
 
+# for sddm to use pam for greeter
+role xdm_r types xdm_t;
+allow system_r xdm_r;
+
 type xdm_lock_t;
 files_lock_file(xdm_lock_t)
 
@@ -848,6 +853,11 @@ manage_files_pattern(xserver_t, xdm_tmp_
 manage_lnk_files_pattern(xserver_t, xdm_tmp_t, xdm_tmp_t)
 manage_sock_files_pattern(xserver_t, xdm_tmp_t, xdm_tmp_t)
 
+# for sddm to use pam for greeter
+corecmd_bin_entry_type(xdm_t)
+# sddm greeter needs execmod
+allow xdm_t xdm_tmpfs_t:file execmod;
+
 # Run Xorg.wrap
 can_exec(xserver_t, xserver_exec_t)
 
Index: refpolicy-2.20210126/config/appconfig-mcs/seusers
===================================================================
--- refpolicy-2.20210126.orig/config/appconfig-mcs/seusers
+++ refpolicy-2.20210126/config/appconfig-mcs/seusers
@@ -1,2 +1,3 @@
 root:unconfined_u:s0-mcs_systemhigh
 __default__:unconfined_u:s0-mcs_systemhigh
+sddm:xdm:s0
Index: refpolicy-2.20210126/policy/users
===================================================================
--- refpolicy-2.20210126.orig/policy/users
+++ refpolicy-2.20210126/policy/users
@@ -27,6 +27,7 @@ gen_user(system_u,, system_r, s0, s0 - m
 gen_user(user_u, user, user_r, s0, s0)
 gen_user(staff_u, staff, staff_r sysadm_r ifdef(`enable_mls',`secadm_r auditadm_r'), s0, s0 - mls_systemhigh, mcs_allcats)
 gen_user(sysadm_u, sysadm, sysadm_r, s0, s0 - mls_systemhigh, mcs_allcats)
+gen_user(xdm, user, xdm_r, s0, s0)
 
 # Until order dependence is fixed for users:
 ifdef(`direct_sysadm_daemon',`
Index: refpolicy-2.20210126/config/appconfig-mcs/xdm_default_contexts
===================================================================
--- /dev/null
+++ refpolicy-2.20210126/config/appconfig-mcs/xdm_default_contexts
@@ -0,0 +1 @@
+system_r:xdm_t:s0		xdm_r:xdm_t:s0
Index: refpolicy-2.20210126/policy/modules/kernel/kernel.te
===================================================================
--- refpolicy-2.20210126.orig/policy/modules/kernel/kernel.te
+++ refpolicy-2.20210126/policy/modules/kernel/kernel.te
@@ -32,6 +32,7 @@ role system_r;
 role sysadm_r;
 role staff_r;
 role user_r;
+role xdm_r;
 
 # here until order dependence is fixed:
 role unconfined_r;

Re: sddm issue and patch not for inclusion

[Dominick Grift]

Russell Coker <russell@coker.com.au> writes:

> In Debian/Unstable (which will soon be frozen and become the next stable
> release) the sddm X login program (the one that's generally recommended
> and specifically known to generally work well with SE Linux) uses PAM to
> start a session for the "greeter" (the program that asks for a password
> before a new session is started).
>
> With the policy currently in Debian that means the sddm user matches
> "__default__" and gets unconfined_u:unconfined_r:unconfined_t, not what
> is desirable for a program that takes input from unauthenticated users.
>
> role xdm_r;
> role xdm_r types xdm_t;
> allow system_r xdm_r;
> allow xdm_t xdm_tmpfs_t:file execmod;

that looks like a bug or at least bad code

> corecmd_bin_entry_type(xdm_t)
>
> To get this working as a test I put the above in a local policy file,
> edited /etc/selinux/default/contexts/default_contexts to add a suitable
> context to the system_r:xdm_t:s0 line, and run the following 2 commands:
> semanage user -a -r s0 -L s0 -R xdm_r -P user xdm
> semanage login -a -s xdm -r s0 sddm
>
> I mention the above for the benefit of people who do web searches for such
> things and get the list archives.
>
> Below is the policy I'm using which will be in the next release of Debian
> if no-one else has a better idea.  NB a "better idea" doesn't mean running
> the greeter as unconfined_t IMHO.  Also while we can debate about whether
> modifying sddm to not use PAM for the greeter session is a good idea, such
> a change would potentially affect people who don't use SE Linux so I won't
> even waste the time of the sddm maintainer by discussing that possibility
> with them before the release.  After the release we can discuss such
> things, but now we need to get things working well in the next few days in
> a manner that will make users happy for the next 2 years.
>
> Index: refpolicy-2.20210126/policy/modules/services/xserver.te
> ===================================================================
> --- refpolicy-2.20210126.orig/policy/modules/services/xserver.te
> +++ refpolicy-2.20210126/policy/modules/services/xserver.te
> @@ -18,6 +18,7 @@ gen_require(`
>  	class x_resource all_x_resource_perms;
>  	class x_event all_x_event_perms;
>  	class x_synthetic_event all_x_synthetic_event_perms;
> +	role xdm_r;
>  ')
>  
>  ########################################
> @@ -152,6 +153,10 @@ init_daemon_domain(xdm_t, xdm_exec_t)
>  xserver_object_types_template(xdm)
>  xserver_common_x_domain_template(xdm, xdm_t)
>  
> +# for sddm to use pam for greeter
> +role xdm_r types xdm_t;
> +allow system_r xdm_r;
> +
>  type xdm_lock_t;
>  files_lock_file(xdm_lock_t)
>  
> @@ -848,6 +853,11 @@ manage_files_pattern(xserver_t, xdm_tmp_
>  manage_lnk_files_pattern(xserver_t, xdm_tmp_t, xdm_tmp_t)
>  manage_sock_files_pattern(xserver_t, xdm_tmp_t, xdm_tmp_t)
>  
> +# for sddm to use pam for greeter
> +corecmd_bin_entry_type(xdm_t)
> +# sddm greeter needs execmod
> +allow xdm_t xdm_tmpfs_t:file execmod;
> +
>  # Run Xorg.wrap
>  can_exec(xserver_t, xserver_exec_t)
>  
> Index: refpolicy-2.20210126/config/appconfig-mcs/seusers
> ===================================================================
> --- refpolicy-2.20210126.orig/config/appconfig-mcs/seusers
> +++ refpolicy-2.20210126/config/appconfig-mcs/seusers
> @@ -1,2 +1,3 @@
>  root:unconfined_u:s0-mcs_systemhigh
>  __default__:unconfined_u:s0-mcs_systemhigh
> +sddm:xdm:s0
> Index: refpolicy-2.20210126/policy/users
> ===================================================================
> --- refpolicy-2.20210126.orig/policy/users
> +++ refpolicy-2.20210126/policy/users
> @@ -27,6 +27,7 @@ gen_user(system_u,, system_r, s0, s0 - m
>  gen_user(user_u, user, user_r, s0, s0)
>  gen_user(staff_u, staff, staff_r sysadm_r ifdef(`enable_mls',`secadm_r auditadm_r'), s0, s0 - mls_systemhigh, mcs_allcats)
>  gen_user(sysadm_u, sysadm, sysadm_r, s0, s0 - mls_systemhigh, mcs_allcats)
> +gen_user(xdm, user, xdm_r, s0, s0)
>  
>  # Until order dependence is fixed for users:
>  ifdef(`direct_sysadm_daemon',`
> Index: refpolicy-2.20210126/config/appconfig-mcs/xdm_default_contexts
> ===================================================================
> --- /dev/null
> +++ refpolicy-2.20210126/config/appconfig-mcs/xdm_default_contexts
> @@ -0,0 +1 @@
> +system_r:xdm_t:s0		xdm_r:xdm_t:s0
> Index: refpolicy-2.20210126/policy/modules/kernel/kernel.te
> ===================================================================
> --- refpolicy-2.20210126.orig/policy/modules/kernel/kernel.te
> +++ refpolicy-2.20210126/policy/modules/kernel/kernel.te
> @@ -32,6 +32,7 @@ role system_r;
>  role sysadm_r;
>  role staff_r;
>  role user_r;
> +role xdm_r;
>  
>  # here until order dependence is fixed:
>  role unconfined_r;
>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: sddm issue and patch not for inclusion

[Dominick Grift]

Dominick Grift <dominick.grift@defensec.nl> writes:

> Russell Coker <russell@coker.com.au> writes:
>
>> In Debian/Unstable (which will soon be frozen and become the next stable
>> release) the sddm X login program (the one that's generally recommended
>> and specifically known to generally work well with SE Linux) uses PAM to
>> start a session for the "greeter" (the program that asks for a password
>> before a new session is started).
>>
>> With the policy currently in Debian that means the sddm user matches
>> "__default__" and gets unconfined_u:unconfined_r:unconfined_t, not what
>> is desirable for a program that takes input from unauthenticated users.
>>
>> role xdm_r;
>> role xdm_r types xdm_t;
>> allow system_r xdm_r;
>> allow xdm_t xdm_tmpfs_t:file execmod;
>
> that looks like a bug or at least bad code
>
>> corecmd_bin_entry_type(xdm_t)

Also wondering what or which bin_t file or files this applies to and if
it instead is not possible to give these a private type 

>>
>> To get this working as a test I put the above in a local policy file,
>> edited /etc/selinux/default/contexts/default_contexts to add a suitable
>> context to the system_r:xdm_t:s0 line, and run the following 2 commands:
>> semanage user -a -r s0 -L s0 -R xdm_r -P user xdm
>> semanage login -a -s xdm -r s0 sddm
>>
>> I mention the above for the benefit of people who do web searches for such
>> things and get the list archives.
>>
>> Below is the policy I'm using which will be in the next release of Debian
>> if no-one else has a better idea.  NB a "better idea" doesn't mean running
>> the greeter as unconfined_t IMHO.  Also while we can debate about whether
>> modifying sddm to not use PAM for the greeter session is a good idea, such
>> a change would potentially affect people who don't use SE Linux so I won't
>> even waste the time of the sddm maintainer by discussing that possibility
>> with them before the release.  After the release we can discuss such
>> things, but now we need to get things working well in the next few days in
>> a manner that will make users happy for the next 2 years.
>>
>> Index: refpolicy-2.20210126/policy/modules/services/xserver.te
>> ===================================================================
>> --- refpolicy-2.20210126.orig/policy/modules/services/xserver.te
>> +++ refpolicy-2.20210126/policy/modules/services/xserver.te
>> @@ -18,6 +18,7 @@ gen_require(`
>>  	class x_resource all_x_resource_perms;
>>  	class x_event all_x_event_perms;
>>  	class x_synthetic_event all_x_synthetic_event_perms;
>> +	role xdm_r;
>>  ')
>>  
>>  ########################################
>> @@ -152,6 +153,10 @@ init_daemon_domain(xdm_t, xdm_exec_t)
>>  xserver_object_types_template(xdm)
>>  xserver_common_x_domain_template(xdm, xdm_t)
>>  
>> +# for sddm to use pam for greeter
>> +role xdm_r types xdm_t;
>> +allow system_r xdm_r;
>> +
>>  type xdm_lock_t;
>>  files_lock_file(xdm_lock_t)
>>  
>> @@ -848,6 +853,11 @@ manage_files_pattern(xserver_t, xdm_tmp_
>>  manage_lnk_files_pattern(xserver_t, xdm_tmp_t, xdm_tmp_t)
>>  manage_sock_files_pattern(xserver_t, xdm_tmp_t, xdm_tmp_t)
>>  
>> +# for sddm to use pam for greeter
>> +corecmd_bin_entry_type(xdm_t)
>> +# sddm greeter needs execmod
>> +allow xdm_t xdm_tmpfs_t:file execmod;
>> +
>>  # Run Xorg.wrap
>>  can_exec(xserver_t, xserver_exec_t)
>>  
>> Index: refpolicy-2.20210126/config/appconfig-mcs/seusers
>> ===================================================================
>> --- refpolicy-2.20210126.orig/config/appconfig-mcs/seusers
>> +++ refpolicy-2.20210126/config/appconfig-mcs/seusers
>> @@ -1,2 +1,3 @@
>>  root:unconfined_u:s0-mcs_systemhigh
>>  __default__:unconfined_u:s0-mcs_systemhigh
>> +sddm:xdm:s0
>> Index: refpolicy-2.20210126/policy/users
>> ===================================================================
>> --- refpolicy-2.20210126.orig/policy/users
>> +++ refpolicy-2.20210126/policy/users
>> @@ -27,6 +27,7 @@ gen_user(system_u,, system_r, s0, s0 - m
>>  gen_user(user_u, user, user_r, s0, s0)
>>  gen_user(staff_u, staff, staff_r sysadm_r ifdef(`enable_mls',`secadm_r auditadm_r'), s0, s0 - mls_systemhigh, mcs_allcats)
>>  gen_user(sysadm_u, sysadm, sysadm_r, s0, s0 - mls_systemhigh, mcs_allcats)
>> +gen_user(xdm, user, xdm_r, s0, s0)
>>  
>>  # Until order dependence is fixed for users:
>>  ifdef(`direct_sysadm_daemon',`
>> Index: refpolicy-2.20210126/config/appconfig-mcs/xdm_default_contexts
>> ===================================================================
>> --- /dev/null
>> +++ refpolicy-2.20210126/config/appconfig-mcs/xdm_default_contexts
>> @@ -0,0 +1 @@
>> +system_r:xdm_t:s0		xdm_r:xdm_t:s0
>> Index: refpolicy-2.20210126/policy/modules/kernel/kernel.te
>> ===================================================================
>> --- refpolicy-2.20210126.orig/policy/modules/kernel/kernel.te
>> +++ refpolicy-2.20210126/policy/modules/kernel/kernel.te
>> @@ -32,6 +32,7 @@ role system_r;
>>  role sysadm_r;
>>  role staff_r;
>>  role user_r;
>> +role xdm_r;
>>  
>>  # here until order dependence is fixed:
>>  role unconfined_r;
>>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: sddm issue and patch not for inclusion

[Russell Coker]

On Thursday, 28 January 2021 10:36:19 PM AEDT Dominick Grift wrote:
> >> In Debian/Unstable (which will soon be frozen and become the next stable
> >> release) the sddm X login program (the one that's generally recommended
> >> and specifically known to generally work well with SE Linux) uses PAM to
> >> start a session for the "greeter" (the program that asks for a password
> >> before a new session is started).
> >> 
> >> With the policy currently in Debian that means the sddm user matches
> >> "__default__" and gets unconfined_u:unconfined_r:unconfined_t, not what
> >> is desirable for a program that takes input from unauthenticated users.
> >> 
> >> role xdm_r;
> >> role xdm_r types xdm_t;
> >> allow system_r xdm_r;
> >> allow xdm_t xdm_tmpfs_t:file execmod;
> > 
> > that looks like a bug or at least bad code

That's a design decision.  One could make a convincing argument that it's a 
good decision.

> >> corecmd_bin_entry_type(xdm_t)
> 
> Also wondering what or which bin_t file or files this applies to and if
> it instead is not possible to give these a private type

/usr/bin/sddm-greeter.  Yes I can give it a private type, might be a good idea 
in any case.

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

Re: sddm issue and patch not for inclusion

[Dominick Grift]

Russell Coker <russell@coker.com.au> writes:

> On Thursday, 28 January 2021 10:36:19 PM AEDT Dominick Grift wrote:
>> >> In Debian/Unstable (which will soon be frozen and become the next stable
>> >> release) the sddm X login program (the one that's generally recommended
>> >> and specifically known to generally work well with SE Linux) uses PAM to
>> >> start a session for the "greeter" (the program that asks for a password
>> >> before a new session is started).
>> >> 
>> >> With the policy currently in Debian that means the sddm user matches
>> >> "__default__" and gets unconfined_u:unconfined_r:unconfined_t, not what
>> >> is desirable for a program that takes input from unauthenticated users.
>> >> 
>> >> role xdm_r;
>> >> role xdm_r types xdm_t;
>> >> allow system_r xdm_r;
>> >> allow xdm_t xdm_tmpfs_t:file execmod;
>> > 
>> > that looks like a bug or at least bad code
>
> That's a design decision.  One could make a convincing argument that it's a 
> good decision.

Not sure whether doing text-relocation on a file you created yourself is
a good decision. From a security perspective does not seem like very
good thing to along, The more because xdm is shared between various
desktop managers.

>
>> >> corecmd_bin_entry_type(xdm_t)
>> 
>> Also wondering what or which bin_t file or files this applies to and if
>> it instead is not possible to give these a private type
>
> /usr/bin/sddm-greeter.  Yes I can give it a private type, might be a good idea 
> in any case.

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: sddm issue and patch not for inclusion

[Russell Coker]

On Friday, 29 January 2021 1:08:56 AM AEDT Dominick Grift wrote:
> > That's a design decision.  One could make a convincing argument that it's
> > a
> > good decision.
> 
> Not sure whether doing text-relocation on a file you created yourself is
> a good decision. From a security perspective does not seem like very
> good thing to along, The more because xdm is shared between various
> desktop managers.

Oh yes, execmod is bad.  I thought your comment was about the use of PAM.

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

Re: sddm issue and patch not for inclusion

[Dominick Grift]

Russell Coker <russell@coker.com.au> writes:

> On Friday, 29 January 2021 1:08:56 AM AEDT Dominick Grift wrote:
>> > That's a design decision.  One could make a convincing argument that it's
>> > a
>> > good decision.
>> 
>> Not sure whether doing text-relocation on a file you created yourself is
>> a good decision. From a security perspective does not seem like very
>> good thing to along, The more because xdm is shared between various
>> desktop managers.
>
> Oh yes, execmod is bad.  I thought your comment was about the use of PAM.

The pam decision is good in my view, i started doing that half a decade
ago.

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift