From: HATAYAMA Daisuke <d.hatay...@jp.fujitsu.com> Subject: Re: [systemd-devel] [PATCH] selinux: fix missing SELinux unit access check Date: Wed, 10 Jun 2015 12:18:48 +0900 (JST)
> From: Lennart Poettering <lenn...@poettering.net> > Subject: Re: [systemd-devel] [PATCH] selinux: fix missing SELinux unit access > check > Date: Mon, 8 Jun 2015 12:37:14 +0200 > >> On Mon, 08.06.15 19:00, HATAYAMA Daisuke (d.hatay...@jp.fujitsu.com) wrote: >> >>> Currently, SELinux unit access check is not performed if a given unit >>> file has not been registered in a hash table. This is because function >>> manager_get_unit() only tries to pick up a Unit object from a Unit >>> hash table. Instead, we use function manager_load_unit() searching >>> Unit file pathes for the given Unit file. >> >> Were precisely is this relevant? I mean, we generally invoke >> operations on units that are already loaded? >> > > The SELinux unit access check is performed in terms of the following rule: > > ~]# sesearch -A -s init_t | grep "systemd_unit_file_type" | grep service > allow init_t systemd_unit_file_type : service { start stop status reload > kill load enable disable } ; > > where systemd_unit_file_type is the attribute that is assigned to > context types that unit files have, e.g. systemd_unit_file_t. > > Because this SELinux unit access check uses context types that unit > files have, we need to be able to refer to given unit files when we > perform requested unit operations. > > Here's how to reproduce this issue. Note that: > > - the systemd used here is with the fixing patch, and > > - test.service has not been loaded (even tried to get loaded) > throughout this boot to guarantee that test.service is not registered > in the unit hash table. > > Then, > > ~]# cat ./test.service > [Unit] > Description=test > > [Service] > Type=oneshot > ExecStart=/usr/bin/true > RemainAfterExit=yes > > [Install] > WantedBy=multi-user.target > ~]# cat ./foo.sh > #! /bin/sh > > mv test.service /etc/systemd/system > systemctl daemon-reload > systemctl enable test.service > ~]# ./foo.sh > > Then, here is a gdb log: > > break mac_selinux_unit_access_check_strv > Breakpoint 1 at 0x7f5339807a28: file src/core/selinux-access.c, line 288. > (gdb) continue > Continuing. > Detaching after fork from child process 2198. > > Breakpoint 1, mac_selinux_unit_access_check_strv (units=0x7f533a1324f0, > message=0x7f533a166a10, m=0x7f533a07c680, permission=0x7f533992bdb6 "enable", > error=0x7ffe80abaec0) at src/core/selinux-access.c:288 > 288 sd_bus_error *error) { > (gdb) l > 283 > 284 int mac_selinux_unit_access_check_strv(char **units, > 285 sd_bus_message *message, > 286 Manager *m, > 287 const char *permission, > 288 sd_bus_error *error) { > 289 #ifdef HAVE_SELINUX > 290 char **i; > 291 Unit *u; > 292 int r; > (gdb) n > 294 STRV_FOREACH(i, units) { > (gdb) n > 295 r = manager_load_unit(m, *i, NULL, error, &u); > (gdb) s > manager_load_unit (m=0x7f533a07c680, name=0x7f533a11ef30 "test.service", > path=0x0, e=0x7ffe80abaec0, _ret=0x7ffe80abad80) at src/core/manager.c:1369 > 1369 assert(m); > (gdb) n > 1374 r = manager_load_unit_prepare(m, name, path, e, _ret); > (gdb) n > 1375 if (r != 0) > (gdb) n > 1378 manager_dispatch_load_queue(m); > (gdb) p r > $1 = 0 > > Here variable r has a return value of manager_load_unit_prepare() and > its value is 0 here, meaning that now there's no unit file with the > requested unit file name in the unit hash table. > > (gdb) p *_ret > $2 = (Unit *) 0x7f533a166ea0 > (gdb) p *_ret->load_state > Attempt to take contents of a non-pointer value. > (gdb) p (*_ret)->load_state > $3 = UNIT_STUB > > Actually, its load_state is still UNIT_STUB. > > (gdb) n > 1380 if (_ret) > (gdb) p (*_ret)->load_state > $4 = UNIT_LOADED > > After dipatching the load queue by manager_dispatch_load_queue(), the > given unit file has just been loaded and the load_state has been > changed into UNIT_LOADED. > > (gdb) finish > Run till exit from #0 manager_load_unit (m=0x7f533a07c680, > name=0x7f533a11ef30 "test.service", path=0x0, e=0x7ffe80abaec0, > _ret=0x7ffe80abad80) at src/core/manager.c:1380 > 0x00007f5339807a6a in mac_selinux_unit_access_check_strv > (units=0x7f533a1324f0, message=0x7f533a166a10, m=0x7f533a07c680, > permission=0x7f533992bdb6 "enable", error=0x7ffe80abaec0) at > src/core/selinux-access.c:295 > 295 r = manager_load_unit(m, *i, NULL, error, &u); > (gdb) continue > Also, I used selinux-context branch of https://github.com/keszybz/systemd.git in this test to avoid the issue in https://bugzilla.redhat.com/show_bug.cgi?id=1224211. -- Thanks. HATAYAMA, Daisuke _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel