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

Run only necessary helpers as root #589

Open
kapsh opened this issue Dec 29, 2020 · 4 comments
Open

Run only necessary helpers as root #589

kapsh opened this issue Dec 29, 2020 · 4 comments

Comments

@kapsh
Copy link
Contributor

kapsh commented Dec 29, 2020

Running GUI applications as root is generally bad idea
(I cannot say about this better than https://bugzilla.gnome.org//show_bug.cgi?id=772875#c5) but hardinfo currently encourages it to show some low-level details (like ones that come from dmidecode output).

Better way: request permission to run only necessary helpers as root using policykit or at least sudo (though former should be present on any modern desktop). I've only peeked into dmiutil.c and it seems that laziest way would be to run pkexec dmidecode ... so polkit will take care about asking password and running requested program as root, maybe more optimal solutions exist too.

Obvious caveats: users will hate to enter password multiple times on different tabs or when generating report. Output from helpers can be cached once to workaround this. Some software avoid similar issues by running separate escalated binary which communicates with main GUI program, too.

@lpereira
Copy link
Owner

Yes, asking to run HardInfo as root is just a crutch until a better way comes along; it's not only because of the GUI libraries that are huge, but also because the code in HardInfo doesn't do a whole lot of validation and is very likely to be easily exploitable. I'm adding this issue as a blocker to the next version.

My preferred method would be invoking pkexec /proc/self/exe --some-argument, and communicate through it with a pipe from the GUI program. Using things other than pkexec (e.g. gksudo), selecting the best method during startup, would be possible; failing to find a possible method, or if the user refuses/fails to authenticate to run the root-only piece, then a suitable message would be shown in place of the current one.

@lpereira
Copy link
Owner

To fix this, we need to comb the code, looking for all commands that need to be executed as root, and come up with a plan. I don't want to make the root-only thing be able to execute anything, just some fixed command set. Maybe we can come up with a macro that helps us?

For instance, instead of calling g_spawn_sync_with_pipes(command, ...), one could call RUN_AS_ROOT(command, ...). This macro would declare a struct in an ELF section with a member pointing to the command string, and expand to a call to a write() that writes the string itself; the part that runs as root can then look in that ELF section if the command was declared in the GUI part (it's the same executable, just not running in GUI mode) and allow running it iff in the table.

@kapsh
Copy link
Contributor Author

kapsh commented Dec 29, 2020

@lpereira thanks for feedback. I'm not very much of developer, but still think that privileged part is better to be separated. Some detailed thoughts.

  1. Throw this binary (may I call it "root helper" for now?) into somewhere like libexec or lib/hardinfo/modules used already for other plugins
  2. Run it when gui starts (using some unknown yet method), talk through pipe as you said. Is this possible for programs running from different users btw?
  3. Provide only selected functionality in root helper: commands "memory_low_details", "cpu_low_details" (example names)
  4. Forbid running main program as root — check uid and exit with error
    4.1. After thinking about special tools like systemrescuecd and similar which often run gui session as root: maybe still allow this but make harder to use accidentaly (hardinfo --really-run-as-root)

@hamishmb
Copy link
Contributor

Accidentally filed a duplicate, so adding my notes here.

Running GUIs as root is also disabled by default with Xorg on some systems, and is entirely unsupported on Wayland.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants