From 564ff73c1807c37a0343389c8abfbca9c3a0bbef Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Tue, 27 Sep 2022 11:55:15 -0700 Subject: [PATCH] Handle more arguments in StartContainer. PiperOrigin-RevId: 477234682 --- pkg/sentry/control/lifecycle.go | 134 ++++++++++++++++++++++---------- pkg/sentry/control/proc.go | 6 +- pkg/sentry/fdimport/fdimport.go | 14 ++-- pkg/sentry/limits/linux.go | 22 +++++- runsc/boot/limits.go | 24 +----- runsc/boot/loader.go | 7 +- 6 files changed, 136 insertions(+), 71 deletions(-) diff --git a/pkg/sentry/control/lifecycle.go b/pkg/sentry/control/lifecycle.go index 7cdf82757..0593e6c96 100644 --- a/pkg/sentry/control/lifecycle.go +++ b/pkg/sentry/control/lifecycle.go @@ -15,12 +15,13 @@ package control import ( + "encoding/json" "fmt" - "strings" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/fdimport" "gvisor.dev/gvisor/pkg/sentry/fs/user" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -90,6 +91,11 @@ type StartContainerArgs struct { // Envv is a list of environment variables. Envv []string `json:"envv"` + // Secret_envv is a list of secret environment variables. + // + // NOTE: This field must never be logged! + SecretEnvv []string `json:"secret_envv"` + // WorkingDirectory defines the working directory for the new process. WorkingDirectory string `json:"wd"` @@ -101,33 +107,44 @@ type StartContainerArgs struct { // the root group if not set explicitly. KGID auth.KGID `json:"KGID"` - // ExtraKGIDs is the list of additional groups to which the user belongs. - ExtraKGIDs []auth.KGID `json:"extraKGID"` + // ContainerID is the container for the process being executed. + ContainerID string `json:"container_id"` - // Capabilities is the list of capabilities to give to the process. - Capabilities *auth.TaskCapabilities `json:"capabilities"` + // Limits is the limit set for the process being executed. + Limits map[string]limits.Limit `json:"limits"` + + // If HOME environment variable is not provided, and this flag is set, + // then the HOME environment variable will be set inside the container + // based on the user's home directory in /etc/passwd. + ResolveHome bool `json:"resolve_home"` + + // If set, attempt to resolve the binary_path via the following procedure: + // 1) If binary_path is absolute, it is used directly. + // 2) If binary_path contains a slash, then it is resolved relative to the + // working_directory (or the root it working_directory is not set). + // 3) Otherwise, search the PATH environment variable for the first directory + // that contains an executable file with name in binary_path. + ResolveBinaryPath bool `json:"resolve_binary_path"` + + // DonatedFDs is the list of sentry-intrenal file descriptors that will + // donated. They correspond to the donated files in FilePayload. + DonatedFDs []int `json:"donated_fds"` // FilePayload determines the files to give to the new process. urpc.FilePayload - - // ContainerID is the container for the process being executed. - ContainerID string `json:"containerID"` - - // Limits is the limit set for the process being executed. - Limits *limits.LimitSet `json:"limits"` } -// String prints the StartContainerArgs.argv as a string. -func (args StartContainerArgs) String() string { - if len(args.Argv) == 0 { - return args.Filename +// String formats the StartContainerArgs without the SecretEnvv field. +func (sca StartContainerArgs) String() string { + sca.SecretEnvv = make([]string, len(sca.SecretEnvv)) + for i := range sca.SecretEnvv { + sca.SecretEnvv[i] = "(hidden)" } - a := make([]string, len(args.Argv)) - copy(a, args.Argv) - if args.Filename != "" { - a[0] = args.Filename + b, err := json.Marshal(sca) + if err != nil { + return fmt.Sprintf("error marshaling: %s", err) } - return strings.Join(a, " ") + return string(b) } func (l *Lifecycle) updateContainerState(containerID string, newState containerState) error { @@ -164,19 +181,28 @@ func (l *Lifecycle) updateContainerState(containerID string, newState containerS // StartContainer will start a new container in the sandbox. func (l *Lifecycle) StartContainer(args *StartContainerArgs, _ *uint32) error { - // Import file descriptors. - fdTable := l.Kernel.NewFDTable() + log.Infof("StartContainer: %v", args) + if len(args.Files) != len(args.DonatedFDs) { + return fmt.Errorf("FilePayload.Files and DonatedFDs must have same number of elements (%d != %d)", len(args.Files), len(args.DonatedFDs)) + } creds := auth.NewUserCredentials( args.KUID, args.KGID, - args.ExtraKGIDs, - args.Capabilities, + nil, /* extraKGIDs */ + nil, /* capabilities */ l.Kernel.RootUserNamespace()) - limitSet := args.Limits - if limitSet == nil { - limitSet = limits.NewLimitSet() + ls, err := limits.NewLinuxDistroLimitSet() + if err != nil { + return fmt.Errorf("error creating default limit set: %w", err) + } + for name, limit := range args.Limits { + lt, ok := limits.FromLinuxResourceName[name] + if !ok { + return fmt.Errorf("unknown limit %q", name) + } + ls.SetUnchecked(lt, limit) } // Create a new pid namespace for the container. Each container must run @@ -184,14 +210,14 @@ func (l *Lifecycle) StartContainer(args *StartContainerArgs, _ *uint32) error { pidNs := l.Kernel.RootPIDNamespace().NewChild(l.Kernel.RootUserNamespace()) initArgs := kernel.CreateProcessArgs{ - Filename: args.Filename, - Argv: args.Argv, - Envv: args.Envv, + Filename: args.Filename, + Argv: args.Argv, + // Order Envv before SecretEnvv. + Envv: append(args.Envv, args.SecretEnvv...), WorkingDirectory: args.WorkingDirectory, Credentials: creds, - FDTable: fdTable, Umask: 0022, - Limits: limitSet, + Limits: ls, MaxSymlinkTraversals: linux.MaxSymlinkTraversals, UTSNamespace: l.Kernel.RootUTSNamespace(), IPCNamespace: l.Kernel.RootIPCNamespace(), @@ -201,7 +227,43 @@ func (l *Lifecycle) StartContainer(args *StartContainerArgs, _ *uint32) error { } ctx := initArgs.NewContext(l.Kernel) + + // Import file descriptors. + fdTable := l.Kernel.NewFDTable() defer fdTable.DecRef(ctx) + hostFDs, err := fd.NewFromFiles(args.Files) + if err != nil { + return fmt.Errorf("error donating host files: %w", err) + } + defer func() { + for _, hfd := range hostFDs { + _ = hfd.Close() + } + }() + fdMap := make(map[int]*fd.FD, len(args.DonatedFDs)) + for i, appFD := range args.DonatedFDs { + fdMap[appFD] = hostFDs[i] + } + if _, _, err := fdimport.Import(ctx, fdTable, false, args.KUID, args.KGID, fdMap); err != nil { + return fmt.Errorf("error importing host files: %w", err) + } + initArgs.FDTable = fdTable + + if args.ResolveBinaryPath { + resolved, err := user.ResolveExecutablePath(ctx, &initArgs) + if err != nil { + return fmt.Errorf("failed to resolve binary path: %w", err) + } + initArgs.Filename = resolved + } + + if args.ResolveHome { + envVars, err := user.MaybeAddExecUserHomeVFS2(ctx, initArgs.MountNamespaceVFS2, creds.RealKUID, initArgs.Envv) + if err != nil { + return fmt.Errorf("failed to get user home dir: %w", err) + } + initArgs.Envv = envVars + } // VFS2 is supported in multi-container mode by default. l.mu.RLock() @@ -214,12 +276,6 @@ func (l *Lifecycle) StartContainer(args *StartContainerArgs, _ *uint32) error { l.mu.RUnlock() initArgs.MountNamespaceVFS2.IncRef() - resolved, err := user.ResolveExecutablePath(ctx, &initArgs) - if err != nil { - return err - } - initArgs.Filename = resolved - fds, err := fd.NewFromFiles(args.Files) if err != nil { return fmt.Errorf("duplicating payload files: %w", err) @@ -289,7 +345,7 @@ func (l *Lifecycle) getInitContainerProcess(containerID string) (*kernel.ThreadG // starting the container. type ContainerArgs struct { // ContainerID. - ContainerID string `json:"containerID"` + ContainerID string `json:"container_id"` Signo int32 `json:"signo"` SignalAll bool `json:"signalAll"` } diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index d1617f909..513e28a49 100644 --- a/pkg/sentry/control/proc.go +++ b/pkg/sentry/control/proc.go @@ -223,7 +223,11 @@ func (proc *Proc) execAsync(args *ExecArgs) (*kernel.ThreadGroup, kernel.ThreadI _ = fd.Close() } }() - ttyFile, ttyFileVFS2, err := fdimport.Import(ctx, fdTable, args.StdioIsPty, args.KUID, args.KGID, fds) + fdMap := make(map[int]*fd.FD, len(fds)) + for appFD, hostFD := range fds { + fdMap[appFD] = hostFD + } + ttyFile, ttyFileVFS2, err := fdimport.Import(ctx, fdTable, args.StdioIsPty, args.KUID, args.KGID, fdMap) if err != nil { return nil, 0, nil, nil, err } diff --git a/pkg/sentry/fdimport/fdimport.go b/pkg/sentry/fdimport/fdimport.go index f2b9630eb..866008854 100644 --- a/pkg/sentry/fdimport/fdimport.go +++ b/pkg/sentry/fdimport/fdimport.go @@ -28,11 +28,11 @@ import ( "gvisor.dev/gvisor/pkg/sentry/vfs" ) -// Import imports a slice of FDs into the given FDTable. If console is true, -// sets up TTY for the first 3 FDs in the slice representing stdin, stdout, -// stderr. Used FDs are either closed or released. It's safe for the caller to -// close any remaining files upon return. -func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth.KUID, gid auth.KGID, fds []*fd.FD) (*host.TTYFileOperations, *hostvfs2.TTYFileDescription, error) { +// Import imports a map of FDs into the given FDTable. If console is true, +// sets up TTY for sentry stdin, stdout, and stderr FDs. Used FDs are either +// closed or released. It's safe for the caller to close any remaining files +// upon return. +func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth.KUID, gid auth.KGID, fds map[int]*fd.FD) (*host.TTYFileOperations, *hostvfs2.TTYFileDescription, error) { if kernel.VFS2Enabled { ttyFile, err := importVFS2(ctx, fdTable, console, uid, gid, fds) return nil, ttyFile, err @@ -41,7 +41,7 @@ func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth return ttyFile, nil, err } -func importFS(ctx context.Context, fdTable *kernel.FDTable, console bool, fds []*fd.FD) (*host.TTYFileOperations, error) { +func importFS(ctx context.Context, fdTable *kernel.FDTable, console bool, fds map[int]*fd.FD) (*host.TTYFileOperations, error) { var ttyFile *fs.File for appFD, hostFD := range fds { var appFile *fs.File @@ -90,7 +90,7 @@ func importFS(ctx context.Context, fdTable *kernel.FDTable, console bool, fds [] return ttyFile.FileOperations.(*host.TTYFileOperations), nil } -func importVFS2(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth.KUID, gid auth.KGID, stdioFDs []*fd.FD) (*hostvfs2.TTYFileDescription, error) { +func importVFS2(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth.KUID, gid auth.KGID, stdioFDs map[int]*fd.FD) (*hostvfs2.TTYFileDescription, error) { k := kernel.KernelFromContext(ctx) if k == nil { return nil, fmt.Errorf("cannot find kernel from context") diff --git a/pkg/sentry/limits/linux.go b/pkg/sentry/limits/linux.go index 3f71abecc..8ce27e92a 100644 --- a/pkg/sentry/limits/linux.go +++ b/pkg/sentry/limits/linux.go @@ -20,7 +20,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" ) -// FromLinuxResource maps linux resources to sentry LimitTypes. +// FromLinuxResource maps linux resources to LimitTypes. var FromLinuxResource = map[int]LimitType{ linux.RLIMIT_CPU: CPU, linux.RLIMIT_FSIZE: FileSize, @@ -40,6 +40,26 @@ var FromLinuxResource = map[int]LimitType{ linux.RLIMIT_RTTIME: Rttime, } +// FromLinuxResourceName maps from linux resource names to LimitTypes. +var FromLinuxResourceName = map[string]LimitType{ + "RLIMIT_AS": AS, + "RLIMIT_CORE": Core, + "RLIMIT_CPU": CPU, + "RLIMIT_DATA": Data, + "RLIMIT_FSIZE": FileSize, + "RLIMIT_LOCKS": Locks, + "RLIMIT_MEMLOCK": MemoryLocked, + "RLIMIT_MSGQUEUE": MessageQueueBytes, + "RLIMIT_NICE": Nice, + "RLIMIT_NOFILE": NumberOfFiles, + "RLIMIT_NPROC": ProcessCount, + "RLIMIT_RSS": Rss, + "RLIMIT_RTPRIO": RealTimePriority, + "RLIMIT_RTTIME": Rttime, + "RLIMIT_SIGPENDING": SignalsPending, + "RLIMIT_STACK": Stack, +} + // FromLinux maps linux rlimit values to sentry Limits, being careful to handle // infinities. func FromLinux(rl uint64) uint64 { diff --git a/runsc/boot/limits.go b/runsc/boot/limits.go index 3d2b3506d..13dccd60a 100644 --- a/runsc/boot/limits.go +++ b/runsc/boot/limits.go @@ -24,28 +24,8 @@ import ( "gvisor.dev/gvisor/pkg/sync" ) -// Mapping from linux resource names to limits.LimitType. -var fromLinuxResource = map[string]limits.LimitType{ - "RLIMIT_AS": limits.AS, - "RLIMIT_CORE": limits.Core, - "RLIMIT_CPU": limits.CPU, - "RLIMIT_DATA": limits.Data, - "RLIMIT_FSIZE": limits.FileSize, - "RLIMIT_LOCKS": limits.Locks, - "RLIMIT_MEMLOCK": limits.MemoryLocked, - "RLIMIT_MSGQUEUE": limits.MessageQueueBytes, - "RLIMIT_NICE": limits.Nice, - "RLIMIT_NOFILE": limits.NumberOfFiles, - "RLIMIT_NPROC": limits.ProcessCount, - "RLIMIT_RSS": limits.Rss, - "RLIMIT_RTPRIO": limits.RealTimePriority, - "RLIMIT_RTTIME": limits.Rttime, - "RLIMIT_SIGPENDING": limits.SignalsPending, - "RLIMIT_STACK": limits.Stack, -} - func findName(lt limits.LimitType) string { - for k, v := range fromLinuxResource { + for k, v := range limits.FromLinuxResourceName { if v == lt { return k } @@ -141,7 +121,7 @@ func createLimitSet(spec *specs.Spec) (*limits.LimitSet, error) { // Then apply overwrites on top of defaults. for _, rl := range spec.Process.Rlimits { - lt, ok := fromLinuxResource[rl.Type] + lt, ok := limits.FromLinuxResourceName[rl.Type] if !ok { return nil, fmt.Errorf("unknown resource %q", rl.Type) } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index f78df13da..de75c4f8a 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -1362,10 +1362,15 @@ func createFDTable(ctx context.Context, console bool, stdioFDs []*fd.FD, user sp if len(stdioFDs) != 3 { return nil, nil, fmt.Errorf("stdioFDs should contain exactly 3 FDs (stdin, stdout, and stderr), but %d FDs received", len(stdioFDs)) } + fdMap := map[int]*fd.FD{ + 0: stdioFDs[0], + 1: stdioFDs[1], + 2: stdioFDs[2], + } k := kernel.KernelFromContext(ctx) fdTable := k.NewFDTable() - _, ttyFile, err := fdimport.Import(ctx, fdTable, console, auth.KUID(user.UID), auth.KGID(user.GID), stdioFDs) + _, ttyFile, err := fdimport.Import(ctx, fdTable, console, auth.KUID(user.UID), auth.KGID(user.GID), fdMap) if err != nil { fdTable.DecRef(ctx) return nil, nil, err