installer: Don't run graphical installer ISOs as root by adisbladis · Pull Request #42610 · NixOS/nixpkgs

adisbladis · 2018-06-26T12:06:05Z

Motivation for this change

This is currently causing issues with some tools like Dolphin (the KDE
file manager) that refuses to run as root.
It's also very rare for graphical install media to be running as root nowadays, all other distros I know of run their installation processes as unprivileged users.
We can instead allow passwordless sudo with similar effect.

On the TTY you are still root, it's just the graphical session user that has changed.

I also took the opportunity to deduplicate some configuration that was the same across KDE/GNOME live ISOs.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

jtojnar

Hmm, did not know about GNOME installer, might be interesting to people at #41797

jtojnar · 2018-06-26T12:13:23Z

nixos/modules/installer/cd-dvd/installation-cd-graphical-gnome.nix

GNOME does not have desktop by default so I guess this is moot. You would need something like:

services.xserver.desktopManager.gnome3 = { extraGSettingsOverridePackages = with pkgs; [ gnome3.gnome_shell ]; extraGSettingsOverrides = '' [org/gnome/shell] favorite-apps=['firefox.desktop', 'nixos-manual.desktop', 'gparted.desktop', 'org.gnome.Terminal.desktop'] ''; };

Ok. My solution for now is to remove gnome desktop entries and let someone more familiar with gnome take care of improving that installer.

worldofpeace · 2018-06-26T23:11:03Z

Thanks for going off of my idea. ❤️

worldofpeace · 2018-06-26T23:22:18Z

nixos/modules/installer/cd-dvd/installation-cd-graphical-kde.nix

Shouldn't this be libinput? I don't think it's a default for plasma5.

I agree. I didn't want to make any major functionality changes right now.
Running as non-root is major enough for a single PR and warrants consideration purely on it's own rights.

worldofpeace

No resistence here.

I don't think a gnome wayland session would have ever worked with root login if there ever was interest to have GNOME the default DE for the graphical installation media.

Other things are:

perhaps a default password
please remove cd-dvd/installation-cd-graphical-kde-new-kernel.nix here.
I don't think it's actually used for anything.

worldofpeace · 2018-06-27T00:30:15Z

nixos/modules/installer/cd-dvd/installation-cd-graphical-base.nix

Drop this I think. This is a graphical media, I would expect it to just start.

I think this belongs in another PR.

I think we should add another boot entry so you can still choose not to start the graphical environment, at the same time we make the one that autostarts the graphical environment the default.

I think we should add another boot entry so you can still choose not to start the graphical environment, at the same time we make the one that autostarts the graphical environment the default.

Why would that be needed? I don't see the graphical env ever being not functional, and you can always just start it and switch to a tty if needed. If someone wanted to not have a graphical env they would use the other iso...

Yeah a discussion for another PR. 👍

davidak · 2018-07-11T15:57:59Z

Great work!

I think the user should be less complex, like just 'live' without -.

adisbladis · 2018-07-13T07:41:31Z

@davidak Thanks for your feedback. I agree.

This is currently causing issues with some tools like Dolphin (the KDE file manager) that refuses to run as root. We can instead allow passwordless sudo with similar effect.

adisbladis · 2018-07-13T07:48:48Z

This should be in a merge-ready state. I just rebased it on master.
Unless someone is opposed I'll merge in a few days.

CC @dezgeg @xeji @grahamc @edolstra

etu · 2018-07-13T07:54:51Z

nixos/modules/installer/cd-dvd/installation-cd-graphical-kde.nix

      Icon=text-html
    '';

+    # Replace default gparted desktop file with one that does "sudo gparted"


Wouldn't this block be useful for both Gnome and KDE images?

Gnome doesn't have files on the desktop anymore.

A desktop file is not a file on the desktop, a desktop file is still used to launch applications from launchers etc.

And from my point of view it would possibly be useful if a user could click "gparted" in the launcher and that it's executed with sudo. But I'm not sure if there's any magic involved.

But these entries would not be visible any more since gnome doesn't read ~/Desktop.

You suggest this would be useful if it used a patched version of gparted that uses sudo?
Currently here it only copies this file to the desktop.

@worldofpeace Yeah, sure. That would be more useful. Especially on an installation media.

Not sure if that's in scope of this PR. But maybe it should be since the behaviour is changed from "I can click that button" to "Clicking that button tells me that it requires root to run".

edolstra · 2018-07-13T08:12:39Z

Not in favor of this. It just seems to make the installation process needlessly complicated. I mean, installing an OS is an inherently privileged operation, so what security is gained by not running as root?

adisbladis · 2018-07-13T08:15:06Z

@edolstra Imo this is not at all about security.
It's about tools like dolphin refusing to run as root and catering to that case.

Currently our live media comes without a working file manager.

edit: Also, running a wayland session as root is not possible.

worldofpeace · 2018-07-13T08:31:59Z

@edolstra Also it is substantiated that running graphical programs as root is insecure.
Here's as statement from a Gnome dev that supports this.

edolstra · 2018-07-13T09:13:41Z

