From 97db45e83d03f735495e899aeebf2069ede728ff Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Mon, 20 Sep 2021 16:20:26 -0700 Subject: [PATCH 1/4] v2 runtime: reduce permissions for bundle dir Bundle directory permissions should be 0700 by default. On Linux with user namespaces enabled, the remapped root also needs access to the bundle directory. In this case, the bundle directory is modified to 0710 and group ownership is changed to the remapped root group. Signed-off-by: Samuel Karp (cherry picked from commit 7d56b24f1a9af82dfaa10ff55a4e3c36a7efd943) --- runtime/v2/bundle.go | 5 +- runtime/v2/bundle_default.go | 24 ++++++ runtime/v2/bundle_linux.go | 74 ++++++++++++++++ runtime/v2/bundle_linux_test.go | 145 ++++++++++++++++++++++++++++++++ runtime/v2/bundle_test.go | 23 +++++ 5 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 runtime/v2/bundle_default.go create mode 100644 runtime/v2/bundle_linux.go create mode 100644 runtime/v2/bundle_linux_test.go create mode 100644 runtime/v2/bundle_test.go diff --git a/runtime/v2/bundle.go b/runtime/v2/bundle.go index 1a58e627b5b5..954163b0f3ea 100644 --- a/runtime/v2/bundle.go +++ b/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) diff --git a/runtime/v2/bundle_default.go b/runtime/v2/bundle_default.go new file mode 100644 index 000000000000..2be40c825415 --- /dev/null +++ b/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 } diff --git a/runtime/v2/bundle_linux.go b/runtime/v2/bundle_linux.go new file mode 100644 index 000000000000..5f1915d77646 --- /dev/null +++ b/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 +} diff --git a/runtime/v2/bundle_linux_test.go b/runtime/v2/bundle_linux_test.go new file mode 100644 index 000000000000..1828fb8cf17a --- /dev/null +++ b/runtime/v2/bundle_linux_test.go @@ -0,0 +1,145 @@ +/* + 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 ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strconv" + "syscall" + "testing" + + "github.com/containerd/containerd/namespaces" + "github.com/containerd/containerd/oci" + "github.com/containerd/containerd/pkg/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") + + ctx := namespaces.WithNamespace(context.TODO(), namespaces.Default) + b, err := NewBundle(ctx, work, state, id, 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") + }) + } +} diff --git a/runtime/v2/bundle_test.go b/runtime/v2/bundle_test.go new file mode 100644 index 000000000000..54e5f24cc2ed --- /dev/null +++ b/runtime/v2/bundle_test.go @@ -0,0 +1,23 @@ +/* + 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 ( + // When testutil is imported for one platform (bundle_linux_test.go) it + // should be imported for all platforms. + _ "github.com/containerd/containerd/pkg/testutil" +) From 68119b417e29c5295a5b8b21b3127f0329ce2090 Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Tue, 21 Sep 2021 13:46:40 -0700 Subject: [PATCH 2/4] v1 runtime: reduce permissions for bundle dir Bundle directory permissions should be 0700 by default. On Linux with user namespaces enabled, the remapped root also needs access to the bundle directory. In this case, the bundle directory is modified to 0710 and group ownership is changed to the remapped root group. Port of the same change for the v2 runtime Signed-off-by: Samuel Karp (cherry picked from commit 6886c6a2ec0c70dde1aa64e77b64a5ad47b983c3) --- runtime/v1/linux/bundle.go | 56 +++++++++++- runtime/v1/linux/bundle_test.go | 145 ++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 runtime/v1/linux/bundle_test.go diff --git a/runtime/v1/linux/bundle.go b/runtime/v1/linux/bundle.go index 9d0a6c447892..48d81e8e0957 100644 --- a/runtime/v1/linux/bundle.go +++ b/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,7 +50,7 @@ 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() { @@ -56,6 +58,9 @@ func newBundle(id, path, workDir string, spec []byte) (b *bundle, err error) { 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 diff --git a/runtime/v1/linux/bundle_test.go b/runtime/v1/linux/bundle_test.go new file mode 100644 index 000000000000..e021dda5f92b --- /dev/null +++ b/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") + }) + } +} From f95fca0790771dc577997b1b12f56791e7a219b7 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Wed, 15 Sep 2021 17:57:13 -0700 Subject: [PATCH 3/4] btrfs: reduce permissions on plugin directories Disallow traversal into directories that may contain unpacked or mounted image filesystems. Signed-off-by: Derek McGowan Signed-off-by: Samuel Karp (cherry picked from commit 7c621e1fcc08bcf5a1a48b837342cc22eada1685) --- snapshots/btrfs/btrfs.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/snapshots/btrfs/btrfs.go b/snapshots/btrfs/btrfs.go index 3096989aee98..dc274ee371f3 100644 --- a/snapshots/btrfs/btrfs.go +++ b/snapshots/btrfs/btrfs.go @@ -50,11 +50,15 @@ type snapshotter struct { // root needs to be a mount point of btrfs. func NewSnapshotter(root string) (snapshots.Snapshotter, error) { // If directory does not exist, create it - if _, err := os.Stat(root); err != nil { + if st, err := os.Stat(root); err != nil { if !os.IsNotExist(err) { return nil, err } - if err := os.Mkdir(root, 0755); err != nil { + if err := os.Mkdir(root, 0700); err != nil { + return nil, err + } + } else if st.Mode()&os.ModePerm != 0700 { + if err := os.Chmod(root, 0700); err != nil { return nil, err } } From bc2f973ffe7125d1082d5df4f956f38115f07919 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Fri, 1 Oct 2021 12:56:03 -0700 Subject: [PATCH 4/4] Prepare release notes for v1.5.7 Signed-off-by: Derek McGowan --- releases/v1.5.7.toml | 19 +++++++++++++++++++ version/version.go | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 releases/v1.5.7.toml diff --git a/releases/v1.5.7.toml b/releases/v1.5.7.toml new file mode 100644 index 000000000000..10c0139cefe1 --- /dev/null +++ b/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""" diff --git a/version/version.go b/version/version.go index dfb7827475f8..b0d1fd48cf67 100644 --- a/version/version.go +++ b/version/version.go @@ -23,7 +23,7 @@ var ( Package = "github.com/containerd/containerd" // Version holds the complete version number. Filled in at linking time. - Version = "1.5.6+unknown" + Version = "1.5.7+unknown" // Revision is filled with the VCS (e.g. git) revision being used to build // the program at linking time.