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

Evaluate adding miniube to sudoers group only for Commands that requires sudo #15847

Open
medyagh opened this issue Feb 13, 2023 · 15 comments
Open
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@medyagh
Copy link
Member

medyagh commented Feb 13, 2023

The idea is for specific minikube commands such as Tunnel on VM drivers, minikube should ask password only once in the life time of the binary, and after that adds minikube to sudoers so we dont have to ask for pssword.

Also we should NOT add minikube to sudoers group if the user does NOT use the special commands that needs sudo (such as tunnel on vm or ...)

whenever minikube needs to run a feature that needs sudo (such as tunnel on VM) it would ask for users password once, and use that to add minikube binary to sudoers list , and after that every other minikube tunnel command will run without asking for password

something like this
https://github.com/lima-vm/socket_vmnet/blob/master/etc_sudoers.d/socket_vmnet

@medyagh medyagh added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 13, 2023
@spowelljr spowelljr added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 13, 2023
@nirs
Copy link
Contributor

nirs commented Feb 14, 2023

Souinds convienet for users, but also for attackers.

One example:

  1. attacker starts command that needs a password
  2. minikube installs suduers file so next run does not need a password
  3. attacker replaces minikube executable
  4. attacker runs the same minikube command to get root

I think the bigget issue is touching sensitive parts of the system without getting
permission from the user. User gave permission to create a tunnel, not to install
suduers files.

It is ok to add a command like minikube install-helper that must run as root and
will allow minikube to do some stuff as root later.

Other issues:

  • suduers syntax is hard to get right, easy to create unsafe rules
  • how minikube deals with suduers files added by other users on the same system?
  • how the suduers files are removed? This is typically done by installer or package scripts
  • why minikube needs to bypass the security mechanisms and policies of the system?
  • can this be done using exiting services (e.g. libvirt)?

@medyagh
Copy link
Member Author

medyagh commented Feb 15, 2023

thats interesting analysis thank you for sharing,

for 3- when you say user replaces minikube exec, woulnd't that need sudo permission itself ? how would they do that?

@nirs
Copy link
Contributor

nirs commented Feb 15, 2023

If minikube was installed by root in (e.g. /usr/bin/minkube) you cannot replace it without root so it is safe. But if minikube was downloaded from web (e.g. /home/user/bin/minikube) the user
can replace it.

@medyagh
Copy link
Member Author

medyagh commented Feb 16, 2023

I see so you are saying if minikube was downloaded as normal user, then minikube asked the user to add it to sudoers group, then someone with "Normal access" can replace minikube binary with a malicious binary ?

so that means if I have any binary that is added to my sudoers group

I think https://github.com/lima-vm/socket_vmnet is using this technic, so it means we should be able to copy any file over that file ? without having to enter sudo's password?

@ComradeProgrammer
Copy link
Member

(maybe I can have a try on this issue?

@nirs
Copy link
Contributor

nirs commented Feb 20, 2023

I see so you are saying if minikube was downloaded as normal user, then minikube asked the user to add it to sudoers group, then someone with "Normal access" can replace minikube binary with a malicious binary ?

Yes

One example, an attacker get the user to run some mallware as regular user (should be safe),
and the malware replaces the minikube executbale with another mallware that run a shell as root.

I think https://github.com/lima-vm/socket_vmnet is using this technic, so it means we should be able to copy any file over that file ? without having to enter sudo's password?

I don't know about socket_vmnet, but in the link you posted in the issue description:
https://github.com/lima-vm/socket_vmnet/blob/master/etc_sudoers.d/socket_vmnet

we see:

# Entries for shared mode (192.168.105.0/24)
%staff ALL=(root:root) NOPASSWD:NOSETENV: /opt/socket_vmnet/bin/socket_vmnet --vmnet-gateway=192.168.105.1 /var/run/socket_vmnet

/opt is usually owned by root and cannot be modified by unprivileged user. If permissions
are set properly, unprivleged user cannot replace /opt/socket_vmnet/bin/socket_vmnet with
mallware. But /home/user/bin/minikube can be replaced by user so it is not safe.

To make this safe, I would extract the code that needs root to a little helper, installed
as root in a normal location with the right permissions. So intead of letting minikube run
a command as root, let the helper run as root. Minikube can run the helper.

@medyagh
Copy link
Member Author

medyagh commented Feb 21, 2023

@nirs thank you for such a detailed example ! that makes sense now,

so what we could do is, identify all minikube commands that run as Sudo and put them in a new minikube binary something like
sudominikube and have it be in /opt/minikube/bin/ ...

addtionally we can keep a Duplicate of the code in main minikube binary for the users that prefer to keep entering their password (and some of the features also only need password on certain drivers/platforms)...

@nirs
Copy link
Contributor

nirs commented Feb 21, 2023

Duplicating the sounds bad, do you mean having single package for the helper code
shipped in 2 executables (minikube, minikube-helper)?

@ComradeProgrammer
Copy link
Member

ComradeProgrammer commented Feb 25, 2023

Well does "duplicate of the code" mean that we build a new binary "sudominikube" for the functions which require sudo, while also keeping those functions in original "minikube"? Surely we don't need to duplicate the source codes right?

Well if so, I think it is really a good idea. But I have some small detailed questions about this.

First, how should we install the "sudominikube"? Well I thought of 2 methods. One possible solution is that we can install it when user execute the installation script. Another one is to add a new command, which downloads the "sudominikube", checks the sha and and installs it to /opt/minikube/bin. Or are there any other methods? (to be honest I prefer to the second method.)

The second question is that we always need to perform checks to ensure that the version of sudominikube and minikube are identical, right? Because if version of sudominikube is older, it may not work properly.

The third question is that, what commands require sudo? Have we already got a list of these commands?

@ComradeProgrammer
Copy link
Member

ComradeProgrammer commented Feb 25, 2023

Besides, maybe we can try to implement serveral commands for sudominkube first, and then gradually add other commands which requires sudo to sudominikube. If possible, maybe I can have a try on this if you consent? @medyagh

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 26, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 25, 2023
@vaibhav2107
Copy link
Member

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 4, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 25, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants