zhaofengli/colmena

Limit access required for SSH

Opened this issue · 4 comments

The issue

I was looking into how to run Colmena without requiring root access. I saw that this was also discussed in #27 and from what I could gather the current solution is to add the privilegeEscalationCommand to the config. Unfortunately this still requires a password, so the only solution I could find was to set

{
  security.sudo.wheelNeedsPassword = false;
}

That solution makes me worried, as that will mean there are now two users with root access instead of just the root account. I think that lowers the security, since, besides the root account, there'll now also be all the accounts in the wheel group that can run whatever sudo command they want without being required a password. What I would like is to only set NOPASSWD on the specific commands that will be run when deploying with Colmena for the users that I will ssh in as.

I therefore worked out a solution that will make this possible:

Suggestion

Looking at the code there's two commands that need sudo privileges; nix-env and /nix/store/<profilePath>/bin/switch-to-configuration. The first one is simple to fix:

users.groups.colmena = {};
security.sudo = {
  extraRules = [
    {
      commands = [
        {
          command = "${pkgs.nix}/bin/nix-env";
          options = ["NOPASSWD"];
        }
      ];
      groups = ["colmena"];
    }
  ];
};

Then it's as simple as adding the desired user to the group and the first issue is resolved.

The second is however a bit more difficult, since profilePath isn't calculated until everything in the config itself has been evaluated. Using something like config.system.build.toplevel to find the path for switch-to-configuration in the store will cause an infinite recursion error.

The alternative I'd suggest is to have a simple wrapper script that can be called from colmena itself. This could look something like:

switch-colmena = pkgs.writeShellScriptBin "switch-colmena" ''
    NEXT_PROFILE=$1
    GOAL=$2

    "$NEXT_PROFILE"/bin/switch-to-configuration "$GOAL"
  '';

The sudo rules can then be changed to:

# This could also be added to the relevant user paths to limit access a bit further.
environment.systemPackages = [
  # ...other packages...
  switch-colmena
]

security.sudo = {
  extraRules = [
    {
      commands = [
        {
          command = "${pkgs.nix}/bin/nix-env";
          options = ["NOPASSWD"];
        }
        {
          command = "${switch-colmena}/bin/switch-colmena";
          options = ["NOPASSWD"];
        }
      ];
      groups = ["colmena"];
    }
  ];
};

To make colmena actually call the desired command I made a small change to the activation_command function in profile.rs

pub fn activation_command(&self, goal: Goal) -> Option<Vec<String>> {
    if let Some(goal) = goal.as_str() {
        let switch_to_configuration = self
            .as_path()
            .to_str()
            .expect("The string should be UTF-8 valid")
            .to_string();

        Some(vec![
            "switch-colmena".to_string(),
            switch_to_configuration,
            goal.to_string(),
        ])
    } else {
        None
    }
}

This is a pretty bare-bones proof-of-concept, so it'll probably require a bit more fleshing out, but I think it would be an improvement overall.

It has been a while since I've used nix-env, but can't it run programs similar to nix run? If so I don't see how this would be an improvement after all, since you could basically run any program nix can fetch with escalated priviledges (through nix-env).

@nrdxp, let me provide an example to illustrate how I believe this approach could enhance security. I'll begin by discussing why I think security.sudo.wheelNeedsPassword = true should be avoided. Then, I'll address the issue you highlighted regarding a dedicated user and nix-env, and finally, I'll propose a solution.

My Concern with wheelNeedsPassword = false

The wheel group is slightly special since its purpose is to grant its members access to the sudo command. I think using it for another purpose (i.e. to run Colmena) should be avoided, since being a member of this group does not equate to being a user who operates Colmena. Consequently, setting wheelNeedsPassword = false undermines the principle of least privilege by allowing users who do not require it the ability to execute sudo commands without a password. Here's an example to illustrate the issue:

Consider a server with three individuals:

  • Alice: The system administrator who executes the Colmena command.
  • Bob: A user who needs sudo access and is a member of the wheel group.
  • Carol: A malicious entity aiming to gain root access to the server.

If security.sudo.wheelNeedsPassword = true, both Bob and Alice must authenticate with their SSH key and then verify their knowledge of the relevant password to execute a sudo command on the server.

In this scenario, Carol must acquire both the SSH key and the sudo password to gain root access.

