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

Add support for ambient capabilities #26979

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Conversation

justincormack
Copy link
Contributor

@justincormack justincormack commented Sep 28, 2016

This change was reverted for docker 1.13 (see #27737)

Linux kernel 4.3 and later supports "ambient capabilities" which are the
only way to pass capabilities to containers running as a non root uid.

Previously there was no way to allow containers not running as root
capabilities in a useful way.

Fix #8460

Signed-off-by: Justin Cormack [email protected]

NOTE: actual runc update was included in #27160 so this is just a docs and tests change now.

NOTE: This will grant the default capabilities to containers that are run as a non root user. This might be surprising, for example we grant cap_chown by default, so with this PR docker run -u 1000 busybox chown 100 /tmp will just work. We could grant fewer capabilities to containers not running as root, by default, but this might break users who expect sudo chown ... to work. In general as running as non root is strictly better than running as root, I think it is best to leave this, as the default capabilities are mostly harmless, and I am planning other patches to allow reducing the default capability set globally. Open to discussion on this.

Chill Out with Ambient Capabilities

chill out

@justincormack
Copy link
Contributor Author

cc @mrunalp @crosbymichael

@cpuguy83
Copy link
Member

ping @tianon too

testRequires(c, DaemonIsLinux, ambientCapabilities)

// test that a non root user can gain capabilities
runCmd := exec.Command(dockerBinary, "run", "--user", "1000", "--cap-add chown", "busybox", "chown", "100", "/tmp")
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a failing test as well, to verify that it won't work without this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@justincormack
Copy link
Contributor Author

The test failures look like they are related to other changes in runc, I will look at them seperately.

@justincormack justincormack mentioned this pull request Sep 28, 2016
@mrunalp
Copy link
Contributor

mrunalp commented Sep 28, 2016

I tested this and it works fine for me 👍

@jeremyeder
Copy link

@mrunalp Ambient Capabilities are available in RHEL7.3.

@mrunalp
Copy link
Contributor

mrunalp commented Sep 29, 2016

@jeremyeder Awesome. I had only tested on RHEL 7.2 and wasn't sure about status in 7.3

justincormack added a commit to justincormack/docker that referenced this pull request Oct 5, 2016
Found some issues when working on moby#26979 which looked unrelated to that PR
but may be related to other changes.

Signed-off-by: Justin Cormack <[email protected]>
to write files belonging to the root uid, for example. User namespaces
also allow capabilities to be granted to processes that are effectively
non root, but these capabilities are limited to resources created in the
user namespace, so they have limitations.
Copy link
Member

Choose a reason for hiding this comment

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

This now needs to be moved somewhere in the https://github.com/docker/docker.gihub.io repository (I'm ok with keeping it here as well, as that repository does not yet have docs for 1.13 it seems)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Linux kernel 4.3 and later supports "ambient capabilities" which are the
only way to pass capabilities to containers running as a non root uid.

Previously there was no way to allow containers not running as root
capabilities in a useful way.

Fix moby#8460

Signed-off-by: Justin Cormack <[email protected]>
@crosbymichael
Copy link
Contributor

LGTM

@thaJeztah thaJeztah added this to the 1.13.0 milestone Oct 10, 2016
@thaJeztah
Copy link
Member

Given that there's no docs for 1.13 yet in the https://github.com/docker/docker.gihub.io repository, I'm moving this to "merge". There'll probably be a sync with that repository in the near future.

/cc @johndmulhausen @mstanleyjones

@justincormack justincormack merged commit 199e195 into moby:master Oct 10, 2016
@justincormack justincormack deleted the ambient branch October 10, 2016 18:29
@mdlinville
Copy link
Contributor

I got this into the vnext-engine branch on docker/docker.github.io repository, which is where new features for Engine should now be documented. That branch currently has all doc-related commits in docker/docker master up to SHA1 4331b62.

@thaJeztah
Copy link
Member

@mstanleyjones I was about to open a pull request to revert the documentation changes in this PR, but I cannot find these changes in your sync of october 10th; https://github.com/docker/docker.github.io/blob/e4bce35ac2d2963e0f52414ca98742c60e8df510/engine/security/seccomp.md

Should we check if things got lost in that sync?

@mdlinville
Copy link
Contributor

Merged #584 which addresses @thaJeztah comment above. Thanks!

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Jul 8, 2019
Removes some test functions that were unused:

- bridgeNfIP6tables
- ambientCapabilities (added to support moby#26979, which was reverted in moby#27737)
- overlay2Supported

Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 25, 2019
Removes some test functions that were unused:

- bridgeNfIP6tables
- ambientCapabilities (added to support moby#26979, which was reverted in moby#27737)
- overlay2Supported

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit c887b09)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
burnMyDread pushed a commit to burnMyDread/moby that referenced this pull request Oct 21, 2019
Removes some test functions that were unused:

- bridgeNfIP6tables
- ambientCapabilities (added to support moby#26979, which was reverted in moby#27737)
- overlay2Supported

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: zach <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Oct 23, 2019
Removes some test functions that were unused:

- bridgeNfIP6tables
- ambientCapabilities (added to support moby#26979, which was reverted in moby#27737)
- overlay2Supported

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit c887b09)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants