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

Add support for non-root deployments #1270

Merged
merged 15 commits into from May 5, 2020
Merged

Conversation

@adisbladis
Copy link
Member

adisbladis commented Mar 29, 2020

This adds a new deployment configuration attribute (targetUser).
To inherit the username from the local user issuing the deployment
set:

deployment.targetUser = null;

Setting this to a string will deploy as that user. This option
defaults to "root".

We assume the following for non-root deploys:

  • Passwordless sudo (no TTY allocation possible)

I'm using the following NixOS configuration

security.pam.services.sudo.sshAgentAuth = true;
security.pam.enableSSHAgentAuth = true;

For this use-case I've introduced:

deployment.sshOptions = [ "-A" ];
  • The deployment user is trusted by the Nix daemon
nix.trustedUsers = [ "adisbladis" ];

This is required because of nix-copy-closure.

Closes #730

@adisbladis adisbladis changed the title Add support for non-root deployments WIP: Add support for non-root deployments Mar 29, 2020
nixops/ssh_util.py Outdated Show resolved Hide resolved
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from 3b5c6e5 to 10cc86e Mar 29, 2020
@adisbladis adisbladis changed the title WIP: Add support for non-root deployments Add support for non-root deployments Mar 29, 2020
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch 3 times, most recently from 7674ab1 to f14c152 Mar 29, 2020
@nixos-discourse
Copy link

nixos-discourse commented Apr 1, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update/6525/1

@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from 6822468 to 7196d19 Apr 14, 2020
nixops/backends/__init__.py Show resolved Hide resolved
nixops/backends/__init__.py Outdated Show resolved Hide resolved
nix/options.nix Outdated Show resolved Hide resolved
nixops/ssh_util.py Outdated Show resolved Hide resolved
@grahamc
Copy link
Member

grahamc commented Apr 15, 2020

I'm thinking we should make two things more configurable, and less "baked-in":

  1. make the escalation command (currently always sudo) configurable as a list of arguments to put before the command being run
  2. make it possible to specify additional options to SSH, so we don't need to explicitly add support for forwarded agents, or other things like GSSAPI.
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from 7196d19 to f91daf8 Apr 16, 2020
@adisbladis
Copy link
Member Author

adisbladis commented Apr 16, 2020

make the escalation command (currently always sudo) configurable as a list of arguments to put before the command being run

Done! The option is in deployment.privilegeEscalationCommand (which defaults to using sudo -H).

make it possible to specify additional options to SSH, so we don't need to explicitly add support for forwarded agents, or other things like GSSAPI.

This is now called deployment.sshOptions.

nix/options.nix Outdated Show resolved Hide resolved
nix/options.nix Outdated Show resolved Hide resolved
@@ -137,6 +145,8 @@ def set_common_state(self, defn) -> None:
if not self.has_fast_connection:
self.ssh.enable_compression()

self.ssh.privilege_escalation_command_set(defn.privilege_escalation_command)

This comment has been minimized.

Copy link
@grahamc

grahamc Apr 16, 2020

Member

Should we put set_ first? What is more pythonic?

This comment has been minimized.

Copy link
@adisbladis

adisbladis Apr 20, 2020

Author Member

I don't know what's more pythonic. Personally I tend to like suffixing the operation so editor completion will suggest related operations.

This comment has been minimized.

Copy link
@adisbladis

adisbladis Apr 20, 2020

Author Member

The most Pythonic way would actually be not to use setters/getters at all but just mutate the attribute.

This comment has been minimized.

Copy link
@adisbladis

adisbladis Apr 20, 2020

Author Member

Let's leave it like this for now and reconsider in the future SSH/SSHMaster refactor you and me discussed.

Copy link

chreekat left a comment

Hi! I made some comments/suggestions. None of them should be considered blockers; feel free to disregard or delay acting on them.

Thanks for chipping away at nixops!

nix/eval-machine-info.nix Outdated Show resolved Hide resolved
nix/options.nix Outdated Show resolved Hide resolved
nix/options.nix Outdated Show resolved Hide resolved
nix/options.nix Outdated Show resolved Hide resolved
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch 2 times, most recently from 6cad1f0 to 9586880 Apr 20, 2020
@grahamc grahamc added this to the 2.0 milestone Apr 20, 2020
@grahamc
Copy link
Member

grahamc commented Apr 21, 2020

I gave this a go on a regular ol' server and it worked well. I then tried it on kif which sends some keys:

$ nixops deploy --deployment personal --include kif
building all machine configurations...
kif.........> copying closure...
personal> closures copied successfully
kif.........> uploading key ‘vault-login’...
kif.........> scp: /run/keys/.vault-login.tmp: Permission denied
kif.........> error: Traceback (most recent call last):
  File "/home/grahamc/projects/github.com/NixOS/nixops/nixops/deployment.py", line 920, in worker
    m.send_keys()
  File "/home/grahamc/projects/github.com/NixOS/nixops/nixops/backends/__init__.py", line 330, in send_keys
    self.upload_file(tmp, tmp_outfile)
  File "/home/grahamc/projects/github.com/NixOS/nixops/nixops/backends/__init__.py", line 478, in upload_file
    return self._logged_exec(cmdline)
  File "/home/grahamc/projects/github.com/NixOS/nixops/nixops/backends/__init__.py", line 413, in _logged_exec
    return nixops.util.logged_exec(command, self.logger, **kwargs)
  File "/home/grahamc/projects/github.com/NixOS/nixops/nixops/util.py", line 207, in logged_exec
    raise CommandFailed(err, res)
nixops.util.CommandFailed: command ‘['scp', '-P', '22', '-o', 'StrictHostKeyChecking=accept-new', '-i', '/run/user/1000/nixops-tmp04t91o0s/id_nixops-kif', '-oControlPath=/run/user/1000/nixops-ssh-tmp7jl7pc7v/master-socket', '/run/user/1000/nixops-tmp04t91o0s/key-kif', 'grahamc@10.5.3.16:/run/keys/.vault-login.tmp']’ failed on machine ‘kif’ (exit code 1)

I think we're going to need to go back to the drawing board a bit and plan this a bit more. In particular:

  • callers of run_command have the assumption they're root
  • upload_file does too

Leaving me with a couple questions:

  • Does this feature make uploading riskier by uploading files as the user and then moving them in to place via sudo?
  • What is the API guarantee we want to retain around run_command?

I'm sorry to drag this PR out.

@grahamc grahamc added this to In progress in kanban Apr 23, 2020
@grahamc grahamc moved this from In progress to To do in kanban Apr 23, 2020
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch 2 times, most recently from c94af80 to 9210af6 Apr 23, 2020
adisbladis added a commit to adisbladis/nixops that referenced this pull request Apr 24, 2020
This will allow us to pass options to openssh and escalate
privileges (see NixOS#1270) more easily.

Closes NixOS#1322
adisbladis added 10 commits Mar 29, 2020
…e escalation command with --
This is so that we won't get inconsistencies between different
subcommands like `nixops send-keys` (which doesn't eval) the
deployment attributes.

This change should be reverted at a later date when we have made these
commands evaluate the configuration.
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from 1851d54 to e3937f9 May 4, 2020
adisbladis added 2 commits May 4, 2020
…chine

Also extend `nixops mount` with full support for all SSH arguments.
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from e3937f9 to 03663db May 4, 2020
Copy link
Member

cole-h left a comment

doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Show resolved Hide resolved
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from e862347 to c2d9d73 May 4, 2020
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Outdated Show resolved Hide resolved
doc/guides/deploy-without-root.rst Show resolved Hide resolved
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch 2 times, most recently from f655a38 to 9f32108 May 4, 2020
doc/guides/deploy-without-root.rst Show resolved Hide resolved
doc/guides/deploy-without-root.rst Show resolved Hide resolved
doc/guides/deploy-without-root.rst Show resolved Hide resolved
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from 91d19d3 to 7d9e7d2 May 4, 2020
grahamc and others added 2 commits May 4, 2020
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
@adisbladis adisbladis force-pushed the adisbladis:non-root-deploys branch from 254b40f to 8818250 May 4, 2020
@grahamc grahamc merged commit d12bac6 into NixOS:master May 5, 2020
6 checks passed
6 checks passed
parsing
Details
build
Details
black
Details
mypy
Details
mypy-ratchet
Details
coverage
Details
kanban automation moved this from To do to Done May 5, 2020
@adisbladis
Copy link
Member Author

adisbladis commented May 5, 2020

🎉 🚀

@nixos-discourse
Copy link

nixos-discourse commented May 11, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-3/7154/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
kanban
  
Done
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.