diff options
author | Nicolas Lacasse <nlacasse@google.com> | 2018-08-24 17:42:30 -0700 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-08-24 17:43:21 -0700 |
commit | 106de2182d34197d76fb68863cd4a102ebac2dbb (patch) | |
tree | 9f3bce620feedb1c7f757c079157538c33b94a5a | |
parent | c48708a041fcc9749e0162a7708f32e5a3d7e526 (diff) |
runsc: Terminal support for "docker exec -ti".
This CL adds terminal support for "docker exec". We previously only supported
consoles for the container process, but not exec processes.
The SYS_IOCTL syscall was added to the default seccomp filter list, but only
for ioctls that get/set winsize and termios structs. We need to allow these
ioctl for all containers because it's possible to run "exec -ti" on a
container that was started without an attached console, after the filters
have been installed.
Note that control-character signals are still not properly supported.
Tested with:
$ docker run --runtime=runsc -it alpine
In another terminial:
$ docker exec -it <containerid> /bin/sh
PiperOrigin-RevId: 210185456
Change-Id: I6d2401e53a7697bb988c120a8961505c335f96d9
-rw-r--r-- | pkg/abi/linux/ioctl.go | 6 | ||||
-rw-r--r-- | pkg/abi/linux/tty.go | 8 | ||||
-rw-r--r-- | pkg/sentry/control/proc.go | 17 | ||||
-rw-r--r-- | pkg/sentry/fs/host/BUILD | 1 | ||||
-rw-r--r-- | pkg/sentry/fs/host/file.go | 19 | ||||
-rw-r--r-- | pkg/sentry/fs/host/ioctl_unsafe.go | 19 | ||||
-rw-r--r-- | runsc/boot/controller.go | 2 | ||||
-rw-r--r-- | runsc/boot/filter/BUILD | 1 | ||||
-rw-r--r-- | runsc/boot/filter/config.go | 38 | ||||
-rw-r--r-- | runsc/boot/filter/filter.go | 6 | ||||
-rw-r--r-- | runsc/boot/loader.go | 2 | ||||
-rw-r--r-- | runsc/cmd/BUILD | 1 | ||||
-rw-r--r-- | runsc/cmd/exec.go | 39 | ||||
-rw-r--r-- | runsc/console/BUILD | 16 | ||||
-rw-r--r-- | runsc/console/console.go (renamed from runsc/sandbox/console.go) | 9 | ||||
-rw-r--r-- | runsc/sandbox/BUILD | 3 | ||||
-rw-r--r-- | runsc/sandbox/sandbox.go | 20 |
17 files changed, 151 insertions, 56 deletions
diff --git a/pkg/abi/linux/ioctl.go b/pkg/abi/linux/ioctl.go index 3ef046562..4d7a2dfd7 100644 --- a/pkg/abi/linux/ioctl.go +++ b/pkg/abi/linux/ioctl.go @@ -21,8 +21,12 @@ const ( TCGETS = 0x00005401 TCSETS = 0x00005402 TCSETSW = 0x00005403 - TIOCINQ = 0x0000541b + TIOCGPGRP = 0x0000540f + TIOCSPGRP = 0x00005410 TIOCOUTQ = 0x00005411 + TIOCGWINSZ = 0x00005413 + TIOCSWINSZ = 0x00005414 + TIOCINQ = 0x0000541b FIONREAD = TIOCINQ FIONBIO = 0x00005421 TIOCGPTN = 0x80045430 diff --git a/pkg/abi/linux/tty.go b/pkg/abi/linux/tty.go index 8c611d22a..81156867c 100644 --- a/pkg/abi/linux/tty.go +++ b/pkg/abi/linux/tty.go @@ -26,6 +26,14 @@ const ( disabledChar = 0 ) +// Winsize is struct winsize, defined in uapi/asm-generic/termios.h. +type Winsize struct { + Row uint16 + Col uint16 + Xpixel uint16 + Ypixel uint16 +} + // Termios is struct termios, defined in uapi/asm-generic/termbits.h. type Termios struct { InputFlags uint32 diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index d94ae560f..2493c5175 100644 --- a/pkg/sentry/control/proc.go +++ b/pkg/sentry/control/proc.go @@ -19,7 +19,6 @@ import ( "encoding/json" "fmt" "sort" - "syscall" "text/tabwriter" "time" @@ -73,6 +72,10 @@ type ExecArgs struct { // Capabilities is the list of capabilities to give to the process. Capabilities *auth.TaskCapabilities + // StdioIsPty indicates that FDs 0, 1, and 2 are connected to a host + // pty fd. + StdioIsPty bool + // FilePayload determines the files to give to the new process. urpc.FilePayload } @@ -108,17 +111,11 @@ func (proc *Proc) Exec(args *ExecArgs, waitStatus *uint32) error { mounter := fs.FileOwnerFromContext(ctx) for appFD, f := range args.FilePayload.Files { - // Copy the underlying FD. - newFD, err := syscall.Dup(int(f.Fd())) - if err != nil { - return err - } - f.Close() + enableIoctl := args.StdioIsPty && appFD <= 2 - // Install the given file as an FD. - file, err := host.NewFile(ctx, newFD, mounter) + // Import the given file FD. This dups the FD as well. + file, err := host.ImportFile(ctx, int(f.Fd()), mounter, enableIoctl) if err != nil { - syscall.Close(newFD) return err } defer file.DecRef() diff --git a/pkg/sentry/fs/host/BUILD b/pkg/sentry/fs/host/BUILD index 29c79284a..f1252b0f2 100644 --- a/pkg/sentry/fs/host/BUILD +++ b/pkg/sentry/fs/host/BUILD @@ -48,7 +48,6 @@ go_library( "//pkg/unet", "//pkg/waiter", "//pkg/waiter/fdnotifier", - "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/pkg/sentry/fs/host/file.go b/pkg/sentry/fs/host/file.go index f9bef6d93..8d2463c78 100644 --- a/pkg/sentry/fs/host/file.go +++ b/pkg/sentry/fs/host/file.go @@ -18,7 +18,6 @@ import ( "fmt" "syscall" - "golang.org/x/sys/unix" "gvisor.googlesource.com/gvisor/pkg/abi/linux" "gvisor.googlesource.com/gvisor/pkg/fd" "gvisor.googlesource.com/gvisor/pkg/log" @@ -296,7 +295,7 @@ func (f *fileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.Sys fd := f.iops.fileState.FD() ioctl := args[1].Uint64() switch ioctl { - case unix.TCGETS: + case linux.TCGETS: termios, err := ioctlGetTermios(fd) if err != nil { return 0, err @@ -306,7 +305,7 @@ func (f *fileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.Sys }) return 0, err - case unix.TCSETS, unix.TCSETSW: + case linux.TCSETS, linux.TCSETSW: var termios linux.Termios if _, err := usermem.CopyObjectIn(ctx, io, args[2].Pointer(), &termios, usermem.IOOpts{ AddressSpaceActive: true, @@ -316,7 +315,7 @@ func (f *fileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.Sys err := ioctlSetTermios(fd, ioctl, &termios) return 0, err - case unix.TIOCGPGRP: + case linux.TIOCGPGRP: // Args: pid_t *argp // When successful, equivalent to *argp = tcgetpgrp(fd). // Get the process group ID of the foreground process group on @@ -332,7 +331,7 @@ func (f *fileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.Sys }) return 0, err - case unix.TIOCSPGRP: + case linux.TIOCSPGRP: // Args: const pid_t *argp // Equivalent to tcsetpgrp(fd, *argp). // Set the foreground process group ID of this terminal. @@ -343,10 +342,10 @@ func (f *fileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.Sys log.Warningf("Ignoring application ioctl(TIOCSPGRP) call") return 0, nil - case unix.TIOCGWINSZ: + case linux.TIOCGWINSZ: // Args: struct winsize *argp // Get window size. - winsize, err := unix.IoctlGetWinsize(fd, unix.TIOCGWINSZ) + winsize, err := ioctlGetWinsize(fd) if err != nil { return 0, err } @@ -355,16 +354,16 @@ func (f *fileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.Sys }) return 0, err - case unix.TIOCSWINSZ: + case linux.TIOCSWINSZ: // Args: const struct winsize *argp // Set window size. - var winsize unix.Winsize + var winsize linux.Winsize if _, err := usermem.CopyObjectIn(ctx, io, args[2].Pointer(), &winsize, usermem.IOOpts{ AddressSpaceActive: true, }); err != nil { return 0, err } - err := unix.IoctlSetWinsize(fd, unix.TIOCSWINSZ, &winsize) + err := ioctlSetWinsize(fd, &winsize) return 0, err default: diff --git a/pkg/sentry/fs/host/ioctl_unsafe.go b/pkg/sentry/fs/host/ioctl_unsafe.go index 3c07c3850..bc965a1c2 100644 --- a/pkg/sentry/fs/host/ioctl_unsafe.go +++ b/pkg/sentry/fs/host/ioctl_unsafe.go @@ -23,7 +23,7 @@ import ( func ioctlGetTermios(fd int) (*linux.Termios, error) { var t linux.Termios - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), syscall.TCGETS, uintptr(unsafe.Pointer(&t))) + _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), linux.TCGETS, uintptr(unsafe.Pointer(&t))) if errno != 0 { return nil, errno } @@ -37,3 +37,20 @@ func ioctlSetTermios(fd int, req uint64, t *linux.Termios) error { } return nil } + +func ioctlGetWinsize(fd int) (*linux.Winsize, error) { + var w linux.Winsize + _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), linux.TIOCGWINSZ, uintptr(unsafe.Pointer(&w))) + if errno != 0 { + return nil, errno + } + return &w, nil +} + +func ioctlSetWinsize(fd int, w *linux.Winsize) error { + _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), linux.TIOCSWINSZ, uintptr(unsafe.Pointer(w))) + if errno != 0 { + return errno + } + return nil +} diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 69e88d8e0..2d6b507b3 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -227,7 +227,7 @@ func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { // Execute runs a command on a created or running sandbox. func (cm *containerManager) Execute(e *control.ExecArgs, waitStatus *uint32) error { - log.Debugf("containerManager.Execute") + log.Debugf("containerManager.Execute: %+v", *e) proc := control.Proc{Kernel: cm.l.k} if err := proc.Exec(e, waitStatus); err != nil { return fmt.Errorf("error executing: %+v: %v", e, err) diff --git a/runsc/boot/filter/BUILD b/runsc/boot/filter/BUILD index c9837c236..96be051fe 100644 --- a/runsc/boot/filter/BUILD +++ b/runsc/boot/filter/BUILD @@ -18,6 +18,7 @@ go_library( "//runsc/boot:__subpackages__", ], deps = [ + "//pkg/abi/linux", "//pkg/log", "//pkg/seccomp", "//pkg/sentry/platform", diff --git a/runsc/boot/filter/config.go b/runsc/boot/filter/config.go index e45e599c3..db2e3f9d8 100644 --- a/runsc/boot/filter/config.go +++ b/runsc/boot/filter/config.go @@ -18,6 +18,7 @@ import ( "syscall" "golang.org/x/sys/unix" + "gvisor.googlesource.com/gvisor/pkg/abi/linux" "gvisor.googlesource.com/gvisor/pkg/seccomp" ) @@ -78,15 +79,36 @@ var allowedSyscalls = seccomp.SyscallRules{ syscall.SYS_TGKILL: {}, syscall.SYS_WRITE: {}, syscall.SYS_WRITEV: {}, -} -// TODO: Ioctl is needed in order to support tty consoles. -// Once filters support argument-checking, we should only allow ioctl -// with tty-related arguments. -func consoleFilters() seccomp.SyscallRules { - return seccomp.SyscallRules{ - syscall.SYS_IOCTL: {}, - } + // SYS_IOCTL is needed for terminal support, but we only allow + // setting/getting termios and winsize. + syscall.SYS_IOCTL: []seccomp.Rule{ + { + seccomp.AllowAny{}, /* fd */ + seccomp.AllowValue(linux.TCGETS), + seccomp.AllowAny{}, /* termios struct */ + }, + { + seccomp.AllowAny{}, /* fd */ + seccomp.AllowValue(linux.TCSETS), + seccomp.AllowAny{}, /* termios struct */ + }, + { + seccomp.AllowAny{}, /* fd */ + seccomp.AllowValue(linux.TCSETSW), + seccomp.AllowAny{}, /* termios struct */ + }, + { + seccomp.AllowAny{}, /* fd */ + seccomp.AllowValue(linux.TIOCSWINSZ), + seccomp.AllowAny{}, /* winsize struct */ + }, + { + seccomp.AllowAny{}, /* fd */ + seccomp.AllowValue(linux.TIOCGWINSZ), + seccomp.AllowAny{}, /* winsize struct */ + }, + }, } // whitelistFSFilters returns syscalls made by whitelistFS. Using WhitelistFS diff --git a/runsc/boot/filter/filter.go b/runsc/boot/filter/filter.go index 6ea9c464e..c57bbd2e5 100644 --- a/runsc/boot/filter/filter.go +++ b/runsc/boot/filter/filter.go @@ -28,7 +28,7 @@ import ( ) // Install installs seccomp filters for based on the given platform. -func Install(p platform.Platform, whitelistFS, console, hostNetwork bool) error { +func Install(p platform.Platform, whitelistFS, hostNetwork bool) error { s := allowedSyscalls // Set of additional filters used by -race and -msan. Returns empty @@ -39,10 +39,6 @@ func Install(p platform.Platform, whitelistFS, console, hostNetwork bool) error Report("direct file access allows unrestricted file access!") s.Merge(whitelistFSFilters()) } - if console { - Report("console is enabled: syscall filters less restrictive!") - s.Merge(consoleFilters()) - } if hostNetwork { Report("host networking enabled: syscall filters less restrictive!") s.Merge(hostInetFilters()) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 2f212c704..0e94cf215 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -340,7 +340,7 @@ func (l *Loader) run() error { } else { whitelistFS := l.conf.FileAccess == FileAccessDirect hostNet := l.conf.Network == NetworkHost - if err := filter.Install(l.k.Platform, whitelistFS, l.console, hostNet); err != nil { + if err := filter.Install(l.k.Platform, whitelistFS, hostNet); err != nil { return fmt.Errorf("Failed to install seccomp filters: %v", err) } } diff --git a/runsc/cmd/BUILD b/runsc/cmd/BUILD index c45784749..b9ef4022f 100644 --- a/runsc/cmd/BUILD +++ b/runsc/cmd/BUILD @@ -38,6 +38,7 @@ go_library( "//pkg/sentry/kernel/auth", "//pkg/urpc", "//runsc/boot", + "//runsc/console", "//runsc/container", "//runsc/fsgofer", "//runsc/specutils", diff --git a/runsc/cmd/exec.go b/runsc/cmd/exec.go index 4ee370656..b84a80119 100644 --- a/runsc/cmd/exec.go +++ b/runsc/cmd/exec.go @@ -35,6 +35,7 @@ import ( "gvisor.googlesource.com/gvisor/pkg/sentry/kernel/auth" "gvisor.googlesource.com/gvisor/pkg/urpc" "gvisor.googlesource.com/gvisor/runsc/boot" + "gvisor.googlesource.com/gvisor/runsc/console" "gvisor.googlesource.com/gvisor/runsc/container" "gvisor.googlesource.com/gvisor/runsc/specutils" ) @@ -50,6 +51,11 @@ type Exec struct { detach bool processPath string pidFile string + + // consoleSocket is the path to an AF_UNIX socket which will receive a + // file descriptor referencing the master end of the console's + // pseudoterminal. + consoleSocket string } // Name implements subcommands.Command.Name. @@ -91,6 +97,7 @@ func (ex *Exec) SetFlags(f *flag.FlagSet) { f.BoolVar(&ex.detach, "detach", false, "detach from the container's process") f.StringVar(&ex.processPath, "process", "", "path to the process.json") f.StringVar(&ex.pidFile, "pid-file", "", "filename that the container pid will be written to") + f.StringVar(&ex.consoleSocket, "console-socket", "", "path to an AF_UNIX socket which will receive a file descriptor referencing the master end of the console's pseudoterminal") } // Execute implements subcommands.Command.Execute. It starts a process in an @@ -178,11 +185,35 @@ func (ex *Exec) execAndWait(waitStatus *syscall.WaitStatus) subcommands.ExitStat args = append(args, a) } } - cmd := exec.Command(binPath, args...) + + // Exec stdio defaults to current process stdio. cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr + + // If the console control socket file is provided, then create a new + // pty master/slave pair and set the tty on the sandbox process. + if ex.consoleSocket != "" { + // Create a new tty pair and send the master on the provided + // socket. + tty, err := console.NewWithSocket(ex.consoleSocket) + if err != nil { + Fatalf("error setting up console with socket %q: %v", ex.consoleSocket, err) + } + defer tty.Close() + + // Set stdio to the new tty slave. + cmd.Stdin = tty + cmd.Stdout = tty + cmd.Stderr = tty + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setsid: true, + Setctty: true, + Ctty: int(tty.Fd()), + } + } + if err := cmd.Start(); err != nil { Fatalf("failure to start child exec process, err: %v", err) } @@ -252,11 +283,12 @@ func (ex *Exec) argsFromCLI(argv []string) (*control.ExecArgs, error) { return &control.ExecArgs{ Argv: argv, WorkingDirectory: ex.cwd, - FilePayload: urpc.FilePayload{Files: []*os.File{os.Stdin, os.Stdout, os.Stderr}}, KUID: ex.user.kuid, KGID: ex.user.kgid, ExtraKGIDs: extraKGIDs, Capabilities: caps, + StdioIsPty: ex.consoleSocket != "", + FilePayload: urpc.FilePayload{[]*os.File{os.Stdin, os.Stdout, os.Stderr}}, }, nil } @@ -292,11 +324,12 @@ func argsFromProcess(p *specs.Process) (*control.ExecArgs, error) { Argv: p.Args, Envv: p.Env, WorkingDirectory: p.Cwd, - FilePayload: urpc.FilePayload{Files: []*os.File{os.Stdin, os.Stdout, os.Stderr}}, KUID: auth.KUID(p.User.UID), KGID: auth.KGID(p.User.GID), ExtraKGIDs: extraKGIDs, Capabilities: caps, + StdioIsPty: p.Terminal, + FilePayload: urpc.FilePayload{Files: []*os.File{os.Stdin, os.Stdout, os.Stderr}}, }, nil } diff --git a/runsc/console/BUILD b/runsc/console/BUILD new file mode 100644 index 000000000..fa1a7d430 --- /dev/null +++ b/runsc/console/BUILD @@ -0,0 +1,16 @@ +package(licenses = ["notice"]) # Apache 2.0 + +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "console", + srcs = ["console.go"], + importpath = "gvisor.googlesource.com/gvisor/runsc/console", + visibility = [ + "//runsc:__subpackages__", + ], + deps = [ + "@com_github_kr_pty//:go_default_library", + "@org_golang_x_sys//unix:go_default_library", + ], +) diff --git a/runsc/sandbox/console.go b/runsc/console/console.go index 3f133e12a..2f2745b2b 100644 --- a/runsc/sandbox/console.go +++ b/runsc/console/console.go @@ -12,7 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -package sandbox +// Package console contains utilities for working with pty consols in runsc. +package console import ( "fmt" @@ -23,9 +24,9 @@ import ( "golang.org/x/sys/unix" ) -// setupConsole creates pty master/slave pair, sends the master FD over the -// given socket, and returns the slave. -func setupConsole(socketPath string) (*os.File, error) { +// NewWithSocket creates pty master/slave pair, sends the master FD over the given +// socket, and returns the slave. +func NewWithSocket(socketPath string) (*os.File, error) { // Create a new pty master and slave. ptyMaster, ptySlave, err := pty.Open() if err != nil { diff --git a/runsc/sandbox/BUILD b/runsc/sandbox/BUILD index d26a4dac6..e9a39f797 100644 --- a/runsc/sandbox/BUILD +++ b/runsc/sandbox/BUILD @@ -5,7 +5,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "sandbox", srcs = [ - "console.go", "namespace.go", "network.go", "sandbox.go", @@ -21,9 +20,9 @@ go_library( "//pkg/sentry/control", "//pkg/urpc", "//runsc/boot", + "//runsc/console", "//runsc/fsgofer", "//runsc/specutils", - "@com_github_kr_pty//:go_default_library", "@com_github_opencontainers_runtime-spec//specs-go:go_default_library", "@com_github_vishvananda_netlink//:go_default_library", "@org_golang_x_sys//unix:go_default_library", diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 7789608f8..e54ba4ba3 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -31,6 +31,7 @@ import ( "gvisor.googlesource.com/gvisor/pkg/sentry/control" "gvisor.googlesource.com/gvisor/pkg/urpc" "gvisor.googlesource.com/gvisor/runsc/boot" + "gvisor.googlesource.com/gvisor/runsc/console" "gvisor.googlesource.com/gvisor/runsc/fsgofer" "gvisor.googlesource.com/gvisor/runsc/specutils" ) @@ -392,7 +393,7 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund "boot", "--bundle", bundleDir, "--controller-fd="+strconv.Itoa(nextFD), - fmt.Sprintf("--console=%t", consoleEnabled)) + "--console="+strconv.FormatBool(consoleEnabled)) nextFD++ controllerFile := os.NewFile(uintptr(fd), "control_server_socket") @@ -407,14 +408,19 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund nextFD++ } + // Sandbox stdio defaults to current process stdio. + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + // If the console control socket file is provided, then create a new // pty master/slave pair and set the tty on the sandbox process. if consoleEnabled { - // setupConsole will send the master on the socket, and return - // the slave. - tty, err := setupConsole(consoleSocket) + // console.NewWithSocket will send the master on the socket, + // and return the slave. + tty, err := console.NewWithSocket(consoleSocket) if err != nil { - return fmt.Errorf("error setting up control socket %q: %v", consoleSocket, err) + return fmt.Errorf("error setting up console with socket %q: %v", consoleSocket, err) } defer tty.Close() @@ -423,10 +429,6 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund cmd.Stderr = tty cmd.SysProcAttr.Setctty = true cmd.SysProcAttr.Ctty = int(tty.Fd()) - } else { - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr } // Detach from this session, otherwise cmd will get SIGHUP and SIGCONT |