[refpolicy] systemd security class and AV's

[refpolicy] systemd security class and AV's

[Laurent Bigonville]

Hi,

This is probably a fist step to have systemd support in the policy.

The following patch is adding the security class and the AV's needed for
systemd.

The list of AV's has been built by grepping the systemd code for the calls to
selinux_unit_access_check() and selinux_access_check() macro but is a bit
different of what Fedora/RHEL have in their own policy and documentation.

For example, Fedora/RHEL have 2 extra AV (kill and load) in the service class
while I cannot find anything in the code. On the other hand, they seems to miss
the start and stop AV in the system class.

Did I overlooked something or is there a bug in that regard in the Fedora/RHEL
policy and documentation?

Cheers,

Laurent Bigonville

[refpolicy] [RFC] Add the security class and AV's needed for systemd

[Laurent Bigonville]

From: Laurent Bigonville <bigon@bigon.be>

The list of AV's has been built by grepping the systemd code for the
calls to selinux_unit_access_check() and selinux_access_check() macro.
---
 policy/flask/access_vectors      | 18 ++++++++++++++++++
 policy/flask/security_classes    |  3 +++
 policy/support/obj_perm_sets.spt |  5 +++++
 3 files changed, 26 insertions(+)

diff --git a/policy/flask/access_vectors b/policy/flask/access_vectors
index a94b169..e0d3768 100644
--- a/policy/flask/access_vectors
+++ b/policy/flask/access_vectors
@@ -393,6 +393,14 @@ class system
 	syslog_mod
 	syslog_console
 	module_request
+	halt
+	reboot
+	status
+	start
+	stop
+	enable
+	disable
+	reload
 }
 
 #
@@ -865,3 +873,13 @@ inherits database
 	implement
 	execute
 }
+
+class service
+{
+	start
+	stop
+	status
+	reload
+	enable
+	disable
+}
diff --git a/policy/flask/security_classes b/policy/flask/security_classes
index 14a4799..fdd6307 100644
--- a/policy/flask/security_classes
+++ b/policy/flask/security_classes
@@ -131,4 +131,7 @@ class db_view			# userspace
 class db_sequence		# userspace
 class db_language		# userspace
 
+# systemd services
+class service
+
 # FLASK
