Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

daemons should not run as root #3383

Open
joseph-reynolds opened this issue Sep 19, 2018 · 20 comments
Open

daemons should not run as root #3383

joseph-reynolds opened this issue Sep 19, 2018 · 20 comments

Comments

@joseph-reynolds
Copy link

This is a meta-issue is to change existing daemons to run as a non-root user.

The security issue:

https://unix.stackexchange.com/questions/206779/is-it-a-good-practice-to-run-a-daemon-under-a-non-root-user-account

TODO: Create an issue (in the correct repo) for each OpenBMC daemon running as root. Each daemon will need to determine what privileges it needs. We'll probably have to create at least one additional user (which might be used for multiple daemons) and change systemd config files.

@edtanous
Copy link
Contributor

edtanous commented Oct 2, 2018

At a system level, we need to solve the dbus permissions issue to close this issue.

bmcweb at one point did run as its own user (and that user still exists on the system) but upon moving from boost-dbus to the sdbusplus interfaces, there was an issue with accessing resources on the system dbus interface when running as non-root. We had someone debug it for close to 2 weeks before giving up and just running the webserver as root.

These permission issues would need to be resolved before any sdbusplus service could be moved to a non-root user.

@joseph-reynolds
Copy link
Author

We'll have to determine appropriate file permissions and group permissions at the same time, and set permissions according to the principle of least privilege.
For example, control applications may need to access sysfs device files but they should not have any access to web application files. Why? Even if the control app would never intentionally access those files, an attacker may cause them to read the files and leak the information, thus causing a security exposure. Similarly, the web server should not have access to sysfs device files. Thus, we might think about having separate security domains.

@edtanous
Copy link
Contributor

edtanous commented Oct 2, 2018

"at the same time" As a matter of rolling out this change slowly, permissions can be overly permissive to start with, then we can slowly transition to more secure filesystem permissions.
Perfect is the enemy of good. Even just getting the applications moved to the correct users, even with a lot of the filesystem set to 0777 has advantages.
Looking forward to seeing your patchsets start showing up on the above.

@jmbills
Copy link
Contributor

jmbills commented Oct 5, 2018

Where we ran into trouble is in https://github.com/systemd/systemd/blob/0a2fcbd2eefe2257f622576e1f9f15608a3b6e19/src/libsystemd/sd-bus/bus-convenience.c#L533

When attempting to start other units or call methods as a non-root daemon, the function sd_bus_query_sender_privilege() is eventually called. This function has UID checks that detect non-root users and block the action.

This caused issues when we had two daemons (bmcweb and a cpu-logger) running as non-root. If bmcweb attempted to call a method on the cpu-logger, the method call was blocked by the above UID check.

@joseph-reynolds
Copy link
Author

Thanks for the updates. My two initial posts were merely to record my intention. I'm glad someone else is interested in this.

