2019-06-20 14:43:26

by Alexander Miroshnichenko

[permalink] [raw]
Subject: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

Create interface ssh_search_dir to allow ssh_server search for keys in non-standard location.

Signed-off-by: Alexander Miroshnichenko <[email protected]>
---
policy/modules/services/ssh.if | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/policy/modules/services/ssh.if b/policy/modules/services/ssh.if
index 0941f133711e..51c64ded00c4 100644
--- a/policy/modules/services/ssh.if
+++ b/policy/modules/services/ssh.if
@@ -680,6 +680,24 @@ interface(`ssh_agent_exec',`
can_exec($1, ssh_agent_exec_t)
')

+########################################
+## <summary>
+## Search for keys in non-standard location
+## </summary>
+## <param name="domain">
+## <summary>
+## Domain allowed access.
+## </summary>
+## </param>
+#
+interface(`ssh_search_dir',`
+ gen_require(`
+ type sshd_t;
+ ')
+
+ allow sshd_t $1:dir search_dir_perms;
+')
+
########################################
## <summary>
## Read ssh home directory content
--
2.21.0


2019-06-20 14:50:18

by Dominick Grift

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

On Thu, Jun 20, 2019 at 05:41:38PM +0300, Alexander Miroshnichenko wrote:
> Create interface ssh_search_dir to allow ssh_server search for keys in non-standard location.
>
> Signed-off-by: Alexander Miroshnichenko <[email protected]>
> ---
> policy/modules/services/ssh.if | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/policy/modules/services/ssh.if b/policy/modules/services/ssh.if
> index 0941f133711e..51c64ded00c4 100644
> --- a/policy/modules/services/ssh.if
> +++ b/policy/modules/services/ssh.if
> @@ -680,6 +680,24 @@ interface(`ssh_agent_exec',`
> can_exec($1, ssh_agent_exec_t)
> ')
>
> +########################################
> +## <summary>
> +## Search for keys in non-standard location
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +## Domain allowed access.
> +## </summary>
> +## </param>
> +#
> +interface(`ssh_search_dir',`
> + gen_require(`
> + type sshd_t;
> + ')
> +
> + allow sshd_t $1:dir search_dir_perms;

This is generally not allowed. The caller should generally be the source.
Regardless of the above. Keys should be in user home directories. I wonder what specific scenario prompted you to propose this interface?

> +')
> +
> ########################################
> ## <summary>
> ## Read ssh home directory content
> --
> 2.21.0
>

--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


Attachments:
(No filename) (1.55 kB)
signature.asc (673.00 B)
Download all attachments

2019-06-20 15:06:12

by Alexander Miroshnichenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

On четверг, 20 июня 2019 г. 17:50:11 MSK, Dominick Grift wrote:
> On Thu, Jun 20, 2019 at 05:41:38PM +0300, Alexander Miroshnichenko wrote:
>> Create interface ssh_search_dir to allow ssh_server search for
>> keys in non-standard location.
>>
>> Signed-off-by: Alexander Miroshnichenko <[email protected]>
>> ---
>> policy/modules/services/ssh.if | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/policy/modules/services/ssh.if
>> b/policy/modules/services/ssh.if
>> index 0941f133711e..51c64ded00c4 100644
>> --- a/policy/modules/services/ssh.if
>> +++ b/policy/modules/services/ssh.if
>> @@ -680,6 +680,24 @@ interface(`ssh_agent_exec',`
>> can_exec($1, ssh_agent_exec_t)
>> ')
>>
>> +########################################
>> +## <summary>
>> +## Search for keys in non-standard location
>> +## </summary>
>> +## <param name="domain">
>> +## <summary>
>> +## Domain allowed access.
>> +## </summary>
>> +## </param>
>> +#
>> +interface(`ssh_search_dir',`
>> + gen_require(`
>> + type sshd_t;
>> + ')
>> +
>> + allow sshd_t $1:dir search_dir_perms;
>
> This is generally not allowed. The caller should generally be the source.
> Regardless of the above. Keys should be in user home
> directories. I wonder what specific scenario prompted you to
> propose this interface?

GIT hosting software like gitolite/gitosis/gitea manage users ssh keys and
store them own location like /var/lib/gitolite/.ssh .
/var/lib/gitolite have gitosis_var_lib_t type,
/var/lib/gitolite/.ssh have gitosis_ssh_home_t type (in patched policy
which
I want to submit).
If sshd does not have { search getattr } permissions to full path to ssh
key
user fail to login.
Can you propose corret way to give such permissions to multiple policies?
It is incorrect to label /var/lib/gitolite as user_home_dir_t type, IMHO.

>> +')
>> +
>> ########################################
>> ## <summary>
>> ## Read ssh home directory content ...
>

2019-06-20 15:27:39

by Dominick Grift

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

On Thu, Jun 20, 2019 at 06:05:57PM +0300, Alexander Miroshnichenko wrote:
> On четверг, 20 июня 2019 г. 17:50:11 MSK, Dominick Grift wrote:
> > On Thu, Jun 20, 2019 at 05:41:38PM +0300, Alexander Miroshnichenko wrote:
> > > Create interface ssh_search_dir to allow ssh_server search for keys
> > > in non-standard location.
> > >
> > > Signed-off-by: Alexander Miroshnichenko <[email protected]>
> > > ---
> > > policy/modules/services/ssh.if | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/policy/modules/services/ssh.if
> > > b/policy/modules/services/ssh.if
> > > index 0941f133711e..51c64ded00c4 100644
> > > --- a/policy/modules/services/ssh.if
> > > +++ b/policy/modules/services/ssh.if
> > > @@ -680,6 +680,24 @@ interface(`ssh_agent_exec',`
> > > can_exec($1, ssh_agent_exec_t)
> > > ')
> > > +########################################
> > > +## <summary>
> > > +## Search for keys in non-standard location
> > > +## </summary>
> > > +## <param name="domain">
> > > +## <summary>
> > > +## Domain allowed access.
> > > +## </summary>
> > > +## </param>
> > > +#
> > > +interface(`ssh_search_dir',`
> > > + gen_require(`
> > > + type sshd_t;
> > > + ')
> > > +
> > > + allow sshd_t $1:dir search_dir_perms;
> >
> > This is generally not allowed. The caller should generally be the source.
> > Regardless of the above. Keys should be in user home directories. I
> > wonder what specific scenario prompted you to propose this interface?
>
> GIT hosting software like gitolite/gitosis/gitea manage users ssh keys and
> store them own location like /var/lib/gitolite/.ssh . /var/lib/gitolite have
> gitosis_var_lib_t type, /var/lib/gitolite/.ssh have gitosis_ssh_home_t type
> (in patched policy which I want to submit).
> If sshd does not have { search getattr } permissions to full path to ssh key
> user fail to login.
> Can you propose corret way to give such permissions to multiple policies?
> It is incorrect to label /var/lib/gitolite as user_home_dir_t type, IMHO.

Yes this sucks. I would probably do the following instead:

1. echo "ignoredirs=/var/lib/gitolite" >> /etc/selinux/semanage.conf
2. semodule -B && restorecon -RvF /var/lib/gitolite
3. gitosis_read_lib_files(sshd_t)

Dont bother with labeling /var/lib/gitolite/.ssh differently

>
> > > +')
> > > +
> > > ########################################
> > > ## <summary>
> > > ## Read ssh home directory content ...
> >
>

--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


Attachments:
(No filename) (2.67 kB)
signature.asc (673.00 B)
Download all attachments

2019-06-20 15:39:59

by Alexander Miroshnichenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

On четверг, 20 июня 2019 г. 18:27:31 MSK, Dominick Grift wrote:
> On Thu, Jun 20, 2019 at 06:05:57PM +0300, Alexander Miroshnichenko wrote:
>> On четверг, 20 июня 2019 г. 17:50:11 MSK, Dominick Grift wrote: ...
>
> Yes this sucks. I would probably do the following instead:
>
> 1. echo "ignoredirs=/var/lib/gitolite" >> /etc/selinux/semanage.conf
> 2. semodule -B && restorecon -RvF /var/lib/gitolite
> 3. gitosis_read_lib_files(sshd_t)

I can't use sshd_t in another policy without require statement.
Or I need to add gitosis_read_lib_files(sshd_t) to ssh.te policy file.
All 3 steps are ugly comparing with new ssh_search_dir() interface.
Why such restrictions where caller must be the source for interface? It is
not flexible.

>
> Dont bother with labeling /var/lib/gitolite/.ssh differently
>
>> ...
>

2019-06-20 15:40:37

by Dominick Grift

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

On Thu, Jun 20, 2019 at 05:27:31PM +0200, Dominick Grift wrote:
> On Thu, Jun 20, 2019 at 06:05:57PM +0300, Alexander Miroshnichenko wrote:
> > On четверг, 20 июня 2019 г. 17:50:11 MSK, Dominick Grift wrote:
> > > On Thu, Jun 20, 2019 at 05:41:38PM +0300, Alexander Miroshnichenko wrote:
> > > > Create interface ssh_search_dir to allow ssh_server search for keys
> > > > in non-standard location.
> > > >
> > > > Signed-off-by: Alexander Miroshnichenko <[email protected]>
> > > > ---
> > > > policy/modules/services/ssh.if | 18 ++++++++++++++++++
> > > > 1 file changed, 18 insertions(+)
> > > >
> > > > diff --git a/policy/modules/services/ssh.if
> > > > b/policy/modules/services/ssh.if
> > > > index 0941f133711e..51c64ded00c4 100644
> > > > --- a/policy/modules/services/ssh.if
> > > > +++ b/policy/modules/services/ssh.if
> > > > @@ -680,6 +680,24 @@ interface(`ssh_agent_exec',`
> > > > can_exec($1, ssh_agent_exec_t)
> > > > ')
> > > > +########################################
> > > > +## <summary>
> > > > +## Search for keys in non-standard location
> > > > +## </summary>
> > > > +## <param name="domain">
> > > > +## <summary>
> > > > +## Domain allowed access.
> > > > +## </summary>
> > > > +## </param>
> > > > +#
> > > > +interface(`ssh_search_dir',`
> > > > + gen_require(`
> > > > + type sshd_t;
> > > > + ')
> > > > +
> > > > + allow sshd_t $1:dir search_dir_perms;
> > >
> > > This is generally not allowed. The caller should generally be the source.
> > > Regardless of the above. Keys should be in user home directories. I
> > > wonder what specific scenario prompted you to propose this interface?
> >
> > GIT hosting software like gitolite/gitosis/gitea manage users ssh keys and
> > store them own location like /var/lib/gitolite/.ssh . /var/lib/gitolite have
> > gitosis_var_lib_t type, /var/lib/gitolite/.ssh have gitosis_ssh_home_t type
> > (in patched policy which I want to submit).
> > If sshd does not have { search getattr } permissions to full path to ssh key
> > user fail to login.
> > Can you propose corret way to give such permissions to multiple policies?
> > It is incorrect to label /var/lib/gitolite as user_home_dir_t type, IMHO.
>
> Yes this sucks. I would probably do the following instead:
>
> 1. echo "ignoredirs=/var/lib/gitolite" >> /etc/selinux/semanage.conf
> 2. semodule -B && restorecon -RvF /var/lib/gitolite
> 3. gitosis_read_lib_files(sshd_t)
>
> Dont bother with labeling /var/lib/gitolite/.ssh differently

But this is just what I would do (if were ever forced to use gitolite). Others may have different opinions.

>
> >
> > > > +')
> > > > +
> > > > ########################################
> > > > ## <summary>
> > > > ## Read ssh home directory content ...
> > >
> >
>
> --
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift



--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


Attachments:
(No filename) (3.15 kB)
signature.asc (673.00 B)
Download all attachments

2019-06-20 15:51:04

by Dominick Grift

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir

On Thu, Jun 20, 2019 at 06:38:10PM +0300, Alexander Miroshnichenko wrote:
> On четверг, 20 июня 2019 г. 18:27:31 MSK, Dominick Grift wrote:
> > On Thu, Jun 20, 2019 at 06:05:57PM +0300, Alexander Miroshnichenko wrote:
> > > On четверг, 20 июня 2019 г. 17:50:11 MSK, Dominick Grift wrote: ...
> >
> > Yes this sucks. I would probably do the following instead:
> >
> > 1. echo "ignoredirs=/var/lib/gitolite" >> /etc/selinux/semanage.conf
> > 2. semodule -B && restorecon -RvF /var/lib/gitolite
> > 3. gitosis_read_lib_files(sshd_t)
>
> I can't use sshd_t in another policy without require statement.
> Or I need to add gitosis_read_lib_files(sshd_t) to ssh.te policy file.
> All 3 steps are ugly comparing with new ssh_search_dir() interface.
> Why such restrictions where caller must be the source for interface? It is
> not flexible.

You would need to add the gitosis_read_var_lib_files(sshd_t) to ssh.te yes.
I agree that this is ugly but the alternative is even more ugly, and I will say that this is just what I would do (you might want to wait for maintainer's advice instead of taking my advice)
This is one of those scenario's that are the exception rather than the rule. All options are bad.
The "restriction" is actually an unwritten rule as I cannot find any references to it in https://github.com/SELinuxProject/refpolicy/wiki/StyleGuide so you might be able to get away with it.

>
> >
> > Dont bother with labeling /var/lib/gitolite/.ssh differently
> >
> > > ...
> >
>

--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


Attachments:
(No filename) (1.67 kB)
signature.asc (673.00 B)
Download all attachments