From 78a609321c6a0a589554b73b0d2e3ab4ff84a1f3 Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Sun, 22 Jan 2017 19:36:22 +0000 Subject: [PATCH] Always use ambient capabilities if no new privileges set, not otherwise Currently we have a build flag to enable ambient capabilities. However this is not a good situation as it means different installations of runc may have significantly different behaviour, which can cause security issues. The reason we introduced this was because that enabling ambient capabilities at all times caused issues with some use cases. There are essentially two ways that capabilities are used, which can be characterised as - "modern" applications which want to grant capabilities directly to applictions, including when they run as a non root user - "traditional" which look more like a traditional Linux distro and wish to mediate capabilities for non root users via suid files or with filesystem capabilities. Granting ambient capabilities causes an issue with the second case as the capabilities are immediately granted to non root processes, rather than only being granted when suid files or files with capabilities are executed, for example `sudo`. This PR removes the build flag, so that all builds are the same. In order to accomodate both types of use case, it makes use of the fact that of the "no new privileges" flag is set in the config, then it is no longer possible for suid or filesystem capabilities to have any effect, so the second use case is not possible. So we only apply ambient capabilities if this is the case. So traditional applications that want to use suid binaries or fs caps should not set the no new privileges flag in the config, and there will be no change in behaviour. Applications that wish to run as a non root user, and grant cpabilities that may be directly used can set the no new provileges flag, and then they will be granted ambient privileges, if the kernel version supports them (since 4.3). Old kernel versions are uncapable of granting direct capabilities to non root processes. The changes that are required for applications using runc after this commit are: - if they were using the old "ambient" build flag, they should set "no new privileges" if they were not before. - programs using the build without ambient capabilities (such as Docker) can use this build. If run without no new capabilities, there will be no change, but with this flag they can grant capabiltiies directly to non root processes. They may wish to consider which capabilties are granted by default in this case; I will make changes to Docker for this. Having all builds have the same security setup is the only safe setup, at present it is easy for a user to accidentally use a build with a different flag setting, causing unexpected privileges to be granted. The setup as in this commit should give a sane setup for all kinds of use case with a single binary build. I will prepare a PR for the runtime specification that says this behaviour is correct. Signed-off-by: Justin Cormack --- Makefile | 2 +- README.md | 1 - libcontainer/capabilities_ambient.go | 7 ------- libcontainer/capabilities_linux.go | 8 +++++++- libcontainer/capabilities_noambient.go | 7 ------- libcontainer/init_linux.go | 2 +- 6 files changed, 9 insertions(+), 18 deletions(-) delete mode 100644 libcontainer/capabilities_ambient.go delete mode 100644 libcontainer/capabilities_noambient.go diff --git a/Makefile b/Makefile index 819bad09ca7..ec61673f7c4 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ static: $(SOURCES) | $(RUNC_LINK) CGO_ENABLED=1 go build -i -tags "$(BUILDTAGS) cgo static_build" -ldflags "-w -extldflags -static -X main.gitCommit=${COMMIT} -X main.version=${VERSION}" -o contrib/cmd/recvtty/recvtty ./contrib/cmd/recvtty release: $(RUNC_LINK) | $(RUNC_LINK) - @flag_list=(seccomp selinux apparmor static ambient); \ + @flag_list=(seccomp selinux apparmor static); \ unset expression; \ for flag in "$${flag_list[@]}"; do \ expression+="' '{'',$${flag}}"; \ diff --git a/README.md b/README.md index ec9b44ed818..d36f2e56e70 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,6 @@ make BUILDTAGS='seccomp apparmor' | seccomp | Syscall filtering | libseccomp | | selinux | selinux process and mount labeling | | | apparmor | apparmor profile support | libapparmor | -| ambient | ambient capability support | kernel 4.3 | ### Running the test suite diff --git a/libcontainer/capabilities_ambient.go b/libcontainer/capabilities_ambient.go deleted file mode 100644 index 50da2832fbb..00000000000 --- a/libcontainer/capabilities_ambient.go +++ /dev/null @@ -1,7 +0,0 @@ -// +build linux,ambient - -package libcontainer - -import "github.com/syndtr/gocapability/capability" - -const allCapabilityTypes = capability.CAPS | capability.BOUNDS | capability.AMBS diff --git a/libcontainer/capabilities_linux.go b/libcontainer/capabilities_linux.go index 31fd0dcf563..455509bc763 100644 --- a/libcontainer/capabilities_linux.go +++ b/libcontainer/capabilities_linux.go @@ -60,7 +60,13 @@ func (w *whitelist) dropBoundingSet() error { } // drop drops all capabilities for the current process except those specified in the whitelist. -func (w *whitelist) drop() error { +// in the case where NoNewPrivileges is set, so sudo and fs capabilities cannot be used, we can +// use ambient capabilities so that non root users will gain capabilities +func (w *whitelist) drop(nnp bool) error { + allCapabilityTypes := capability.CAPS | capability.BOUNDS + if nnp { + allCapabilityTypes |= capability.AMBS + } w.pid.Clear(allCapabilityTypes) w.pid.Set(allCapabilityTypes, w.keep...) return w.pid.Apply(allCapabilityTypes) diff --git a/libcontainer/capabilities_noambient.go b/libcontainer/capabilities_noambient.go deleted file mode 100644 index 752c4e51676..00000000000 --- a/libcontainer/capabilities_noambient.go +++ /dev/null @@ -1,7 +0,0 @@ -// +build !ambient,linux - -package libcontainer - -import "github.com/syndtr/gocapability/capability" - -const allCapabilityTypes = capability.CAPS | capability.BOUNDS diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 9d5f068025f..f93be066087 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -141,7 +141,7 @@ func finalizeNamespace(config *initConfig) error { return err } // drop all other capabilities - if err := w.drop(); err != nil { + if err := w.drop(config.NoNewPrivileges); err != nil { return err } if config.Cwd != "" {