Conversely, if security.sudo.wheelNeedsPassword = false, Carol only needs to obtain the SSH key of either Alice or Bob to gain complete server access. This scenario increases both the attack surface and the potential damage from a compromised SSH key.

Implementing a Dedicated User or Group for Colmena

Introducing a specific user or group for Colmena indeed presents an additional vector for Carol to exploit.

With a dedicated group where Alice is a member, Carol could:

  • Acquire the SSH key and password from Bob.
  • Obtain an SSH key from Alice.

With a dedicated user, she could either:

  • Acquire the SSH key and password from Alice or Bob.
  • Obtain an SSH key from the dedicated user.

Despite these risks, I believe both scenarios offer an improvement by limiting the target to one user's SSH key, as opposed to any user in the wheel group. The security benefit diminishes only if the wheel group exclusively contains users who will run Colmena, and it's assured that all future members will do the same.

Mitigation Strategy

Based on your description of the issue, the core problem appears to be that allowing a user to run nix-env without sudo enables them to execute arbitrary code on the system. If I've misunderstood, please correct me.

As I explained above, this does not mean security is not improved by avoiding wheelNeedsPassword = false, but it does not follow the principle of least privilege, since it's possible to run any nix-env command. Access to nix-env could be restricted similarly to how switch-to-configuration is managed, but I'll go through why I don't think this solution is an improvement at the end of the comment. The nix-env command can be restricted as follows:

set-system-path = pkgs.writeShellScriptBin "set-system-path" ''
  PATH=$1

  ${pkgs.nix}/bin/nix-env --profile /nix/var/nix/profiles/system --path $PATH
'';

Then, adjust the extraRules to permit only the execution of the restricted command instead of nix-env directly:

environment.systemPackages = [
  # ...other packages...
  switch-colmena
  set-system-path
];

security.sudo = {
  extraRules = [
    {
      commands = [
        {
          command = "${lib.getExe set-system-path}";
          options = ["NOPASSWD"];
        },
        {
          command = "${lib.getExe switch-colmena}";
          options = ["NOPASSWD"];
        }
      ];
      groups = ["colmena"];
    }
  ];
};

It's still possible for a malicious actor that has gained access to the users account to change the system path to whatever they want, since they can simply point the script to a path of a build containing whatever code they want to run, so it'll only be an improvement if the attacker can't be bothered to build and push their own closure.

As I mentioned above however, the main goal of my suggestion is to limit how many users has this privilege without altering the wheel group's intended function.

I agree that have passwordless sudo is far from ideal.

But yeah, my basic concern is that if nix-env has sudo access, that you can basically run any program you want through it, which is basically the same as passwordless sudo in practice.

I know the upstream nixos-rebuild script resolves this by adding a --use-remote-sudo flag which only calls sudo directly in the places where the script actually needs it. I'm curious why exactly nix-env would even need sudo at all? I don't think anything it does would really require it. Perhaps the code could be rewritten to avoid it. If we could just limit the call to sudo when calling the "switch to" script alone, (essentially mimicing the upstream --use-remote-sudo flag) then I think that'd be the way to go.

I am decent with Rust but not too familiar with the codebase here, so maybe I can take a look at it soonish and see if I can figure it out.

@nrdxp thanks for the reply. It sounds like we might be in agreement. My issue description might have lacked clarity in its description and focused too much on a solution without clearly establishing the problem I'd like to see solved.

My main concern is that depending on the wheel group to run Colmena leaves a pretty dangerous foot-gun lying around - especially on multi-user systems. It's primarily a UX concern in terms of how Colmena might be used in practice, since a dependency on wheelNeedsPassword = false in my opinion makes it unsuitable for production environments.

Removing the need for password-less sudo for nix-env would be a better solution than only the privilege of running it to a dedicated user/group. I still think a good solution allows for the user to configure a specific user/group for Colmena, so I hope both can be done.

I'm somewhat familiar with Rust and I did spend some time looking through the codebase when creating the ticket. I'd be happy to contribute to a solution if it'll be helpful. Just let me know if I can help out with anything either here in the ticket or reach out directly through the email in my GitHub profile.

Thanks for taking the time to provide some really good feedback to my proposal. I can now see that i had a blind spot in regards to nix-env that would have resulted in a less than ideal solution.