Skip to content

Commit 6fc1914

Browse files
cypharcyphar
committed
internal: move utils.MkdirAllInRoot to internal/pathrs
We will have more wrappers around filepath-securejoin, and so move them to their own specific package so that we can eventually use libpathrs fairly cleanly (by swapping out the implementation). Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
1 parent db19bbe commit 6fc1914

File tree

6 files changed

+214
-89
lines changed

6 files changed

+214
-89
lines changed

‎internal/pathrs/doc.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
4+
* Copyright (C) 2024-2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
// Package pathrs provides wrappers around filepath-securejoin to add the
20+
// minimum set of features needed from libpathrs that are not provided by
21+
// filepath-securejoin, with the eventual goal being that these can be used to
22+
// ease the transition by converting them stubs when enabling libpathrs builds.
23+
package pathrs

‎internal/pathrs/mkdirall_securejoin.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
4+
* Copyright (C) 2024-2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package pathrs
20+
21+
import (
22+
"fmt"
23+
"os"
24+
"path/filepath"
25+
26+
securejoin "github.com/cyphar/filepath-securejoin"
27+
"github.com/sirupsen/logrus"
28+
"golang.org/x/sys/unix"
29+
)
30+
31+
// MkdirAllInRootOpen attempts to make
32+
//
33+
// path, _ := securejoin.SecureJoin(root, unsafePath)
34+
// os.MkdirAll(path, mode)
35+
// os.Open(path)
36+
//
37+
// safer against attacks where components in the path are changed between
38+
// SecureJoin returning and MkdirAll (or Open) being called. In particular, we
39+
// try to detect any symlink components in the path while we are doing the
40+
// MkdirAll.
41+
//
42+
// NOTE: If unsafePath is a subpath of root, we assume that you have already
43+
// called SecureJoin and so we use the provided path verbatim without resolving
44+
// any symlinks (this is done in a way that avoids symlink-exchange races).
45+
// This means that the path also must not contain ".." elements, otherwise an
46+
// error will occur.
47+
//
48+
// This uses securejoin.MkdirAllHandle under the hood, but it has special
49+
// handling if unsafePath has already been scoped within the rootfs (this is
50+
// needed for a lot of runc callers and fixing this would require reworking a
51+
// lot of path logic).
52+
func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (*os.File, error) {
53+
// If the path is already "within" the root, get the path relative to the
54+
// root and use that as the unsafe path. This is necessary because a lot of
55+
// MkdirAllInRootOpen callers have already done SecureJoin, and refactoring
56+
// all of them to stop using these SecureJoin'd paths would require a fair
57+
// amount of work.
58+
// TODO(cyphar): Do the refactor to libpathrs once it's ready.
59+
if IsLexicallyInRoot(root, unsafePath) {
60+
subPath, err := filepath.Rel(root, unsafePath)
61+
if err != nil {
62+
return nil, err
63+
}
64+
unsafePath = subPath
65+
}
66+
67+
// Check for any silly mode bits.
68+
if mode&^0o7777 != 0 {
69+
return nil, fmt.Errorf("tried to include non-mode bits in MkdirAll mode: 0o%.3o", mode)
70+
}
71+
// Linux (and thus os.MkdirAll) silently ignores the suid and sgid bits if
72+
// passed. While it would make sense to return an error in that case (since
73+
// the user has asked for a mode that won't be applied), for compatibility
74+
// reasons we have to ignore these bits.
75+
if ignoredBits := mode &^ 0o1777; ignoredBits != 0 {
76+
logrus.Warnf("MkdirAll called with no-op mode bits that are ignored by Linux: 0o%.3o", ignoredBits)
77+
mode &= 0o1777
78+
}
79+
80+
rootDir, err := os.OpenFile(root, unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
81+
if err != nil {
82+
return nil, fmt.Errorf("open root handle: %w", err)
83+
}
84+
defer rootDir.Close()
85+
86+
return securejoin.MkdirAllHandle(rootDir, unsafePath, mode)
87+
}
88+
89+
// MkdirAllInRoot is a wrapper around MkdirAllInRootOpen which closes the
90+
// returned handle, for callers that don't need to use it.
91+
func MkdirAllInRoot(root, unsafePath string, mode os.FileMode) error {
92+
f, err := MkdirAllInRootOpen(root, unsafePath, mode)
93+
if err == nil {
94+
_ = f.Close()
95+
}
96+
return err
97+
}

‎internal/pathrs/path.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
4+
* Copyright (C) 2024-2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package pathrs
20+
21+
import (
22+
"strings"
23+
)
24+
25+
// IsLexicallyInRoot is shorthand for strings.HasPrefix(path+"/", root+"/"),
26+
// but properly handling the case where path or root have a "/" suffix.
27+
//
28+
// NOTE: The return value only make sense if the path is already mostly cleaned
29+
// (i.e., doesn't contain "..", ".", nor unneeded "/"s).
30+
func IsLexicallyInRoot(root, path string) bool {
31+
root = strings.TrimRight(root, "/")
32+
path = strings.TrimRight(path, "/")
33+
return strings.HasPrefix(path+"/", root+"/")
34+
}

‎internal/pathrs/path_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
4+
* Copyright (C) 2024-2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package pathrs
20+
21+
import "testing"
22+
23+
func TestIsLexicallyInRoot(t *testing.T) {
24+
for _, test := range []struct {
25+
name string
26+
root, path string
27+
expected bool
28+
}{
29+
{"Equal1", "/foo", "/foo", true},
30+
{"Equal2", "/bar/baz", "/bar/baz", true},
31+
{"Equal3", "/bar/baz/", "/bar/baz/", true},
32+
{"Root", "/", "/foo/bar", true},
33+
{"Root-Equal", "/", "/", true},
34+
{"InRoot-Basic1", "/foo/bar", "/foo/bar/baz/abcd", true},
35+
{"InRoot-Basic2", "/a/b/c/d", "/a/b/c/d/e/f/g/h", true},
36+
{"InRoot-Long", "/var/lib/docker/container/1234abcde/rootfs", "/var/lib/docker/container/1234abcde/rootfs/a/b/c", true},
37+
{"InRoot-TrailingSlash1", "/foo/bar/", "/foo/bar", true},
38+
{"InRoot-TrailingSlash2", "/foo/", "/foo/bar/baz/boop", true},
39+
{"NotInRoot-Basic1", "/foo", "/bar", false},
40+
{"NotInRoot-Basic2", "/foo", "/bar", false},
41+
{"NotInRoot-Basic3", "/foo/bar/baz", "/foo/boo/baz/abc", false},
42+
{"NotInRoot-Long", "/var/lib/docker/container/1234abcde/rootfs", "/a/b/c", false},
43+
{"NotInRoot-Tricky1", "/foo/bar", "/foo/bara", false},
44+
{"NotInRoot-Tricky2", "/foo/bar", "/foo/ba/r", false},
45+
} {
46+
t.Run(test.name, func(t *testing.T) {
47+
got := IsLexicallyInRoot(test.root, test.path)
48+
if test.expected != got {
49+
t.Errorf("IsLexicallyInRoot(%q, %q) = %v (expected %v)", test.root, test.path, got, test.expected)
50+
}
51+
})
52+
}
53+
}

‎libcontainer/rootfs_linux.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
devices "github.com/opencontainers/cgroups/devices/config"
2626
"github.com/opencontainers/cgroups/fs2"
2727
"github.com/opencontainers/runc/internal/linux"
28+
"github.com/opencontainers/runc/internal/pathrs"
2829
"github.com/opencontainers/runc/libcontainer/configs"
2930
"github.com/opencontainers/runc/libcontainer/utils"
3031
)
@@ -327,7 +328,7 @@ func mountCgroupV1(m *configs.Mount, c *mountConfig) error {
327328
// inside the tmpfs, so we don't want to resolve symlinks).
328329
subsystemPath := filepath.Join(c.root, b.Destination)
329330
subsystemName := filepath.Base(b.Destination)
330-
if err := utils.MkdirAllInRoot(c.root, subsystemPath, 0o755); err != nil {
331+
if err := pathrs.MkdirAllInRoot(c.root, subsystemPath, 0o755); err != nil {
331332
return err
332333
}
333334
if err := utils.WithProcfd(c.root, b.Destination, func(dstFd string) error {
@@ -520,7 +521,7 @@ func createMountpoint(rootfs string, m mountEntry) (string, error) {
520521
}
521522
// Make the parent directory.
522523
destDir, destBase := filepath.Split(dest)
523-
destDirFd, err := utils.MkdirAllInRootOpen(rootfs, destDir, 0o755)
524+
destDirFd, err := pathrs.MkdirAllInRootOpen(rootfs, destDir, 0o755)
524525
if err != nil {
525526
return "", fmt.Errorf("make parent dir of file bind-mount: %w", err)
526527
}
@@ -557,7 +558,7 @@ func createMountpoint(rootfs string, m mountEntry) (string, error) {
557558
}
558559
}
559560

560-
if err := utils.MkdirAllInRoot(rootfs, dest, 0o755); err != nil {
561+
if err := pathrs.MkdirAllInRoot(rootfs, dest, 0o755); err != nil {
561562
return "", err
562563
}
563564
return dest, nil
@@ -576,7 +577,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
576577
// TODO: This won't be necessary once we switch to libpathrs and we can
577578
// stop all of these symlink-exchange attacks.
578579
dest := filepath.Clean(m.Destination)
579-
if !utils.IsLexicallyInRoot(rootfs, dest) {
580+
if !pathrs.IsLexicallyInRoot(rootfs, dest) {
580581
// Do not use securejoin as it resolves symlinks.
581582
dest = filepath.Join(rootfs, dest)
582583
}
@@ -590,7 +591,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
590591
} else if !fi.IsDir() {
591592
return fmt.Errorf("filesystem %q must be mounted on ordinary directory", m.Device)
592593
}
593-
if err := utils.MkdirAllInRoot(rootfs, dest, 0o755); err != nil {
594+
if err := pathrs.MkdirAllInRoot(rootfs, dest, 0o755); err != nil {
594595
return err
595596
}
596597
// Selinux kernels do not support labeling of /proc or /sys.
@@ -956,7 +957,7 @@ func createDeviceNode(rootfs string, node *devices.Device, bind bool) error {
956957
if dest == rootfs {
957958
return fmt.Errorf("%w: mknod over rootfs", errRootfsToFile)
958959
}
959-
if err := utils.MkdirAllInRoot(rootfs, filepath.Dir(dest), 0o755); err != nil {
960+
if err := pathrs.MkdirAllInRoot(rootfs, filepath.Dir(dest), 0o755); err != nil {
960961
return err
961962
}
962963
if bind {

‎libcontainer/utils/utils_unix.go‎

Lines changed: 0 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"path/filepath"
1010
"runtime"
1111
"strconv"
12-
"strings"
1312
"sync"
1413
_ "unsafe" // for go:linkname
1514

@@ -269,88 +268,6 @@ func ProcThreadSelfFd(fd uintptr) (string, ProcThreadSelfCloser) {
269268
return ProcThreadSelf("fd/" + strconv.FormatUint(uint64(fd), 10))
270269
}
271270

272-
// IsLexicallyInRoot is shorthand for strings.HasPrefix(path+"/", root+"/"),
273-
// but properly handling the case where path or root are "/".
274-
//
275-
// NOTE: The return value only make sense if the path doesn't contain "..".
276-
func IsLexicallyInRoot(root, path string) bool {
277-
if root != "/" {
278-
root += "/"
279-
}
280-
if path != "/" {
281-
path += "/"
282-
}
283-
return strings.HasPrefix(path, root)
284-
}
285-
286-
// MkdirAllInRootOpen attempts to make
287-
//
288-
// path, _ := securejoin.SecureJoin(root, unsafePath)
289-
// os.MkdirAll(path, mode)
290-
// os.Open(path)
291-
//
292-
// safer against attacks where components in the path are changed between
293-
// SecureJoin returning and MkdirAll (or Open) being called. In particular, we
294-
// try to detect any symlink components in the path while we are doing the
295-
// MkdirAll.
296-
//
297-
// NOTE: If unsafePath is a subpath of root, we assume that you have already
298-
// called SecureJoin and so we use the provided path verbatim without resolving
299-
// any symlinks (this is done in a way that avoids symlink-exchange races).
300-
// This means that the path also must not contain ".." elements, otherwise an
301-
// error will occur.
302-
//
303-
// This uses securejoin.MkdirAllHandle under the hood, but it has special
304-
// handling if unsafePath has already been scoped within the rootfs (this is
305-
// needed for a lot of runc callers and fixing this would require reworking a
306-
// lot of path logic).
307-
func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (_ *os.File, Err error) {
308-
// If the path is already "within" the root, get the path relative to the
309-
// root and use that as the unsafe path. This is necessary because a lot of
310-
// MkdirAllInRootOpen callers have already done SecureJoin, and refactoring
311-
// all of them to stop using these SecureJoin'd paths would require a fair
312-
// amount of work.
313-
// TODO(cyphar): Do the refactor to libpathrs once it's ready.
314-
if IsLexicallyInRoot(root, unsafePath) {
315-
subPath, err := filepath.Rel(root, unsafePath)
316-
if err != nil {
317-
return nil, err
318-
}
319-
unsafePath = subPath
320-
}
321-
322-
// Check for any silly mode bits.
323-
if mode&^0o7777 != 0 {
324-
return nil, fmt.Errorf("tried to include non-mode bits in MkdirAll mode: 0o%.3o", mode)
325-
}
326-
// Linux (and thus os.MkdirAll) silently ignores the suid and sgid bits if
327-
// passed. While it would make sense to return an error in that case (since
328-
// the user has asked for a mode that won't be applied), for compatibility
329-
// reasons we have to ignore these bits.
330-
if ignoredBits := mode &^ 0o1777; ignoredBits != 0 {
331-
logrus.Warnf("MkdirAll called with no-op mode bits that are ignored by Linux: 0o%.3o", ignoredBits)
332-
mode &= 0o1777
333-
}
334-
335-
rootDir, err := os.OpenFile(root, unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
336-
if err != nil {
337-
return nil, fmt.Errorf("open root handle: %w", err)
338-
}
339-
defer rootDir.Close()
340-
341-
return securejoin.MkdirAllHandle(rootDir, unsafePath, mode)
342-
}
343-
344-
// MkdirAllInRoot is a wrapper around MkdirAllInRootOpen which closes the
345-
// returned handle, for callers that don't need to use it.
346-
func MkdirAllInRoot(root, unsafePath string, mode os.FileMode) error {
347-
f, err := MkdirAllInRootOpen(root, unsafePath, mode)
348-
if err == nil {
349-
_ = f.Close()
350-
}
351-
return err
352-
}
353-
354271
// Openat is a Go-friendly openat(2) wrapper.
355272
func Openat(dir *os.File, path string, flags int, mode uint32) (*os.File, error) {
356273
dirFd := unix.AT_FDCWD

0 commit comments

Comments
 (0)