From 1a913c7b896387b6341fef1bedac8b4d8cdd290e Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sat, 23 Apr 2016 23:39:38 +1000 Subject: [PATCH] *: correctly chown() consoles In user namespaces, we need to make sure we don't chown() the console to unmapped users. This means we need to get both the UID and GID of the root user in the container when changing the owner. Signed-off-by: Aleksa Sarai --- libcontainer/integration/execin_test.go | 2 +- libcontainer/process.go | 4 ++-- libcontainer/process_linux.go | 4 ++-- restore.go | 3 ++- tty.go | 8 ++++---- utils_linux.go | 19 ++++++++++++------- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index da920ddb..cd101cc2 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -218,7 +218,7 @@ func TestExecInTTY(t *testing.T) { Args: []string{"ps"}, Env: standardEnvironment, } - console, err := ps.NewConsole(0) + console, err := ps.NewConsole(0, 0) copy := make(chan struct{}) go func() { io.Copy(&stdout, console) diff --git a/libcontainer/process.go b/libcontainer/process.go index 91e8ef56..46fb2749 100644 --- a/libcontainer/process.go +++ b/libcontainer/process.go @@ -102,8 +102,8 @@ type IO struct { } // NewConsole creates new console for process and returns it -func (p *Process) NewConsole(rootuid int) (Console, error) { - console, err := NewConsole(rootuid, rootuid) +func (p *Process) NewConsole(rootuid, rootgid int) (Console, error) { + console, err := NewConsole(rootuid, rootgid) if err != nil { return nil, err } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 2b6b29b0..3a34b130 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -447,7 +447,7 @@ func getPipeFds(pid int) ([]string, error) { // InitializeIO creates pipes for use with the process's STDIO // and returns the opposite side for each -func (p *Process) InitializeIO(rootuid int) (i *IO, err error) { +func (p *Process) InitializeIO(rootuid, rootgid int) (i *IO, err error) { var fds []uintptr i = &IO{} // cleanup in case of an error @@ -479,7 +479,7 @@ func (p *Process) InitializeIO(rootuid int) (i *IO, err error) { p.Stderr, i.Stderr = w, r // change ownership of the pipes incase we are in a user namespace for _, fd := range fds { - if err := syscall.Fchown(int(fd), rootuid, rootuid); err != nil { + if err := syscall.Fchown(int(fd), rootuid, rootgid); err != nil { return nil, err } } diff --git a/restore.go b/restore.go index f176c414..3f1a2c0d 100644 --- a/restore.go +++ b/restore.go @@ -117,6 +117,7 @@ using the runc checkpoint command.`, func restoreContainer(context *cli.Context, spec *specs.Spec, config *configs.Config, imagePath string) (code int, err error) { var ( rootuid = 0 + rootgid = 0 id = context.Args().First() ) factory, err := loadFactory(context) @@ -149,7 +150,7 @@ func restoreContainer(context *cli.Context, spec *specs.Spec, config *configs.Co defer destroy(container) } process := &libcontainer.Process{} - tty, err := setupIO(process, rootuid, "", false, detach) + tty, err := setupIO(process, rootuid, rootgid, "", false, detach) if err != nil { return -1, err } diff --git a/tty.go b/tty.go index 80c65515..5a76ebe3 100644 --- a/tty.go +++ b/tty.go @@ -14,8 +14,8 @@ import ( // setup standard pipes so that the TTY of the calling runc process // is not inherited by the container. -func createStdioPipes(p *libcontainer.Process, rootuid int) (*tty, error) { - i, err := p.InitializeIO(rootuid) +func createStdioPipes(p *libcontainer.Process, rootuid, rootgid int) (*tty, error) { + i, err := p.InitializeIO(rootuid, rootgid) if err != nil { return nil, err } @@ -52,14 +52,14 @@ func (t *tty) copyIO(w io.Writer, r io.ReadCloser) { r.Close() } -func createTty(p *libcontainer.Process, rootuid int, consolePath string) (*tty, error) { +func createTty(p *libcontainer.Process, rootuid, rootgid int, consolePath string) (*tty, error) { if consolePath != "" { if err := p.ConsoleFromPath(consolePath); err != nil { return nil, err } return &tty{}, nil } - console, err := p.NewConsole(rootuid) + console, err := p.NewConsole(rootuid, rootgid) if err != nil { return nil, err } diff --git a/utils_linux.go b/utils_linux.go index bafb6417..c71df0d5 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -92,7 +92,7 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) { return lp, nil } -func dupStdio(process *libcontainer.Process, rootuid int) error { +func dupStdio(process *libcontainer.Process, rootuid, rootgid int) error { process.Stdin = os.Stdin process.Stdout = os.Stdout process.Stderr = os.Stderr @@ -101,7 +101,7 @@ func dupStdio(process *libcontainer.Process, rootuid int) error { os.Stdout.Fd(), os.Stderr.Fd(), } { - if err := syscall.Fchown(int(fd), rootuid, rootuid); err != nil { + if err := syscall.Fchown(int(fd), rootuid, rootgid); err != nil { return err } } @@ -123,22 +123,22 @@ func destroy(container libcontainer.Container) { // setupIO sets the proper IO on the process depending on the configuration // If there is a nil error then there must be a non nil tty returned -func setupIO(process *libcontainer.Process, rootuid int, console string, createTTY, detach bool) (*tty, error) { +func setupIO(process *libcontainer.Process, rootuid, rootgid int, console string, createTTY, detach bool) (*tty, error) { // detach and createTty will not work unless a console path is passed // so error out here before changing any terminal settings if createTTY && detach && console == "" { return nil, fmt.Errorf("cannot allocate tty if runc will detach") } if createTTY { - return createTty(process, rootuid, console) + return createTty(process, rootuid, rootgid, console) } if detach { - if err := dupStdio(process, rootuid); err != nil { + if err := dupStdio(process, rootuid, rootgid); err != nil { return nil, err } return &tty{}, nil } - return createStdioPipes(process, rootuid) + return createStdioPipes(process, rootuid, rootgid) } // createPidFile creates a file with the processes pid inside it atomically @@ -215,7 +215,12 @@ func (r *runner) run(config *specs.Process) (int, error) { r.destroy() return -1, err } - tty, err := setupIO(process, rootuid, r.console, config.Terminal, r.detach) + rootgid, err := r.container.Config().HostGID() + if err != nil { + r.destroy() + return -1, err + } + tty, err := setupIO(process, rootuid, rootgid, r.console, config.Terminal, r.detach) if err != nil { r.destroy() return -1, err