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

installer: Don't run as root #66338

Merged
merged 3 commits into from Aug 14, 2019

Conversation

@worldofpeace
Copy link
Member

commented Aug 8, 2019

Motivation for this change

There's many reason why it is and is going to
continue to be difficult to do this:

  1. All display-managers (excluding slim) default PAM rules
    disallow root auto login.

  2. We can't use wayland

  3. We have to use system-wide pulseaudio

  4. It could break applications in the session.
    This happened to dolphin in plasma5
    in the past.

This is a growing technical debt, let's just use
passwordless sudo.


This is a per-requisite to having a GNOME3 iso.
Broken up from #66313

I've supplied all the requested changes to what was raised on this commit.
In particular #66313 (review).

Things done

I've built iso_minimal and it auto logs in as live when tested
in qemu.

  • 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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@@ -17,7 +27,7 @@ with lib;
# Automatically login as root.
displayManager.slim = {

This comment has been minimized.

Copy link
@grahamc

grahamc Aug 8, 2019

Member

I won't ask it of this PR, but slim is long abandoned. We should get off of it sometime :)

This comment has been minimized.

Copy link
@worldofpeace

worldofpeace Aug 8, 2019

Author Member

Yep, I raised this in #66313 (comment) at the very end. Some people had strong opinions though in the past.

@grahamc

grahamc approved these changes Aug 8, 2019

Copy link
Member

left a comment

Shall we go for slightly more branding and call the "live" user "nixos"?

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

Shall we go for slightly more branding and call the "live" user "nixos"?

When adisbladis and I discussed this and he authored the original change
I think @davidak suggested to use live #42610 (comment).

@grahamc

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

It sounds like the name then was something with a hyphen, but I can't see what it was. I still like nixos as the name, but 🤷‍♂ 😉 .

One thing I spotted in that linked PR is the suggestion of sudo -i, which we probably should document.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

It sounds like the name then was something with a hyphen, but I can't see what it was. I still like nixos as the name, but man_shrugging wink .

One thing I spotted in that linked PR is the suggestion of sudo -i, which we probably should document.

I think I'll do this 👍

@@ -30,15 +30,27 @@ with lib;
Version=1.0
Type=Application
Name=NixOS Manual
Exec=firefox ${config.system.build.manual.manualHTMLIndex}

This comment has been minimized.

Copy link
@worldofpeace

worldofpeace Aug 8, 2019

Author Member

Just realized that nixos-manual.desktop generation isn't needed anymore,
as nixos-manual has a desktop item that will intelligently launch the default browser.

This comment has been minimized.

Copy link
@lheckemann

lheckemann Aug 14, 2019

Member

So this can be removed?

@samueldr

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

(Noting for myself, mainly) this will affect sd_image, meaning both that it needs to be tested, and (external) documentation amended as needed.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

bd59a8b was needed so users can use gparted and it will call into pkexec so we don't have to use a custom sudo desktop item.

They'll need no kind of authentication because of the polkit rule we added.

@worldofpeace worldofpeace force-pushed the worldofpeace:installer/no-root branch 2 times, most recently from 912a080 to 58ea6b5 Aug 8, 2019

@bachp

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2019

I remember that I once added the possibility do do root login into the install media via SSH. If the default user is no longer root than this is probably no longer needed.

The SSH install workflow would then just be as follows:

  1. Set a password (or public key) for user nixos
  2. Enable SSH daemon
  3. Login via ssh nixos@<target>
  4. Run sudo -i

So only password login for normal users is required.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2019

I remember that I once added the possibility do do root login into the install media via SSH. If the default user is no longer root than this is probably no longer needed.

The SSH install workflow would then just be as follows:

1. Set a password (or public key) for user `nixos`

2. Enable SSH daemon

3. Login via `ssh nixos@<target>`

4. Run `sudo -i`

So only password login for normal users is required.

Guess that means you authored

 <para>
    If you would like to continue the installation from a different machine you
    need to activate the SSH daemon via <literal>systemctl start
    sshd</literal>. In order to be able to login you also need to set a
    password for <literal>root</literal> using <literal>passwd</literal>.
   </para>

I guess we should document it this way, but I believe we should still permit root login with ssh.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

I've now tested the graphical iso.

Things done

  1. can sudo -i on tty
  2. can run sudo systemctl start display-manager on tty
  3. graphical session starts
  4. all desktop items launched properly (gparted without password prompt from polkit)
  5. can use sudo without password in konsole

Did the procedure to login via ssh for root and nixos.

@@ -29,13 +29,14 @@
</para>

<para>
You are logged-in automatically as <literal>root</literal>. (The
<literal>root</literal> user account has an empty password.)
You are logged-in automatically as <literal>nixos</literal>.

This comment has been minimized.

Copy link
@grahamc

grahamc Aug 12, 2019

Member
Suggested change
You are logged-in automatically as <literal>nixos</literal>.
You are logged-in automatically as the <literal>nixos</literal> user.
@@ -33,6 +33,12 @@
PHP 7.1 is no longer supported due to upstream not supporting this version for the entire lifecycle of the 19.09 release.
</para>
</listitem>
<listitem>
<para>
The installer now uses a less privileged <literal>nixos</literal> user whereas before we logged in as root.

This comment has been minimized.

Copy link
@grahamc

grahamc Aug 12, 2019

Member
Suggested change
The installer now uses a less privileged <literal>nixos</literal> user whereas before we logged in as root.
The installer now uses the <literal>nixos</literal> user instead of <literal>root</literal>.

This comment has been minimized.

Copy link
@grahamc

grahamc Aug 12, 2019

Member

I drop "less privileged" because they're not actually less privileged., and saying it is might lead to questions about well how do I do the thing then.

@grahamc
Copy link
Member

left a comment

I haven't tested it, but I am quite sure it has been tested :) Looks great.

worldofpeace added some commits Aug 8, 2019

installer: Don't run as root
There's many reason why it is and is going to
continue to be difficult to do this:

1. All display-managers (excluding slim) default PAM rules
   disallow root auto login.

2. We can't use wayland

3. We have to use system-wide pulseaudio

4. It could break applications in the session.
   This happened to dolphin in plasma5
   in the past.

This is a growing technical debt, let's just use
passwordless sudo.
gparted: correct polkit support
Use wrapGAppsHook as well
gparted: add adwaita-icon-theme
This adds the icon theme to XDG_DATA_DIRS.
It doesn't appear Plasma5 is properly configured for gtk
apps so this works around there being no icon theme installed
for it.

@worldofpeace worldofpeace force-pushed the worldofpeace:installer/no-root branch from 3852b0c to 15f5535 Aug 12, 2019

@samueldr

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

Just validated that sd_image works as expected.

@worldofpeace worldofpeace merged commit dd49cf7 into NixOS:master Aug 14, 2019

15 of 16 checks passed

gparted on x86_64-darwin No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
gparted on aarch64-linux Success
Details
gparted on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details

@worldofpeace worldofpeace deleted the worldofpeace:installer/no-root branch Aug 14, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

Thanks everyone.

worldofpeace added a commit to worldofpeace/nixpkgs that referenced this pull request Aug 14, 2019

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2019

There was some documentation fixes missing in nixos/doc/manual/configuration/profiles/installation-device.xml - I included them in #63773 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.