RedHatInsights / yggdrasil

GNU General Public License v3.0
21 stars 37 forks source link

yggdrasil.service should use non-root user #152

Closed jirihnidek closed 3 months ago

jirihnidek commented 1 year ago

Describe the bug The yggdrasil.service is run using root user ATM. This fact can bring unnecessary security risks. Some non-root service user (e.g. yggd) should be created and used for running this service.

To Reproduce Steps to reproduce the behavior:

  1. Start yggdrasil.service using: systemctl start yggdrasil.service
  2. Check that root user is used for running the yggd process using e.g. ps aux | grep yggd

Current behavior root 3923 0.0 0.6 1312120 13976 ? Ssl 10:56 0:00 /usr/bin/yggd

Expected behavior yggd 3923 0.0 0.6 1312120 13976 ? Ssl 10:56 0:00 /usr/bin/yggd

Additional Information

jirihnidek commented 1 year ago

BTW: I like the approach that nginx uses. They do not set User and Group in service file, but administrator of system can configure user in configuration file /etc/nginx/nginx.conf. The nginx process is started as root user, but later they call setuid() & setgid() system calls. This approach gives you strength to set ownership and permissions to all files that are necessary for running yggd process as non-root user and later call setuid() & setgid() to non-root UID.

subpop commented 1 year ago

Yes, I've seen approaches like this. There is also seteuid that can be used to set an effective UID of a process. This technique could be used to start the process as root, drop root privileges, and only escalate privileged access to root when needed.

subpop commented 1 year ago

Have you read up on systemd's sysusers? Could this be of any help? https://www.freedesktop.org/software/systemd/man/systemd-sysusers.html

jirihnidek commented 1 year ago

Yes, I've seen approaches like this. There is also seteuid that can be used to set an effective UID of a process. This technique could be used to start the process as root, drop root privileges, and only escalate privileged access to root when needed.

I think that we will need only setuid(), but it is good to know about seteuid() for the case we will need it.

jirihnidek commented 1 year ago

Have you read up on systemd's sysusers? Could this be of any help? https://www.freedesktop.org/software/systemd/man/systemd-sysusers.html

Yeah, we will need to create some service user. It is better to use something like systemd-sysusers than use some own script and useradd. I tkink that we will need to create such user & greoup during meson install.

subpop commented 6 months ago

There are two PRs that propose implementations addressing this feature (#157 and #218). Both link back to this original issue. Both explore different possible solutions, but I don't feel like either landed on the ideal yet. We need to continue discussing the design before we will arrive at the right solution, so I'd like to continue that discussion here in the issue (rather than across two different PRs).

That being said, I agree that yggd should be fully capable of running as non-root, but connecting to the system D-Bus instance. However, I don't think that the user yggd runs as needs to be coded into the yggd binary. A distribution should be free to choose whatever user they want to run yggd as. This means a few statements can be asserted to be true, which will help us find the right solution:

  1. The username should not be coded into the yggd source files.
  2. The username should not be configurable as a configuration file option.
  3. The program should support running as both root (uid 0) and non-root (uid > 0) users.

I think (3) is already mostly true, since yggd can run as a "non-root, non-system" user and operate over the session bus instance. This means that we need to focus on (1) and (2). I think we can do this by implementing a solution for distributions to configure the desired user at build time; the distribution is then responsible for creating the user during program installation.

jirihnidek commented 6 months ago

It will be necessary to specify some non-root user in the file /usr/share/dbus-1/system.d/yggd.conf defining D-Bus policy (policy file could be extended to support root and non-root user). Some default non-root user will have to be coded in this configuration file. This file has to be installed to the system. Otherwise yggdrasil.service will not work when it runs as non-root user. For this reason it will be necessary to code this non-root user to source code or configuration file. Thus, IMHO we will have to break your request (1) or (2).

Another issue is with file /var/lib/yggdrasil/client-id. The problem is described here: https://github.com/RedHatInsights/yggdrasil/pull/218#pullrequestreview-1957274723 Problem with system user is that environment variable $XDG_STATE_HOME is empty and yggdrasil tries to write some directory that does not exists (something like /.local/state).

subpop commented 6 months ago

You are correct that those are problems, however, they're not insurmountable. I know we have two open PRs addressing this issue already, but I found the best way to express my approach was to create a PR with my own proposal. See #224. This approach addresses the requirement that the D-Bus policy file has to encode the username by using meson to configure the file at compile time. It introduces a meson build option that is used to configure this file. The client-id file being created in non-existent paths like /.local/state was a bug that I fixed in a commit included in #224.

My experimentation with the implementation in #224 is positive so far. I can run both the system service and the user service simultaneously. The system service runs as a systemd-sysuser-created user and gets directories configured under the appropriate locations via the RuntimeDirectory= family of unit exec directives. The same directives are used in the user unit, but because it is run as a local user, the values set by those directives are appropriate to the user session.