1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
|
From d75b05441772417a0828465a9483f16287937724 Mon Sep 17 00:00:00 2001
From: Michael Crosby <crosbymichael@gmail.com>
Date: Mon, 23 Sep 2019 16:45:45 -0400
Subject: [PATCH] Only allow proc mount if it is procfs
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>
Upstream-Status: Backport [https://github.com/opencontainers/runc/pull/2129/commits/331692baa7afdf6c186f8667cb0e6362ea0802b3]
CVE: CVE-2019-16884
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
libcontainer/container_linux.go | 4 +--
libcontainer/rootfs_linux.go | 50 +++++++++++++++++++++++--------
libcontainer/rootfs_linux_test.go | 8 ++---
3 files changed, 43 insertions(+), 19 deletions(-)
diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
index 7e58e5e0..d51e35df 100644
--- a/src/import/libcontainer/container_linux.go
+++ b/src/import/libcontainer/container_linux.go
@@ -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"
@@ -1160,7 +1160,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
diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go
index f13b226e..5650b0ac 100644
--- a/src/import/libcontainer/rootfs_linux.go
+++ b/src/import/libcontainer/rootfs_linux.go
@@ -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"
@@ -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.
@@ -388,7 +388,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.
@@ -435,12 +435,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,
@@ -463,16 +463,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 {
diff --git a/libcontainer/rootfs_linux_test.go b/libcontainer/rootfs_linux_test.go
index d755984b..1bfe7c66 100644
--- a/src/import/libcontainer/rootfs_linux_test.go
+++ b/src/import/libcontainer/rootfs_linux_test.go
@@ -10,7 +10,7 @@ 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")
}
@@ -18,7 +18,7 @@ func TestCheckMountDestOnProc(t *testing.T) {
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")
}
@@ -26,7 +26,7 @@ func TestCheckMountDestOnProcChroot(t *testing.T) {
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")
}
@@ -34,7 +34,7 @@ func TestCheckMountDestInSys(t *testing.T) {
func TestCheckMountDestFalsePositive(t *testing.T) {
dest := "/rootfs/sysfiles/fs/cgroup"
- err := checkMountDestination("/rootfs", dest)
+ err := checkProcMount("/rootfs", dest, "")
if err != nil {
t.Fatal(err)
}
--
2.17.1
|
