[PATCH v2 0/2] ssh policy new interfaces

[PATCH v2 0/2] ssh policy new interfaces

[Alexander Miroshnichenko]

New interfaces for ssh policy which can be used with git hosting
software.

Alexander Miroshnichenko (2):
  ssh: Add ssh_exec_keygen interface
  ssh: Add interface ssh_search_dir

 policy/modules/services/ssh.if | 37 ++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

-- 
2.21.0

[PATCH v2 1/2] ssh: Add ssh_exec_keygen interface

[Alexander Miroshnichenko]

Signed-off-by: Alexander Miroshnichenko <alex@millerson.name>
---
 policy/modules/services/ssh.if | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/policy/modules/services/ssh.if b/policy/modules/services/ssh.if
index b5bd2762ef96..0941f133711e 100644
--- a/policy/modules/services/ssh.if
+++ b/policy/modules/services/ssh.if
@@ -701,6 +701,25 @@ interface(`ssh_read_user_home_files',`
 	userdom_search_user_home_dirs($1)
 ')
 
+########################################
+## <summary>
+##      Execute the ssh key generator in the caller domain.
+## </summary>
+## <param name="domain">
+##      <summary>
+##      Domain allowed to transition.
+##      </summary>
+## </param>
+#
+interface(`ssh_exec_keygen',`
+        gen_require(`
+                type ssh_keygen_exec_t;
+        ')
+
+	corecmd_search_bin($1)
+	can_exec($1, ssh_keygen_exec_t)
+')
+
 ########################################
 ## <summary>
 ##	Execute the ssh key generator in the ssh keygen domain.
-- 
2.21.0

[PATCH v2 2/2] ssh: Add interface ssh_search_dir

[Alexander Miroshnichenko]

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

Signed-off-by: Alexander Miroshnichenko <alex@millerson.name>
---
 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

Re: [PATCH v2 1/2] ssh: Add ssh_exec_keygen interface

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On Thu, Jun 20, 2019 at 05:41:37PM +0300, Alexander Miroshnichenko wrote:
> Signed-off-by: Alexander Miroshnichenko <alex@millerson.name>
> ---
>  policy/modules/services/ssh.if | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/policy/modules/services/ssh.if b/policy/modules/services/ssh.if
> index b5bd2762ef96..0941f133711e 100644
> --- a/policy/modules/services/ssh.if
> +++ b/policy/modules/services/ssh.if
> @@ -701,6 +701,25 @@ interface(`ssh_read_user_home_files',`
>  	userdom_search_user_home_dirs($1)
>  ')
>  
> +########################################
> +## <summary>
> +##      Execute the ssh key generator in the caller domain.
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed to transition.

"Domain allowed access." Its not a transition. I suppose you have a use for this interface?

> +##      </summary>
> +## </param>
> +#
> +interface(`ssh_exec_keygen',`
> +        gen_require(`
> +                type ssh_keygen_exec_t;
> +        ')
> +
> +	corecmd_search_bin($1)
> +	can_exec($1, ssh_keygen_exec_t)
> +')
> +
>  ########################################
>  ## <summary>
>  ##	Execute the ssh key generator in the ssh keygen domain.
> -- 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

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 <alex@millerson.name>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Alexander Miroshnichenko]

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 <alex@millerson.name>
>> ---
>>  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 ...
>

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

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 2739 bytes --]

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 <alex@millerson.name>
> > > ---
> > >  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Alexander Miroshnichenko]

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

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

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]

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 <alex@millerson.name>
> > > > ---
> > > >  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]