Ú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");
+ 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..296bada 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; \
+ int _r; \
+ DBusConnection *_c = (connection); \
+ DBusMessage *_m = (message); \
+ dbus_error_init(&_error); \
_r = selinux_access_check(_c, _m, NULL, (permission), &_error); \
if (_r < 0) \
return bus_send_error_reply(_c, _m, &_error, _r); \
@@ -57,6 +69,7 @@ int selinux_access_check(DBusConnection
*connection, DBusMessage *message, const
#else
#define SELINUX_ACCESS_CHECK(connection, message, permission) do { }
while (false)
+#define SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message,
permission) do { } while (false)
#define SELINUX_UNIT_ACCESS_CHECK(unit, connection, message,
permission) do { } while (false)
#endif
--
1.8.4.2
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>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; \
+ int _r; \
+ DBusConnection *_c = (connection); \
+ DBusMessage *_m = (message); \
+ dbus_error_init(&_error); \
_r = selinux_access_check(_c, _m, NULL, (permission), &_error); \
if (_r < 0) \
return bus_send_error_reply(_c, _m, &_error, _r); \
--
1.8.4.2
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
