Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Support mount, uts and ipc namespaces in the host #847

Closed
wants to merge 5 commits into from

Conversation

devimc
Copy link

@devimc devimc commented Oct 19, 2018

Unlike other runtimes, we have several components like qemu, kata-shim and
kata-proxy that should be placed in new namespace to improve the isolation and
security. The problem is that we need persistent namespaces and in the case of
the mount namespace (aka headache namespace ;)), it MUST BE created before
starting golang execution and in a new mount point with a propagation different
to shared, even worse, to make it persistent the new namespace
(/proc/PID/ns/mnt) MUST be 'bind mounted' by a process that doesn't know of the
existence of it. To get more information about this, see
github.com/karelzak/util-linux/commit/c84f2590dfdb8570beeb731e0105f8fe597443d1

With these new functions now is possible to join, create, remove and make
persistent namespaces. Next diagram can help us to understand how it works.

 ============================================================================
| Host's namespace                                    | New namespace!       |
|                                                     |                      |
| ---------------------------------------------------------------------------|
| C (no threads, no goroutines, NO GOLANG!)           |                      |
|                                                     |                      |
|  ---------   ---------------                        |                      |
| | Runtime |-|listen children|--|fork-setns-unshare|-|-------.              |
|  ---------   ---------------\                       |       |              |
|      |               |       \                      |       |              |
|    fork              |        Wait children &       |       |              |
|      |               |             exit             |       |              |
|----------------------------------------------------------------------------|
| Golang               |                              |       |              |
|      |        join,create,make,remove               |       |              |
|  ---------    persistent namespace                  |   ---------          |
| | Runtime |          |                              |  | Runtime |         |
|  ---------        --------                          |   ---------          |
|      |           |-Create |                         |       |              |
|  parse jsons     |-Start  |                         | Create,start,etc.    |
|  and cmdline     |-Delete |                         |   Containers         |
|       \          |-etc    |                         |       |              |
|        \         |--------                          |      exit            |
|         --subcmd-'                                  |                      |
|          \                                          |                      |
|           - exit                                    |                      |
============================================================================

fixes #160

Depends-on: github.com/kata-containers/tests#946

Signed-off-by: Julio Montes julio.montes@intel.com

@devimc
Copy link
Author

devimc commented Oct 19, 2018

/test

@devimc
Copy link
Author

devimc commented Oct 19, 2018

/test

@sboeuf
Copy link

sboeuf commented Oct 19, 2018

@devimc thanks for the PR, but please be more verbose about the problem statement, and the reasons behind the choices you made. It is a pretty huge PR affecting main components, and it needs to be clear about the why and how.

@devimc devimc changed the title Support mount namespace in the host [WIP] Support mount namespace in the host Oct 19, 2018
@devimc
Copy link
Author

devimc commented Oct 19, 2018

@sboeuf sure, I still need to write a document

@devimc
Copy link
Author

devimc commented Oct 19, 2018

/test

@jshachm
Copy link
Member

jshachm commented Oct 20, 2018

waiting for the doc~ @devimc and maybe we need to take mount ns leak into consideration

@devimc devimc force-pushed the topic/mntns branch 5 times, most recently from 536e3b6 to f3f421c Compare October 23, 2018 15:20
@devimc
Copy link
Author

devimc commented Oct 23, 2018

/test

@devimc
Copy link
Author

devimc commented Oct 23, 2018

/test

@devimc devimc force-pushed the topic/mntns branch 11 times, most recently from cf39256 to 8ffdab1 Compare October 25, 2018 18:32
@devimc devimc changed the title [WIP] Support mount namespace in the host Support mount, utc and ipc namespaces in the host Oct 25, 2018
@devimc
Copy link
Author

devimc commented Nov 26, 2018

/test

@devimc
Copy link
Author

devimc commented Nov 27, 2018

/test

devimc pushed a commit to devimc/kata-tests that referenced this pull request Nov 27, 2018
Skip crio test 'ctr list filtering' to allow
kata-containers/runtime#847 lands

fixes kata-containers#945

Signed-off-by: Julio Montes <julio.montes@intel.com>
Julio Montes added 5 commits November 27, 2018 10:36
Unlike other runtimes, we have several components like qemu, kata-shim and
kata-proxy that should be placed in new namespace to improve the isolation and
security. The problem is that we need persistent namespaces and in the case of
the mount namespace (aka headache namespace ;)), it MUST BE created before
starting golang execution and in a new mount point with a propagation different
to shared, even worse, to make it persistent the new namespace
(/proc/PID/ns/mnt) MUST be 'bind mounted' by a process that doesn't know of the
existence of it. To get more information about this, see
github.com/karelzak/util-linux/commit/c84f2590dfdb8570beeb731e0105f8fe597443d1

With these new functions now is possible to join, create, remove and make
persistent namespaces. Next diagram can help us to understand how it works.

```
 ============================================================================
| Host's namespace                                    | New namespace!       |
|                                                     |                      |
| ---------------------------------------------------------------------------|
| C (no threads, no goroutines, NO GOLANG!)           |                      |
|                                                     |                      |
|  ---------   ---------------                        |                      |
| | Runtime |-|listen children|--|fork-setns-unshare|-|-------.              |
|  ---------   ---------------\                       |       |              |
|      |               |       \                      |       |              |
|    fork              |        Wait children &       |       |              |
|      |               |             exit             |       |              |
|----------------------------------------------------------------------------|
| Golang               |                              |       |              |
|      |        join,create,make,remove               |       |              |
|  ---------    persistent namespace                  |   ---------          |
| | Runtime |          |                              |  | Runtime |         |
|  ---------        --------                          |   ---------          |
|      |           |-Create |                         |       |              |
|  parse jsons     |-Start  |                         | Create,start,etc.    |
|  and cmdline     |-Delete |                         |   Containers         |
|       \          |-etc    |                         |       |              |
|        \         |--------                          |      exit            |
|         --subcmd-'                                  |                      |
|          \                                          |                      |
|           - exit                                    |                      |
 ============================================================================
```

fixes kata-containers#160

Depends-on: github.com/kata-containers/tests#946

Signed-off-by: Julio Montes <julio.montes@intel.com>
setup the console in create, after tracing and once the container ID
has been obtained.

Signed-off-by: Julio Montes <julio.montes@intel.com>
Depending of the command and the container type the runtime
creates, joins, makes or removes a persistent namespace.
When container type is a container, the rootfs for this container
must be mounted on create and unmounted on kill or delete, since
it's not visible in the sanbox namespace.

Signed-off-by: Julio Montes <julio.montes@intel.com>
shim is already started in a new UTC and IPC namespace, hence clone flags are
no more needed when it's started.

Signed-off-by: Julio Montes <julio.montes@intel.com>
In order to avoid file descriptor leaks, the communication channel with the
parent must be closed.

Signed-off-by: Julio Montes <julio.montes@intel.com>
@devimc
Copy link
Author

devimc commented Nov 27, 2018

/test

@devimc
Copy link
Author

devimc commented Nov 27, 2018

@kata-containers/runtime @bergwolf @gnawux @sboeuf please take a look

@amshinde
Copy link
Member

amshinde commented Dec 4, 2018

@devimc This needs a rebase.
We talked about having this in the architecture committee, I know I missed the last one. Was this discussed at all?

@raravena80
Copy link
Member

@devimc any updates?

@devimc
Copy link
Author

devimc commented Dec 18, 2018

@raravena80 ehmm nop, I'd like to present this in the kata arch meeting

@jodh-intel
Copy link
Contributor

Hi @devimc - Thanks for raising! As @sboeuf has noted, this is a complex one. It does kind of feel like this issue should be handled by fixing golang rather than us having to jump through "hoops of fire" as it were ;) On that topic, is anyone aware of a golang github issue regarding the namespace issues? I took a quick look but didn't find anything useful (ironically, searching on google lead me back to the Kata github project! ;)

Awesome ASCII art btw, but it would be helpful if you could provide a step-by-step explanation of how this works to allow us all to "digest" this PR more quickly.

@caoruidong
Copy link
Member

What issue? The namespace problem fixed in golang 1.10? golang/go#20676 #148

@jodh-intel
Copy link
Contributor

@caoruidong - I was referring to @sboeuf's comment:

It's very interesting and complex as Golang is really not friendly with namespaces.

@devimc
Copy link
Author

devimc commented Jan 8, 2019

@jodh-intel golang/go#8676 Status changed to WontFix

@jodh-intel
Copy link
Contributor

@devimc - This has been open for a while now (and the branch is now conflicted).

I think there is a general feeling that although this PR solves a problem, it introduces more complexity that we are comfortable with, unless there is a very good reason for it. It also introduces C code. I understand why that is, but again, I think we'd rather not use C unless there is a super-compelling reason for it due to the potential security implications. It would also require us to setup new static analysis, etc, etc.

Hence, please could you / @amshinde provide further details on why we need this with, ideally with some examples. Trying to trace back through the issues leads to the initial issue... containing no details :-((

I think you did present this at the Architecture Committee meeting? Please could you add details here of what the feedback was. If there wasn't any, or wasn't enough, I think it might be worth you raising this topic again in the AC meeting. Now that folk have had time to consider the impact of adding this change we might get more feedback.

@devimc devimc closed this Jan 23, 2019
@devimc devimc deleted the topic/mntns branch April 8, 2019 14:52
egernst added a commit to egernst/runtime that referenced this pull request Feb 9, 2021
github: Remove issue template and use central one
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable mount namespace
10 participants