From 84bd7e4c9eee9aa419cd4e7658dfba831dea4aa1 Mon Sep 17 00:00:00 2001 From: Dinesh Subhraveti Date: Thu, 19 Jun 2014 02:17:13 -0400 Subject: [PATCH 1/3] Maintain a whitelist of capabilities rather than droplist This fixes 6/18 vulnerability Docker-DCO-1.1-Signed-off-by: Dinesh Subhraveti (github: dineshs-altiscale) Upstream-commit: cf331cdd6ad35c6e0d291df51b49aef5909671f5 Component: engine --- .../engine/daemon/execdriver/lxc/driver.go | 10 -- .../engine/daemon/execdriver/lxc/init.go | 112 ++++++++++++++---- 2 files changed, 87 insertions(+), 35 deletions(-) diff --git a/components/engine/daemon/execdriver/lxc/driver.go b/components/engine/daemon/execdriver/lxc/driver.go index 118eab1987..634ccbd143 100644 --- a/components/engine/daemon/execdriver/lxc/driver.go +++ b/components/engine/daemon/execdriver/lxc/driver.go @@ -19,7 +19,6 @@ import ( "github.com/docker/libcontainer/label" "github.com/docker/libcontainer/mount/nodes" "github.com/dotcloud/docker/daemon/execdriver" - "github.com/dotcloud/docker/pkg/system" "github.com/dotcloud/docker/utils" ) @@ -40,15 +39,6 @@ func init() { if err := setupCapabilities(args); err != nil { return err } - if err := setupWorkingDirectory(args); err != nil { - return err - } - if err := system.CloseFdsFrom(3); err != nil { - return err - } - if err := changeUser(args); err != nil { - return err - } path, err := exec.LookPath(args.Args[0]) if err != nil { diff --git a/components/engine/daemon/execdriver/lxc/init.go b/components/engine/daemon/execdriver/lxc/init.go index e687a9db4d..2f871b5286 100644 --- a/components/engine/daemon/execdriver/lxc/init.go +++ b/components/engine/daemon/execdriver/lxc/init.go @@ -11,6 +11,7 @@ import ( "github.com/docker/libcontainer/netlink" "github.com/dotcloud/docker/daemon/execdriver" + "github.com/dotcloud/docker/pkg/system" "github.com/dotcloud/docker/pkg/user" "github.com/syndtr/gocapability/capability" ) @@ -130,39 +131,100 @@ func changeUser(args *execdriver.InitArgs) error { return nil } -func setupCapabilities(args *execdriver.InitArgs) error { - if args.Privileged { - return nil - } - - drop := []capability.Cap{ - capability.CAP_SETPCAP, - capability.CAP_SYS_MODULE, - capability.CAP_SYS_RAWIO, - capability.CAP_SYS_PACCT, - capability.CAP_SYS_ADMIN, - capability.CAP_SYS_NICE, - capability.CAP_SYS_RESOURCE, - capability.CAP_SYS_TIME, - capability.CAP_SYS_TTY_CONFIG, - capability.CAP_AUDIT_WRITE, - capability.CAP_AUDIT_CONTROL, - capability.CAP_MAC_OVERRIDE, - capability.CAP_MAC_ADMIN, - capability.CAP_NET_ADMIN, - capability.CAP_SYSLOG, - } +var whiteList = []capability.Cap{ + capability.CAP_MKNOD, + capability.CAP_SETUID, + capability.CAP_SETGID, + capability.CAP_CHOWN, + capability.CAP_NET_RAW, + capability.CAP_DAC_OVERRIDE, + capability.CAP_FOWNER, + capability.CAP_FSETID, + capability.CAP_KILL, + capability.CAP_SETGID, + capability.CAP_SETUID, + capability.CAP_LINUX_IMMUTABLE, + capability.CAP_NET_BIND_SERVICE, + capability.CAP_NET_BROADCAST, + capability.CAP_IPC_LOCK, + capability.CAP_IPC_OWNER, + capability.CAP_SYS_CHROOT, + capability.CAP_SYS_PTRACE, + capability.CAP_SYS_BOOT, + capability.CAP_LEASE, + capability.CAP_SETFCAP, + capability.CAP_WAKE_ALARM, + capability.CAP_BLOCK_SUSPEND, +} +func dropBoundingSet() error { c, err := capability.NewPid(os.Getpid()) if err != nil { return err } + c.Clear(capability.BOUNDS) + c.Set(capability.BOUNDS, whiteList...) - c.Unset(capability.CAPS|capability.BOUNDS, drop...) - - if err := c.Apply(capability.CAPS | capability.BOUNDS); err != nil { + if err := c.Apply(capability.BOUNDS); err != nil { return err } + + return nil +} + +const allCapabilityTypes = capability.CAPS | capability.BOUNDS + +func dropCapabilities() error { + c, err := capability.NewPid(os.Getpid()) + if err != nil { + return err + } + c.Clear(allCapabilityTypes) + c.Set(allCapabilityTypes, whiteList...) + + if err := c.Apply(allCapabilityTypes); err != nil { + return err + } + + return nil +} + +func setupCapabilities(args *execdriver.InitArgs) error { + if err := system.CloseFdsFrom(3); err != nil { + return err + } + + if !args.Privileged { + // drop capabilities in bounding set before changing user + if err := dropBoundingSet(); err != nil { + return fmt.Errorf("drop bounding set %s", err) + } + + // preserve existing capabilities while we change users + if err := system.SetKeepCaps(); err != nil { + return fmt.Errorf("set keep caps %s", err) + } + } + + if err := changeUser(args); err != nil { + return err + } + + if !args.Privileged { + if err := system.ClearKeepCaps(); err != nil { + return fmt.Errorf("clear keep caps %s", err) + } + + // drop all other capabilities + if err := dropCapabilities(); err != nil { + return fmt.Errorf("drop capabilities %s", err) + } + } + + if err := setupWorkingDirectory(args); err != nil { + return err + } + return nil } From 1316dc9e2d238c767ef017adb3d864fe42952af7 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Thu, 19 Jun 2014 11:07:57 -0700 Subject: [PATCH 2/3] Use libcontainer cap drop method Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) Upstream-commit: d31ae5aed80eeb40a461930776ad2b507804bf4e Component: engine --- .../engine/daemon/execdriver/lxc/driver.go | 9 +- .../engine/daemon/execdriver/lxc/init.go | 123 ------------------ .../daemon/execdriver/lxc/lxc_init_linux.go | 42 ++++++ .../execdriver/lxc/lxc_init_unsupported.go | 6 + components/engine/pkg/system/unsupported.go | 8 ++ 5 files changed, 64 insertions(+), 124 deletions(-) diff --git a/components/engine/daemon/execdriver/lxc/driver.go b/components/engine/daemon/execdriver/lxc/driver.go index 634ccbd143..24144dc194 100644 --- a/components/engine/daemon/execdriver/lxc/driver.go +++ b/components/engine/daemon/execdriver/lxc/driver.go @@ -19,6 +19,7 @@ import ( "github.com/docker/libcontainer/label" "github.com/docker/libcontainer/mount/nodes" "github.com/dotcloud/docker/daemon/execdriver" + "github.com/dotcloud/docker/pkg/system" "github.com/dotcloud/docker/utils" ) @@ -36,7 +37,13 @@ func init() { if err := setupNetworking(args); err != nil { return err } - if err := setupCapabilities(args); err != nil { + if err := setupWorkingDirectory(args); err != nil { + return err + } + if err := system.CloseFdsFrom(3); err != nil { + return err + } + if err := finalizeNamespace(args); err != nil { return err } diff --git a/components/engine/daemon/execdriver/lxc/init.go b/components/engine/daemon/execdriver/lxc/init.go index 2f871b5286..1af7730cae 100644 --- a/components/engine/daemon/execdriver/lxc/init.go +++ b/components/engine/daemon/execdriver/lxc/init.go @@ -11,9 +11,6 @@ import ( "github.com/docker/libcontainer/netlink" "github.com/dotcloud/docker/daemon/execdriver" - "github.com/dotcloud/docker/pkg/system" - "github.com/dotcloud/docker/pkg/user" - "github.com/syndtr/gocapability/capability" ) // Clear environment pollution introduced by lxc-start @@ -108,126 +105,6 @@ func setupWorkingDirectory(args *execdriver.InitArgs) error { return nil } -// Takes care of dropping privileges to the desired user -func changeUser(args *execdriver.InitArgs) error { - uid, gid, suppGids, err := user.GetUserGroupSupplementary( - args.User, - syscall.Getuid(), syscall.Getgid(), - ) - if err != nil { - return err - } - - if err := syscall.Setgroups(suppGids); err != nil { - return fmt.Errorf("Setgroups failed: %v", err) - } - if err := syscall.Setgid(gid); err != nil { - return fmt.Errorf("Setgid failed: %v", err) - } - if err := syscall.Setuid(uid); err != nil { - return fmt.Errorf("Setuid failed: %v", err) - } - - return nil -} - -var whiteList = []capability.Cap{ - capability.CAP_MKNOD, - capability.CAP_SETUID, - capability.CAP_SETGID, - capability.CAP_CHOWN, - capability.CAP_NET_RAW, - capability.CAP_DAC_OVERRIDE, - capability.CAP_FOWNER, - capability.CAP_FSETID, - capability.CAP_KILL, - capability.CAP_SETGID, - capability.CAP_SETUID, - capability.CAP_LINUX_IMMUTABLE, - capability.CAP_NET_BIND_SERVICE, - capability.CAP_NET_BROADCAST, - capability.CAP_IPC_LOCK, - capability.CAP_IPC_OWNER, - capability.CAP_SYS_CHROOT, - capability.CAP_SYS_PTRACE, - capability.CAP_SYS_BOOT, - capability.CAP_LEASE, - capability.CAP_SETFCAP, - capability.CAP_WAKE_ALARM, - capability.CAP_BLOCK_SUSPEND, -} - -func dropBoundingSet() error { - c, err := capability.NewPid(os.Getpid()) - if err != nil { - return err - } - c.Clear(capability.BOUNDS) - c.Set(capability.BOUNDS, whiteList...) - - if err := c.Apply(capability.BOUNDS); err != nil { - return err - } - - return nil -} - -const allCapabilityTypes = capability.CAPS | capability.BOUNDS - -func dropCapabilities() error { - c, err := capability.NewPid(os.Getpid()) - if err != nil { - return err - } - c.Clear(allCapabilityTypes) - c.Set(allCapabilityTypes, whiteList...) - - if err := c.Apply(allCapabilityTypes); err != nil { - return err - } - - return nil -} - -func setupCapabilities(args *execdriver.InitArgs) error { - if err := system.CloseFdsFrom(3); err != nil { - return err - } - - if !args.Privileged { - // drop capabilities in bounding set before changing user - if err := dropBoundingSet(); err != nil { - return fmt.Errorf("drop bounding set %s", err) - } - - // preserve existing capabilities while we change users - if err := system.SetKeepCaps(); err != nil { - return fmt.Errorf("set keep caps %s", err) - } - } - - if err := changeUser(args); err != nil { - return err - } - - if !args.Privileged { - if err := system.ClearKeepCaps(); err != nil { - return fmt.Errorf("clear keep caps %s", err) - } - - // drop all other capabilities - if err := dropCapabilities(); err != nil { - return fmt.Errorf("drop capabilities %s", err) - } - } - - if err := setupWorkingDirectory(args); err != nil { - return err - } - - return nil -} - func getEnv(args *execdriver.InitArgs, key string) string { for _, kv := range args.Env { parts := strings.SplitN(kv, "=", 2) diff --git a/components/engine/daemon/execdriver/lxc/lxc_init_linux.go b/components/engine/daemon/execdriver/lxc/lxc_init_linux.go index 7288f5877b..6069c4becc 100644 --- a/components/engine/daemon/execdriver/lxc/lxc_init_linux.go +++ b/components/engine/daemon/execdriver/lxc/lxc_init_linux.go @@ -3,9 +3,51 @@ package lxc import ( + "fmt" "syscall" + + "github.com/docker/libcontainer/namespaces" + "github.com/docker/libcontainer/security/capabilities" + "github.com/dotcloud/docker/daemon/execdriver" + "github.com/dotcloud/docker/daemon/execdriver/native/template" + "github.com/dotcloud/docker/pkg/system" ) func setHostname(hostname string) error { return syscall.Sethostname([]byte(hostname)) } + +func finalizeNamespace(args *execdriver.InitArgs) error { + // We use the native drivers default template so that things like caps are consistent + // across both drivers + container := template.New() + + if !args.Privileged { + // drop capabilities in bounding set before changing user + if err := capabilities.DropBoundingSet(container); err != nil { + return fmt.Errorf("drop bounding set %s", err) + } + + // preserve existing capabilities while we change users + if err := system.SetKeepCaps(); err != nil { + return fmt.Errorf("set keep caps %s", err) + } + } + + if err := namespaces.SetupUser(args.User); err != nil { + return fmt.Errorf("setup user %s", err) + } + + if !args.Privileged { + if err := system.ClearKeepCaps(); err != nil { + return fmt.Errorf("clear keep caps %s", err) + } + + // drop all other capabilities + if err := capabilities.DropCapabilities(container); err != nil { + return fmt.Errorf("drop capabilities %s", err) + } + } + + return nil +} diff --git a/components/engine/daemon/execdriver/lxc/lxc_init_unsupported.go b/components/engine/daemon/execdriver/lxc/lxc_init_unsupported.go index d68cb91a1e..079446e186 100644 --- a/components/engine/daemon/execdriver/lxc/lxc_init_unsupported.go +++ b/components/engine/daemon/execdriver/lxc/lxc_init_unsupported.go @@ -2,6 +2,12 @@ package lxc +import "github.com/dotcloud/docker/daemon/execdriver" + func setHostname(hostname string) error { panic("Not supported on darwin") } + +func finalizeNamespace(args *execdriver.InitArgs) error { + panic("Not supported on darwin") +} diff --git a/components/engine/pkg/system/unsupported.go b/components/engine/pkg/system/unsupported.go index 96ebc858f5..aea4b69f97 100644 --- a/components/engine/pkg/system/unsupported.go +++ b/components/engine/pkg/system/unsupported.go @@ -28,3 +28,11 @@ func GetClockTicks() int { func CreateMasterAndConsole() (*os.File, string, error) { return nil, "", ErrNotSupportedPlatform } + +func SetKeepCaps() error { + return ErrNotSupportedPlatform +} + +func ClearKeepCaps() error { + return ErrNotSupportedPlatform +} From 16f6e0948828114d403d577e7bd26ea555625e98 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Thu, 19 Jun 2014 11:57:09 -0700 Subject: [PATCH 3/3] Update close fd issues for lxc Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) Upstream-commit: 707ef9618b3b26a0534a0af732a22f159eccfaa5 Component: engine --- components/engine/daemon/execdriver/lxc/driver.go | 7 ------- .../engine/daemon/execdriver/lxc/lxc_init_linux.go | 9 +++++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/components/engine/daemon/execdriver/lxc/driver.go b/components/engine/daemon/execdriver/lxc/driver.go index 24144dc194..59daf1afe1 100644 --- a/components/engine/daemon/execdriver/lxc/driver.go +++ b/components/engine/daemon/execdriver/lxc/driver.go @@ -19,7 +19,6 @@ import ( "github.com/docker/libcontainer/label" "github.com/docker/libcontainer/mount/nodes" "github.com/dotcloud/docker/daemon/execdriver" - "github.com/dotcloud/docker/pkg/system" "github.com/dotcloud/docker/utils" ) @@ -37,12 +36,6 @@ func init() { if err := setupNetworking(args); err != nil { return err } - if err := setupWorkingDirectory(args); err != nil { - return err - } - if err := system.CloseFdsFrom(3); err != nil { - return err - } if err := finalizeNamespace(args); err != nil { return err } diff --git a/components/engine/daemon/execdriver/lxc/lxc_init_linux.go b/components/engine/daemon/execdriver/lxc/lxc_init_linux.go index 6069c4becc..3b15d096af 100644 --- a/components/engine/daemon/execdriver/lxc/lxc_init_linux.go +++ b/components/engine/daemon/execdriver/lxc/lxc_init_linux.go @@ -8,6 +8,7 @@ import ( "github.com/docker/libcontainer/namespaces" "github.com/docker/libcontainer/security/capabilities" + "github.com/docker/libcontainer/utils" "github.com/dotcloud/docker/daemon/execdriver" "github.com/dotcloud/docker/daemon/execdriver/native/template" "github.com/dotcloud/docker/pkg/system" @@ -18,6 +19,10 @@ func setHostname(hostname string) error { } func finalizeNamespace(args *execdriver.InitArgs) error { + if err := utils.CloseExecFrom(3); err != nil { + return err + } + // We use the native drivers default template so that things like caps are consistent // across both drivers container := template.New() @@ -49,5 +54,9 @@ func finalizeNamespace(args *execdriver.InitArgs) error { } } + if err := setupWorkingDirectory(args); err != nil { + return err + } + return nil }