pkg/process: Only use idmap mounts if runc supports it

rata · rata · commit 2e13d39546cf · 2023-09-13T16:44:54.000+02:00
runc, as mandated by the runtime-spec, ignores unknown fields in the
config.json. This is unfortunate for cases where we _must_ enable that
feature or fail.

For example, if we want to start a container with user namespaces and
volumes, using the uidMappings/gidMappings field is needed so the
UID/GIDs in the volume don't end up with garbage. However, if we don't
fail when runc will ignore these fields (because they are unknown to
runc), we will just start a container without using the mappings and the
UID/GIDs the container will persist to volumes the hostUID/GID, that can
change if the container is re-scheduled by Kubernetes.

This will end up in volumes having "garbage" and unmapped UIDs that the
container can no longer change. So, let's avoid this entirely by just
checking that runc supports idmap mounts if the container we are about
to create needs them.

Please note that the "runc features" subcommand is only run when we are
using idmap mounts. If idmap mounts are not used, the subcommand is not
run and therefore this should not affect containers that don't use idmap
mounts in any way.

Signed-off-by: Rodrigo Campos &lt;rodrigoca@microsoft.com&gt;
diff --git a/integration/pod_userns_linux_test.go b/integration/pod_userns_linux_test.go
@@ -17,6 +17,8 @@
 package integration
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/user"
@@ -27,6 +29,7 @@ import (
 	"time"
 
 	"github.com/containerd/containerd/integration/images"
+	runc "github.com/containerd/go-runc"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	exec "golang.org/x/sys/execabs"
@@ -234,6 +237,9 @@ func TestPodUserNS(t *testing.T) {
 			if test.hostVolumes && !supportsIDMap(volumeHostPath) {
 				t.Skipf("ID mappings are not supported host volume filesystem: %v", volumeHostPath)
 			}
+			if err := supportsRuncIDMap(); err != nil {
+				t.Skipf("OCI runtime doesn't support idmap mounts: %v", err)
+			}
 
 			testPodLogDir := t.TempDir()
 			sandboxOpts := append(test.sandboxOpts, WithPodLogDirectory(testPodLogDir))
@@ -297,3 +303,22 @@ func TestPodUserNS(t *testing.T) {
 		})
 	}
 }
+
+func supportsRuncIDMap() error {
+	var r runc.Runc
+	features, err := r.Features(context.Background())
+	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
+}
diff --git a/pkg/process/init.go b/pkg/process/init.go
@@ -21,6 +21,7 @@ package process
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -140,6 +141,13 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error {
 	if socket != nil {
 		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")
 	}
@@ -173,6 +181,56 @@ 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 !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 {
diff --git a/pkg/process/utils.go b/pkg/process/utils.go
@@ -21,6 +21,7 @@ package process
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -31,6 +32,7 @@ import (
 
 	"github.com/containerd/containerd/errdefs"
 	runc "github.com/containerd/go-runc"
+	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"golang.org/x/sys/unix"
 )
 
@@ -39,6 +41,8 @@ const (
 	RuncRoot = "/run/containerd/runc"
 	// InitPidFile name of the file that contains the init pid
 	InitPidFile = "init.pid"
+	// configFile is the name of the runc config file
+	configFile = "config.json"
 )
 
 // safePid is a thread safe wrapper for pid.
@@ -184,3 +188,23 @@ 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
+}
