Skip to content

Commit

Permalink
core/runtime: Check shim PluginInfo to enforce idmap support
Browse files Browse the repository at this point in the history 
This commit gets rid of the TODO by moving the check to use the
pluginInfo() infrastructure.

The check is only enforced for shims that return info that can be read
as type runtime.Features. For shims that don't provide that, we just
ignore it, as those shims might not be affected by this.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
  • Loading branch information
@rata
rata committed Feb 7, 2024
1 parent 805ed8e commit 9243ae1
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 92 deletions.
70 changes: 0 additions & 70 deletions cmd/containerd-shim-runc-v2/process/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package process
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -142,12 +141,6 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
opts.ConsoleSocket = socket
}

// runc ignores silently features it doesn't know about, so for things that this is
// problematic let's check if this runc version supports them.
if err := p.validateRuncFeatures(ctx, r.Bundle); err != nil {
return fmt.Errorf("failed to detect OCI runtime features: %w", err)
}

if err := p.runtime.Create(ctx, r.ID, r.Bundle, opts); err != nil {
return p.runtimeError(err, "OCI runtime create failed")
}
Expand Down Expand Up @@ -181,60 +174,6 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
return nil
}

func (p *Init) validateRuncFeatures(ctx context.Context, bundle string) error {
// TODO: We should remove the logic from here and rebase on #8509.
// This way we can avoid the call to readConfig() here and the call to p.runtime.Features()
// in validateIDMapMounts().
// But that PR is not yet merged nor it is clear if it will be refactored.
// Do this contained hack for now.
spec, err := readConfig(bundle)
if err != nil {
return fmt.Errorf("failed to read config: %w", err)
}

if err := p.validateIDMapMounts(ctx, spec); err != nil {
return fmt.Errorf("OCI runtime doesn't support idmap mounts: %w", err)
}

return nil
}

func (p *Init) validateIDMapMounts(ctx context.Context, spec *specs.Spec) error {
var used bool
for _, m := range spec.Mounts {
if m.UIDMappings != nil || m.GIDMappings != nil {
used = true
break
}
if sliceContainsStr(m.Options, "idmap") || sliceContainsStr(m.Options, "ridmap") {
used = true
break
}
}

if !used {
return nil
}

// From here onwards, we require idmap mounts. So if we fail to check, we return an error.
features, err := p.runtime.Features(ctx)
if err != nil {
// If the features command is not implemented, then runc is too old.
return fmt.Errorf("features command failed: %w", err)

}

if features.Linux.MountExtensions == nil || features.Linux.MountExtensions.IDMap == nil {
return errors.New("missing `mountExtensions.idmap` entry in `features` command")
}

if enabled := features.Linux.MountExtensions.IDMap.Enabled; enabled == nil || !*enabled {
return errors.New("idmap mounts not supported")
}

return nil
}

func (p *Init) openStdin(path string) error {
sc, err := fifo.OpenFifo(context.Background(), path, unix.O_WRONLY|unix.O_NONBLOCK, 0)
if err != nil {
Expand Down Expand Up @@ -556,12 +495,3 @@ func withConditionalIO(c stdio.Stdio) runc.IOOpt {
o.OpenStderr = c.Stderr != ""
}
}

func sliceContainsStr(s []string, str string) bool {
for _, s := range s {
if s == str {
return true
}
}
return false
}
22 changes: 0 additions & 22 deletions cmd/containerd-shim-runc-v2/process/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package process
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand All @@ -32,7 +31,6 @@ import (

"github.com/containerd/errdefs"
runc "github.com/containerd/go-runc"
specs "github.com/opencontainers/runtime-spec/specs-go"
"golang.org/x/sys/unix"
)

Expand Down Expand Up @@ -188,23 +186,3 @@ func stateName(v interface{}) string {
}
panic(fmt.Errorf("invalid state %v", v))
}

func readConfig(path string) (spec *specs.Spec, err error) {
cfg := filepath.Join(path, configFile)
f, err := os.Open(cfg)
if err != nil {
if os.IsNotExist(err) {
return nil, fmt.Errorf("JSON specification file %s not found", cfg)
}
return nil, err
}
defer f.Close()

if err = json.NewDecoder(f).Decode(&spec); err != nil {
return nil, fmt.Errorf("failed to parse config: %w", err)
}
if spec == nil {
return nil, errors.New("config cannot be null")
}
return spec, nil
}
87 changes: 87 additions & 0 deletions core/runtime/v2/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ import (
"github.com/containerd/platforms"
"github.com/containerd/plugin"
"github.com/containerd/plugin/registry"
"github.com/containerd/typeurl/v2"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/runtime-spec/specs-go/features"
)

// Config for the v2 runtime
Expand Down Expand Up @@ -455,6 +458,10 @@ func (m *TaskManager) Create(ctx context.Context, taskID string, opts runtime.Cr
return nil, err
}

if err := m.validateRuntimeFeatures(ctx, opts); err != nil {
return nil, fmt.Errorf("failed to validate OCI runtime features: %w", err)
}

t, err := shimTask.Create(ctx, opts)
if err != nil {
// NOTE: ctx contains required namespace information.
Expand Down Expand Up @@ -568,3 +575,83 @@ func (m *TaskManager) PluginInfo(ctx context.Context, request interface{}) (inte
}
return &info, nil
}

func (m *TaskManager) validateRuntimeFeatures(ctx context.Context, opts runtime.CreateOpts) error {
var spec specs.Spec
if err := typeurl.UnmarshalTo(opts.Spec, &spec); err != nil {
return fmt.Errorf("unmarshal spec: %w", err)
}

// Only ask for the PluginInfo if idmap mounts are used.
if !usesIDMapMounts(spec) {
return nil
}

pInfo, err := m.PluginInfo(ctx, &apitypes.RuntimeRequest{RuntimePath: opts.Runtime})
if err != nil {
return fmt.Errorf("runtime info: %w", err)
}

pluginInfo, ok := pInfo.(*apitypes.RuntimeInfo)
if !ok {
return fmt.Errorf("invalid runtime info type: %T", pInfo)
}

feat, err := typeurl.UnmarshalAny(pluginInfo.Features)
if err != nil {
return fmt.Errorf("unmarshal runtime features: %w", err)
}

// runc-compatible runtimes silently ignores features it doesn't know about. But ignoring
// our request to use idmap mounts can break permissions in the volume, so let's make sure
// it supports it. For more info, see:
// https://github.com/opencontainers/runtime-spec/pull/1219
//
features, ok := feat.(*features.Features)
if !ok {
// Leave alone non runc-compatible runtimes that don't provide the features info,
// they might not be affected by this.
return nil
}

if err := supportsIDMapMounts(features); err != nil {
return fmt.Errorf("idmap mounts: %w", err)
}

return nil
}

func usesIDMapMounts(spec specs.Spec) bool {
for _, m := range spec.Mounts {
if m.UIDMappings != nil || m.GIDMappings != nil {
return true
}
// TODO (rata): When we switch to go 1.21, we can use the slices package from stdlib
// instead of sliceContainsStr().
if sliceContainsStr(m.Options, "idmap") || sliceContainsStr(m.Options, "ridmap") {
return true
}

}
return false
}

func supportsIDMapMounts(features *features.Features) error {
if features.Linux.MountExtensions == nil || features.Linux.MountExtensions.IDMap == nil {
return errors.New("missing `mountExtensions.idmap` entry in `features` command")

}
if enabled := features.Linux.MountExtensions.IDMap.Enabled; enabled == nil || !*enabled {
return errors.New("not supported or disabled")
}
return nil
}

func sliceContainsStr(s []string, str string) bool {
for _, s := range s {
if s == str {
return true
}
}
return false
}

0 comments on commit 9243ae1

Please sign in to comment.