From d7d847c90b964088ddf5b3b51a5fcebc5a1f9130 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Thu, 22 Dec 2022 10:41:18 +0000 Subject: [PATCH 1/3] chore: refactor runtime spec handling out of bundle Bring all handling of the runtime spec out of the native oci bundle package, up into the oci launcher. We obtain the bundle's image spec after it is downloaded / extracted. The launcher then computes the correct Process config and updates the bundle with it. This is required so that we can handle an image USER in the launcher. Signed-off-by: Edita Kizinevic --- LICENSE_THIRD_PARTY.md | 1 + .../runtime/launcher/oci/launcher_linux.go | 44 ++- .../pkg/runtime/launcher/oci/process_linux.go | 133 ++++++++++ .../launcher/oci/process_linux_test.go | 234 ++++++++++++++++ pkg/ocibundle/bundle.go | 3 + pkg/ocibundle/native/bundle_linux.go | 129 +-------- pkg/ocibundle/native/bundle_linux_test.go | 250 ------------------ pkg/ocibundle/sif/bundle_linux.go | 10 + 8 files changed, 410 insertions(+), 394 deletions(-) diff --git a/LICENSE_THIRD_PARTY.md b/LICENSE_THIRD_PARTY.md index 052d722b85..3f827622e9 100644 --- a/LICENSE_THIRD_PARTY.md +++ b/LICENSE_THIRD_PARTY.md @@ -238,6 +238,7 @@ The source files: * `pkg/sypgp/testdata_test.go` * `internal/pkg/util/user/cgo_lookup_unix.go` +* `internal/pkg/util/passwdfile/passwdfile_unix.go` Contain code from the Go project. diff --git a/internal/pkg/runtime/launcher/oci/launcher_linux.go b/internal/pkg/runtime/launcher/oci/launcher_linux.go index 8210d42f4c..8264a63dd9 100644 --- a/internal/pkg/runtime/launcher/oci/launcher_linux.go +++ b/internal/pkg/runtime/launcher/oci/launcher_linux.go @@ -20,7 +20,6 @@ import ( "os/exec" "path/filepath" "strings" - "syscall" "github.com/apptainer/apptainer/internal/pkg/buildcfg" "github.com/apptainer/apptainer/internal/pkg/cache" @@ -36,7 +35,6 @@ import ( "github.com/containers/image/v5/types" "github.com/google/uuid" "github.com/opencontainers/runtime-spec/specs-go" - "golang.org/x/term" ) var ( @@ -217,19 +215,12 @@ func checkOpts(lo launcher.Options) error { } // createSpec produces an OCI runtime specification, suitable to launch a -// container. This spec excludes ProcessArgs and Env, as these have to be +// container. This spec excludes the Process config, as this have to be // computed where the image config is available, to account for the image's CMD // / ENTRYPOINT / ENV. func (l *Launcher) createSpec() (*specs.Spec, error) { spec := minimalSpec() - // Override the default Process.Terminal to false if our stdin is not a terminal. - if !term.IsTerminal(syscall.Stdin) { - spec.Process.Terminal = false - } - - spec.Process.User = l.getProcessUser() - // If we are *not* requesting fakeroot, then we need to map the container // uid back to host uid, through the initial fakeroot userns. if !l.cfg.Fakeroot && os.Getuid() != 0 { @@ -338,43 +329,38 @@ func (l *Launcher) Exec(ctx context.Context, image string, process string, args } } + // Create OCI runtime spec, excluding the Process settings which must consider the image spec. spec, err := l.createSpec() if err != nil { return fmt.Errorf("while creating OCI spec: %w", err) } - // Assemble the runtime & user-requested environment, which will be merged - // with the image ENV and set in the container at runtime. - rtEnv := defaultEnv(image, bundleDir) - // APPTAINERENV_ has lowest priority - rtEnv = mergeMap(rtEnv, apptainerEnvMap()) - // --env-file can override APPTAINERENV_ - if l.cfg.EnvFile != "" { - e, err := envFileMap(ctx, l.cfg.EnvFile) - if err != nil { - return err - } - rtEnv = mergeMap(rtEnv, e) - } - // --env flag can override --env-file and APPTAINERENV_ - rtEnv = mergeMap(rtEnv, l.cfg.Env) - + // Create a bundle - obtain and extract the image. b, err := native.New( native.OptBundlePath(bundleDir), native.OptImageRef(image), native.OptSysCtx(sysCtx), native.OptImgCache(imgCache), - native.OptProcessArgs(process, args), - native.OptProcessEnv(rtEnv), ) if err != nil { return err } - if err := b.Create(ctx, spec); err != nil { return err } + // With reference to the bundle's image spec, now set the process configuration. + imgSpec := b.ImageSpec() + if imgSpec == nil { + return fmt.Errorf("bundle has no image spec") + } + specProcess, err := l.getProcess(ctx, *imgSpec, image, b.Path(), process, args) + if err != nil { + return err + } + spec.Process = specProcess + b.Update(ctx, spec) + if err := l.updatePasswdGroup(tools.RootFs(b.Path()).Path()); err != nil { return err } diff --git a/internal/pkg/runtime/launcher/oci/process_linux.go b/internal/pkg/runtime/launcher/oci/process_linux.go index 9390935f70..b864a1bcaa 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux.go +++ b/internal/pkg/runtime/launcher/oci/process_linux.go @@ -14,14 +14,84 @@ import ( "fmt" "os" "strings" + "syscall" "github.com/apptainer/apptainer/internal/pkg/fakeroot" + "github.com/apptainer/apptainer/internal/pkg/runtime/engine/config/oci/generate" "github.com/apptainer/apptainer/internal/pkg/util/env" "github.com/apptainer/apptainer/internal/pkg/util/shell/interpreter" "github.com/apptainer/apptainer/internal/pkg/util/user" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/term" ) +const apptainerLibs = "/.singularity.d/libs" + +func (l *Launcher) getProcess(ctx context.Context, imgSpec imgspecv1.Image, image, bundle, process string, args []string) (*specs.Process, error) { + // Assemble the runtime & user-requested environment, which will be merged + // with the image ENV and set in the container at runtime. + rtEnv := defaultEnv(image, bundle) + // APPTAINERENV_ has lowest priority + rtEnv = mergeMap(rtEnv, apptainerEnvMap()) + // --env-file can override APPTAINERENV_ + if l.cfg.EnvFile != "" { + e, err := envFileMap(ctx, l.cfg.EnvFile) + if err != nil { + return nil, err + } + rtEnv = mergeMap(rtEnv, e) + } + // --env flag can override --env-file and APPTAINERENV_ + rtEnv = mergeMap(rtEnv, l.cfg.Env) + + cwd, err := l.getProcessCwd() + if err != nil { + return nil, err + } + + p := specs.Process{ + Args: getProcessArgs(imgSpec, process, args), + Cwd: cwd, + Env: getProcessEnv(imgSpec, rtEnv), + User: l.getProcessUser(), + Terminal: getProcessTerminal(), + } + + return &p, nil +} + +// getProcessTerminal determines whether the container process should run with a terminal. +func getProcessTerminal() bool { + // Override the default Process.Terminal to false if our stdin is not a terminal. + if term.IsTerminal(syscall.Stdin) { + return true + } + return false +} + +// getProcessArgs returns the process args for a container, with reference to the OCI Image Spec. +// The process and image parameters may override the image CMD and/or ENTRYPOINT. +func getProcessArgs(imageSpec imgspecv1.Image, process string, args []string) []string { + var processArgs []string + + if process != "" { + processArgs = []string{process} + } else { + processArgs = imageSpec.Config.Entrypoint + } + + if len(args) > 0 { + processArgs = append(processArgs, args...) + } else { + if process == "" { + processArgs = append(processArgs, imageSpec.Config.Cmd...) + } + } + + return processArgs +} + // getProcessUser computes the uid/gid(s) to be set on process execution. // Currently this only supports the same uid / primary gid as on the host. // TODO - expand for fakeroot, and arbitrary mapped user. @@ -146,6 +216,69 @@ func (l *Launcher) getReverseUserMaps() (uidMap, gidMap []specs.LinuxIDMapping, return uidMap, gidMap, nil } +// getProcessEnv combines the image config ENV with the ENV requested at runtime. +// APPEND_PATH and PREPEND_PATH are honored as with the native apptainer runtime. +// LD_LIBRARY_PATH is modified to always include the apptainer lib bind directory. +func getProcessEnv(imageSpec imgspecv1.Image, runtimeEnv map[string]string) []string { + path := "" + appendPath := "" + prependPath := "" + ldLibraryPath := "" + + // Start with the environment from the image config. + g := generate.New(nil) + g.Config.Process = &specs.Process{Env: imageSpec.Config.Env} + + // Obtain PATH, and LD_LIBRARY_PATH if set in the image config, for special handling. + for _, env := range imageSpec.Config.Env { + e := strings.SplitN(env, "=", 2) + if len(e) < 2 { + continue + } + if e[0] == "PATH" { + path = e[1] + } + if e[0] == "LD_LIBRARY_PATH" { + ldLibraryPath = e[1] + } + } + + // Apply env vars from runtime, except PATH and LD_LIBRARY_PATH releated. + for k, v := range runtimeEnv { + switch k { + case "PATH": + path = v + case "APPEND_PATH": + appendPath = v + case "PREPEND_PATH": + prependPath = v + case "LD_LIBRARY_PATH": + ldLibraryPath = v + default: + g.SetProcessEnv(k, v) + } + } + + // Compute and set optionally APPEND-ed / PREPEND-ed PATH. + if appendPath != "" { + path = path + ":" + appendPath + } + if prependPath != "" { + path = prependPath + ":" + path + } + if path != "" { + g.SetProcessEnv("PATH", path) + } + + // Ensure LD_LIBRARY_PATH always contains apptainer lib binding dir. + if !strings.Contains(ldLibraryPath, apptainerLibs) { + ldLibraryPath = strings.TrimPrefix(ldLibraryPath+":"+apptainerLibs, ":") + } + g.SetProcessEnv("LD_LIBRARY_PATH", ldLibraryPath) + + return g.Config.Process.Env +} + // defaultEnv returns default environment variables set in the container. func defaultEnv(image, bundle string) map[string]string { return map[string]string{ diff --git a/internal/pkg/runtime/launcher/oci/process_linux_test.go b/internal/pkg/runtime/launcher/oci/process_linux_test.go index 9fffa596d3..8163158ea2 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux_test.go +++ b/internal/pkg/runtime/launcher/oci/process_linux_test.go @@ -15,6 +15,8 @@ import ( "path/filepath" "reflect" "testing" + + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" ) func TestApptainerEnvMap(t *testing.T) { @@ -146,3 +148,235 @@ func TestEnvFileMap(t *testing.T) { }) } } + +func TestGetProcessArgs(t *testing.T) { + tests := []struct { + name string + imgEntrypoint []string + imgCmd []string + bundleProcess string + bundleArgs []string + expectProcessArgs []string + }{ + { + name: "imageEntrypointOnly", + imgEntrypoint: []string{"ENTRYPOINT"}, + imgCmd: []string{}, + bundleProcess: "", + bundleArgs: []string{}, + expectProcessArgs: []string{"ENTRYPOINT"}, + }, + { + name: "imageCmdOnly", + imgEntrypoint: []string{}, + imgCmd: []string{"CMD"}, + bundleProcess: "", + bundleArgs: []string{}, + expectProcessArgs: []string{"CMD"}, + }, + { + name: "imageEntrypointCMD", + imgEntrypoint: []string{"ENTRYPOINT"}, + imgCmd: []string{"CMD"}, + bundleProcess: "", + bundleArgs: []string{}, + expectProcessArgs: []string{"ENTRYPOINT", "CMD"}, + }, + { + name: "ProcessOnly", + imgEntrypoint: []string{}, + imgCmd: []string{}, + bundleProcess: "PROCESS", + bundleArgs: []string{}, + expectProcessArgs: []string{"PROCESS"}, + }, + { + name: "ArgsOnly", + imgEntrypoint: []string{}, + imgCmd: []string{}, + bundleProcess: "", + bundleArgs: []string{"ARGS"}, + expectProcessArgs: []string{"ARGS"}, + }, + { + name: "ProcessArgs", + imgEntrypoint: []string{}, + imgCmd: []string{}, + bundleProcess: "PROCESS", + bundleArgs: []string{"ARGS"}, + expectProcessArgs: []string{"PROCESS", "ARGS"}, + }, + { + name: "overrideEntrypointOnlyProcess", + imgEntrypoint: []string{"ENTRYPOINT"}, + imgCmd: []string{}, + bundleProcess: "PROCESS", + bundleArgs: []string{}, + expectProcessArgs: []string{"PROCESS"}, + }, + { + name: "overrideCmdOnlyArgs", + imgEntrypoint: []string{}, + imgCmd: []string{"CMD"}, + bundleProcess: "", + bundleArgs: []string{"ARGS"}, + expectProcessArgs: []string{"ARGS"}, + }, + { + name: "overrideBothProcess", + imgEntrypoint: []string{"ENTRYPOINT"}, + imgCmd: []string{"CMD"}, + bundleProcess: "PROCESS", + bundleArgs: []string{}, + expectProcessArgs: []string{"PROCESS"}, + }, + { + name: "overrideBothArgs", + imgEntrypoint: []string{"ENTRYPOINT"}, + imgCmd: []string{"CMD"}, + bundleProcess: "", + bundleArgs: []string{"ARGS"}, + expectProcessArgs: []string{"ENTRYPOINT", "ARGS"}, + }, + { + name: "overrideBothProcessArgs", + imgEntrypoint: []string{"ENTRYPOINT"}, + imgCmd: []string{"CMD"}, + bundleProcess: "PROCESS", + bundleArgs: []string{"ARGS"}, + expectProcessArgs: []string{"PROCESS", "ARGS"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + i := imgspecv1.Image{ + Config: imgspecv1.ImageConfig{ + Entrypoint: tt.imgEntrypoint, + Cmd: tt.imgCmd, + }, + } + args := getProcessArgs(i, tt.bundleProcess, tt.bundleArgs) + if !reflect.DeepEqual(args, tt.expectProcessArgs) { + t.Errorf("Expected: %v, Got: %v", tt.expectProcessArgs, args) + } + }) + } +} + +func TestGetProcessEnv(t *testing.T) { + tests := []struct { + name string + imageEnv []string + bundleEnv map[string]string + wantEnv []string + }{ + { + name: "Default", + imageEnv: []string{}, + bundleEnv: map[string]string{}, + wantEnv: []string{"LD_LIBRARY_PATH=/.singularity.d/libs"}, + }, + { + name: "ImagePath", + imageEnv: []string{"PATH=/foo"}, + bundleEnv: map[string]string{}, + wantEnv: []string{ + "PATH=/foo", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + { + name: "OverridePath", + imageEnv: []string{"PATH=/foo"}, + bundleEnv: map[string]string{"PATH": "/bar"}, + wantEnv: []string{ + "PATH=/bar", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + { + name: "AppendPath", + imageEnv: []string{"PATH=/foo"}, + bundleEnv: map[string]string{"APPEND_PATH": "/bar"}, + wantEnv: []string{ + "PATH=/foo:/bar", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + { + name: "PrependPath", + imageEnv: []string{"PATH=/foo"}, + bundleEnv: map[string]string{"PREPEND_PATH": "/bar"}, + wantEnv: []string{ + "PATH=/bar:/foo", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + { + name: "ImageLdLibraryPath", + imageEnv: []string{"LD_LIBRARY_PATH=/foo"}, + bundleEnv: map[string]string{}, + wantEnv: []string{ + "LD_LIBRARY_PATH=/foo:/.singularity.d/libs", + }, + }, + { + name: "BundleLdLibraryPath", + imageEnv: []string{}, + bundleEnv: map[string]string{"LD_LIBRARY_PATH": "/foo"}, + wantEnv: []string{ + "LD_LIBRARY_PATH=/foo:/.singularity.d/libs", + }, + }, + { + name: "OverrideLdLibraryPath", + imageEnv: []string{"LD_LIBRARY_PATH=/foo"}, + bundleEnv: map[string]string{"LD_LIBRARY_PATH": "/bar"}, + wantEnv: []string{ + "LD_LIBRARY_PATH=/bar:/.singularity.d/libs", + }, + }, + { + name: "ImageVar", + imageEnv: []string{"FOO=bar"}, + bundleEnv: map[string]string{}, + wantEnv: []string{ + "FOO=bar", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + { + name: "ImageOverride", + imageEnv: []string{"FOO=bar"}, + bundleEnv: map[string]string{"FOO": "baz"}, + wantEnv: []string{ + "FOO=baz", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + { + name: "ImageAdditional", + imageEnv: []string{"FOO=bar"}, + bundleEnv: map[string]string{"ABC": "123"}, + wantEnv: []string{ + "FOO=bar", + "ABC=123", + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + imgSpec := imgspecv1.Image{ + Config: imgspecv1.ImageConfig{Env: tt.imageEnv}, + } + + env := getProcessEnv(imgSpec, tt.bundleEnv) + + if !reflect.DeepEqual(env, tt.wantEnv) { + t.Errorf("want: %v, got: %v", tt.wantEnv, env) + } + }) + } +} diff --git a/pkg/ocibundle/bundle.go b/pkg/ocibundle/bundle.go index a1e7dc9631..f96695f3a3 100644 --- a/pkg/ocibundle/bundle.go +++ b/pkg/ocibundle/bundle.go @@ -12,12 +12,15 @@ package ocibundle import ( "context" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-spec/specs-go" ) // Bundle defines an OCI bundle interface to create/delete OCI bundles type Bundle interface { Create(context.Context, *specs.Spec) error + Update(context.Context, *specs.Spec) error + ImageSpec() *imgspecv1.Image Delete() error Path() string } diff --git a/pkg/ocibundle/native/bundle_linux.go b/pkg/ocibundle/native/bundle_linux.go index 203eb89b65..f848efab18 100644 --- a/pkg/ocibundle/native/bundle_linux.go +++ b/pkg/ocibundle/native/bundle_linux.go @@ -39,8 +39,6 @@ import ( "github.com/opencontainers/umoci/pkg/idtools" ) -const apptainerLibs = "/.singularity.d/libs" - // Bundle is a native OCI bundle, created from imageRef. type Bundle struct { // imageRef is the reference to the OCI image source, e.g. docker://ubuntu:latest. @@ -56,11 +54,6 @@ type Bundle struct { // OCI->SIF conversions, which are not used here. imgCache *cache.Handle // process is the command to execute, which may override the image's ENTRYPOINT / CMD. - process string - // args are the command arguments, which may override the image's CMD. - args []string - // env is the container environment to set, which will be merged with the image's env. - env map[string]string // Generic bundle properties ocibundle.Bundle } @@ -103,23 +96,6 @@ func OptImgCache(ic *cache.Handle) Option { } } -// OptProcessArgs sets the command and arguments to run in the container. -func OptProcessArgs(process string, args []string) Option { - return func(b *Bundle) error { - b.process = process - b.args = args - return nil - } -} - -// OptEnv sets the environment to be set, merged with the image ENV. -func OptProcessEnv(env map[string]string) Option { - return func(b *Bundle) error { - b.env = env - return nil - } -} - // New returns a bundle interface to create/delete an OCI bundle from an OCI image ref. func New(opts ...Option) (ocibundle.Bundle, error) { b := Bundle{ @@ -168,104 +144,27 @@ func (b *Bundle) Create(ctx context.Context, ociConfig *specs.Spec) error { if err := os.RemoveAll(tmpDir); err != nil { return err } - // ProcessArgs are set here, rather than in the launcher spec generation, as we need to - // consult the image Config to handle combining ENTRYPOINT/CMD with user - // provided args. - b.setProcessArgs(g) - // Ditto for environment handling (merge image and user/rt requested). - b.setProcessEnv(g) - return b.writeConfig(g) } -// Path returns the bundle's path on disk. -func (b *Bundle) Path() string { - return b.bundlePath -} - -func (b *Bundle) setProcessArgs(g *generate.Generator) { - var processArgs []string - - if b.process != "" { - processArgs = []string{b.process} - } else { - processArgs = b.imageSpec.Config.Entrypoint - } - - if len(b.args) > 0 { - processArgs = append(processArgs, b.args...) - } else { - if b.process == "" { - processArgs = append(processArgs, b.imageSpec.Config.Cmd...) - } +// Update will update the OCI config for the OCI bundle, so that it is ready for execution. +func (b *Bundle) Update(ctx context.Context, ociConfig *specs.Spec) error { + // generate OCI bundle directory and config + g, err := tools.GenerateBundleConfig(b.bundlePath, ociConfig) + if err != nil { + return fmt.Errorf("failed to generate OCI bundle/config: %s", err) } - - g.SetProcessArgs(processArgs) + return b.writeConfig(g) } -// setProcessEnv combines the image config ENV with the ENV requested in the runtime provided spec. -// APPEND_PATH and PREPEND_PATH are honored as with the native apptainer runtime. -// LD_LIBRARY_PATH is modified to always include the apptainer lib bind directory. -func (b *Bundle) setProcessEnv(g *generate.Generator) { - if g.Config == nil { - g.Config = &specs.Spec{} - } - if g.Config.Process == nil { - g.Config.Process = &specs.Process{} - } - g.Config.Process.Env = b.imageSpec.Config.Env - - path := "" - appendPath := "" - prependPath := "" - ldLibraryPath := "" - - // Obtain PATH, and LD_LIBRARY_PATH if set in the image config. - for _, env := range b.imageSpec.Config.Env { - e := strings.SplitN(env, "=", 2) - if len(e) < 2 { - continue - } - if e[0] == "PATH" { - path = e[1] - } - if e[0] == "LD_LIBRARY_PATH" { - ldLibraryPath = e[1] - } - } - - // Apply env vars from spec, except PATH and LD_LIBRARY_PATH releated. - for k, v := range b.env { - switch k { - case "PATH": - path = v - case "APPEND_PATH": - appendPath = v - case "PREPEND_PATH": - prependPath = v - case "LD_LIBRARY_PATH": - ldLibraryPath = v - default: - g.SetProcessEnv(k, v) - } - } - - // Compute and set optionally APPEND-ed / PREPEND-ed PATH. - if appendPath != "" { - path = path + ":" + appendPath - } - if prependPath != "" { - path = prependPath + ":" + path - } - if path != "" { - g.SetProcessEnv("PATH", path) - } +// ImageSpec returns the OCI image spec associated with the bundle. +func (b *Bundle) ImageSpec() (imgSpec *imgspecv1.Image) { + return b.imageSpec +} - // Ensure LD_LIBRARY_PATH always contains apptainer lib binding dir. - if !strings.Contains(ldLibraryPath, apptainerLibs) { - ldLibraryPath = strings.TrimPrefix(ldLibraryPath+":"+apptainerLibs, ":") - } - g.SetProcessEnv("LD_LIBRARY_PATH", ldLibraryPath) +// Path returns the bundle's path on disk. +func (b *Bundle) Path() string { + return b.bundlePath } func (b *Bundle) writeConfig(g *generate.Generator) error { diff --git a/pkg/ocibundle/native/bundle_linux_test.go b/pkg/ocibundle/native/bundle_linux_test.go index ee29e9f7b9..91e195d407 100644 --- a/pkg/ocibundle/native/bundle_linux_test.go +++ b/pkg/ocibundle/native/bundle_linux_test.go @@ -16,13 +16,9 @@ import ( "net/http" "os" "os/exec" - "reflect" "testing" "github.com/apptainer/apptainer/internal/pkg/cache" - "github.com/apptainer/apptainer/internal/pkg/runtime/engine/config/oci" - "github.com/apptainer/apptainer/internal/pkg/runtime/engine/config/oci/generate" - v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-tools/validate" ) @@ -145,249 +141,3 @@ func TestFromImageRef(t *testing.T) { }) } } - -func TestSetProcessArgs(t *testing.T) { - tests := []struct { - name string - imgEntrypoint []string - imgCmd []string - bundleProcess string - bundleArgs []string - expectProcessArgs []string - }{ - { - name: "imageEntrypointOnly", - imgEntrypoint: []string{"ENTRYPOINT"}, - imgCmd: []string{}, - bundleProcess: "", - bundleArgs: []string{}, - expectProcessArgs: []string{"ENTRYPOINT"}, - }, - { - name: "imageCmdOnly", - imgEntrypoint: []string{}, - imgCmd: []string{"CMD"}, - bundleProcess: "", - bundleArgs: []string{}, - expectProcessArgs: []string{"CMD"}, - }, - { - name: "imageEntrypointCMD", - imgEntrypoint: []string{"ENTRYPOINT"}, - imgCmd: []string{"CMD"}, - bundleProcess: "", - bundleArgs: []string{}, - expectProcessArgs: []string{"ENTRYPOINT", "CMD"}, - }, - { - name: "ProcessOnly", - imgEntrypoint: []string{}, - imgCmd: []string{}, - bundleProcess: "PROCESS", - bundleArgs: []string{}, - expectProcessArgs: []string{"PROCESS"}, - }, - { - name: "ArgsOnly", - imgEntrypoint: []string{}, - imgCmd: []string{}, - bundleProcess: "", - bundleArgs: []string{"ARGS"}, - expectProcessArgs: []string{"ARGS"}, - }, - { - name: "ProcessArgs", - imgEntrypoint: []string{}, - imgCmd: []string{}, - bundleProcess: "PROCESS", - bundleArgs: []string{"ARGS"}, - expectProcessArgs: []string{"PROCESS", "ARGS"}, - }, - { - name: "overrideEntrypointOnlyProcess", - imgEntrypoint: []string{"ENTRYPOINT"}, - imgCmd: []string{}, - bundleProcess: "PROCESS", - bundleArgs: []string{}, - expectProcessArgs: []string{"PROCESS"}, - }, - { - name: "overrideCmdOnlyArgs", - imgEntrypoint: []string{}, - imgCmd: []string{"CMD"}, - bundleProcess: "", - bundleArgs: []string{"ARGS"}, - expectProcessArgs: []string{"ARGS"}, - }, - { - name: "overrideBothProcess", - imgEntrypoint: []string{"ENTRYPOINT"}, - imgCmd: []string{"CMD"}, - bundleProcess: "PROCESS", - bundleArgs: []string{}, - expectProcessArgs: []string{"PROCESS"}, - }, - { - name: "overrideBothArgs", - imgEntrypoint: []string{"ENTRYPOINT"}, - imgCmd: []string{"CMD"}, - bundleProcess: "", - bundleArgs: []string{"ARGS"}, - expectProcessArgs: []string{"ENTRYPOINT", "ARGS"}, - }, - { - name: "overrideBothProcessArgs", - imgEntrypoint: []string{"ENTRYPOINT"}, - imgCmd: []string{"CMD"}, - bundleProcess: "PROCESS", - bundleArgs: []string{"ARGS"}, - expectProcessArgs: []string{"PROCESS", "ARGS"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - b := Bundle{ - imageSpec: &v1.Image{ - Config: v1.ImageConfig{ - Entrypoint: tt.imgEntrypoint, - Cmd: tt.imgCmd, - }, - }, - process: tt.bundleProcess, - args: tt.bundleArgs, - } - - g, err := oci.DefaultConfig() - if err != nil { - t.Fatal(err) - } - b.setProcessArgs(g) - if !reflect.DeepEqual(g.Config.Process.Args, tt.expectProcessArgs) { - t.Errorf("Expected: %v, Got: %v", tt.expectProcessArgs, g.Config.Process.Args) - } - }) - } -} - -func TestSetProcessEnv(t *testing.T) { - tests := []struct { - name string - imageEnv []string - bundleEnv map[string]string - wantEnv []string - }{ - { - name: "Default", - imageEnv: []string{}, - bundleEnv: map[string]string{}, - wantEnv: []string{"LD_LIBRARY_PATH=/.singularity.d/libs"}, - }, - { - name: "ImagePath", - imageEnv: []string{"PATH=/foo"}, - bundleEnv: map[string]string{}, - wantEnv: []string{ - "PATH=/foo", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - { - name: "OverridePath", - imageEnv: []string{"PATH=/foo"}, - bundleEnv: map[string]string{"PATH": "/bar"}, - wantEnv: []string{ - "PATH=/bar", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - { - name: "AppendPath", - imageEnv: []string{"PATH=/foo"}, - bundleEnv: map[string]string{"APPEND_PATH": "/bar"}, - wantEnv: []string{ - "PATH=/foo:/bar", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - { - name: "PrependPath", - imageEnv: []string{"PATH=/foo"}, - bundleEnv: map[string]string{"PREPEND_PATH": "/bar"}, - wantEnv: []string{ - "PATH=/bar:/foo", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - { - name: "ImageLdLibraryPath", - imageEnv: []string{"LD_LIBRARY_PATH=/foo"}, - bundleEnv: map[string]string{}, - wantEnv: []string{ - "LD_LIBRARY_PATH=/foo:/.singularity.d/libs", - }, - }, - { - name: "BundleLdLibraryPath", - imageEnv: []string{}, - bundleEnv: map[string]string{"LD_LIBRARY_PATH": "/foo"}, - wantEnv: []string{ - "LD_LIBRARY_PATH=/foo:/.singularity.d/libs", - }, - }, - { - name: "OverrideLdLibraryPath", - imageEnv: []string{"LD_LIBRARY_PATH=/foo"}, - bundleEnv: map[string]string{"LD_LIBRARY_PATH": "/bar"}, - wantEnv: []string{ - "LD_LIBRARY_PATH=/bar:/.singularity.d/libs", - }, - }, - { - name: "ImageVar", - imageEnv: []string{"FOO=bar"}, - bundleEnv: map[string]string{}, - wantEnv: []string{ - "FOO=bar", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - { - name: "ImageOverride", - imageEnv: []string{"FOO=bar"}, - bundleEnv: map[string]string{"FOO": "baz"}, - wantEnv: []string{ - "FOO=baz", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - { - name: "ImageAdditional", - imageEnv: []string{"FOO=bar"}, - bundleEnv: map[string]string{"ABC": "123"}, - wantEnv: []string{ - "FOO=bar", - "ABC=123", - "LD_LIBRARY_PATH=/.singularity.d/libs", - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - imgSpec := &v1.Image{ - Config: v1.ImageConfig{Env: tt.imageEnv}, - } - - b := &Bundle{ - imageSpec: imgSpec, - env: tt.bundleEnv, - } - g := &generate.Generator{} - b.setProcessEnv(g) - - if !reflect.DeepEqual(g.Config.Process.Env, tt.wantEnv) { - t.Errorf("want: %v, got: %v", tt.wantEnv, g.Config.Process.Env) - } - }) - } -} diff --git a/pkg/ocibundle/sif/bundle_linux.go b/pkg/ocibundle/sif/bundle_linux.go index 5b46feb208..abaa7f45f5 100644 --- a/pkg/ocibundle/sif/bundle_linux.go +++ b/pkg/ocibundle/sif/bundle_linux.go @@ -157,6 +157,11 @@ func (s *sifBundle) Create(ctx context.Context, ociConfig *specs.Spec) error { return nil } +// Update will update the OCI config for the OCI bundle, so that it is ready for execution. +func (s *sifBundle) Update(ctx context.Context, ociConfig *specs.Spec) error { + return fmt.Errorf("cannot update config of a SIF OCI bundle: not implemented") +} + // Delete erases OCI bundle create from SIF image func (s *sifBundle) Delete() error { if s.writable { @@ -173,6 +178,11 @@ func (s *sifBundle) Delete() error { return tools.DeleteBundle(s.bundlePath) } +// ImageSpec returns nil for SIF bundles, as they currently do not carry an OCI image spec. +func (s *sifBundle) ImageSpec() (imgSpec *imageSpecs.Image) { + return nil +} + func (s *sifBundle) Path() string { return s.bundlePath } From 8f624b248875d6c74aa8863b13f4ffb599cfdac4 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Thu, 29 Dec 2022 14:07:33 +0000 Subject: [PATCH 2/3] oci: inspection of image user in bundle Signed-off-by: Edita Kizinevic --- .../runtime/launcher/oci/launcher_linux.go | 35 ++-- .../pkg/util/passwdfile/passwdfile_unix.go | 180 ++++++++++++++++++ pkg/ocibundle/tools/oci.go | 18 ++ 3 files changed, 221 insertions(+), 12 deletions(-) create mode 100644 internal/pkg/util/passwdfile/passwdfile_unix.go diff --git a/internal/pkg/runtime/launcher/oci/launcher_linux.go b/internal/pkg/runtime/launcher/oci/launcher_linux.go index 8264a63dd9..f399c0e65b 100644 --- a/internal/pkg/runtime/launcher/oci/launcher_linux.go +++ b/internal/pkg/runtime/launcher/oci/launcher_linux.go @@ -26,6 +26,7 @@ import ( "github.com/apptainer/apptainer/internal/pkg/runtime/launcher" "github.com/apptainer/apptainer/internal/pkg/util/fs/files" "github.com/apptainer/apptainer/internal/pkg/util/user" + "github.com/apptainer/apptainer/pkg/ocibundle" "github.com/apptainer/apptainer/pkg/ocibundle/native" "github.com/apptainer/apptainer/pkg/ocibundle/tools" "github.com/apptainer/apptainer/pkg/syfs" @@ -249,6 +250,27 @@ func (l *Launcher) createSpec() (*specs.Spec, error) { return &spec, nil } +func (l *Launcher) updateSpecFromImage(ctx context.Context, b ocibundle.Bundle, spec *specs.Spec, image string, process string, args []string) error { + imgSpec := b.ImageSpec() + if imgSpec == nil { + return fmt.Errorf("bundle has no image spec") + } + + specProcess, err := l.getProcess(ctx, *imgSpec, image, b.Path(), process, args) + if err != nil { + return err + } + spec.Process = specProcess + if err := b.Update(ctx, spec); err != nil { + return err + } + + if err := l.updatePasswdGroup(tools.RootFs(b.Path()).Path()); err != nil { + return err + } + return nil +} + func (l *Launcher) updatePasswdGroup(rootfs string) error { uid := os.Getuid() gid := os.Getgid() @@ -350,18 +372,7 @@ func (l *Launcher) Exec(ctx context.Context, image string, process string, args } // With reference to the bundle's image spec, now set the process configuration. - imgSpec := b.ImageSpec() - if imgSpec == nil { - return fmt.Errorf("bundle has no image spec") - } - specProcess, err := l.getProcess(ctx, *imgSpec, image, b.Path(), process, args) - if err != nil { - return err - } - spec.Process = specProcess - b.Update(ctx, spec) - - if err := l.updatePasswdGroup(tools.RootFs(b.Path()).Path()); err != nil { + if err := l.updateSpecFromImage(ctx, b, spec, image, process, args); err != nil { return err } diff --git a/internal/pkg/util/passwdfile/passwdfile_unix.go b/internal/pkg/util/passwdfile/passwdfile_unix.go new file mode 100644 index 0000000000..5f70e2c2d3 --- /dev/null +++ b/internal/pkg/util/passwdfile/passwdfile_unix.go @@ -0,0 +1,180 @@ +// Copyright (c) Contributors to the Apptainer project, established as +// Apptainer a Series of LF Projects LLC. +// For website terms of use, trademark policy, privacy policy and other +// project policies see https://lfprojects.org/policies +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. +// +// This source code is an adaptation of: +// https://go.dev/src/os/user/lookup_unix.go +// to provide user lookup functionality against an arbitrary password file. + +package passwdfile + +import ( + "bufio" + "bytes" + "errors" + "io" + "os" + "os/user" + "strconv" + "strings" +) + +var colon = []byte(":") + +// lineFunc returns a value, an error, or (nil, nil) to skip the row. +type lineFunc func(line []byte) (v any, err error) + +// readColonFile parses r as an /etc/group or /etc/passwd style file, running +// fn for each row. readColonFile returns a value, an error, or (nil, nil) if +// the end of the file is reached without a match. +// +// readCols is the minimum number of colon-separated fields that will be passed +// to fn; in a long line additional fields may be silently discarded. +func readColonFile(r io.Reader, fn lineFunc, readCols int) (v any, err error) { + rd := bufio.NewReader(r) + + // Read the file line-by-line. + for { + var isPrefix bool + var wholeLine []byte + + // Read the next line. We do so in chunks (as much as reader's + // buffer is able to keep), check if we read enough columns + // already on each step and store final result in wholeLine. + for { + var line []byte + line, isPrefix, err = rd.ReadLine() + + if err != nil { + // We should return (nil, nil) if EOF is reached + // without a match. + if err == io.EOF { + err = nil + } + return nil, err + } + + // Simple common case: line is short enough to fit in a + // single reader's buffer. + if !isPrefix && len(wholeLine) == 0 { + wholeLine = line + break + } + + wholeLine = append(wholeLine, line...) + + // Check if we read the whole line (or enough columns) + // already. + if !isPrefix || bytes.Count(wholeLine, []byte{':'}) >= readCols { + break + } + } + + // There's no spec for /etc/passwd or /etc/group, but we try to follow + // the same rules as the glibc parser, which allows comments and blank + // space at the beginning of a line. + wholeLine = bytes.TrimSpace(wholeLine) + if len(wholeLine) == 0 || wholeLine[0] == '#' { + continue + } + v, err = fn(wholeLine) + if v != nil || err != nil { + return + } + + // If necessary, skip the rest of the line + for ; isPrefix; _, isPrefix, err = rd.ReadLine() { + if err != nil { + // We should return (nil, nil) if EOF is reached without a match. + if err == io.EOF { + err = nil + } + return nil, err + } + } + } +} + +// returns a *User for a row if that row's has the given value at the +// given index. +func matchUserIndexValue(value string, idx int) lineFunc { + var leadColon string + if idx > 0 { + leadColon = ":" + } + substr := []byte(leadColon + value + ":") + return func(line []byte) (v any, err error) { + if !bytes.Contains(line, substr) || bytes.Count(line, colon) < 6 { + return + } + // kevin:x:1005:1006::/home/kevin:/usr/bin/zsh + parts := strings.SplitN(string(line), ":", 7) + if len(parts) < 6 || parts[idx] != value || parts[0] == "" || + parts[0][0] == '+' || parts[0][0] == '-' { + return + } + if _, err := strconv.Atoi(parts[2]); err != nil { + return nil, nil + } + if _, err := strconv.Atoi(parts[3]); err != nil { + return nil, nil + } + u := &user.User{ + Username: parts[0], + Uid: parts[2], + Gid: parts[3], + Name: parts[4], + HomeDir: parts[5], + } + // The pw_gecos field isn't quite standardized. Some docs + // say: "It is expected to be a comma separated list of + // personal data where the first item is the full name of the + // user." + u.Name, _, _ = strings.Cut(u.Name, ",") + return u, nil + } +} + +func findUserID(uid string, r io.Reader) (*user.User, error) { + i, e := strconv.Atoi(uid) + if e != nil { + return nil, errors.New("user: invalid userid " + uid) + } + if v, err := readColonFile(r, matchUserIndexValue(uid, 2), 6); err != nil { + return nil, err + } else if v != nil { + return v.(*user.User), nil + } + return nil, user.UnknownUserIdError(i) +} + +func findUsername(name string, r io.Reader) (*user.User, error) { + if v, err := readColonFile(r, matchUserIndexValue(name, 0), 6); err != nil { + return nil, err + } else if v != nil { + return v.(*user.User), nil + } + return nil, user.UnknownUserError(name) +} + +func LookupUserInFile(userFile, username string) (*user.User, error) { + f, err := os.Open(userFile) + if err != nil { + return nil, err + } + defer f.Close() + return findUsername(username, f) +} + +func LookupUserIDInFile(userFile, uid string) (*user.User, error) { + f, err := os.Open(userFile) + if err != nil { + return nil, err + } + defer f.Close() + return findUserID(uid, f) +} diff --git a/pkg/ocibundle/tools/oci.go b/pkg/ocibundle/tools/oci.go index 5989c4f041..c1cad97ae4 100644 --- a/pkg/ocibundle/tools/oci.go +++ b/pkg/ocibundle/tools/oci.go @@ -12,11 +12,14 @@ package tools import ( "fmt" "os" + "os/user" "path/filepath" + "strconv" "syscall" "github.com/apptainer/apptainer/internal/pkg/runtime/engine/config/oci" "github.com/apptainer/apptainer/internal/pkg/runtime/engine/config/oci/generate" + "github.com/apptainer/apptainer/internal/pkg/util/passwdfile" "github.com/opencontainers/runtime-spec/specs-go" ) @@ -107,3 +110,18 @@ func DeleteBundle(bundlePath string) error { } return nil } + +// BundleUser returns a user struct for the specified user, from the bundle passwd file. +func Bundle(bundlePath, user string) (u *user.User, err error) { + passwd := filepath.Join(RootFs(bundlePath).Path(), "etc", "passwd") + if _, err := os.Stat(passwd); err != nil { + return nil, fmt.Errorf("cannot access container passwd file: %w", err) + } + + // We have a numeric container uid + if _, err := strconv.Atoi(user); err == nil { + return passwdfile.LookupUserIDInFile(passwd, user) + } + // We have a container username + return passwdfile.LookupUserInFile(passwd, user) +} From b1451d523c7e480ea18bab84711b5b54c39a5b17 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Thu, 29 Dec 2022 14:53:18 +0000 Subject: [PATCH 3/3] feat: oci: honor USER in image config When a USER is specified in the image config: * If running unprivileged, ensure the inner uid / gid mapping results in the container process running as the USER, by default. * If running privileged, run as the USER, by default. Fixes sylabs/singularity#77 Signed-off-by: Edita Kizinevic --- LICENSE.md | 2 +- e2e/docker/docker.go | 51 ++++++++- .../runtime/launcher/oci/launcher_linux.go | 102 ++++++++++++------ .../pkg/runtime/launcher/oci/process_linux.go | 35 ++---- .../launcher/oci/process_linux_test.go | 2 +- pkg/ocibundle/native/bundle_linux.go | 2 +- pkg/ocibundle/native/bundle_linux_test.go | 2 +- pkg/ocibundle/sif/bundle_linux.go | 2 +- pkg/ocibundle/tools/oci.go | 4 +- 9 files changed, 135 insertions(+), 67 deletions(-) diff --git a/LICENSE.md b/LICENSE.md index f6910faeaa..692e688a2a 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -23,7 +23,7 @@ reserved. Copyright (c) 2017, SingularityWare, LLC. All rights reserved. -Copyright (c) 2018-2022, Sylabs, Inc. All rights reserved. +Copyright (c) 2018-2023, Sylabs, Inc. All rights reserved. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: diff --git a/e2e/docker/docker.go b/e2e/docker/docker.go index 9a83f66141..a806869c7d 100644 --- a/e2e/docker/docker.go +++ b/e2e/docker/docker.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2019-2022 Sylabs Inc. All rights reserved. +// Copyright (c) 2019-2023 Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -827,6 +827,54 @@ func (c ctx) testDockerCMDENTRYPOINT(t *testing.T) { } } +// Check that the USER in a docker container is honored under --oci mode +func (c ctx) testDockerUSER(t *testing.T) { + tests := []struct { + name string + expectOutput string + profile e2e.Profile + }{ + // Sanity check apptainer native engine... no support for USER + { + name: "default", + profile: e2e.UserProfile, + expectOutput: fmt.Sprintf("uid=%d(%s) gid=%d", + e2e.UserProfile.ContainerUser(t).UID, + e2e.UserProfile.ContainerUser(t).Name, + e2e.UserProfile.ContainerUser(t).GID), + }, + // `--oci` modes (USER honored by default) + { + name: "OCIUser", + profile: e2e.OCIUserProfile, + expectOutput: `uid=2000(testuser) gid=2000(testgroup)`, + }, + { + name: "OCIFakeroot", + profile: e2e.OCIFakerootProfile, + expectOutput: `uid=0(root) gid=0(root)`, + }, + { + name: "OCIRoot", + profile: e2e.OCIRootProfile, + expectOutput: `uid=2000(testuser) gid=2000(testgroup)`, + }, + } + + for _, tt := range tests { + c.env.RunApptainer( + t, + e2e.AsSubtest(tt.name), + e2e.WithProfile(tt.profile), + e2e.WithCommand("run"), + e2e.WithArgs("docker://ghcr.io/apptainer/docker-user"), + e2e.ExpectExit(0, + e2e.ExpectOutput(e2e.ContainMatch, tt.expectOutput), + ), + ) + } +} + // E2ETests is the main func to trigger the test suite func E2ETests(env e2e.TestEnv) testhelper.Tests { c := ctx{ @@ -848,6 +896,7 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests { t.Run("entrypoint", c.testDockerENTRYPOINT) t.Run("cmdentrypoint", c.testDockerCMDENTRYPOINT) t.Run("cmd quotes", c.testDockerCMDQuotes) + t.Run("user", c.testDockerUSER) // Regressions t.Run("issue 4524", c.issue4524) }, diff --git a/internal/pkg/runtime/launcher/oci/launcher_linux.go b/internal/pkg/runtime/launcher/oci/launcher_linux.go index f399c0e65b..8b4360a36c 100644 --- a/internal/pkg/runtime/launcher/oci/launcher_linux.go +++ b/internal/pkg/runtime/launcher/oci/launcher_linux.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -19,6 +19,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "github.com/apptainer/apptainer/internal/pkg/buildcfg" @@ -215,32 +216,15 @@ func checkOpts(lo launcher.Options) error { return nil } -// createSpec produces an OCI runtime specification, suitable to launch a -// container. This spec excludes the Process config, as this have to be -// computed where the image config is available, to account for the image's CMD -// / ENTRYPOINT / ENV. +// createSpec creates an initial OCI runtime specification, suitable to launch a +// container. This spec excludes the Process config, as this has to be computed +// where the image config is available, to account for the image's CMD / +// ENTRYPOINT / ENV / USER. func (l *Launcher) createSpec() (*specs.Spec, error) { spec := minimalSpec() - // If we are *not* requesting fakeroot, then we need to map the container - // uid back to host uid, through the initial fakeroot userns. - if !l.cfg.Fakeroot && os.Getuid() != 0 { - uidMap, gidMap, err := l.getReverseUserMaps() - if err != nil { - return nil, err - } - spec.Linux.UIDMappings = uidMap - spec.Linux.GIDMappings = gidMap - } - spec = addNamespaces(spec, l.cfg.Namespaces) - cwd, err := l.getProcessCwd() - if err != nil { - return nil, err - } - spec.Process.Cwd = cwd - mounts, err := l.getMounts() if err != nil { return nil, err @@ -250,13 +234,64 @@ func (l *Launcher) createSpec() (*specs.Spec, error) { return &spec, nil } -func (l *Launcher) updateSpecFromImage(ctx context.Context, b ocibundle.Bundle, spec *specs.Spec, image string, process string, args []string) error { +// finalizeSpec updates the bundle config, filling in Process config that depends on the image spec. +func (l *Launcher) finalizeSpec(ctx context.Context, b ocibundle.Bundle, spec *specs.Spec, image string, process string, args []string) (err error) { imgSpec := b.ImageSpec() if imgSpec == nil { return fmt.Errorf("bundle has no image spec") } - specProcess, err := l.getProcess(ctx, *imgSpec, image, b.Path(), process, args) + // In the absence of a USER in the OCI image config, we will run the + // container process as our current user / group. + currentUID := uint32(os.Getuid()) + currentGID := uint32(os.Getgid()) + targetUID := currentUID + targetGID := currentGID + containerUser := false + + // If the OCI image config specifies a USER we will: + // * When unprivileged - run as that user, via nested subuid/gid mappings (host user -> userns root -> OCI USER) + // * When privileged - directly run as that user, as a host uid/gid. + if imgSpec.Config.User != "" { + imgUser, err := tools.BundleUser(b.Path(), imgSpec.Config.User) + if err != nil { + return err + } + imgUID, err := strconv.ParseUint(imgUser.Uid, 10, 32) + if err != nil { + return err + } + imgGID, err := strconv.ParseUint(imgUser.Gid, 10, 32) + if err != nil { + return err + } + targetUID = uint32(imgUID) + targetGID = uint32(imgGID) + containerUser = true + sylog.Debugf("Running as USER specified in OCI image config %d:%d", targetUID, targetGID) + } + + // Fakeroot always overrides to give us root in the container (via userns & idmap if unprivileged). + if l.cfg.Fakeroot { + targetUID = 0 + targetGID = 0 + } + + if targetUID != 0 && currentUID != 0 { + uidMap, gidMap, err := l.getReverseUserMaps(targetUID, targetGID) + if err != nil { + return err + } + spec.Linux.UIDMappings = uidMap + spec.Linux.GIDMappings = gidMap + } + + u := specs.User{ + UID: targetUID, + GID: targetGID, + } + + specProcess, err := l.getProcess(ctx, *imgSpec, image, b.Path(), process, args, u) if err != nil { return err } @@ -265,16 +300,19 @@ func (l *Launcher) updateSpecFromImage(ctx context.Context, b ocibundle.Bundle, return err } - if err := l.updatePasswdGroup(tools.RootFs(b.Path()).Path()); err != nil { + // If we are entering as root, or a USER defined in the container, then passwd/group + // information should be present already. + if targetUID == 0 || containerUser { + return nil + } + // Otherewise, add to the passwd and group files in the container. + if err := l.updatePasswdGroup(tools.RootFs(b.Path()).Path(), targetUID, targetGID); err != nil { return err } return nil } -func (l *Launcher) updatePasswdGroup(rootfs string) error { - uid := os.Getuid() - gid := os.Getgid() - +func (l *Launcher) updatePasswdGroup(rootfs string, uid, gid uint32) error { if os.Getuid() == 0 || l.cfg.Fakeroot { return nil } @@ -288,7 +326,7 @@ func (l *Launcher) updatePasswdGroup(rootfs string) error { } sylog.Debugf("Updating passwd file: %s", containerPasswd) - content, err := files.Passwd(containerPasswd, pw.Dir, uid) + content, err := files.Passwd(containerPasswd, pw.Dir, int(uid)) if err != nil { return fmt.Errorf("while creating passwd file: %w", err) } @@ -297,7 +335,7 @@ func (l *Launcher) updatePasswdGroup(rootfs string) error { } sylog.Debugf("Updating group file: %s", containerGroup) - content, err = files.Group(containerGroup, uid, []int{gid}) + content, err = files.Group(containerGroup, int(uid), []int{int(gid)}) if err != nil { return fmt.Errorf("while creating group file: %w", err) } @@ -372,7 +410,7 @@ func (l *Launcher) Exec(ctx context.Context, image string, process string, args } // With reference to the bundle's image spec, now set the process configuration. - if err := l.updateSpecFromImage(ctx, b, spec, image, process, args); err != nil { + if err := l.finalizeSpec(ctx, b, spec, image, process, args); err != nil { return err } diff --git a/internal/pkg/runtime/launcher/oci/process_linux.go b/internal/pkg/runtime/launcher/oci/process_linux.go index b864a1bcaa..59b268eb9b 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux.go +++ b/internal/pkg/runtime/launcher/oci/process_linux.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -28,7 +28,7 @@ import ( const apptainerLibs = "/.singularity.d/libs" -func (l *Launcher) getProcess(ctx context.Context, imgSpec imgspecv1.Image, image, bundle, process string, args []string) (*specs.Process, error) { +func (l *Launcher) getProcess(ctx context.Context, imgSpec imgspecv1.Image, image, bundle, process string, args []string, u specs.User) (*specs.Process, error) { // Assemble the runtime & user-requested environment, which will be merged // with the image ENV and set in the container at runtime. rtEnv := defaultEnv(image, bundle) @@ -54,7 +54,7 @@ func (l *Launcher) getProcess(ctx context.Context, imgSpec imgspecv1.Image, imag Args: getProcessArgs(imgSpec, process, args), Cwd: cwd, Env: getProcessEnv(imgSpec, rtEnv), - User: l.getProcessUser(), + User: u, Terminal: getProcessTerminal(), } @@ -63,11 +63,8 @@ func (l *Launcher) getProcess(ctx context.Context, imgSpec imgspecv1.Image, imag // getProcessTerminal determines whether the container process should run with a terminal. func getProcessTerminal() bool { - // Override the default Process.Terminal to false if our stdin is not a terminal. - if term.IsTerminal(syscall.Stdin) { - return true - } - return false + // Sets the default Process.Terminal to false if our stdin is not a terminal. + return term.IsTerminal(syscall.Stdin) } // getProcessArgs returns the process args for a container, with reference to the OCI Image Spec. @@ -92,22 +89,6 @@ func getProcessArgs(imageSpec imgspecv1.Image, process string, args []string) [] return processArgs } -// getProcessUser computes the uid/gid(s) to be set on process execution. -// Currently this only supports the same uid / primary gid as on the host. -// TODO - expand for fakeroot, and arbitrary mapped user. -func (l *Launcher) getProcessUser() specs.User { - if l.cfg.Fakeroot { - return specs.User{ - UID: 0, - GID: 0, - } - } - return specs.User{ - UID: uint32(os.Getuid()), - GID: uint32(os.Getgid()), - } -} - // getProcessCwd computes the Cwd that the container process should start in. // Currently this is the user's tmpfs home directory (see --containall). func (l *Launcher) getProcessCwd() (dir string, err error) { @@ -122,12 +103,12 @@ func (l *Launcher) getProcessCwd() (dir string, err error) { return pw.Dir, nil } -// getReverseUserMaps returns uid and gid mappings that re-map container uid to host +// getReverseUserMaps returns uid and gid mappings that re-map container uid to target // uid. This 'reverses' the host user to container root mapping in the initial // userns from which the OCI runtime is launched. // -// host 1001 -> fakeroot userns 0 -> container 1001 -func (l *Launcher) getReverseUserMaps() (uidMap, gidMap []specs.LinuxIDMapping, err error) { +// e.g. host 1001 -> fakeroot userns 0 -> container targetUID +func (l *Launcher) getReverseUserMaps(targetUID, targetGID uint32) (uidMap, gidMap []specs.LinuxIDMapping, err error) { uid := uint32(os.Getuid()) gid := uint32(os.Getgid()) // Get user's configured subuid & subgid ranges diff --git a/internal/pkg/runtime/launcher/oci/process_linux_test.go b/internal/pkg/runtime/launcher/oci/process_linux_test.go index 8163158ea2..6c4050e159 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux_test.go +++ b/internal/pkg/runtime/launcher/oci/process_linux_test.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. diff --git a/pkg/ocibundle/native/bundle_linux.go b/pkg/ocibundle/native/bundle_linux.go index f848efab18..f3cb82390a 100644 --- a/pkg/ocibundle/native/bundle_linux.go +++ b/pkg/ocibundle/native/bundle_linux.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. diff --git a/pkg/ocibundle/native/bundle_linux_test.go b/pkg/ocibundle/native/bundle_linux_test.go index 91e195d407..e52c438c79 100644 --- a/pkg/ocibundle/native/bundle_linux_test.go +++ b/pkg/ocibundle/native/bundle_linux_test.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. diff --git a/pkg/ocibundle/sif/bundle_linux.go b/pkg/ocibundle/sif/bundle_linux.go index abaa7f45f5..63e3f8caa6 100644 --- a/pkg/ocibundle/sif/bundle_linux.go +++ b/pkg/ocibundle/sif/bundle_linux.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2019-2020, Sylabs Inc. All rights reserved. +// Copyright (c) 2019-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. diff --git a/pkg/ocibundle/tools/oci.go b/pkg/ocibundle/tools/oci.go index c1cad97ae4..74a7b7e247 100644 --- a/pkg/ocibundle/tools/oci.go +++ b/pkg/ocibundle/tools/oci.go @@ -2,7 +2,7 @@ // Apptainer a Series of LF Projects LLC. // For website terms of use, trademark policy, privacy policy and other // project policies see https://lfprojects.org/policies -// Copyright (c) 2019-2020, Sylabs Inc. All rights reserved. +// Copyright (c) 2019-2023, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE.md file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -112,7 +112,7 @@ func DeleteBundle(bundlePath string) error { } // BundleUser returns a user struct for the specified user, from the bundle passwd file. -func Bundle(bundlePath, user string) (u *user.User, err error) { +func BundleUser(bundlePath, user string) (u *user.User, err error) { passwd := filepath.Join(RootFs(bundlePath).Path(), "etc", "passwd") if _, err := os.Stat(passwd); err != nil { return nil, fmt.Errorf("cannot access container passwd file: %w", err)