-----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) {
@@ -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
