On Fri, 16 Jun 2023 at 07:43, Petr Lautrbach <plautrba@xxxxxxxxxx> wrote: > > Paul Moore <paul@xxxxxxxxxxxxxx> writes: > > > Hello all, > > > > Amongst Christian's various other SELinux contributions, over the past > > several years Christian has been working on improving the SELinux > > integration in systemd. One of the things that Christian has been > > working on is revamping the SELinux permissions that systemd uses for > > unitfile operations, both to resolve problems and generally improve > > the mapping of permissions to systemd operations. As this work has > > been languishing for several years, I would like to see if we can get > > things "unstuck" by proposing two things: > > > > 1. I've provided links to the systemd GH PRs below, but I think it > > might be helpful if Christian could provide a quick summary of the new > > permissions, how they map to systemd operations, and how they map to > > the existing SELinux/systemd permissions with a focus on helping > > policy developers migrate existing SELinux policies. > > > > 2. Given the significance of systemd to modern Linux distributions, I > > think it might be a good idea if we selected a SELinux "liaison" for > > the systemd project. This person, or group of people, would work with > > the systemd folks to keep the SELinux integration in good working > > order, review systemd code as necessary, and help represent the > > SELinux project within systemd. > > > > How does that sound to everyone? If we are in agreement on #2, and > > assuming he would be willing to help out, I would like to nominate > > Christian as our SELinux liaison to systemd; any objections? Anyone > > else interested in helping out? > > I agree with the Christian's nomination. > > As for #1, I looked on both, but I have to admit that I had a lack of > understanding of the problem and so I would need some time to get > into it. Therefore I postponed it due to other priorities, (but never > come back). If it's still open I'll focus on it next week. > > > > For reference, Christian's systemd PRs on GH: > > * https://github.com/systemd/systemd/pull/10023 > > * https://github.com/systemd/systemd/pull/20387 > > > > Thanks, > > Petr > Hi all, Thanks Paul for attracting some attention to the SELinux integration of systemd. It has been several months since I last worked on the systemd related patches, so they all need a rebase and another round of testing. Addressing the first point, here are some details about the changes I would like to see in systemd. There are some systemd specific ones and some which also affect other object managers, e.g. D-Bus. I. Re-add SELinux checks for unit install operations systemd v190 introduced SELinux checks for unit install operations, like enabling a unit. This is useful to limit the access of daemons, which are running as root and having access to systemd via the D-Bus connection. The related security class is `service { start stop status reload enable disable }` used by both the Reference Policy and the Fedora one. Due to an internal refactoring the checks for unit install operations were dropped in systemd v225[1], so starting or stopping a unit is still subject to a SELinux check but en-/disabling or un-/masking are not. Reintroducing those checks was the initial intention for my patches. The most recent pull request targeting this issue is #20387[2] (it contains some additional changes I am going to talk about in the next section). Migration and compatibility with regard to SELinux policies depend on the individual policy. When they do not define the security class `service` or its permissions `enable` and `disable` the policy setting deny_unknown will determine whether the proposed checks are granted or denied. Both the Reference Policy and the Fedora one however do define those two permissions. For generic access they use the permission macros `manage_service_perms` and `all_system_perms ` (which include `enable` and `disable`), so the sysadm and unconfined domains should continue to work fine. Individual module interfaces, especially in the reference policy, might need some tweaks in the case a caller is actually en-/disabling a unit, as they commonly use the lone permissions `start` and/or `stop` directly. II. Introduction of two new unit permissions Even with the re-introduction of the unit install operations several other unit operations are still not subject to any SELinux check: * applying a preset to a unit (`systemctl preset <unit>`) * reverting a unit (`systemctl revert <unit>`) * modifying dependencies of a unit (`systemctl add-wants/add-requires <unit>`) All of them might influence the behavior of a unit and therefore should from a SELinux point of view not be granted by default to all root user D-Bus clients of systemd. To address them the last commit of #20387[2] introduces two new permissions to the security class `service` and `system`: `preset` and `modify`. The same commit also changes the permission for modifying systemd internal settings (via the D-Bus interface org.freedesktop.DBus.Properties) from currently `reload` to the newly added permission `modify`. This allows a more fine-grained access. During my last test run setting the log level or target was an example for such a setting modification (I'll retest after the rebase). Regarding compatibility any policy with a policy setting of `deny_unknown allow` will not break, such as the Reference Policy and the Fedora Policy. They should however consider adding and using the added permissions (mainly by adding both to the permission macro `manage_service_perms`), otherwise the previously checked setting of systemd properties will be allowed by default (for root user D-Bus clients of systemd). Confined usage of the new permissions might hopefully be quite uncommon affecting only very few domains. III. Add SELinux checks for systemd-logind D-Bus interfaces Over the years logind gained a lot of functionality[3]. Also from my experience various services talk to logind over D-Bus. So it might to be a good candidate to add some checks around the more invasive D-Bus interfaces of logind, especially everything related to reboot handling and session management (used by pam_systemd(8) clients like sshd or local_login), again to limit the default access of root user D-Bus clients. There is no actual proposal patch yet for this, only an ancient work-in-progress branch[4]. Compatibility should be trivial for policies with `deny_unknown` set to `allow` (like Reference Policy and Fedora Policy) since all potential checks will be against an entire new security class. Otherwise mainly the sysadm and unconfined domains as well as pam_systemd(8) clients need adjusting. IV. Improve audit fields on SELinux denials This topic is not specific to systemd but affects all SELinux object managers which perform access control on third party subjects, but systemd is probably the most affected one (together with D-Bus (reference implementation and dbus-broker). Today's audit messages from SELinux denials in object manager look like the following: # dbus-broker type=USER_AVC msg=audit(22/09/22 16:58:47.569:313) : pid=789 uid=dbus auid=unset ses=unset subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc: granted { send_msg } for scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 tclass=dbus exe=/usr/bin/dbus-broker sauid=dbus hostname=? addr=? terminal=?' # systemd type=USER_AVC msg=audit(22/09/22 15:35:47.847:132) : pid=1 uid=root auid=unset ses=unset subj=system_u:system_r:init_t:s0 msg='avc: granted { reload } for auid=unset uid=root gid=root cmdline="" function="method_reload" scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=system_u:system_r:init_t:s0 tclass=system exe=/usr/lib/systemd/systemd sauid=root hostname=? addr=? terminal=?' They contain the subject and target context relevant for policy rules, but are missing some information to improve understanding the cause of the denial. For example the PID, command or executable name of the actual subject is not included. Also in the case of systemd the embedded message contains a duplicated `auid` field and reuses the field names `uid` and `gid`. The kernel part of the USER_AVC messages provide the following: - pid=789 : the PID of the message issuer (not of the process subject to the failed SELinux check) - uid=dbus : UID of the message issuer - auid=unset : AUID of the message issuer - ses=unset : session ID of the message issuer - subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 : security context of the message issuer - msg='...' : the message of the issuer Libselinux (src/avc.c:avc_audit()) provides a message formatting of avc: granted { send_msg } for <<<callback-generated-content>>> scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 tclass=dbus and libaudit (lib/audit_logging.c:audit_log_user_avc_message()) appends the following into the msg field: exe=/usr/bin/dbus-broker sauid=dbus hostname=? addr=? terminal=? So I would like to introduce a recommended way for SELinux object managers to format the embedded message, supplied via the selinux_set_callback(3) option SELINUX_CB_AUDIT. They are recommended to supply the uid, gid, pid, exe and cmdline of the actual subject. Also they should supply the auid of the actual subject as the last argument of audit_log_user_avc_message(3) (and not their own), since the auid of the issuer is already supplied by the kernel. To distinguish the subject related fields they should be unique, either via a common prefix of just "s" or "subj_", inspired by the current list of audit event fields[5]. I tried to start a discussion about that in the audit issue tracker[6] and proposed two pull requests against systemd[7,8]. A more standardized and verbose USER_AVC output would benefit SELinux reporting tools like setroubleshoot[9]. Some miscellaneous notes at the end: The initial pull request of the re-addition of SELinux checks for unit install operations[10] renamed the security class for systemd manager access control (currently "system"). This was to split the userspace portion of this class from the original kernel portion. It was dropped to simplify the transition and minimize the code changes. I am not familiar with all features of systemd and might not be aware of some classes of issues. For example there exists a pull request from Daniel Walsh regarding labels of nspawn containers[11]. Regards, Christian [1]: https://github.com/systemd/systemd/pull/1044 [2]: https://github.com/systemd/systemd/pull/20387 [3]: https://www.freedesktop.org/wiki/Software/systemd/logind/ [4]: https://github.com/systemd/systemd/pull/15245 [5]: https://access.redhat.com/articles/4409591#audit-event-fields-1 [6]: https://github.com/linux-audit/audit-userspace/issues/283 [7]: https://github.com/systemd/systemd/pull/25264 [8]: https://github.com/systemd/systemd/pull/25322 [9]: https://gitlab.com/setroubleshoot/setroubleshoot/-/merge_requests/19 [10]: https://github.com/systemd/systemd/pull/10023 [11]: https://github.com/systemd/systemd/pull/26141
