Handle running nested in a user namespace #458
Conversation
|
Note there is no technical or security reason why these could not be allowed in a user namespace, but the kernel patch to change that was rejected by the kernel cgroups maintainer. |
|
We had a related discussion in the OCI weekly call yesterday. I proposed separation of the devices cgroup configuration from the other device configuration. That way we can just exclude the device cgroup manipulation when nesting. However, this can also work before that lands. Your commit is missing the actual Godep vendored files, though. Thanks! |
|
On Thu, Jan 07, 2016 at 05:48:57PM -0800, Mrunal Patel wrote:
yeah sorry wasn't sure what the right step is. Should I 'go get' the code with runc/Godeps/_workspace as my GOPATH ? |
|
@hallyn runc uses godep, you can follow the instruction from: https://github.com/tools/godep#add-a-dependency |
hallyn
changed the title
hallyn
force-pushed
the
userns
branch
3 times, most recently
from
51b08c4 to
9ca6225
Compare
|
@hallyn is it possible to move the required functions to another small-ish package ? Importing all the lxd/shared dependencies seem rather expensive for this small change. |
|
Hi - certainly possible :) Ideally it would be defined in libcontainer and (see moby/moby#19182) I'll go ahead and update both requests with something shared that works. |
|
@hallyn Thanks! Maybe in this file https://github.com/opencontainers/runc/blob/master/libcontainer/utils/utils.go |
|
I went with system instead of util, since it is linux-specific. Should I move |
|
@hallyn I am fine with system. |
|
|
||
| line := string(l) | ||
| var a, b, c int64 | ||
| fmt.Sscanf(line, "%d %d %d", &a, &b, &c) |
There was a problem hiding this comment.
Can we add a comment that this function assumes that uid_map has been written in the new user namespace (which is a fair assumption)?
There was a problem hiding this comment.
On Fri, Jan 08, 2016 at 11:51:26AM -0800, Mrunal Patel wrote:
+func RunningInUserNS() bool {
- file, err := os.Open("/proc/self/uid_map")
- if err != nil {
return false- }
- defer file.Close()
- buf := bufio.NewReader(file)
- l, _, err := buf.ReadLine()
- if err != nil {
return false- }
- line := string(l)
- var a, b, c int64
- fmt.Sscanf(line, "%d %d %d", &a, &b, &c)
Can we add a comment that this function assumes that uid_map has been written in the new user namespace (which is a fair assumption)?
I don't understand what you mean. The file is a procfile, served by the
kernel. If it's not there we assume the kernel doesn't support user
namespaces and return false at the top.
Happy to add a comment but not sure what you are referring to.
There was a problem hiding this comment.
I meant that we are checking for the default values in the uid_map file to check user namespace is in use. So the assumption is that new values that differ from the default values in that file have been written. Adding this is discretionary but I think might clarify the behavior. What do you think?
On Jan 8, 2016, at 11:55 AM, Serge Hallyn notifications@github.com wrote:
In libcontainer/system/linux.go:
+func RunningInUserNS() bool {
- file, err := os.Open("/proc/self/uid_map")
- if err != nil {
return false- }
- defer file.Close()
- buf := bufio.NewReader(file)
- l, _, err := buf.ReadLine()
- if err != nil {
return false- }
- line := string(l)
- var a, b, c int64
- fmt.Sscanf(line, "%d %d %d", &a, &b, &c)
On Fri, Jan 08, 2016 at 11:51:26AM -0800, Mrunal Patel wrote: > +func RunningInUserNS() bool { > + file, err := os.Open("/proc/self/uid_map") > + if err != nil { > + return false > + } > + defer file.Close() > + > + buf := bufio.NewReader(file) > + l, _, err := buf.ReadLine() > + if err != nil { > + return false > + } > + > + line := string(l) > + var a, b, c int64 > + fmt.Sscanf(line, "%d %d %d", &a, &b, &c) Can we add a comment that this function assumes that uid_map has been written in the new user namespace (which is a fair assumption)?
I don't understand what you mean. The file is a procfile, served by the kernel. If it's not there we assume the kernel doesn't support user namespaces and return false at the top. Happy to add a comment but not sure what you are referring to.
—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
On Fri, Jan 08, 2016 at 12:47:24PM -0800, Mrunal Patel wrote:
+func RunningInUserNS() bool {
- file, err := os.Open("/proc/self/uid_map")
- if err != nil {
return false- }
- defer file.Close()
- buf := bufio.NewReader(file)
- l, _, err := buf.ReadLine()
- if err != nil {
return false- }
- line := string(l)
- var a, b, c int64
- fmt.Sscanf(line, "%d %d %d", &a, &b, &c)
I meant that we are checking for the default values in the uid_map file to check user namespace is in use. So the assumption is that new values that differ from the default values in that file have been written. Adding this is discretionary but I think might clarify the behavior. What do you think?
Gotcha - will do
When we launch a container in a new user namespace, we cannot create devices, so we bind mount the host's devices into place instead. If we are running in a user namespace (i.e. nested in a container), then we need to do the same thing. Add a function to detect that and check for it before doing mknod. Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- Changelog - add a comment clarifying what's going on with the uidmap file.
|
Is travis being fragile, or am I just failing to see what i did wrong? |
|
@hallyn Thanks! LGTM |
|
LGTM |
Handle running nested in a user namespace
Add PullApprove checks
Therefore check for that. This is a step to being able to run docker/runc containers inside a user namespaced container.
No change in behavior when running in the initial user namespace.
Signed-off-by: Serge Hallyn serge.hallyn@ubuntu.com