@worldofpeace That's cargo cult security. If a user account can sudo to root without a password, then that user account is pretty much equivalent to root in security terms.

edolstra · 2018-07-13T09:14:35Z

nixos/doc/manual/release-notes/rl-1809.xml

+   <listitem>
+    <para>
+     The installer is no longer running the graphical session as root.
+     To gain root privileges in the graphical session use <literal>sudo</literal> without a password.


This should probably be mentioned in the installation section of the manual as well.

Also, it probably should be sudo -i.

samueldr · 2018-09-09T00:42:25Z

Other than the file manager not being usable as root, which real other reasons are there for the installer environment to run in that double-faced manner? I will continue with arguments against, but I am open to arguments for.

1. Dolphin is fixed

This was taken from a freshly built graphical iso at the current tip of channels/nixos-18.09. We don't need this for 18.09.

2. Makes documentation harder to write

This is because anything related to manipulations using the installer image now has to account for the possibility of having the user not be root. The existing instructions and the existing user knowledge all assume this is always done under the root user account.

This will also cause issues when troubleshooting (discourse, IRC) where it will become plausible an issue a user has is caused by being in the graphical interface instead of being in a VT logged-in as root.

3. Wayland

I lied, this is not an argument against, but more of an open-ended question: when is it realistic to see Wayland on the installer image? I'm not up to speed with the state of Wayland, it may be more likely than I think!

I'm thinking this should be delayed, because we may need to figure out a better alternative than having the installer image sometimes be root, sometimes not be. It may be better (imho) to figure out a way so that all installer images behave the same way.

Since Wayland isn't (probably) for the next release, and definitely not this one, let's work on a proactive better integrated solution instead of a reactive (to dolphin) solution.

adisbladis · 2018-09-11T03:35:12Z

@samueldr I completely share your concerns. I also noticed the change in behaviour of dolphing and was deliberately holding this off for after 18.09.

Now that my immediate reason for this change is no longer valid I think it's a good idea to close this and rethink how we can get a wayland session installer up and running for a future release.

adisbladis requested review from edolstra, jtojnar, lucabrunox and ttuegel June 26, 2018 12:06

GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jun 26, 2018

jtojnar reviewed Jun 26, 2018

View reviewed changes

GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jun 26, 2018

adisbladis changed the title ~~installer: Don't run graphical installer ISOs as root~~ WIP: installer: Don't run graphical installer ISOs as root Jun 26, 2018

worldofpeace reviewed Jun 26, 2018

View reviewed changes

worldofpeace suggested changes Jun 26, 2018

View reviewed changes

worldofpeace reviewed Jun 27, 2018

View reviewed changes

adisbladis force-pushed the installer-noroot branch 2 times, most recently from 3a088a7 to ff7247c Compare July 13, 2018 07:08

adisbladis changed the title ~~WIP: installer: Don't run graphical installer ISOs as root~~ installer: Don't run graphical installer ISOs as root Jul 13, 2018

installer: Don't run graphical installer ISOs as root

b48a82f

This is currently causing issues with some tools like Dolphin (the KDE file manager) that refuses to run as root. We can instead allow passwordless sudo with similar effect.

adisbladis force-pushed the installer-noroot branch from ff7247c to b48a82f Compare July 13, 2018 07:48

GrahamcOfBorg added 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Jul 13, 2018

etu reviewed Jul 13, 2018

View reviewed changes

worldofpeace approved these changes Jul 13, 2018

View reviewed changes

edolstra reviewed Jul 13, 2018

View reviewed changes

ttuegel removed request for lucabrunox and ttuegel July 14, 2018 18:07

etu approved these changes Jul 23, 2018

View reviewed changes

adisbladis closed this Sep 11, 2018

worldofpeace mentioned this pull request Sep 11, 2018

installer: refactor #46510

Merged

9 tasks

worldofpeace mentioned this pull request Sep 20, 2018

installer: autorun xserver in graphical media #46997

Merged

9 tasks

worldofpeace mentioned this pull request Feb 21, 2019

installer: Enable pulseaudio in all graphical iso's #56167

Merged

1 task

This was referenced Aug 8, 2019

Add GNOME3 ISO, Don't run graphical installer ISOs as root #66313

Closed

installer: Don't run as root #66338

Merged

Uh oh!

Conversation

adisbladis commented Jun 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

jtojnar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

worldofpeace commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

worldofpeace left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

worldofpeace Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidak commented Jul 11, 2018

Uh oh!

adisbladis commented Jul 13, 2018

Uh oh!

adisbladis commented Jul 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etu Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edolstra commented Jul 13, 2018

Uh oh!

adisbladis commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

worldofpeace commented Jul 13, 2018

Uh oh!

edolstra commented Jul 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samueldr commented Sep 9, 2018

1. Dolphin is fixed

2. Makes documentation harder to write

3. Wayland

Uh oh!

adisbladis commented Sep 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

adisbladis commented Jun 26, 2018 •

edited

Loading

worldofpeace left a comment •

edited

Loading

worldofpeace Jun 27, 2018 •

edited

Loading

etu Jul 13, 2018 •

edited

Loading

adisbladis commented Jul 13, 2018 •

edited

Loading