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

[WIP] CVE-2019-16884: container: use open fd to host /proc and /sys #109

Closed
wants to merge 1 commit into from

Conversation

giuseppe
Copy link
Member

use an open fd to refer to the host /proc and /sys when setting up the
SELinux and the AppArmor labels.

Addresses CVE-2019-16884.

Signed-off-by: Giuseppe Scrivano giuseppe@scrivano.org

@giuseppe
Copy link
Member Author

marked as WIP as I've only tested the SELinux part so far.

/CC @crosbymichael @cyphar @rhatdan

@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2019

This pull request introduces 1 alert when merging 93b4bac into 423bc44 - view on LGTM.com

new alerts:

  • 1 for Lossy pointer cast

use an open fd to refer to the host /proc and /sys when setting up the
SELinux and the AppArmor labels.

Addresses CVE-2019-16884.

Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
@cyphar
Copy link

cyphar commented Sep 28, 2019

I really don't like the idea of using a host mountpoint for this -- it means that any possible bug which causes the fd to be leaked into the container allows for full access to the host filesystem. If stashing away an fd to the container's /proc mount is out of the question (if conmon was integrated into crun -- effectively copying the LXC model for container management -- this would be trivial) then at the very least this should be done with fds to the specific files that need to be written to instead of passing directory fds.

(As an aside, the new mount API would allow you to do this even more trivially by doing fsopen("proc") -- but obviously this would massively limit the number of kernels you could run on.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants