R-1214: the capped copy refuses any path with a symlink in it (security review: a link under /mnt pointing outside resolves elsewhere for the daemon); red-proved; unreleased
gates / gates (push) Successful in 1m53s
gates / gates (push) Successful in 1m53s
This commit is contained in:
1 parent
03c0a693b8
commit
ae394670c1
3 files changed
+33
-3
No files matched your search
+4
-1
@@ -12,7 +12,10 @@
|
|||||||
`/mnt/`, where the controller and the Docker daemon see the same paths: a path on the controller's own data volume
|
`/mnt/`, where the controller and the Docker daemon see the same paths: a path on the controller's own data volume
|
||||||
differs for the daemon, and a capped `--delete` there would mirror an EMPTY folder over the copy. Those paths, a box
|
differs for the daemon, and a capped `--delete` there would mirror an EMPTY folder over the copy. Those paths, a box
|
||||||
whose image cannot be read and a container that cannot start (exit 125) keep the plain in-process rsync; an rsync
|
whose image cannot be read and a container that cannot start (exit 125) keep the plain in-process rsync; an rsync
|
||||||
failure inside the container is an error. Tests `TestR1214_*` (3, Docker-free seams), red-proved both ways.
|
failure inside the container is an error. A path with a SYMLINK anywhere in it never takes the container either
|
||||||
|
(a link under /mnt pointing outside resolves elsewhere for the daemon — found by the commit's security review the
|
||||||
|
same night; `filepath.EvalSymlinks` must return the path itself). Tests `TestR1214_*` (4, Docker-free seams),
|
||||||
|
red-proved each way.
|
||||||
|
|
||||||
## Unreleased — two data-loss candidates fixed (R-1187 c, R-1189 d), 2026-10-22 night
|
## Unreleased — two data-loss candidates fixed (R-1187 c, R-1189 d), 2026-10-22 night
|
||||||
|
|
||||||
|
|||||||
@@ -91,3 +91,24 @@ func TestR1214_ANotStartedContainerFallsBackAnRsyncErrorDoesNot(t *testing.T) {
|
|||||||
t.Fatalf("no readable image → the plain copy: %v", err)
|
t.Fatalf("no readable image → the plain copy: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A symlink under the capped root that points elsewhere must never reach the container (the daemon would resolve it
|
||||||
|
// somewhere else, and `--delete` would mirror that over the copy) — the security review's case.
|
||||||
|
// RED-PROOF: with the EvalSymlinks check removed, this fails — seen failing 2026-10-22.
|
||||||
|
func TestR1214_ASymlinkedPathNeverGoesToDocker(t *testing.T) {
|
||||||
|
root := t.TempDir() + "/"
|
||||||
|
elsewhere := t.TempDir()
|
||||||
|
os.WriteFile(filepath.Join(elsewhere, "f"), []byte("x"), 0644)
|
||||||
|
link := filepath.Join(root, "drive1", "unit")
|
||||||
|
os.MkdirAll(filepath.Dir(link), 0755)
|
||||||
|
if err := os.Symlink(elsewhere, link); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
rec := withSeams(t, root, "img", func([]string) ([]byte, int, error) { return nil, 0, nil })
|
||||||
|
if err := rsyncMirror(link, filepath.Join(root, "drive2", "unit")); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if len(rec.calls) != 0 {
|
||||||
|
t.Fatalf("a symlinked path went to the container: %v", rec.calls)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -818,9 +818,15 @@ func rsyncMirror(src, dst string) error {
|
|||||||
const tier2CopyMemCap = "512m"
|
const tier2CopyMemCap = "512m"
|
||||||
|
|
||||||
// tier2CappedPath: a path the controller and the Docker daemon see identically (the guest's /mnt is bind-mounted into
|
// tier2CappedPath: a path the controller and the Docker daemon see identically (the guest's /mnt is bind-mounted into
|
||||||
// the controller at /mnt). Never a relative path, never `..`.
|
// the controller at /mnt). Never a relative path, never `..`, and NO SYMLINK anywhere in it: a link under /mnt that
|
||||||
|
// points outside /mnt resolves to a different place for the daemon (found by the commit's security review,
|
||||||
|
// 2026-10-22) — the path must resolve to itself. The path must exist (rsyncMirror creates dst first).
|
||||||
func tier2CappedPath(p string) bool {
|
func tier2CappedPath(p string) bool {
|
||||||
return strings.HasPrefix(p, tier2CappedRoot) && filepath.Clean(p) == p
|
if !strings.HasPrefix(p, tier2CappedRoot) || filepath.Clean(p) != p {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
real, err := filepath.EvalSymlinks(p)
|
||||||
|
return err == nil && real == p
|
||||||
}
|
}
|
||||||
|
|
||||||
// tier2CappedRoot is "/mnt/" in production (a test seam: tests point it at a temp dir).
|
// tier2CappedRoot is "/mnt/" in production (a test seam: tests point it at a temp dir).
|
||||||
|
|||||||
Reference in new issue
Block a user