I am agreed that we can change to non-root as a first step and curious what you tried. Did you switch from root to nonroot (uid=1000 or whatever) in the daemon to retain all the Linux process capabilities (http://man7.org/linux/man-pages/man7/capabilities.7.html)? Or did you switch before exec'ing the daemon like in bmcweb.service?

@joseph-reynolds
Copy link
Author

I'm not sure if looking at Linux process capabilities is the right direction.
I need to understand how OpenBMC uses the D-Bus authorization model. My understanding is:

  1. We are using the system bus (not session buses).
  2. All of our daemons currently run as root (but we want to change them so they run as non-root).
  3. D-Bus treats a process running as root (uid=0) as special, so it can message anywhere it likes.
  4. Messages from daemons run as non-root are blocked because of the authorization failure.

If my understanding is correct, can we somehow authorize these non-root daemons to write messages to other applications on the system bus? I imagine that we'll want multiple non-root users, each from a separate set of processes. Example domains: one set for all control apps that need write authority to sysfs files, another set for control apps that go between D-Bus and REST APIs, and a third set for web apps that don't need any D-Bus or sysfs authorization.

I checked the D-Bus spec (https://dbus.freedesktop.org/doc/dbus-specification.html) which mentions authentication protocols, but I couldn't figure out how to use the protocol to allow a non-root process to write message to a root process D-Bus app. The D-Bus man pages talk about sd_bus_creds, but I couldn't figure out how to create the creds needed to talk to my root-owned app.

Specifically, my question is: how do I change one of my daemons which used to run as root to run as non-root and still be able to send and receive D-Bus messages from different non-root users?

@joseph-reynolds
Copy link
Author

Per https://unix.stackexchange.com/questions/194308/d-bus-authentication-and-authorization, I think we just need the right UNIX domain credentials (SCM_CREDENTIALS or SO_PEERCRED). For example, in each systemd .service file:

[Service]
User=someuser
Group=root   --or--
SupplementaryGroups=root

If this works, daemon processes do not run as root (and can be run as different users), and they can exchange dbus messages with each other. So this is a step in the right direction. (However, because group=root the processes can access more files than we would like, further work is needed to shut off that access.)

@joseph-reynolds
Copy link
Author

We discussed this briefly at the 2019-03-06 security working group meeting. Maybe this is a bug that nobody else sees because they use Policy Kits (polkit)?

@stale
Copy link

stale bot commented Sep 5, 2019

This issue has been automatically marked as stale because no activity has occurred in the last 6 months. It will be closed if no activity occurs in the next 30 days. If this issue should not be closed please add a comment. Thank you for your understanding and contributions.

@stale stale bot added the stale label Sep 5, 2019
@stale
Copy link

stale bot commented Oct 5, 2019

This issue has been closed because no activity has occurred in the last 7 months. Please reopen if this issue should not have been closed. Thank you for your contributions.

@stale stale bot closed this as completed Oct 5, 2019
@bradbishop
Copy link
Member

This is still needed.

@alext-w
Copy link

alext-w commented Jan 10, 2020

Looking at the https://github.com/systemd/systemd/blob/0a2fcbd2eefe2257f622576e1f9f15608a3b6e19/src/libsystemd/sd-bus/bus-convenience.c#L533 function listed by @jmbills as the culprit, I see it actually checks for process capabilities first, only then resorts to simplified E/UID checks. So probably capabilities could help us here at the end of the day. Or maybe better understanding of the D-Bus authentication and whether it could be used here at all. All in all, it, I believe, is a quite important thing to achieve, to have most or our daemons running as non-root, because currently that's clearly surplus and it opens up a necessarily wide attack capabilities for any compromised process.

@joseph-reynolds, I'd propose to revisit this during the next Security WG meeting and maybe try to define more specific next steps to start moving here.

@joseph-reynolds
Copy link
Author

This is an instance of CWE-250: Execution with Unnecessary Privileges.

@joseph-reynolds
Copy link
Author

Copied from #openbmc IRC chat: Would it help to use systemd-nspawn to help isolate processes?

@alext-w
Copy link

alext-w commented Apr 22, 2020

Would it help to use systemd-nspawn to help isolate processes?

This looks like yet another containerization solution. While it would indeed provide the root powers isolation, would it really allow the containerized workload to communicate over hosts' D-Bus? My understanding from some reading (I don't have any first-hand experience with this one) is that it won't. It also performs full filesystem isolation, as they claim, including /dev so I wonder whether systemd-nspawn'ed daemons will actually be able to do anything useful.

And of course performance is another concern here, while being lightweight, this is still some additional work for the CPU to do, so it should at least be benchmarked.

All in all, it IMHO doesn't looks like something that would help, but some actual trial may be useful here.

@stale
Copy link

stale bot commented Oct 23, 2020

This issue has been automatically marked as stale because no activity has occurred in the last 6 months. It will be closed if no activity occurs in the next 30 days. If this issue should not be closed please add a comment. Thank you for your understanding and contributions.

@stale stale bot added the stale label Oct 23, 2020
@ya-mouse
Copy link
Contributor

I currently working on this issue.

@stale stale bot removed the stale label Oct 23, 2020
geissonator pushed a commit that referenced this issue Apr 12, 2021
DynamicUsers flag in systemd service configuration file required to create,
handle and recycle temporary users.

This is essential module for upcoming daemons' privilege separation work.

Reference: #3383

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: Iabd709c4a20f754fc6ea505e640b2d361aba0be2
geissonator pushed a commit to openbmc/phosphor-hwmon that referenced this issue May 6, 2021
This change required as a part of privilege separation work:
  openbmc/openbmc#3383

The dependant change in openbmc repo:
  https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/40359

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I89aec71b446e0b7be40a10e1cd94ea16d891a277
geissonator pushed a commit to openbmc/phosphor-hwmon that referenced this issue May 7, 2021
Introduce `-i|--sensor-id` flag to explicitly set sensor
suffix for dbus name.

Sample usage:

   phosphor-hwmon-readd -i test_sensor_id -o /apb/...

Will register the service with the following busname:

   xyz.openbmc_project.Hwmon-test_sensor_id.Hwmon1

This change required as a part of privilege separation work:
  openbmc/openbmc#3383

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I48ff9c3efe0edb84718ff8f695e7e932af5445de
geissonator pushed a commit that referenced this issue May 7, 2021
Use meson to build the package.

This change required as a part of privilege separation work:
  #3383

This change should be merged after individual repo change:
  https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-hwmon/+/40277

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I542645d55b8646e0ddb3fb95f50a1cd87334706e
geissonator pushed a commit that referenced this issue May 7, 2021
Anton D. Kachalov (1):
      Add sensor instance id command line argument.

This change will generate busconfig ACLs from the provided hwmon
environment files from /etc/default/obmc/hwmon that will be
supplied as "sensor-id" argument to hwmon daemon.

This change required as a part of privilege separation work:
  #3383

This change should be merged after individual repo change:
  https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-hwmon/+/40214

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I37c31d6f2f74041bfee453bdf338d4c7e148c791
geissonator pushed a commit to openbmc/phosphor-logging that referenced this issue May 10, 2021
This change required as a part of privilege separation work:
  openbmc/openbmc#3383

This change has dependant change:
  https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/41834

Change-Id: Ib4744ac69428af1a9625c3e99004a4cfd857ba4b
Signed-off-by: Anton D. Kachalov <gmouse@google.com>
geissonator pushed a commit that referenced this issue May 13, 2021
This change required is a part of privilege separation work:
  #3383

This change should be merged after individual repo change:
  https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-logging/+/41835

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I6243736cd0ce1ecccb2b164068dccf05d01ab6ad
geissonator pushed a commit to openbmc/phosphor-user-manager that referenced this issue May 27, 2021
This change required as a part of privilege separation work:
  openbmc/openbmc#3383

This change required by the following openbmc meta change:
  https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/42672

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: Iad476fc32f9df6fe5ceb51e8eea2c798dcc51252
@stale
Copy link

stale bot commented Jun 3, 2021

This issue has been automatically marked as stale because no activity has occurred in the last 6 months. It will be closed if no activity occurs in the next 30 days. If this issue should not be closed please add a comment. Thank you for your understanding and contributions.

@stale stale bot added the stale label Jun 3, 2021
@geissonator
Copy link
Contributor

@ya-mouse still chugging away on this one stalebot

@stale stale bot removed the stale label Jun 7, 2021
bradbishop pushed a commit to openbmc/phosphor-certificate-manager that referenced this issue Jul 1, 2021
This change required as a part of privilege separation work:
  openbmc/openbmc#3383

Dependant meta-phosphor change:
  https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/41430

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: Ic0b1b57f8a088defe096f1ab793efa1f015ca5be
geissonator pushed a commit that referenced this issue Aug 30, 2021
This change required as a part of privilege separation work:
  #3383

Seccomp support enables sandboxing in systemd:
  https://www.freedesktop.org/software/systemd/man/systemd.exec.html#Sandboxing

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: If7ff257103c4aa40dae5f64256bf60d8a30fbf59
geissonator pushed a commit that referenced this issue Oct 4, 2021
This change is a part of the privilege seperation work
which is tracked in:
  #3383

This change should be merged after individual repo change:
  https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-certificate-manager/+/41166

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I72e4842e7aa6de2ae4bcbdbf00953b7a79a0f414
geissonator pushed a commit that referenced this issue Oct 4, 2021
This change required as a part of privilege separation work:
   #3383

This change should be merged after individual repo change:
   https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-user-manager/+/42633

Signed-off-by: Anton D. Kachalov <gmouse@google.com>
Change-Id: I3d68a3cb27f822b05027ef07a89e6c65f6859178
@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because no activity has occurred in the last 6 months. It will be closed if no activity occurs in the next 30 days. If this issue should not be closed please add a comment. Thank you for your understanding and contributions.

@stale stale bot added the stale label Apr 17, 2022
@stale stale bot removed the stale label May 23, 2022
@ya-mouse ya-mouse removed their assignment Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants