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

about: correct references to runc #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cyphar
Copy link

@cyphar cyphar commented Oct 2, 2017

It appears this document was drafted some time before runc gained
support for rootless containers. dfba702 ("adding fork me on
github, page on environment metadata, and making tons of changes for
2.3!") removed some other out-dated information but this section
remained.

Signed-off-by: Aleksa Sarai [email protected]

@cyphar cyphar force-pushed the remove-incorrect-runc-statements branch from 114d1a0 to e9715ee Compare October 2, 2017 15:02
@cyphar
Copy link
Author

cyphar commented Oct 2, 2017

It's also quite frustrating that this document doesn't mention bubblewrap nor LXC. Both of these have had support for rootless containers longer than runc.

@vsoch
Copy link
Member

vsoch commented Oct 2, 2017

hey @cyphar! I'll be sure to set some time to review this soon, it's really great to have your contribution. Definitely add add more on LXC and bubble wrap if you would like! It might be very useful learning for our users.

Copy link
Member

@vsoch vsoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we talk about what it means to have setuid binaries?

@@ -69,7 +69,7 @@ Singularity does not utilize a daemon process to manage the containers. While da

Additionally, securing a root owned daemon process which is designed to manipulate the host's environment becomes tricky. In currently implemented models, it is possible to grant permissions to users to control the daemon, or not. There is no sense of ACL's or access of what users can and can not do.

While there are some other container implementations that do not leverage a daemon, they lack other features necessary to be considered as reasonable user facing solution without having root access. For example, there has been a standing unimplemented patch to RunC (already daemon-less) which allows for root-less usage (no root). But, user contexts are not maintained, and it will only work with chroot directories (instead of an image) where files must be owned and manipulated by the root user!
There are several container implementations that do not leverage a daemon. In addition, they do not require the installation of setuid binaries to operate. An example of this is the runc project which has merged a long-standing patch. In addition, runc is the de-facto implementation of the Open Container Initiative's runtime specification, making it compatible with a wide variety of other systems. With the use of tools such as umoci, a user can create a chroot enviornment without privileges, removing previous concerns about requiring root privileges to set up a container's rootfs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the user will likely want links to these tools, and to open in a new page to not leave their current reading

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would be doing a disservice to our users if we direct them to other projects which are not proven out on or designed for HPC computational systems, which is why most users are here.

Additionally, Singularity can create a chroot as a user without using root privileges. Granted pedantically, that would be using DockerHub so it is not truly creating a container from scratch as to build a container from scratch would require use of fakeroot and debootstrap and does not work with yum and some other distribution packaging systems (unless umoci has another method, if so I'd love to know more). The caveat is that the files created will be owned by the user, and not root. This maybe a red herring for a single user running locally, but it is not a method that can be used for creating reproducible container images.

@gmkurtzer
Copy link
Member

There are a few modifications to your wording that I would like to make to articulate the usage delta between HPC and enterprise and applicability for our use-cases, but we are happy to fix the wording as no ill-intent was intended.

Thanks for the PR!

@@ -69,7 +69,7 @@ Singularity does not utilize a daemon process to manage the containers. While da

Additionally, securing a root owned daemon process which is designed to manipulate the host's environment becomes tricky. In currently implemented models, it is possible to grant permissions to users to control the daemon, or not. There is no sense of ACL's or access of what users can and can not do.

While there are some other container implementations that do not leverage a daemon, they lack other features necessary to be considered as reasonable user facing solution without having root access. For example, there has been a standing unimplemented patch to RunC (already daemon-less) which allows for root-less usage (no root). But, user contexts are not maintained, and it will only work with chroot directories (instead of an image) where files must be owned and manipulated by the root user!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I see it:

  1. The first sentence would be more accurate if we add "...user facing HPC solution..."
  2. The second sentence is dated, as the user namespace patch has been merged into runc
  3. User contexts are not maintained in other solutions (inside the container, user seemingly becomes root) so this is accurate.
  4. As far as I know, no other container system leverages verifiable container image files. Without this, every file within the container must be owned by the user for it to be writable/modifiable/editable by the user. This is not the appropriate permissions of a root file system, so while this point is not wrong, it could be clarified.

Please feel free to challenge these points if you wish, but as I see it, aside from temporal modifications, this paragraph is accurate.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, no other container system leverages verifiable container image files. Without this, every file within the container must be owned by the user for it to be writable/modifiable/editable by the user. This is not the appropriate permissions of a root file system, so while this point is not wrong, it could be clarified.

Does "verifiable" in this context mean that you use a loopback device so that there's no extraction of the rootfs? If so, I agree most other runtimes don't do root filesystems in that way due to the concerns I outlined in our email thread (mainly related to the security of exposing the filesystem parsers to unprivileged users).

@vsoch
Copy link
Member

vsoch commented Oct 3, 2017

hey @gmkurtzer and @cyphar this is great discussion! For a quick update from me (and ETA for you) we are doing some re-organization of the docs first for 2.4, and when we finish that up we will better be able to work on the changes here (i.e., with some file renaming it will be messy if do it the other way around.) So stay tuned! :)

@gmkurtzer
Copy link
Member

You are amazing @vsoch! I'm starting to go through it, and it looks fantastic!

@cyphar cyphar force-pushed the remove-incorrect-runc-statements branch from e9715ee to f6da638 Compare October 3, 2017 17:17
@cyphar
Copy link
Author

cyphar commented Oct 3, 2017

I've pushed an updated paragraph, but sure I can wait until you make the changes for 2.4. Sorry again for the over-reaction. Thanks!

@cyphar cyphar changed the title about: remove blantly incorrect information about runc about: correct references to runc Oct 3, 2017
It appears this document was drafted some time before runc gained
support for rootless containers. dfba702 ("adding fork me on
github, page on environment metadata, and making tons of changes for
2.3!") removed some other out-dated information but this section
remained.

Signed-off-by: Aleksa Sarai <[email protected]>
@cyphar cyphar force-pushed the remove-incorrect-runc-statements branch from f6da638 to 67074cc Compare October 3, 2017 17:26
@vsoch
Copy link
Member

vsoch commented Oct 3, 2017

hey it's ok, this kind of thing has happened to me too :) It just means that you care a lot about what you do, and really that's a good thing. In the end, I think all parties (and documentation involved) will be the better for the experience.

We will get them consolidated likely later today, stay tuned!

@vsoch
Copy link
Member

vsoch commented Oct 4, 2017

quick update - we merged in (major) changes to the structure, and @GodloveD is going to do some final work and additions. The release is planned tentatively for tomorrow PM, so @gmkurtzer and @cyphar let's review the content here before that so it goes out with the new docs.

@cyphar
Copy link
Author

cyphar commented Oct 5, 2017

Where are the updates? I can't see them on master.

@vsoch
Copy link
Member

vsoch commented Oct 5, 2017

check out the docs/2.4 branch: https://github.com/singularityware/singularityware.github.io/tree/docs/2.4 We will merge into master when 2.4 is released, and then docs/2.4 will be more of an archive so down the line when we need to know, we can correspond with the various versions.

@gmkurtzer
Copy link
Member

I am fixing some things too in my branch. It might be better just to wait until we are done, and then take a look @cyphar (that way we don't waste your time looking at it before we are done).

Thanks!

@vsoch
Copy link
Member

vsoch commented Oct 30, 2017

hey @cyphar apologies I didn't ping you when we finished up the docs! They are online (current master and on the main site) if you want to take a look - likely we have some tweaks to do, but if you'd like to contribute some about runc it would be appreciated.

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.

3 participants