diff --git a/executor/oci/mounts.go b/executor/oci/mounts.go index dddfb8469..62360f466 100644 --- a/executor/oci/mounts.go +++ b/executor/oci/mounts.go @@ -5,75 +5,50 @@ import ( "path/filepath" "strings" + "github.com/containerd/containerd/containers" + "github.com/containerd/containerd/oci" specs "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pkg/errors" ) -// MountOpts sets oci spec specific info for mount points -type MountOpts func([]specs.Mount) ([]specs.Mount, error) - -//GetMounts returns default required for buildkit -// https://github.com/moby/buildkit/issues/429 -func GetMounts(ctx context.Context, mountOpts ...MountOpts) ([]specs.Mount, error) { - mounts := []specs.Mount{ - { - Destination: "/proc", - Type: "proc", - Source: "proc", - }, - { - Destination: "/dev", - Type: "tmpfs", - Source: "tmpfs", - Options: []string{"nosuid", "strictatime", "mode=755", "size=65536k"}, - }, - { - Destination: "/dev/pts", - Type: "devpts", - Source: "devpts", - Options: []string{"nosuid", "noexec", "newinstance", "ptmxmode=0666", "mode=0620", "gid=5"}, - }, - { - Destination: "/dev/shm", - Type: "tmpfs", - Source: "shm", - Options: []string{"nosuid", "noexec", "nodev", "mode=1777", "size=65536k"}, - }, - { - Destination: "/dev/mqueue", - Type: "mqueue", - Source: "mqueue", - Options: []string{"nosuid", "noexec", "nodev"}, - }, - { - Destination: "/sys", - Type: "sysfs", - Source: "sysfs", - Options: []string{"nosuid", "noexec", "nodev", "ro"}, - }, - } - var err error - for _, o := range mountOpts { - mounts, err = o(mounts) - if err != nil { - return nil, err +func withRemovedMount(destination string) oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + newMounts := []specs.Mount{} + for _, o := range s.Mounts { + if o.Destination != destination { + newMounts = append(newMounts, o) + } } + s.Mounts = newMounts + + return nil } - return mounts, nil } -func withROBind(src, dest string) func(m []specs.Mount) ([]specs.Mount, error) { - return func(m []specs.Mount) ([]specs.Mount, error) { - m = append(m, specs.Mount{ +func withROBind(src, dest string) oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + s.Mounts = append(s.Mounts, specs.Mount{ Destination: dest, Type: "bind", Source: src, Options: []string{"nosuid", "noexec", "nodev", "rbind", "ro"}, }) - return m, nil + return nil } } +func withCGroup() oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + s.Mounts = append(s.Mounts, specs.Mount{ + Destination: "/sys/fs/cgroup", + Type: "cgroup", + Source: "cgroup", + Options: []string{"ro", "nosuid", "noexec", "nodev"}, + }) + return nil + } + +} + func hasPrefix(p, prefixDir string) bool { prefixDir = filepath.Clean(prefixDir) if filepath.Base(prefixDir) == string(filepath.Separator) { @@ -93,25 +68,35 @@ func removeMountsWithPrefix(mounts []specs.Mount, prefixDir string) []specs.Moun return ret } -func withProcessMode(processMode ProcessMode) func([]specs.Mount) ([]specs.Mount, error) { - return func(m []specs.Mount) ([]specs.Mount, error) { - switch processMode { - case ProcessSandbox: - // keep the default - case NoProcessSandbox: - m = removeMountsWithPrefix(m, "/proc") - procMount := specs.Mount{ - Destination: "/proc", - Type: "bind", - Source: "/proc", - // NOTE: "rbind"+"ro" does not make /proc read-only recursively. - // So we keep maskedPath and readonlyPaths (although not mandatory for rootless mode) - Options: []string{"rbind"}, - } - m = append([]specs.Mount{procMount}, m...) - default: - return nil, errors.Errorf("unknown process mode: %v", processMode) +func withBoundProc() oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + s.Mounts = removeMountsWithPrefix(s.Mounts, "/proc") + procMount := specs.Mount{ + Destination: "/proc", + Type: "bind", + Source: "/proc", + // NOTE: "rbind"+"ro" does not make /proc read-only recursively. + // So we keep maskedPath and readonlyPaths (although not mandatory for rootless mode) + Options: []string{"rbind"}, } - return m, nil + s.Mounts = append([]specs.Mount{procMount}, s.Mounts...) + + var maskedPaths []string + for _, s := range s.Linux.MaskedPaths { + if !hasPrefix(s, "/proc") { + maskedPaths = append(maskedPaths, s) + } + } + s.Linux.MaskedPaths = maskedPaths + + var readonlyPaths []string + for _, s := range s.Linux.ReadonlyPaths { + if !hasPrefix(s, "/proc") { + readonlyPaths = append(readonlyPaths, s) + } + } + s.Linux.ReadonlyPaths = readonlyPaths + + return nil } } diff --git a/executor/oci/mounts_test.go b/executor/oci/mounts_test.go index ec30a111a..0e6900a83 100644 --- a/executor/oci/mounts_test.go +++ b/executor/oci/mounts_test.go @@ -4,6 +4,9 @@ import ( "runtime" "testing" + "github.com/containerd/containerd/oci" + "github.com/moby/buildkit/util/appcontext" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" ) @@ -94,3 +97,58 @@ func TestHasPrefix(t *testing.T) { assert.Equal(t, tc.expected, actual, "#%d: under(%q,%q)", i, tc.path, tc.prefix) } } + +func TestWithRemovedMounts(t *testing.T) { + // The default mount-list from containerd + s := oci.Spec{ + Mounts: []specs.Mount{ + { + Destination: "/proc", + Type: "proc", + Source: "proc", + Options: []string{"nosuid", "noexec", "nodev"}, + }, + { + Destination: "/dev", + Type: "tmpfs", + Source: "tmpfs", + Options: []string{"nosuid", "strictatime", "mode=755", "size=65536k"}, + }, + { + Destination: "/dev/pts", + Type: "devpts", + Source: "devpts", + Options: []string{"nosuid", "noexec", "newinstance", "ptmxmode=0666", "mode=0620", "gid=5"}, + }, + { + Destination: "/dev/shm", + Type: "tmpfs", + Source: "shm", + Options: []string{"nosuid", "noexec", "nodev", "mode=1777", "size=65536k"}, + }, + { + Destination: "/dev/mqueue", + Type: "mqueue", + Source: "mqueue", + Options: []string{"nosuid", "noexec", "nodev"}, + }, + { + Destination: "/sys", + Type: "sysfs", + Source: "sysfs", + Options: []string{"nosuid", "noexec", "nodev", "ro"}, + }, + { + Destination: "/run", + Type: "tmpfs", + Source: "tmpfs", + Options: []string{"nosuid", "strictatime", "mode=755", "size=65536k"}, + }, + }, + } + + oldLen := len(s.Mounts) + err := withRemovedMount("/run")(appcontext.Context(), nil, nil, &s) + assert.NoError(t, err) + assert.Equal(t, oldLen-1, len(s.Mounts)) +} diff --git a/executor/oci/spec.go b/executor/oci/spec.go index 830dca8f9..40ea26c6c 100644 --- a/executor/oci/spec.go +++ b/executor/oci/spec.go @@ -14,7 +14,6 @@ import ( "github.com/mitchellh/hashstructure" "github.com/moby/buildkit/executor" "github.com/moby/buildkit/snapshot" - "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/network" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" @@ -47,6 +46,12 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou ctx = namespaces.WithNamespace(ctx, "buildkit") } + if mountOpts, err := generateMountOpts(resolvConf, hostsFile); err == nil { + opts = append(opts, mountOpts...) + } else { + return nil, nil, err + } + if securityOpts, err := generateSecurityOpts(meta.SecurityMode); err == nil { opts = append(opts, securityOpts...) } else { @@ -59,6 +64,20 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou return nil, nil, err } + if idmapOpts, err := generateIDmapOpts(idmap); err == nil { + opts = append(opts, idmapOpts...) + } else { + return nil, nil, err + } + + opts = append(opts, + oci.WithProcessArgs(meta.Args...), + oci.WithEnv(meta.Env), + oci.WithProcessCwd(meta.Cwd), + oci.WithNewPrivileges, + oci.WithHostname("buildkitsandbox"), + ) + s, err := oci.GenerateSpec(ctx, nil, c, opts...) if err != nil { return nil, nil, err @@ -66,65 +85,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou // set the networking information on the spec namespace.Set(s) - s.Process.Args = meta.Args - s.Process.Env = meta.Env - s.Process.Cwd = meta.Cwd - s.Process.Rlimits = nil // reset open files limit - s.Process.NoNewPrivileges = false // reset nonewprivileges - s.Hostname = "buildkitsandbox" - - // Setup for a Linux-based container (includes LCOW) - if s.Linux != nil { - s.Mounts, err = GetMounts(ctx, - withProcessMode(processMode), - withROBind(resolvConf, "/etc/resolv.conf"), - withROBind(hostsFile, "/etc/hosts"), - ) - if err != nil { - return nil, nil, err - } - - s.Mounts = append(s.Mounts, specs.Mount{ - Destination: "/sys/fs/cgroup", - Type: "cgroup", - Source: "cgroup", - Options: []string{"ro", "nosuid", "noexec", "nodev"}, - }) - - if processMode == NoProcessSandbox { - var maskedPaths []string - for _, s := range s.Linux.MaskedPaths { - if !hasPrefix(s, "/proc") { - maskedPaths = append(maskedPaths, s) - } - } - s.Linux.MaskedPaths = maskedPaths - var readonlyPaths []string - for _, s := range s.Linux.ReadonlyPaths { - if !hasPrefix(s, "/proc") { - readonlyPaths = append(readonlyPaths, s) - } - } - s.Linux.ReadonlyPaths = readonlyPaths - } - - if meta.SecurityMode == pb.SecurityMode_INSECURE { - if err = oci.WithWriteableCgroupfs(ctx, nil, c, s); err != nil { - return nil, nil, err - } - if err = oci.WithWriteableSysfs(ctx, nil, c, s); err != nil { - return nil, nil, err - } - } - - if idmap != nil { - s.Linux.Namespaces = append(s.Linux.Namespaces, specs.LinuxNamespace{ - Type: specs.UserNamespace, - }) - s.Linux.UIDMappings = specMapping(idmap.UIDs()) - s.Linux.GIDMappings = specMapping(idmap.GIDs()) - } - } + s.Process.Rlimits = nil // reset open files limit sm := &submounts{} diff --git a/executor/oci/spec_unix.go b/executor/oci/spec_unix.go index 9bd1a14b2..de36195c3 100644 --- a/executor/oci/spec_unix.go +++ b/executor/oci/spec_unix.go @@ -5,26 +5,54 @@ package oci import ( "github.com/containerd/containerd/contrib/seccomp" "github.com/containerd/containerd/oci" + "github.com/docker/docker/pkg/idtools" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/entitlements/security" "github.com/moby/buildkit/util/system" specs "github.com/opencontainers/runtime-spec/specs-go" ) +func generateMountOpts(resolvConf, hostsFile string) ([]oci.SpecOpts, error) { + return []oci.SpecOpts{ + // https://github.com/moby/buildkit/issues/429 + withRemovedMount("/run"), + withROBind(resolvConf, "/etc/resolv.conf"), + withROBind(hostsFile, "/etc/hosts"), + withCGroup(), + }, nil +} + +// generateSecurityOpts may affect mounts, so must be called after generateMountOpts func generateSecurityOpts(mode pb.SecurityMode) ([]oci.SpecOpts, error) { if mode == pb.SecurityMode_INSECURE { - return []oci.SpecOpts{security.WithInsecureSpec()}, nil + return []oci.SpecOpts{ + security.WithInsecureSpec(), + oci.WithWriteableCgroupfs, + oci.WithWriteableSysfs, + }, nil } else if system.SeccompSupported() && mode == pb.SecurityMode_SANDBOX { return []oci.SpecOpts{seccomp.WithDefaultProfile()}, nil } return nil, nil } +// generateProcessModeOpts may affect mounts, so must be called after generateMountOpts func generateProcessModeOpts(mode ProcessMode) ([]oci.SpecOpts, error) { if mode == NoProcessSandbox { - // Mount for /proc is replaced in GetMounts() anyway - return []oci.SpecOpts{oci.WithHostNamespace(specs.PIDNamespace)}, nil + return []oci.SpecOpts{ + oci.WithHostNamespace(specs.PIDNamespace), + withBoundProc(), + }, nil // TODO(AkihiroSuda): Configure seccomp to disable ptrace (and prctl?) explicitly } return nil, nil } + +func generateIDmapOpts(idmap *idtools.IdentityMapping) ([]oci.SpecOpts, error) { + if idmap == nil { + return nil, nil + } + return []oci.SpecOpts{ + oci.WithUserNamespace(specMapping(idmap.UIDs()), specMapping(idmap.GIDs())), + }, nil +} diff --git a/executor/oci/spec_windows.go b/executor/oci/spec_windows.go index 794b48c1e..4589c9d98 100644 --- a/executor/oci/spec_windows.go +++ b/executor/oci/spec_windows.go @@ -5,11 +5,17 @@ package oci import ( "github.com/containerd/containerd/contrib/seccomp" "github.com/containerd/containerd/oci" + "github.com/docker/docker/pkg/idtools" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/system" "github.com/pkg/errors" ) +func generateMountOpts(resolvConf, hostsFile string) ([]oci.SpecOpts, error) { + return nil, nil +} + +// generateSecurityOpts may affect mounts, so must be called after generateMountOpts func generateSecurityOpts(mode pb.SecurityMode) ([]oci.SpecOpts, error) { if mode == pb.SecurityMode_INSECURE { return nil, errors.New("no support for running in insecure mode on Windows") @@ -20,9 +26,17 @@ func generateSecurityOpts(mode pb.SecurityMode) ([]oci.SpecOpts, error) { return nil, nil } +// generateProcessModeOpts may affect mounts, so must be called after generateMountOpts func generateProcessModeOpts(mode ProcessMode) ([]oci.SpecOpts, error) { if mode == NoProcessSandbox { return nil, errors.New("no support for NoProcessSandbox on Windows") } return nil, nil } + +func generateIDmapOpts(idmap *idtools.IdentityMapping) ([]oci.SpecOpts, error) { + if idmap == nil { + return nil, nil + } + return nil, errors.New("no support for IdentityMapping on Windows") +}