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 >> Signed-off-by: HATAYAMA Daisuke <d.hatay...@jp.fujitsu.com> > > We don't use S-o-b in systemd. > Sorry, I'll not use the tag next. I decided to use it becuase I found commits with the tag in git log (of course, I found the commits without the tag, too). -- Thanks. HATAYAMA, Daisuke _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel