Merge pull request from GHSA-c2h3-6mxw-7mvq

[release/1.5] v1 & v2 runtimes: reduce permissions for bundle dir

releases/v1.5.7.toml

@@ -0,0 +1,19 @@
+# commit to be tagged for new release
+commit = "HEAD"
+
+project_name = "containerd"
+github_repo = "containerd/containerd"
+match_deps = "^github.com/(containerd/[a-zA-Z0-9-]+)$"
+
+# previous release
+previous = "v1.5.6"
+
+pre_release = false
+
+preface = """\
+The seventh patch release for containerd 1.5 is a security release to fix CVE-2021-41103.
+
+### Notable Updates
+* **Fix insufficiently restricted permissions on container root and plugin directories** [GHSA-c2h3-6mxw-7mvq](https://github.com/containerd/containerd/security/advisories/GHSA-c2h3-6mxw-7mvq)
+
+See the changelog for complete list of changes"""

runtime/v1/linux/bundle.go

@@ -21,6 +21,7 @@ package linux
 import (
 	"context"
 	"crypto/sha256"
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -30,6 +31,7 @@ import (
 	"github.com/containerd/containerd/runtime/linux/runctypes"
 	"github.com/containerd/containerd/runtime/v1/shim"
 	"github.com/containerd/containerd/runtime/v1/shim/client"
+	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 )
 
@@ -48,14 +50,17 @@ func newBundle(id, path, workDir string, spec []byte) (b *bundle, err error) {
 		return nil, err
 	}
 	path = filepath.Join(path, id)
-	if err := os.Mkdir(path, 0711); err != nil {
+	if err := os.Mkdir(path, 0700); err != nil {
 		return nil, err
 	}
 	defer func() {
 		if err != nil {
 			os.RemoveAll(path)
 		}
 	}()
+	if err := prepareBundleDirectoryPermissions(path, spec); err != nil {
+		return nil, err
+	}
 	workDir = filepath.Join(workDir, id)
 	if err := os.MkdirAll(workDir, 0711); err != nil {
 		return nil, err
@@ -77,6 +82,55 @@ func newBundle(id, path, workDir string, spec []byte) (b *bundle, err error) {
 	}, err
 }
 
+// prepareBundleDirectoryPermissions prepares the permissions of the bundle
+// directory. When user namespaces are enabled, the permissions are modified
+// to allow the remapped root GID to access the bundle.
+func prepareBundleDirectoryPermissions(path string, spec []byte) error {
+	gid, err := remappedGID(spec)
+	if err != nil {
+		return err
+	}
+	if gid == 0 {
+		return nil
+	}
+	if err := os.Chown(path, -1, int(gid)); err != nil {
+		return err
+	}
+	return os.Chmod(path, 0710)
+}
+
+// ociSpecUserNS is a subset of specs.Spec used to reduce garbage during
+// unmarshal.
+type ociSpecUserNS struct {
+	Linux *linuxSpecUserNS
+}
+
+// linuxSpecUserNS is a subset of specs.Linux used to reduce garbage during
+// unmarshal.
+type linuxSpecUserNS struct {
+	GIDMappings []specs.LinuxIDMapping
+}
+
+// remappedGID reads the remapped GID 0 from the OCI spec, if it exists. If
+// there is no remapping, remappedGID returns 0. If the spec cannot be parsed,
+// remappedGID returns an error.
+func remappedGID(spec []byte) (uint32, error) {
+	var ociSpec ociSpecUserNS
+	err := json.Unmarshal(spec, &ociSpec)
+	if err != nil {
+		return 0, err
+	}
+	if ociSpec.Linux == nil || len(ociSpec.Linux.GIDMappings) == 0 {
+		return 0, nil
+	}
+	for _, mapping := range ociSpec.Linux.GIDMappings {
+		if mapping.ContainerID == 0 {
+			return mapping.HostID, nil
+		}
+	}
+	return 0, nil
+}
+
 type bundle struct {
 	id      string
 	path    string

runtime/v1/linux/bundle_test.go

@@ -0,0 +1,145 @@
+//go:build linux
+// +build linux
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package linux
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strconv"
+	"syscall"
+	"testing"
+
+	"github.com/containerd/containerd/oci"
+	"github.com/containerd/continuity/testutil"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewBundle(t *testing.T) {
+	testutil.RequiresRoot(t)
+	tests := []struct {
+		userns bool
+	}{{
+		userns: false,
+	}, {
+		userns: true,
+	}}
+	const usernsGID = 4200
+
+	for i, tc := range tests {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			dir, err := ioutil.TempDir("", "test-new-bundle")
+			require.NoError(t, err, "failed to create test directory")
+			defer os.RemoveAll(dir)
+			work := filepath.Join(dir, "work")
+			state := filepath.Join(dir, "state")
+			id := fmt.Sprintf("new-bundle-%d", i)
+			spec := oci.Spec{}
+			if tc.userns {
+				spec.Linux = &specs.Linux{
+					GIDMappings: []specs.LinuxIDMapping{{ContainerID: 0, HostID: usernsGID}},
+				}
+			}
+			specBytes, err := json.Marshal(&spec)
+			require.NoError(t, err, "failed to marshal spec")
+
+			b, err := newBundle(id, work, state, specBytes)
+			require.NoError(t, err, "newBundle should succeed")
+			require.NotNil(t, b, "bundle should not be nil")
+
+			fi, err := os.Stat(b.path)
+			assert.NoError(t, err, "should be able to stat bundle path")
+			if tc.userns {
+				assert.Equal(t, os.ModeDir|0710, fi.Mode(), "bundle path should be a directory with perm 0710")
+			} else {
+				assert.Equal(t, os.ModeDir|0700, fi.Mode(), "bundle path should be a directory with perm 0700")
+			}
+			stat, ok := fi.Sys().(*syscall.Stat_t)
+			require.True(t, ok, "should assert to *syscall.Stat_t")
+			expectedGID := uint32(0)
+			if tc.userns {
+				expectedGID = usernsGID
+			}
+			assert.Equal(t, expectedGID, stat.Gid, "gid should match")
+
+		})
+	}
+}
+
+func TestRemappedGID(t *testing.T) {
+	tests := []struct {
+		spec oci.Spec
+		gid  uint32
+	}{{
+		// empty spec
+		spec: oci.Spec{},
+		gid:  0,
+	}, {
+		// empty Linux section
+		spec: oci.Spec{
+			Linux: &specs.Linux{},
+		},
+		gid: 0,
+	}, {
+		// empty ID mappings
+		spec: oci.Spec{
+			Linux: &specs.Linux{
+				GIDMappings: make([]specs.LinuxIDMapping, 0),
+			},
+		},
+		gid: 0,
+	}, {
+		// valid ID mapping
+		spec: oci.Spec{
+			Linux: &specs.Linux{
+				GIDMappings: []specs.LinuxIDMapping{{
+					ContainerID: 0,
+					HostID:      1000,
+				}},
+			},
+		},
+		gid: 1000,
+	}, {
+		// missing ID mapping
+		spec: oci.Spec{
+			Linux: &specs.Linux{
+				GIDMappings: []specs.LinuxIDMapping{{
+					ContainerID: 100,
+					HostID:      1000,
+				}},
+			},
+		},
+		gid: 0,
+	}}
+
+	for i, tc := range tests {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			s, err := json.Marshal(tc.spec)
+			require.NoError(t, err, "failed to marshal spec")
+			gid, err := remappedGID(s)
+			assert.NoError(t, err, "should unmarshal successfully")
+			assert.Equal(t, tc.gid, gid, "expected GID to match")
+		})
+	}
+}

runtime/v2/bundle.go

@@ -72,7 +72,10 @@ func NewBundle(ctx context.Context, root, state, id string, spec []byte) (b *Bun
 	if err := os.MkdirAll(filepath.Dir(b.Path), 0711); err != nil {
 		return nil, err
 	}
-	if err := os.Mkdir(b.Path, 0711); err != nil {
+	if err := os.Mkdir(b.Path, 0700); err != nil {
+		return nil, err
+	}
+	if err := prepareBundleDirectoryPermissions(b.Path, spec); err != nil {
 		return nil, err
 	}
 	paths = append(paths, b.Path)

runtime/v2/bundle_default.go

@@ -0,0 +1,24 @@
+//go:build !linux
+// +build !linux
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package v2
+
+// prepareBundleDirectoryPermissions prepares the permissions of the bundle
+// directory according to the needs of the current platform.
+func prepareBundleDirectoryPermissions(path string, spec []byte) error { return nil }

runtime/v2/bundle_linux.go

@@ -0,0 +1,74 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package v2
+
+import (
+	"encoding/json"
+	"os"
+
+	"github.com/opencontainers/runtime-spec/specs-go"
+)
+
+// prepareBundleDirectoryPermissions prepares the permissions of the bundle
+// directory according to the needs of the current platform.
+// On Linux when user namespaces are enabled, the permissions are modified to
+// allow the remapped root GID to access the bundle.
+func prepareBundleDirectoryPermissions(path string, spec []byte) error {
+	gid, err := remappedGID(spec)
+	if err != nil {
+		return err
+	}
+	if gid == 0 {
+		return nil
+	}
+	if err := os.Chown(path, -1, int(gid)); err != nil {
+		return err
+	}
+	return os.Chmod(path, 0710)
+}
+
+// ociSpecUserNS is a subset of specs.Spec used to reduce garbage during
+// unmarshal.
+type ociSpecUserNS struct {
+	Linux *linuxSpecUserNS
+}
+
+// linuxSpecUserNS is a subset of specs.Linux used to reduce garbage during
+// unmarshal.
+type linuxSpecUserNS struct {
+	GIDMappings []specs.LinuxIDMapping
+}
+
+// remappedGID reads the remapped GID 0 from the OCI spec, if it exists. If
+// there is no remapping, remappedGID returns 0. If the spec cannot be parsed,
+// remappedGID returns an error.
+func remappedGID(spec []byte) (uint32, error) {
+	var ociSpec ociSpecUserNS
+	err := json.Unmarshal(spec, &ociSpec)
+	if err != nil {
+		return 0, err
+	}
+	if ociSpec.Linux == nil || len(ociSpec.Linux.GIDMappings) == 0 {
+		return 0, nil
+	}
+	for _, mapping := range ociSpec.Linux.GIDMappings {
+		if mapping.ContainerID == 0 {
+			return mapping.HostID, nil
+		}
+	}
+	return 0, nil
+}