Skip to content

Commit

Permalink
Only allow proc mount if it is procfs
Browse files Browse the repository at this point in the history
Fixes #2128

This allows proc to be bind mounted for host and rootless namespace usecases but
it removes the ability to mount over the top of proc with a directory.

```bash
> sudo docker run --rm  apparmor
docker: Error response from daemon: OCI runtime create failed:
container_linux.go:346: starting container process caused "process_linux.go:449:
container init caused \"rootfs_linux.go:58: mounting
\\\"/var/lib/docker/volumes/aae28ea068c33d60e64d1a75916cf3ec2dc3634f97571854c9ed30c8401460c1/_data\\\"
to rootfs
\\\"/var/lib/docker/overlay2/a6be5ae911bf19f8eecb23a295dec85be9a8ee8da66e9fb55b47c841d1e381b7/merged\\\"
at \\\"/proc\\\" caused
\\\"\\\\\\\"/var/lib/docker/overlay2/a6be5ae911bf19f8eecb23a295dec85be9a8ee8da66e9fb55b47c841d1e381b7/merged/proc\\\\\\\"
cannot be mounted because it is not of type proc\\\"\"": unknown.

> sudo docker run --rm -v /proc:/proc apparmor

docker-default (enforce)        root     18989  0.9  0.0   1288     4 ?
Ss   16:47   0:00 sleep 20
```

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
  • Loading branch information
crosbymichael committed Sep 24, 2019
1 parent 7507c64 commit 331692b
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 19 deletions.
4 changes: 2 additions & 2 deletions libcontainer/container_linux.go
Expand Up @@ -19,7 +19,7 @@ import (
"syscall" // only for SysProcAttr and Signal
"time"

"github.com/cyphar/filepath-securejoin"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/intelrdt"
Expand Down Expand Up @@ -1176,7 +1176,7 @@ func (c *linuxContainer) makeCriuRestoreMountpoints(m *configs.Mount) error {
if err != nil {
return err
}
if err := checkMountDestination(c.config.Rootfs, dest); err != nil {
if err := checkProcMount(c.config.Rootfs, dest, ""); err != nil {
return err
}
m.Destination = dest
Expand Down
50 changes: 37 additions & 13 deletions libcontainer/rootfs_linux.go
Expand Up @@ -13,7 +13,7 @@ import (
"strings"
"time"

"github.com/cyphar/filepath-securejoin"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/mrunalp/fileutils"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
Expand Down Expand Up @@ -197,7 +197,7 @@ func prepareBindMount(m *configs.Mount, rootfs string) error {
if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
return err
}
if err := checkMountDestination(rootfs, dest); err != nil {
if err := checkProcMount(rootfs, dest, m.Source); err != nil {
return err
}
// update the mount with the correct dest after symlinks are resolved.
Expand Down Expand Up @@ -414,7 +414,7 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string, enableCgroupns b
if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
return err
}
if err := checkMountDestination(rootfs, dest); err != nil {
if err := checkProcMount(rootfs, dest, m.Source); err != nil {
return err
}
// update the mount with the correct dest after symlinks are resolved.
Expand Down Expand Up @@ -461,12 +461,12 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
return binds, nil
}

// checkMountDestination checks to ensure that the mount destination is not over the top of /proc.
// checkProcMount checks to ensure that the mount destination is not over the top of /proc.
// dest is required to be an abs path and have any symlinks resolved before calling this function.
func checkMountDestination(rootfs, dest string) error {
invalidDestinations := []string{
"/proc",
}
//
// if source is nil, don't stat the filesystem. This is used for restore of a checkpoint.
func checkProcMount(rootfs, dest, source string) error {
const procPath = "/proc"
// White list, it should be sub directories of invalid destinations
validDestinations := []string{
// These entries can be bind mounted by files emulated by fuse,
Expand All @@ -489,16 +489,40 @@ func checkMountDestination(rootfs, dest string) error {
return nil
}
}
for _, invalid := range invalidDestinations {
path, err := filepath.Rel(filepath.Join(rootfs, invalid), dest)
path, err := filepath.Rel(filepath.Join(rootfs, procPath), dest)
if err != nil {
return err
}
// pass if the mount path is located outside of /proc
if strings.HasPrefix(path, "..") {
return nil
}
if path == "." {
// an empty source is pasted on restore
if source == "" {
return nil
}
// only allow a mount on-top of proc if it's source is "proc"
isproc, err := isProc(source)
if err != nil {
return err
}
if path != "." && !strings.HasPrefix(path, "..") {
return fmt.Errorf("%q cannot be mounted because it is located inside %q", dest, invalid)
// pass if the mount is happening on top of /proc and the source of
// the mount is a proc filesystem
if isproc {
return nil
}
return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
}
return nil
return fmt.Errorf("%q cannot be mounted because it is inside /proc", dest)
}

func isProc(path string) (bool, error) {
var s unix.Statfs_t
if err := unix.Statfs(path, &s); err != nil {
return false, err
}
return s.Type == unix.PROC_SUPER_MAGIC, nil
}

func setupDevSymlinks(rootfs string) error {
Expand Down
8 changes: 4 additions & 4 deletions libcontainer/rootfs_linux_test.go
Expand Up @@ -10,31 +10,31 @@ import (

func TestCheckMountDestOnProc(t *testing.T) {
dest := "/rootfs/proc/sys"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "")
if err == nil {
t.Fatal("destination inside proc should return an error")
}
}

func TestCheckMountDestOnProcChroot(t *testing.T) {
dest := "/rootfs/proc/"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "/proc")
if err != nil {
t.Fatal("destination inside proc when using chroot should not return an error")
}
}

func TestCheckMountDestInSys(t *testing.T) {
dest := "/rootfs//sys/fs/cgroup"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "")
if err != nil {
t.Fatal("destination inside /sys should not return an error")
}
}

func TestCheckMountDestFalsePositive(t *testing.T) {
dest := "/rootfs/sysfiles/fs/cgroup"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "")
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit 331692b

Please sign in to comment.