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

[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 (dwalsh at 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-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-SELinux-check-for-snapshot-and-transitent-unit-c.patch
Type: text/x-patch
Size: 4312 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131119/a4f0d6eb/attachment.bin>