diff --git a/policy/support/obj_perm_sets.spt b/policy/support/obj_perm_sets.spt
index 6e91317..38ae511 100644
--- a/policy/support/obj_perm_sets.spt
+++ b/policy/support/obj_perm_sets.spt
@@ -271,3 +271,8 @@ define(`server_stream_socket_perms', `{ client_stream_socket_perms listen accept
 # Keys
 #
 define(`manage_key_perms', `{ create link read search setattr view write } ')
+
+#
+# Service
+#
+define(`manage_service_perms', `{ start stop status reload enable disable } ')
-- 
2.0.1

[refpolicy] [RFC] Add the security class and AV's needed for systemd

[Christopher J. PeBenito]

On 7/5/2014 9:59 AM, Laurent Bigonville wrote:
> From: Laurent Bigonville <bigon@bigon.be>
> 
> The list of AV's has been built by grepping the systemd code for the
> calls to selinux_unit_access_check() and selinux_access_check() macro.
> ---
>  policy/flask/access_vectors      | 18 ++++++++++++++++++
>  policy/flask/security_classes    |  3 +++
>  policy/support/obj_perm_sets.spt |  5 +++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/policy/flask/access_vectors b/policy/flask/access_vectors
> index a94b169..e0d3768 100644
> --- a/policy/flask/access_vectors
> +++ b/policy/flask/access_vectors
> @@ -393,6 +393,14 @@ class system
>  	syslog_mod
>  	syslog_console
>  	module_request
> +	halt
> +	reboot
> +	status
> +	start
> +	stop
> +	enable
> +	disable
> +	reload

This doesn't look right.  There shouldn't be userspace permissions mixed
in with a kernel object class.  Are these really used or are they
compatibility for old versions of systemd?


> @@ -865,3 +873,13 @@ inherits database
>  	implement
>  	execute
>  }
> +
> +class service
> +{
> +	start
> +	stop
> +	status
> +	reload
> +	enable
> +	disable
> +}

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

[refpolicy] [RFC] Add the security class and AV's needed for systemd

[Laurent Bigonville]

Le Tue, 8 Jul 2014 08:30:32 -0400,
"Christopher J. PeBenito" <cpebenito@tresys.com> a ?crit :

> On 7/5/2014 9:59 AM, Laurent Bigonville wrote:
> > From: Laurent Bigonville <bigon@bigon.be>
> > 
> > The list of AV's has been built by grepping the systemd code for the
> > calls to selinux_unit_access_check() and selinux_access_check()
> > macro. ---
> >  policy/flask/access_vectors      | 18 ++++++++++++++++++
> >  policy/flask/security_classes    |  3 +++
> >  policy/support/obj_perm_sets.spt |  5 +++++
> >  3 files changed, 26 insertions(+)
> > 
> > diff --git a/policy/flask/access_vectors
> > b/policy/flask/access_vectors index a94b169..e0d3768 100644
> > --- a/policy/flask/access_vectors
> > +++ b/policy/flask/access_vectors
> > @@ -393,6 +393,14 @@ class system
> >  	syslog_mod
> >  	syslog_console
> >  	module_request
> > +	halt
> > +	reboot
> > +	status
> > +	start
> > +	stop
> > +	enable
> > +	disable
> > +	reload
> 
> This doesn't look right.  There shouldn't be userspace permissions
> mixed in with a kernel object class.  Are these really used or are
> they compatibility for old versions of systemd?

I searched the code that is currently in the HEAD of the master branch
in the systemd git repository and the code path still seems to be used
ATM.

Dominick even had issue with the "start" AV not being associated to the
system class when developing his own policy

> 
> 
> > @@ -865,3 +873,13 @@ inherits database
> >  	implement
> >  	execute
> >  }
> > +
> > +class service
> > +{
> > +	start
> > +	stop
> > +	status
> > +	reload
> > +	enable
> > +	disable
> > +}
> 

[refpolicy] [RFC] Add the security class and AV's needed for systemd

[Christopher J. PeBenito]

On 7/8/2014 9:16 AM, Laurent Bigonville wrote:
> Le Tue, 8 Jul 2014 08:30:32 -0400,
> "Christopher J. PeBenito" <cpebenito@tresys.com> a ?crit :
> 
>> On 7/5/2014 9:59 AM, Laurent Bigonville wrote:
>>> From: Laurent Bigonville <bigon@bigon.be>
>>>
>>> The list of AV's has been built by grepping the systemd code for the
>>> calls to selinux_unit_access_check() and selinux_access_check()
>>> macro. ---
>>>  policy/flask/access_vectors      | 18 ++++++++++++++++++
>>>  policy/flask/security_classes    |  3 +++
>>>  policy/support/obj_perm_sets.spt |  5 +++++
>>>  3 files changed, 26 insertions(+)
>>>
>>> diff --git a/policy/flask/access_vectors
>>> b/policy/flask/access_vectors index a94b169..e0d3768 100644
>>> --- a/policy/flask/access_vectors
>>> +++ b/policy/flask/access_vectors
>>> @@ -393,6 +393,14 @@ class system
>>>  	syslog_mod
>>>  	syslog_console
>>>  	module_request
>>> +	halt
>>> +	reboot
>>> +	status
>>> +	start
>>> +	stop
>>> +	enable
>>> +	disable
>>> +	reload
>>
>> This doesn't look right.  There shouldn't be userspace permissions
>> mixed in with a kernel object class.  Are these really used or are
>> they compatibility for old versions of systemd?
> 
> I searched the code that is currently in the HEAD of the master branch
> in the systemd git repository and the code path still seems to be used
> ATM.
> 
> Dominick even had issue with the "start" AV not being associated to the
> system class when developing his own policy

I just looked at the bug you're referring to.  From Dominick's
description, it sounds like it's buggy code being covered up by the
unknown permissions=allow functionality.

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

[refpolicy] [RFC] Add the security class and AV's needed for systemd

[Dominick Grift]

On Tue, 2014-07-08 at 15:16 +0200, Laurent Bigonville wrote:
> Le Tue, 8 Jul 2014 08:30:32 -0400,
> "Christopher J. PeBenito" <cpebenito@tresys.com> a ?crit :
> 
> > On 7/5/2014 9:59 AM, Laurent Bigonville wrote:
> > > From: Laurent Bigonville <bigon@bigon.be>
> > > 
> > > The list of AV's has been built by grepping the systemd code for the
> > > calls to selinux_unit_access_check() and selinux_access_check()
> > > macro. ---
> > >  policy/flask/access_vectors      | 18 ++++++++++++++++++
> > >  policy/flask/security_classes    |  3 +++
> > >  policy/support/obj_perm_sets.spt |  5 +++++
> > >  3 files changed, 26 insertions(+)
> > > 
> > > diff --git a/policy/flask/access_vectors
> > > b/policy/flask/access_vectors index a94b169..e0d3768 100644
> > > --- a/policy/flask/access_vectors
> > > +++ b/policy/flask/access_vectors
> > > @@ -393,6 +393,14 @@ class system
> > >  	syslog_mod
> > >  	syslog_console
> > >  	module_request
> > > +	halt
> > > +	reboot
> > > +	status
> > > +	start
> > > +	stop
> > > +	enable
> > > +	disable
> > > +	reload
> > 
> > This doesn't look right.  There shouldn't be userspace permissions
> > mixed in with a kernel object class.  Are these really used or are
> > they compatibility for old versions of systemd?
> 
> I searched the code that is currently in the HEAD of the master branch
> in the systemd git repository and the code path still seems to be used
> ATM.
> 
> Dominick even had issue with the "start" AV not being associated to the
> system class when developing his own policy
> 
> > 
> > 
> > > @@ -865,3 +873,13 @@ inherits database
> > >  	implement
> > >  	execute
> > >  }
> > > +
> > > +class service
> > > +{
> > > +	start
> > > +	stop
> > > +	status
> > > +	reload
> > > +	enable
> > > +	disable
> > > +}
> > 
> 

Systemd has many issues with SELinux still, and it object managers in
core components in a system also exposes issues in libselinux.

Some of the issues:

1. systemd uses the linux system object for its some of it's access
vector permissions.
2. systemd creates objects on behalf of real users without setting
security contexts as specified in policy. (needs to use lsetfilecon)
3. systemd runs programs on behalf of real users without setting
security contexts as specified in policy. (needs to hook into
pam_selinux)
4. libselinux does not expose unknown user space access vectors. in the
past this has not proved to be a problem but now that core components
are object managers (systemd), and that dbus has become a core component
this proves to be a serious issue

These are some of the more pressing issues currently with systemd and
selinux. I have files bug reports for most of these issues (except issue
#1). However, i am having difficulties explaining what the problem is
here (or they have difficulties understanding or they just dont care).

[refpolicy] [RFC] Add the security class and AV's needed for systemd

[Laurent Bigonville]

Le Tue, 8 Jul 2014 08:30:32 -0400,
"Christopher J. PeBenito" <cpebenito@tresys.com> a ?crit :

> On 7/5/2014 9:59 AM, Laurent Bigonville wrote:
> > From: Laurent Bigonville <bigon@bigon.be>
> > 
> > The list of AV's has been built by grepping the systemd code for the
> > calls to selinux_unit_access_check() and selinux_access_check()
> > macro. ---
> >  policy/flask/access_vectors      | 18 ++++++++++++++++++
> >  policy/flask/security_classes    |  3 +++
> >  policy/support/obj_perm_sets.spt |  5 +++++
> >  3 files changed, 26 insertions(+)
> > 
> > diff --git a/policy/flask/access_vectors
> > b/policy/flask/access_vectors index a94b169..e0d3768 100644
> > --- a/policy/flask/access_vectors
> > +++ b/policy/flask/access_vectors
> > @@ -393,6 +393,14 @@ class system
> >  	syslog_mod
> >  	syslog_console
> >  	module_request
> > +	halt
> > +	reboot
> > +	status
> > +	start
> > +	stop
> > +	enable
> > +	disable
> > +	reload
> 
> This doesn't look right.  There shouldn't be userspace permissions
> mixed in with a kernel object class.  Are these really used or are
> they compatibility for old versions of systemd?

I've opened a bug about this:
https://bugs.freedesktop.org/show_bug.cgi?id=81105

[refpolicy] systemd security class and AV's

[Nicolas Iooss]

Hello,

2014-07-05 15:59 GMT+02:00 Laurent Bigonville <bigon@debian.org>:
> Hi,
>
> This is probably a fist step to have systemd support in the policy.
>
> The following patch is adding the security class and the AV's needed for
> systemd.

Thanks! As a systemd user I've been using a policy built from merging
part of Fedora's policy into refpolicy and then I've improved it to
make it work on my system (which is running Arch Linux). With your
work it would be easier to upstream fixes such as stopping labeling
vsftpd service as "iptables_unit_file_t" (cf [1]). While speaking
about unit files, the label on these files is quite important because
it is the target of the "service:start/stop/status" checks (according
to [2]). Fedora policy uses "systemd_unit_file" interface to define
unit files types and there have been emails in February against this
interface name [3]. Has a new name been chosen?
>
> The list of AV's has been built by grepping the systemd code for the calls to
> selinux_unit_access_check() and selinux_access_check() macro but is a bit
> different of what Fedora/RHEL have in their own policy and documentation.
>
> For example, Fedora/RHEL have 2 extra AV (kill and load) in the service class
> while I cannot find anything in the code. On the other hand, they seems to miss
> the start and stop AV in the system class.
>
> Did I overlooked something or is there a bug in that regard in the Fedora/RHEL
> policy and documentation?

I have the same question since the moment I began cleaning up
systemd-related things in my policy a few weeks ago. To find where the
start, stop, enable, disable and reload permission could be used with
the "system" class (not to be confused with the "service" one), I
removed all of these from the system block in
policy/flask/access_vectors and nothing went wrong. I use "UNK_PERMS =
deny" in my build.conf and run in permissive mode and no AVC denials
show up in audit.log when I start a transient unit.

More precisely, here is the relevant code in systemd:
method_start_transient_unit() calls selinux_access_check(message,
"start", error) [4] which is a expanded by a macro to
selinux_generic_access_check(message, NULL, "start", error) [5]. The
second argument of this function is named "path" and is NULL so the
"else" branch is taken on line 219 and selinux_check_access is called
with "system" class [6]. In short, AFAIU, starting a transient unit
leads to an access check of permission "start" in the class "system".
However, when running "systemd-run /usr/bin/ls" (which calls
org.freedesktop.systemd1.Manager.StartTransientUnit DBus method
according to strace), I get the following log:

  type=USER_AVC msg=audit(1404575782.104:353): pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t msg='avc:
 denied  { start } for auid=1000 uid=0 gid=0
path="/run/systemd/system/run-12142.service" cmdline="systemd-run
/usr/bin/ls" scontext=unconfined_u:unconfined_r:unconfined_t
tcontext=system_u:object_r:init_var_run_t tclass=service
exe="/usr/lib/systemd/systemd" sauid=0 hostname=? addr=? terminal=?'

... so the generated user AVC has class "service" and not "system".
There are at least two ways of explaining this:

* either I missed something in libselinux and there is a check with
"system:start" which is not audited even though this permission
doesn't exist (and the previous AVC is a second check which happens
once the unit is configured),
* or I misunderstood systemd's code and selinux_access_check() can do
checks in service class too.

Could someone who understands the interface between systemd and
SELinux explain what's happening?

Thanks,

Nicolas

[1] https://git.fedorahosted.org/cgit/selinux-policy.git/tree/ftp.fc?h=master_contrib&id=3f08711eadbf3d74113cd4c9a36ab84c36a0ec8c#n3
[2] http://cgit.freedesktop.org/systemd/systemd/tree/src/core/selinux-access.c#n212
[3] http://oss.tresys.com/pipermail/refpolicy/2014-February/006910.html
[4] http://cgit.freedesktop.org/systemd/systemd/tree/src/core/dbus-manager.c?id=108e8cd11e88bd4795a62bf335921d438592601c#n607
[5] http://cgit.freedesktop.org/systemd/systemd/tree/src/core/selinux-access.h?id=108e8cd11e88bd4795a62bf335921d438592601c#n34
[6] http://cgit.freedesktop.org/systemd/systemd/tree/src/core/selinux-access.c?id=108e8cd11e88bd4795a62bf335921d438592601c#n209