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

dangerous default behaviour (mounting home directory inside minikube VM) #6788

Closed
starbassma opened this issue Feb 24, 2020 · 11 comments
Closed
Labels
area/mount co/virtualbox kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.

Comments

@starbassma
Copy link

starbassma commented Feb 24, 2020

Hello,
When starting a minikube instance
minikube start
by default, the --mount-string= is set to home directory ~ of the host machine, this argument is passed to the minikube mount command on start, the home directory is mounted under /minikube-host (mode -493 =>).
executing rm -rf / inside the minikube VM (or with ssh) would have a catastrophic effect (~ directory of the host machine deleted).
Since minikube is made for testing purposes, executing deletion command is highly probable.
I think this default behavior is very dangerous and should be changed.
Regards
UPDATE:
This behavior is related to VirtualBox's driver, not to minikube's mount, disabling the fs mounts provided by the hypervisors should fix this behavior (by setting disable-driver-mounts to true)
thanks to @afbjorklund for his remarks

@medyagh
Copy link
Member

medyagh commented Feb 25, 2020

I believe the mount is only for minikube HOME folder so that would be
~/.minikube not ~.

@medyagh medyagh added the priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. label Feb 25, 2020
@afbjorklund
Copy link
Collaborator

Mounting the (actual, real) home directory is a “feature” inherited from libmachine. Like most of Docker, it prefers convenience over security.

On Mac I think it was even worse, it mounted /Users (similar to /home on Linux). So not only $HOME, but all users’ homes...

@starbassma
Copy link
Author

Hello @medyagh
These are the the values of the default mount directory :

Linux

var DefaultMountDir = homedir.HomeDir()

Darwin

var DefaultMountDir = "/Users"

Windows

var DefaultMountDir = homedir.HomeDir()

working on a fix, i will send a PR soon.
Regards,

@afbjorklund
Copy link
Collaborator

About machine:

The defaulting mounting is only implemented for the VirtualBox driver, I believe (not KVM)

And it actually mounts /home on Linux too, I misremembered when I said it was only Mac.

https://github.com/docker/machine/blob/master/drivers/virtualbox/virtualbox_linux.go#L7

https://github.com/docker/machine/blob/master/drivers/virtualbox/virtualbox_darwin.go#L7

In minikube, the setting was called --virtualbox-no-share, now it is --disable-driver-mounts:

      --disable-driver-mounts=false: Disables the filesystem mounts provided by the hypervisors

This is not the minikube mount, though.

But the mount behaviour is the same.

@afbjorklund
Copy link
Collaborator

Wonder why it was not simply using homedir.HomeDir() on darwin too ? Is it broken somehow

@medyagh, what does homedir.HomeDir() return on your Mac ? Thought it was getpwnam_r

@starbassma
Copy link
Author

starbassma commented Feb 25, 2020

Wonder why it was not simply using homedir.HomeDir() on darwin too ? Is it broken somehow

@medyagh, what does homedir.HomeDir() return on your Mac ? Thought it was getpwnam_r

@afbjorklund

homedir.HomeDir() returns the expected result (home directory of the OS user) ( tested under Darwin Kernel V 19.2.0) i don't think it's broken, using different values ​​remains incomprehensible

@afbjorklund
Copy link
Collaborator

afbjorklund commented Mar 1, 2020

@starbassma : there are two different types of mounts here, both could mount your home...

The first parameter you mention, --mount-string, is indeed for minikube mount - any driver.

This means that if you run virtualbox and you run minikube start --mount, you will have two

One under /hosthome (vboxsf) and one under /minikube-host (9p). Both from your $HOME:

Shared folders:  

Name: 'hosthome', Host path: '/home' (machine mapping), writable
📁  Creating mount /home/anders:/minikube-host ...

As per above, it can be disabled. And the --mount=false is default already.

Only virtualbox implements --disable-driver-mounts now, with xhyve gone.

See #5012 and #1646

Originally 4545f8d

@fejta-bot

This comment was marked as outdated.

@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 30, 2020
@fejta-bot

This comment was marked as outdated.

@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 29, 2020
@Fedcomp
Copy link

Fedcomp commented Mar 15, 2023

@sharifelgamal i see you closed the issue. Is there a way to properly reopen it? i am using minikube

$ minikube version
minikube version: v1.29.0
commit: ddac20b4b34a9c8c857fc602203b6ba2679794d3-dirty

And i still see this behavior (and was shocked with such defaults for test environments where everything can go wrong).
I believe it should not be default behavior. Reason to use virtual machines is host system safety.

@afbjorklund
Copy link
Collaborator

afbjorklund commented Mar 15, 2023

This default is being carried forward into systems such as Docker Desktop and Podman Desktop, as a part of the Docker legacy. Other systems mount the home directory as read-only, same privacy implications but less chances of destruction.

For now, it's opt-out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mount co/virtualbox kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants