Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-19 Thread Daniel J Walsh 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/18/2013 05:45 PM, Michal Sekletar wrote:
 On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
 11/16/2013 08:10 AM, Lennart Poettering wrote:
 On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
 
 
 -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
 
 On 11/14/2013 12:50 PM, Harald Hoyer wrote:
 On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
 On 11/05/2013 12:22 PM, Lennart Poettering wrote:
 
 Ok lets add a check that checks for start on a service labeled
 with the remote process label, then we can add rules like
 
 allow systemd_logind_t self:service start
 
 Or we can make it simpler and have the local end check against
 the init_t process.
 
 allow systemd_logind_t init_t:service start;
 
 Which is probably a better solution, if we have no way of 
 differentiating the services.
 
 Machineid usually runs as init_t now.
 
 systemd-run runs as the label of the process that executes it, 
 Usually unconfined_t, and sysadm_t.
 
 
 has any solution been found for this?
 
 seems like one is needed for 
 https://bugzilla.redhat.com/show_bug.cgi?id=1008864
 
 
 I guess the question I have is do you expect a patch from me?  Or
 are you guys working on it?  I would go with the checking based on
 process label.
 
 I am hoping for a patch for this!
 
 Thanks,
 
 Lennart
 
 
 This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I 
 believe this error will happen when a snapshot is created.  Also now pass
 in system when doing a system check, if it is doing a service check and
 does not pass in a unit file we will default the target to the label that
 systemd is running with.
 
 Hi,
 
 Maybe I am missing something but isn't this about transient units in
 general, i.e. what about StartTransient()? What is going to change in
 this case after applying this patch? tclass will be system since in
 SELINUX_ACCESS_CHECK you now pass system as path and you will set
 tclass in else branch to system which is afaik same as before.
 
In the current code, passing a unit_file of NULL (StartTransients has a NULL
UnitFile) will indicate that it should do a system check.  My patch is
intended to change this so a NULL UnitFile will indicate that systemd should
check the access between the calling process label and the current process
label for a service access.  Where as the SELINUX_ACCESS_CHECK will now pass
a system flag to the function to make it do a system check.
 On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as
 do {} while (false) in case if we don't HAVE_SELINUX.
 
 It might be the case that I completely misunderstood what's this about,
 in such case ignore this email.
 
 Michal
 

Thanks added  SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in case if
we don't HAVE_SELINUX.

Updated patch.

snip

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-
From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit creation.

SELinux does not have a path to check for a snapshot or transient
unit files service creation.  Currently systemd does a bogus check.

If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.

Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
 src/core/dbus-manager.c   |  2 +-
 src/core/selinux-access.c |  9 +
 src/core/selinux-access.h | 13 +
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
 dbus_bool_t cleanup;
 Snapshot *s;
 
-SELINUX_ACCESS_CHECK(connection, message, start);
+SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, start);
 
 if (!dbus_message_get_args(
 message,
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index c7e951c..af34b9e 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -374,8 +374,9 @@ int selinux_access_check(
 goto finish;
 }
 
-if (path) {
-tclass = service;
+
+tclass = service;
+if (path  strneq(path,system, strlen(system))) {
 /* get the file context of the unit file */
 r = getfilecon(path, fcon);
 if (r  0)

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-19 Thread Michal Sekletar 
On Tue, Nov 19, 2013 at 08:54:41AM -0500, Daniel J Walsh wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 11/18/2013 05:45 PM, Michal Sekletar wrote:
  On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
  11/16/2013 08:10 AM, Lennart Poettering wrote:
  On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
  
  
  -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
  
  On 11/14/2013 12:50 PM, Harald Hoyer wrote:
  On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
  On 11/05/2013 12:22 PM, Lennart Poettering wrote:
  
  Ok lets add a check that checks for start on a service labeled
  with the remote process label, then we can add rules like
  
  allow systemd_logind_t self:service start
  
  Or we can make it simpler and have the local end check against
  the init_t process.
  
  allow systemd_logind_t init_t:service start;
  
  Which is probably a better solution, if we have no way of 
  differentiating the services.
  
  Machineid usually runs as init_t now.
  
  systemd-run runs as the label of the process that executes it, 
  Usually unconfined_t, and sysadm_t.
  
  
  has any solution been found for this?
  
  seems like one is needed for 
  https://bugzilla.redhat.com/show_bug.cgi?id=1008864
  
  
  I guess the question I have is do you expect a patch from me?  Or
  are you guys working on it?  I would go with the checking based on
  process label.
  
  I am hoping for a patch for this!
  
  Thanks,
  
  Lennart
  
  
  This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I 
  believe this error will happen when a snapshot is created.  Also now pass
  in system when doing a system check, if it is doing a service check and
  does not pass in a unit file we will default the target to the label that
  systemd is running with.
  
  Hi,
  
  Maybe I am missing something but isn't this about transient units in
  general, i.e. what about StartTransient()? What is going to change in
  this case after applying this patch? tclass will be system since in
  SELINUX_ACCESS_CHECK you now pass system as path and you will set
  tclass in else branch to system which is afaik same as before.
  
 In the current code, passing a unit_file of NULL (StartTransients has a NULL
 UnitFile) will indicate that it should do a system check.  My patch is
 intended to change this so a NULL UnitFile will indicate that systemd should
 check the access between the calling process label and the current process
 label for a service access.  Where as the SELINUX_ACCESS_CHECK will now pass
 a system flag to the function to make it do a system check.

Hi Dan,

Agreed, I get the idea, but this means that SELINUX_SNAPSHOT_ACCESS_CHECK should
be performed in StartTransient branch in dbus-manager.c too and macro should be
probably renamed to something like SELINUX_RUNTIME_UNIT_ACCESS_CHECK.

Hope that makes sense.

Michal

  On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as
  do {} while (false) in case if we don't HAVE_SELINUX.
  
  It might be the case that I completely misunderstood what's this about,
  in such case ignore this email.
  
  Michal
  
 
 Thanks added  SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in case if
 we don't HAVE_SELINUX.
 
 Updated patch.
 
 snip
 
 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.15 (GNU/Linux)
 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
 
 iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
 PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
 =9I5G
 -END PGP SIGNATURE-

 From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
 From: Dan Walsh dwa...@redhat.com
 Date: Mon, 18 Nov 2013 15:52:37 -0500
 Subject: [PATCH] Fix SELinux check for snapshot and transitent unit creation.
 
 SELinux does not have a path to check for a snapshot or transient
 unit files service creation.  Currently systemd does a bogus check.
 
 If we don't have a unit file for a snapshot or transient unit we i
 should check if the remote process label, has the required access
 for a service with the SELinux label that systemd is running with.
 
 Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
 ---
  src/core/dbus-manager.c   |  2 +-
  src/core/selinux-access.c |  9 +
  src/core/selinux-access.h | 13 +
  3 files changed, 19 insertions(+), 5 deletions(-)
 
 diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
 index 747bcfc..a60a568 100644
 --- a/src/core/dbus-manager.c
 +++ b/src/core/dbus-manager.c
 @@ -1112,7 +1112,7 @@ static DBusHandlerResult 
 bus_manager_message_handler(DBusConnection *connection,
  dbus_bool_t cleanup;
  Snapshot *s;
  
 -SELINUX_ACCESS_CHECK(connection, message, start);
 +SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, start);
  
  if (!dbus_message_get_args(
  message,
 diff --git a/src/core/selinux-access.c

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-19 Thread Václav Pavlín 



Út 19. listopad 2013, 15:16:47 CET, Michal Sekletar napsal:


On Tue, Nov 19, 2013 at 08:54:41AM -0500, Daniel J Walsh wrote:


-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/18/2013 05:45 PM, Michal Sekletar wrote:


On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
11/16/2013 08:10 AM, Lennart Poettering wrote:






On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:




-BEGIN PGP SIGNED MESSAGE- Hash: SHA1

On 11/14/2013 12:50 PM, Harald Hoyer wrote:


On 11/05/2013 11:12 PM, Daniel J Walsh wrote:


On 11/05/2013 12:22 PM, Lennart Poettering wrote:





Ok lets add a check that checks for start on a service labeled
with the remote process label, then we can add rules like





allow systemd_logind_t self:service start





Or we can make it simpler and have the local end check against
the init_t process.





allow systemd_logind_t init_t:service start;





Which is probably a better solution, if we have no way of
differentiating the services.





Machineid usually runs as init_t now.





systemd-run runs as the label of the process that executes it,
Usually unconfined_t, and sysadm_t.




has any solution been found for this?

seems like one is needed for
https://bugzilla.redhat.com/show_bug.cgi?id=1008864




I guess the question I have is do you expect a patch from me? Or
are you guys working on it? I would go with the checking based on
process label.



I am hoping for a patch for this!

Thanks,

Lennart








This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I
believe this error will happen when a snapshot is created. Also now pass
in system when doing a system check, if it is doing a service 
check and
does not pass in a unit file we will default the target to the label 
that

systemd is running with.



Hi,





Maybe I am missing something but isn't this about transient units in
general, i.e. what about StartTransient()? What is going to change in
this case after applying this patch? tclass will be system since in
SELINUX_ACCESS_CHECK you now pass system as path and you will set
tclass in else branch to system which is afaik same as before.





In the current code, passing a unit_file of NULL (StartTransients has 
a NULL

UnitFile) will indicate that it should do a system check. My patch is
intended to change this so a NULL UnitFile will indicate that systemd 
should
check the access between the calling process label and the current 
process
label for a service access. Where as the SELINUX_ACCESS_CHECK will 
now pass

a system flag to the function to make it do a system check.



Hi Dan,

Agreed, I get the idea, but this means that 
SELINUX_SNAPSHOT_ACCESS_CHECK should
be performed in StartTransient branch in dbus-manager.c too and macro 
should be

probably renamed to something like SELINUX_RUNTIME_UNIT_ACCESS_CHECK.

Hope that makes sense.

Michal


Hi,

I tried to improve Dan's patch, so I added an empty call if selinux is 
not supported, renamed the function so it doesn't imply it's only for 
snapshots and used it in StartTransient and RemoveSnapshot as well. I 
wanted to test it, but I am not sure if the policy is already updated.


Vaclav








On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as
do {} while (false) in case if we don't HAVE_SELINUX.





It might be the case that I completely misunderstood what's this about,
in such case ignore this email.





Michal






Thanks added SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in 
case if

we don't HAVE_SELINUX.

Updated patch.

snip

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-





From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit 
creation.


SELinux does not have a path to check for a snapshot or transient
unit files service creation. Currently systemd does a bogus check.

If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.

Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
src/core/dbus-manager.c | 2 +-
src/core/selinux-access.c | 9 +
src/core/selinux-access.h | 13 +
3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult 
bus_manager_message_handler(DBusConnection *connection,

dbus_bool_t cleanup;
Snapshot *s;

- SELINUX_ACCESS_CHECK(connection, message, start);
+

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-19 Thread Václav Pavlín 

And I obviously attached wrong file...this is the right one, sorry

St 20. listopad 2013, 05:47:36 CET, Václav Pavlín napsal:




Út 19. listopad 2013, 15:16:47 CET, Michal Sekletar napsal:



On Tue, Nov 19, 2013 at 08:54:41AM -0500, Daniel J Walsh wrote:



-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/18/2013 05:45 PM, Michal Sekletar wrote:



On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
11/16/2013 08:10 AM, Lennart Poettering wrote:









On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:





-BEGIN PGP SIGNED MESSAGE- Hash: SHA1

On 11/14/2013 12:50 PM, Harald Hoyer wrote:



On 11/05/2013 11:12 PM, Daniel J Walsh wrote:



On 11/05/2013 12:22 PM, Lennart Poettering wrote:







Ok lets add a check that checks for start on a service labeled
with the remote process label, then we can add rules like







allow systemd_logind_t self:service start







Or we can make it simpler and have the local end check against
the init_t process.







allow systemd_logind_t init_t:service start;







Which is probably a better solution, if we have no way of
differentiating the services.







Machineid usually runs as init_t now.







systemd-run runs as the label of the process that executes it,
Usually unconfined_t, and sysadm_t.





has any solution been found for this?

seems like one is needed for
https://bugzilla.redhat.com/show_bug.cgi?id=1008864





I guess the question I have is do you expect a patch from me? Or
are you guys working on it? I would go with the checking based on
process label.




I am hoping for a patch for this!

Thanks,

Lennart











This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK,
because I
believe this error will happen when a snapshot is created. Also now
pass
in system when doing a system check, if it is doing a service
check and
does not pass in a unit file we will default the target to the
label that
systemd is running with.




Hi,







Maybe I am missing something but isn't this about transient units in
general, i.e. what about StartTransient()? What is going to change in
this case after applying this patch? tclass will be system since in
SELINUX_ACCESS_CHECK you now pass system as path and you will set
tclass in else branch to system which is afaik same as before.







In the current code, passing a unit_file of NULL (StartTransients
has a NULL
UnitFile) will indicate that it should do a system check. My patch is
intended to change this so a NULL UnitFile will indicate that
systemd should
check the access between the calling process label and the current
process
label for a service access. Where as the SELINUX_ACCESS_CHECK will
now pass
a system flag to the function to make it do a system check.




Hi Dan,

Agreed, I get the idea, but this means that
SELINUX_SNAPSHOT_ACCESS_CHECK should
be performed in StartTransient branch in dbus-manager.c too and macro
should be
probably renamed to something like SELINUX_RUNTIME_UNIT_ACCESS_CHECK.

Hope that makes sense.

Michal



Hi,

I tried to improve Dan's patch, so I added an empty call if selinux is
not supported, renamed the function so it doesn't imply it's only for
snapshots and used it in StartTransient and RemoveSnapshot as well. I
wanted to test it, but I am not sure if the policy is already updated.

Vaclav












On the side note, you forgot to define
SELINUX_SNAPSHOT_ACCESS_CHECK as
do {} while (false) in case if we don't HAVE_SELINUX.







It might be the case that I completely misunderstood what's this
about,
in such case ignore this email.







Michal








Thanks added SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in
case if
we don't HAVE_SELINUX.

Updated patch.

snip

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-







From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit
creation.

SELinux does not have a path to check for a snapshot or transient
unit files service creation. Currently systemd does a bogus check.

If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.

Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
src/core/dbus-manager.c | 2 +-
src/core/selinux-access.c | 9 +
src/core/selinux-access.h | 13 +
3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-18 Thread Daniel J Walsh 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/16/2013 08:10 AM, Lennart Poettering wrote:
 On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
 
 
 -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
 
 On 11/14/2013 12:50 PM, Harald Hoyer wrote:
 On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
 On 11/05/2013 12:22 PM, Lennart Poettering wrote:
 
 Ok lets add a check that checks for start on a service labeled with
 the remote process label, then we can add rules like
 
 allow systemd_logind_t self:service start
 
 Or we can make it simpler and have the local end check against the
 init_t process.
 
 allow systemd_logind_t init_t:service start;
 
 Which is probably a better solution, if we have no way of
 differentiating the services.
 
 Machineid usually runs as init_t now.
 
 systemd-run runs as the label of the process that executes it,
 Usually unconfined_t, and sysadm_t.
 
 
 has any solution been found for this?
 
 seems like one is needed for 
 https://bugzilla.redhat.com/show_bug.cgi?id=1008864
 
 
 I guess the question I have is do you expect a patch from me?  Or are you
 guys working on it?  I would go with the checking based on process
 label.
 
 I am hoping for a patch for this!
 
 Thanks,
 
 Lennart
 

This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I
believe this error will happen when a snapshot is created.  Also now pass in
system
when doing a system check, if it is doing a service check and does not pass in
a unit file we will default the target to the label that systemd is running 
with.

How does this patch look?

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlKKhFgACgkQrlYvE4MpobNd1ACbBrwtl5T/CEhCttI9QZ3IZbes
k8UAoODr9J6D0CoyZfpdpvILU7QpxOD2
=0zYK
-END PGP SIGNATURE-
From d8e5784f64a68580f136b99cd5e3fd173586312f Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot creation.

SELinux does not have a path to check for a snapshot servic creation.
This ends up giving us a bogus check.

On snapshot creation we should check if the remote process type, has the ability to start a service with the type that systemd is running with.
---
 src/core/dbus-manager.c   |  2 +-
 src/core/selinux-access.c |  9 +
 src/core/selinux-access.h | 12 
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
 dbus_bool_t cleanup;
 Snapshot *s;
 
-SELINUX_ACCESS_CHECK(connection, message, start);
+SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, start);
 
 if (!dbus_message_get_args(
 message,
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index c7e951c..af34b9e 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -374,8 +374,9 @@ int selinux_access_check(
 goto finish;
 }
 
-if (path) {
-tclass = service;
+
+tclass = service;
+if (path  strneq(path,system, strlen(system))) {
 /* get the file context of the unit file */
 r = getfilecon(path, fcon);
 if (r  0) {
@@ -384,9 +385,9 @@ int selinux_access_check(
 log_error(Failed to get security context on %s: %m,path);
 goto finish;
 }
-
 } else {
-tclass = system;
+if (path)
+tclass = system;
 r = getcon(fcon);
 if (r  0) {
 dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, Failed to get current context.);
diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
index 2d7ac64..53bb9b0 100644
--- a/src/core/selinux-access.h
+++ b/src/core/selinux-access.h
@@ -36,6 +36,18 @@ int selinux_access_check(DBusConnection *connection, DBusMessage *message, const
 DBusConnection *_c = (connection);  \
 DBusMessage *_m = (message);\
 dbus_error_init(_error);   \
+_r = selinux_access_check(_c, _m, system, (permission), _error); \
+if (_r  0) \
+return bus_send_error_reply(_c, _m, _error, _r); \
+} while (false)
+
+#define SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, permission) \
+do {\
+DBusError _error;

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-18 Thread Michal Sekletar 
On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 11/16/2013 08:10 AM, Lennart Poettering wrote:
  On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
  
  
  -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
  
  On 11/14/2013 12:50 PM, Harald Hoyer wrote:
  On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
  On 11/05/2013 12:22 PM, Lennart Poettering wrote:
  
  Ok lets add a check that checks for start on a service labeled with
  the remote process label, then we can add rules like
  
  allow systemd_logind_t self:service start
  
  Or we can make it simpler and have the local end check against the
  init_t process.
  
  allow systemd_logind_t init_t:service start;
  
  Which is probably a better solution, if we have no way of
  differentiating the services.
  
  Machineid usually runs as init_t now.
  
  systemd-run runs as the label of the process that executes it,
  Usually unconfined_t, and sysadm_t.
  
  
  has any solution been found for this?
  
  seems like one is needed for 
  https://bugzilla.redhat.com/show_bug.cgi?id=1008864
  
  
  I guess the question I have is do you expect a patch from me?  Or are you
  guys working on it?  I would go with the checking based on process
  label.
  
  I am hoping for a patch for this!
  
  Thanks,
  
  Lennart
  
 
 This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I
 believe this error will happen when a snapshot is created.  Also now pass in
 system
 when doing a system check, if it is doing a service check and does not pass in
 a unit file we will default the target to the label that systemd is running
 with.

Hi,

Maybe I am missing something but isn't this about transient units in general,
i.e. what about StartTransient()? What is going to change in this case after 
applying
this patch? tclass will be system since in SELINUX_ACCESS_CHECK you now pass
system as path and you will set tclass in else branch to system which is
afaik same as before.

On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as do {}
while (false) in case if we don't HAVE_SELINUX.

It might be the case that I completely misunderstood what's this about, in such
case ignore this email.

Michal

 
 How does this patch look?
 
 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.15 (GNU/Linux)
 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
 
 iEYEARECAAYFAlKKhFgACgkQrlYvE4MpobNd1ACbBrwtl5T/CEhCttI9QZ3IZbes
 k8UAoODr9J6D0CoyZfpdpvILU7QpxOD2
 =0zYK
 -END PGP SIGNATURE-

 From d8e5784f64a68580f136b99cd5e3fd173586312f Mon Sep 17 00:00:00 2001
 From: Dan Walsh dwa...@redhat.com
 Date: Mon, 18 Nov 2013 15:52:37 -0500
 Subject: [PATCH] Fix SELinux check for snapshot creation.
 
 SELinux does not have a path to check for a snapshot servic creation.
 This ends up giving us a bogus check.
 
 On snapshot creation we should check if the remote process type, has the 
 ability to start a service with the type that systemd is running with.
 ---
  src/core/dbus-manager.c   |  2 +-
  src/core/selinux-access.c |  9 +
  src/core/selinux-access.h | 12 
  3 files changed, 18 insertions(+), 5 deletions(-)
 
 diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
 index 747bcfc..a60a568 100644
 --- a/src/core/dbus-manager.c
 +++ b/src/core/dbus-manager.c
 @@ -1112,7 +1112,7 @@ static DBusHandlerResult 
 bus_manager_message_handler(DBusConnection *connection,
  dbus_bool_t cleanup;
  Snapshot *s;
  
 -SELINUX_ACCESS_CHECK(connection, message, start);
 +SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, start);
  
  if (!dbus_message_get_args(
  message,
 diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
 index c7e951c..af34b9e 100644
 --- a/src/core/selinux-access.c
 +++ b/src/core/selinux-access.c
 @@ -374,8 +374,9 @@ int selinux_access_check(
  goto finish;
  }
  
 -if (path) {
 -tclass = service;
 +
 +tclass = service;
 +if (path  strneq(path,system, strlen(system))) {
  /* get the file context of the unit file */
  r = getfilecon(path, fcon);
  if (r  0) {
 @@ -384,9 +385,9 @@ int selinux_access_check(
  log_error(Failed to get security context on %s: 
 %m,path);
  goto finish;
  }
 -
  } else {
 -tclass = system;
 +if (path)
 +tclass = system;
  r = getcon(fcon);
  if (r  0) {
  dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, 
 Failed to get current context.);
 diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
 index 2d7ac64..53bb9b0 100644
 --- a/src/core/selinux-access.h
 +++

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-16 Thread Lennart Poettering 
On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:

 
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 11/14/2013 12:50 PM, Harald Hoyer wrote:
  On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
  On 11/05/2013 12:22 PM, Lennart Poettering wrote:
  
  Ok lets add a check that checks for start on a service labeled with the
  remote process label, then we can add rules like
  
  allow systemd_logind_t self:service start
  
  Or we can make it simpler and have the local end check against the init_t
  process.
  
  allow systemd_logind_t init_t:service start;
  
  Which is probably a better solution, if we have no way of differentiating
  the services.
  
  Machineid usually runs as init_t now.
  
  systemd-run runs as the label of the process that executes it,  Usually 
  unconfined_t, and sysadm_t.
  
  
  has any solution been found for this?
  
  seems like one is needed for
  https://bugzilla.redhat.com/show_bug.cgi?id=1008864
  
 
 I guess the question I have is do you expect a patch from me?  Or are you guys
 working on it?  I would go with the checking based on process label.

I am hoping for a patch for this!

Thanks,

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-14 Thread Harald Hoyer 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
 On 11/05/2013 12:22 PM, Lennart Poettering wrote:
 
 Ok lets add a check that checks for start on a service labeled with the remote
 process label, then we can add rules like
 
 allow systemd_logind_t self:service start
 
 Or we can make it simpler and have the local end check against the init_t 
 process.
 
 allow systemd_logind_t init_t:service start;
 
 Which is probably a better solution, if we have no way of differentiating the
 services.
 
 Machineid usually runs as init_t now.
 
 systemd-run runs as the label of the process that executes it,  Usually
 unconfined_t, and sysadm_t.
 

has any solution been found for this?

seems like one is needed for https://bugzilla.redhat.com/show_bug.cgi?id=1008864
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJShQ19AAoJEANOs3ABTfJwqLQQAMCwlsVui5GxrAIgm1pnRYy0
1vBZRJombQ93cYr5RbXcQzo9raGwD9C/TOA6PXJNyIKoxCh5unCMGdUB1pwR6y57
o1Uql1V1wVXPKqlsDsRdiKi14Qdz6e2CBqNQ1Gn1wx1JPvKPVy52iIMWjnFUCTzM
FzKoX+CJlbR77tOgn24WxhP+Zll4QFqBAAwgptKBCyZf8lyHgsgTSgS7KDKAr/Jr
v9BumGS9210fEHSQBJdkoaoYnMZOHPpaDxSDjZ76AqBw0MOksQsiamCRr20gbWnQ
a8wvguQzTXhjKLeM0rX9x5hwrCI2Q4YL+VMsr5I1GPfR5GBIldmPhe8V4SYrJQY4
zDa+pHc1ubsuv/b4c9mHS/4Wl6IY8Nz6AVIAjvM0wR6cKJ+Ip09bEEeyIFWk7oRa
RxNnFLwnUW4yweGCq4HlVv8r+SLpXIFkW9HkG1tr1UswB/jEC13wXW8WLfemGhuk
XjMus/oMdQkd79wPvlfKF9JT4xTtx0u613kDEc/A6uEEoR4dwIeuLFf1Lss+zydg
okLbsc8pLEFzXj1JKMut3DMEuhZss+WhLslGFEPKPpsAbHMEf42gJjuvMFb9aRTW
V1qNW9adpMbqYC4MEzEV9SD4rwfDk1RQPW8iNEfm4XINcF1X2HK7+DGvCgVVeK8d
tSM5P0ZhmXwimbrU63EH
=IedG
-END PGP SIGNATURE-
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-14 Thread Daniel J Walsh 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/14/2013 12:50 PM, Harald Hoyer wrote:
 On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
 On 11/05/2013 12:22 PM, Lennart Poettering wrote:
 
 Ok lets add a check that checks for start on a service labeled with the
 remote process label, then we can add rules like
 
 allow systemd_logind_t self:service start
 
 Or we can make it simpler and have the local end check against the init_t
 process.
 
 allow systemd_logind_t init_t:service start;
 
 Which is probably a better solution, if we have no way of differentiating
 the services.
 
 Machineid usually runs as init_t now.
 
 systemd-run runs as the label of the process that executes it,  Usually 
 unconfined_t, and sysadm_t.
 
 
 has any solution been found for this?
 
 seems like one is needed for
 https://bugzilla.redhat.com/show_bug.cgi?id=1008864
 

I guess the question I have is do you expect a patch from me?  Or are you guys
working on it?  I would go with the checking based on process label.

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlKFNdUACgkQrlYvE4MpobNuXACg1eKUvMGKMv5zuwKHDvj44K+F
L6gAn3sQtD0QvGUUmJWRGRSolZTdOqN0
=pYrx
-END PGP SIGNATURE-
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-04 Thread Lennart Poettering 
On Thu, 31.10.13 15:51, Vaclav Pavlin (vpav...@redhat.com) wrote:

 From: Václav Pavlín vpav...@redhat.com

Sorry, I don't understand what this patch is doing. Please explain in a
commit message!

 
 ---
  src/core/selinux-access.c | 59 
 ++-
  src/core/selinux-access.h | 15 +---
  2 files changed, 70 insertions(+), 4 deletions(-)
 
 diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
 index 0a3ee18..5908a79 100644
 --- a/src/core/selinux-access.c
 +++ b/src/core/selinux-access.c
 @@ -333,6 +333,50 @@ static int get_calling_context(
  return 0;
  }
  
 +static int load_context(
 +const char *path,
 +const char *name,
 +char **context) {
 +
 +int r = 0;
 +char l[LINE_MAX];
 +char *p;
 +FILE *fd;
 +
 +fd = fopen(path, r);

Please always use re rather than r. Not that it would matter much
here, but Please always do.

 +if (!fd)
 +return -EEXIST;
 +
 +if (!fgets(l, sizeof(l), fd)) {

Hmm, isn't this a job for read_one_line_file()?

 +if (feof(fd)) {
 +r = -EINVAL;
 +goto finish;
 +}
 +
 +log_error(Failed to read context file '%s': %m, path);
 +r = -errno;
 +goto finish;
 +}
 +
 +p = strtok (l, =);

strtok() is evil, as it keeps internal state, please don't use it. Also
one space too much...

 +if (p  streq(p, name)) {
 +p = strtok (NULL, =);
 +if (!p) {
 +r = -EINVAL;
 +goto finish;
 +}
 +
 +*context = strdup(p);

Missing OOM check.

 +} else
 +r = -EINVAL;
 +
 +finish:
 +if (fd)
 +fclose(fd);

Please use _cleanup_close_ for cases like this.

 +} else if (class == CLASS_TRANSIENT) {
 +const char *context_path =
 selinux_systemd_contexts_path();

Where is selinux_systemd_contexts_path() define? I cant find it in this
patch and neither in the sources? What am I missing?

 +if (_u-source_path || _u-fragment_path)  \
 +_r = selinux_access_check(_c, _m, CLASS_SERVICE, 
 _u-source_path ?: _u-fragment_path, (permission), _error); \
 +else\
 +_r = selinux_access_check(_c, _m, CLASS_TRANSIENT, 
 _u-id, (permission), _error); \

You can recognize transient units via the transient bool on the Unit structure.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-04 Thread Kay Sievers 
On Mon, Nov 4, 2013 at 5:06 PM, Lennart Poettering
lenn...@poettering.net wrote:

 Sorry, I don't understand what this patch is doing. Please explain in a
 commit message!

The file format should also be documented in the code itself, if not
done by selinx, then we need to add the link to the doc.

 ---
  src/core/selinux-access.c | 59 
 ++-
  src/core/selinux-access.h | 15 +---
  2 files changed, 70 insertions(+), 4 deletions(-)

 diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
 index 0a3ee18..5908a79 100644
 --- a/src/core/selinux-access.c
 +++ b/src/core/selinux-access.c
 @@ -333,6 +333,50 @@ static int get_calling_context(
  return 0;
  }

 +static int load_context(
 +const char *path,
 +const char *name,
 +char **context) {
 +
 +int r = 0;
 +char l[LINE_MAX];
 +char *p;
 +FILE *fd;
 +
 +fd = fopen(path, r);

 Please always use re rather than r. Not that it would matter much
 here, but Please always do.

 +if (!fd)
 +return -EEXIST;
 +
 +if (!fgets(l, sizeof(l), fd)) {

 Hmm, isn't this a job for read_one_line_file()?

 +if (feof(fd)) {
 +r = -EINVAL;
 +goto finish;
 +}
 +
 +log_error(Failed to read context file '%s': %m, path);
 +r = -errno;
 +goto finish;
 +}
 +
 +p = strtok (l, =);

 strtok() is evil, as it keeps internal state, please don't use it. Also
 one space too much...

 +if (p  streq(p, name)) {
 +p = strtok (NULL, =);
 +if (!p) {
 +r = -EINVAL;
 +goto finish;
 +}
 +
 +*context = strdup(p);

 Missing OOM check.

 +} else
 +r = -EINVAL;
 +
 +finish:
 +if (fd)
 +fclose(fd);

 Please use _cleanup_close_ for cases like this.

It should probably just use:
  parse_env_file()
it does all that.

 +} else if (class == CLASS_TRANSIENT) {
 +const char *context_path =
 selinux_systemd_contexts_path();

 Where is selinux_systemd_contexts_path() define? I cant find it in this
 patch and neither in the sources? What am I missing?

http://people.fedoraproject.org/~dwalsh/SELinux/Patches/0008-Add-selinux_systemd_contexts_path.patch

 +if (_u-source_path || _u-fragment_path)  \
 +_r = selinux_access_check(_c, _m, CLASS_SERVICE, 
 _u-source_path ?: _u-fragment_path, (permission), _error); \
 +else\
 +_r = selinux_access_check(_c, _m, CLASS_TRANSIENT, 
 _u-id, (permission), _error); \

 You can recognize transient units via the transient bool on the Unit 
 structure.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-04 Thread Lennart Poettering 
On Mon, 04.11.13 17:06, Lennart Poettering (lenn...@poettering.net) wrote:

 On Thu, 31.10.13 15:51, Vaclav Pavlin (vpav...@redhat.com) wrote:
 
  From: Václav Pavlín vpav...@redhat.com
 
 Sorry, I don't understand what this patch is doing. Please explain in a
 commit message!

Hmm, so, here's another idea. The transient units are created by a
client process. We could easily determine the label of that client
process. Wouldn't it a better approach to calculate the label of the
transient units somehow from the client process' label? This way
wouldn't need any additional systemd-specific infrastructure in
libselinux.

Dan, could that work?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-11-04 Thread Daniel J Walsh 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/04/2013 02:05 PM, Lennart Poettering wrote:
 On Mon, 04.11.13 17:06, Lennart Poettering (lenn...@poettering.net) wrote:
 
 On Thu, 31.10.13 15:51, Vaclav Pavlin (vpav...@redhat.com) wrote:
 
 From: Václav Pavlín vpav...@redhat.com
 
 Sorry, I don't understand what this patch is doing. Please explain in a 
 commit message!
 
 Hmm, so, here's another idea. The transient units are created by a client
 process. We could easily determine the label of that client process.
 Wouldn't it a better approach to calculate the label of the transient units
 somehow from the client process' label? This way wouldn't need any
 additional systemd-specific infrastructure in libselinux.
 
 Dan, could that work?
 
 Lennart
 
I suppose it would.  The only label we have the the clients is the process 
label.

What process types create these runtime objects and what do they request to do
with them?

Currently systemd asks for permissions on system class and service class, where
class system
{
ipc_info
syslog_read
syslog_mod
syslog_console
module_request
halt
reboot
status
undefined
enable
disable
reload
}

class service
{
start
stop
status
reload
kill
load
enable
disable
}

Do we have to add a rule like

allow sysadm_t networkmanager_t:service start;

Were networkmanager_t is a process type?
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlJ3/gsACgkQrlYvE4MpobPWbQCfWElx/pR6cOjQKM1Ad0cE/eU1
cAcAoJ1k49KbB143/NJH/DEfl0aRLhnn
=eao5
-END PGP SIGNATURE-
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] selinux: fix selinux check for transient units

2013-10-31 Thread Vaclav Pavlin 
From: Václav Pavlín vpav...@redhat.com

---
 src/core/selinux-access.c | 59 ++-
 src/core/selinux-access.h | 15 +---
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index 0a3ee18..5908a79 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -333,6 +333,50 @@ static int get_calling_context(
 return 0;
 }
 
+static int load_context(
+const char *path,
+const char *name,
+char **context) {
+
+int r = 0;
+char l[LINE_MAX];
+char *p;
+FILE *fd;
+
+fd = fopen(path, r);
+if (!fd)
+return -EEXIST;
+
+if (!fgets(l, sizeof(l), fd)) {
+if (feof(fd)) {
+r = -EINVAL;
+goto finish;
+}
+
+log_error(Failed to read context file '%s': %m, path);
+r = -errno;
+goto finish;
+}
+
+p = strtok (l, =);
+if (p  streq(p, name)) {
+p = strtok (NULL, =);
+if (!p) {
+r = -EINVAL;
+goto finish;
+}
+
+*context = strdup(p);
+} else
+r = -EINVAL;
+
+finish:
+if (fd)
+fclose(fd);
+
+return r;
+}
+
 /*
This function communicates with the kernel to check whether or not it should
allow the access.
@@ -342,6 +386,7 @@ static int get_calling_context(
 int selinux_access_check(
 DBusConnection *connection,
 DBusMessage *message,
+int class,
 const char *path,
 const char *permission,
 DBusError *error) {
@@ -374,7 +419,7 @@ int selinux_access_check(
 goto finish;
 }
 
-if (path) {
+if (class == CLASS_SERVICE) {
 tclass = service;
 /* get the file context of the unit file */
 r = getfilecon(path, fcon);
@@ -384,7 +429,19 @@ int selinux_access_check(
 log_error(Failed to get security context on %s: 
%m,path);
 goto finish;
 }
+} else if (class == CLASS_TRANSIENT) {
+const char *context_path = selinux_systemd_contexts_path();
+
+tclass = service;
 
+/* get the context transient unit */
+r = load_context(context_path, runtime, fcon);
+if (r  0) {
+dbus_set_error(error, DBUS_ERROR_ACCESS_DENIED, 
Failed to get context for transient unit);
+r = -errno;
+log_error(Failed to get security context on transient 
unit %s: %m, context_path);
+goto finish;
+}
 } else {
 tclass = system;
 r = getcon(fcon);
diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
index 2d7ac64..79eea6a 100644
--- a/src/core/selinux-access.h
+++ b/src/core/selinux-access.h
@@ -25,7 +25,13 @@
 
 void selinux_access_free(void);
 
-int selinux_access_check(DBusConnection *connection, DBusMessage *message, 
const char *path, const char *permission, DBusError *error);
+int selinux_access_check(DBusConnection *connection, DBusMessage *message, int 
class, const char *path, const char *permission, DBusError *error);
+
+enum {
+CLASS_SYSTEM,
+CLASS_SERVICE,
+CLASS_TRANSIENT
+};
 
 #ifdef HAVE_SELINUX
 
@@ -36,7 +42,7 @@ int selinux_access_check(DBusConnection *connection, 
DBusMessage *message, const
 DBusConnection *_c = (connection);  \
 DBusMessage *_m = (message);\
 dbus_error_init(_error);   \
-_r = selinux_access_check(_c, _m, NULL, (permission), 
_error); \
+_r = selinux_access_check(_c, _m, CLASS_SYSTEM, NULL, 
(permission), _error); \
 if (_r  0) \
 return bus_send_error_reply(_c, _m, _error, _r); \
 } while (false)
@@ -49,7 +55,10 @@ int selinux_access_check(DBusConnection *connection, 
DBusMessage *message, const
 DBusMessage *_m = (message);\
 Unit *_u = (unit);  \
 dbus_error_init(_error);   \
-_r = selinux_access_check(_c, _m, _u-source_path ?: 
_u-fragment_path, (permission), _error); \
+if (_u-source_path || _u-fragment_path)  \
+_r = selinux_access_check(_c, _m, CLASS_SERVICE, 
_u-source_path ?: _u-fragment_path,