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

Don't allow an edge service to bind root-privileged files/dirs from the host #960

Closed
bmpotter opened this issue May 30, 2019 · 6 comments
Closed
Assignees
Labels
Milestone

Comments

@bmpotter
Copy link
Member

bmpotter commented May 30, 2019

There are currently a few serious security holes in the sandbox that anax puts an edge service into. By the service putting a few specific files/dirs in the deployment binds array in its service def, it can do things on the edge host it shouldn't be allowed to do:

  • /var/run/docker.sock: if the service can start any docker container, it essentially has root access to the host. See If you have access to docker run you have root access on the host moby/moby#1655
  • Any root protected directory: anax should not allow the service to mount rw any dir that only root on the host can write. Similarly, it should not allow the service to mount ro any dir that only root on the host can read.

The checks should be done for both hzn dev service start and hzn register.

@bmpotter bmpotter added this to the Sprint 14 milestone May 30, 2019
@linggao linggao assigned nhjabbour and unassigned linggao May 30, 2019
@linggao
Copy link
Member

linggao commented May 30, 2019

@bmpotter Should we fail the container bring up or should we bring it up without these invalid bings?

@linggao
Copy link
Member

linggao commented May 30, 2019

@bmpotter there is privileged setting for the container deployment string? If the container is going to run the privileged mode, do we still preventing it from binding the directories you have mentioned?

@bmpotter
Copy link
Member Author

If they have invalid binds, fail the container bring up. One of the reasons for also doing this check in hzn dev service start is that they will readily see that msg.

No, i don't think privileged is as dangerous as the binds addressed in this issue. So prevent them even if privileged:true is specified.

@nhjabbour
Copy link
Contributor

I have added support to fail the container bring up if certain keywords are detected. So far I have docker.sock and /etc. Is there a list of certain directories we want to restrict? I am thinking the list of directories might differ based on what OS our code is running on.

@bmpotter
Copy link
Member Author

bmpotter commented Jun 3, 2019

Nadim,

Yes, it will differ. The way to handle this is to not have a list of dirs, but instead look at the permissions of the dir or file. For example:

  • if the other-write bit of the dir or file permission isn't on, and the bind mode is rw don't allow it
  • of if the other read and execute bits of the dir permission aren't on, and the bind mode is ro don't allow it
  • of if the other-read bits of the file permission aren't on, and the bind mode is ro don't allow it

Since there are other typical system usernames (bin, daemon, sys, ...) in addition to root, and we don't even know if regular user-owned dirs/files should be accessible by edge services, because they don't have any host system creds, I don't think it is worth checking the owner and group of the dir/file.

Please survey all of the new hire samples to get an idea if this new restriction is going to be a problem, and let's discuss if it is.

As glen has noted, this won't catch cases in which the service bind mounts host dir /foo which is world readable, but host dir /foo/bar is not, but those cases are rare, so the above is a lot better than doing nothing.

@jiportilla
Copy link

Dave to update this issue.

@jiportilla jiportilla modified the milestones: Sprint 14, Sprint 15 Jun 7, 2019
@dabooz dabooz self-assigned this Jun 25, 2019
@dabooz dabooz closed this as completed Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants