diff --git a/dockivad/api.go b/dockivad/api.go index 3ae0b00..acfccfa 100644 --- a/dockivad/api.go +++ b/dockivad/api.go @@ -118,6 +118,9 @@ func (b *engine) removeImage(ref string, force bool) []obj { continue } p := filepath.Join(b.Cache, entry.Name()) + if !exists(filepath.Join(p, "image.json")) { // half removed by an earlier engine: goes at the next start (migrateNames) + continue + } meta := readObject(filepath.Join(p, "image.json")) d := str(meta["digest"]) if digest == "" && strings.HasPrefix(d, "sha256:"+strings.TrimPrefix(ref, "sha256:")) { @@ -150,8 +153,7 @@ func (b *engine) removeImage(ref string, force bool) []obj { } out := []obj{} for p, name := range dirs { - retainLowers(b.storeDir(), b.State, p) // containers keep running over its files - check(os.RemoveAll(p)) + removeImageDir(b.storeDir(), b.State, p) // containers keep running over its files out = append(out, obj{"Untagged": name}) b.events.publish(imageEvent("untag", digest, digest)) // Docker names both by the image ID } @@ -344,7 +346,7 @@ func (b *engine) createWith(name string, body obj, prepared *image) obj { if filesDone != nil { <-filesDone } - _ = os.RemoveAll(stage) + _ = removeTree(stage) } }() allowed := map[string]bool{} @@ -910,8 +912,7 @@ func (b *engine) api(authorized func(*http.Request) bool) http.Handler { check(e) rows := []any{} for _, entry := range entries { - if entry.IsDir() && len(entry.Name()) == 64 { - p := filepath.Join(b.Cache, entry.Name(), "image.json") + if p := filepath.Join(b.Cache, entry.Name(), "image.json"); entry.IsDir() && len(entry.Name()) == 64 && exists(p) { meta := readObject(p) if !imageFilter(str(meta["name"]), filters) { continue diff --git a/dockivad/archive.go b/dockivad/archive.go index 68df097..832b496 100644 --- a/dockivad/archive.go +++ b/dockivad/archive.go @@ -179,7 +179,7 @@ func tarEntry(out *tar.Writer, name, p string, info os.FileInfo) error { return e } if info.Mode().IsRegular() { - src, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + src, e := openGuest(p) check(e) defer src.Close() _, e = io.CopyN(out, src, h.Size) @@ -239,7 +239,9 @@ func (b *engine) changes(name string) []obj { } var visit func(rel string) bool visit = func(rel string) bool { - entries, e := os.ReadDir(filepath.Join(r.upper, filepath.FromSlash(rel))) + dir := filepath.Join(r.upper, filepath.FromSlash(rel)) + defer unlock(dir, 0o500)() + entries, e := os.ReadDir(dir) check(e) changed := false for _, entry := range entries { @@ -311,7 +313,8 @@ func (b *engine) putArchive(name, requested string, body io.Reader, noOverwriteD if to.files != nil { to.files.remove(to.rel) } else { - check(os.RemoveAll(dest)) + defer unlock(filepath.Dir(dest), 0o700)() + check(removeTree(dest)) } exists = false } @@ -323,6 +326,9 @@ func (b *engine) putArchive(name, requested string, body io.Reader, noOverwriteD } defer to.files.settle(to.rel, h.Typeflag == tar.TypeDir) } + // Docker's root writes into a directory without write permission: it has it until + // the archive is in (restricted.go). + defer unlock(filepath.Dir(dest), 0o700)() mode := tarMode(h) switch h.Typeflag { case tar.TypeDir: diff --git a/dockivad/build.go b/dockivad/build.go index 16a5ea5..2232321 100644 --- a/dockivad/build.go +++ b/dockivad/build.go @@ -117,7 +117,7 @@ func (b *engine) build(w http.ResponseWriter, r *http.Request, q url.Values) { }() work, e := os.MkdirTemp(private(filepath.Join(b.storeDir(), "build")), "b-") check(e) - defer func() { go os.RemoveAll(work) }() + defer func() { go removeTree(work) }() quiet := q.Get("q") == "1" || q.Get("q") == "true" var memory int64 if raw := q.Get("memory"); raw != "" && raw != "0" { // docker build -m @@ -707,7 +707,7 @@ func (bd *builder) runStep(st *buildStage, o runOptions) { func (bd *builder) discardRoot(st *buildStage) { old := filepath.Join(bd.work, "old-"+token()[:12]) check(os.Rename(st.root, old)) - go os.RemoveAll(old) + go removeTree(old) } // The engine writes these into each container; a build keeps the stage's own. @@ -920,7 +920,7 @@ func isGuestDir(root, guest string) bool { // copyIntoGuest copies a directory's contents into a guest directory, merging. func copyIntoGuest(src, root, dest string, mode os.FileMode, chmod bool) { guestDir(root, dest) - check(filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error { + check(walkTree(src, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(src, p) check(e) @@ -948,6 +948,7 @@ func copyFileTo(src, root, guest string, mode os.FileMode, chmod bool) { parent, base := path.Split(path.Clean("/" + guest)) reject(base != "", "Cannot copy a file onto /") dst := filepath.Join(guestDir(root, parent), base) + defer unlock(filepath.Dir(dst), 0o700)() if old, e := os.Lstat(dst); e == nil { reject(!old.IsDir(), "Cannot replace the directory "+guest+" with a file") check(os.Remove(dst)) diff --git a/dockivad/buildcache.go b/dockivad/buildcache.go index ea5c33b..fbc68c2 100644 --- a/dockivad/buildcache.go +++ b/dockivad/buildcache.go @@ -97,7 +97,7 @@ func (bd *builder) store(st *buildStage, in instruction, id, dir string) { _ = safeRun(func() { tmp, e := os.MkdirTemp(bd.work, "cache-") check(e) - defer os.RemoveAll(tmp) + defer removeTree(tmp) cloneTree(st.root, filepath.Join(tmp, "rootfs")) now := time.Now().UTC().Format(time.RFC3339Nano) save(filepath.Join(tmp, "config.json"), st.config) @@ -175,7 +175,7 @@ func (bd *builder) release() { // changed, so the sum overstates the disk used. Ask APFS for private sizes if it matters. func changedSize(root, base string) int64 { var n int64 - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) if !d.Type().IsRegular() { return nil @@ -185,7 +185,7 @@ func changedSize(root, base string) int64 { if base != "" { rel, e := filepath.Rel(root, p) check(e) - if old, e := os.Lstat(filepath.Join(base, rel)); e == nil && old.Mode().IsRegular() && old.Size() == info.Size() && old.ModTime().Equal(info.ModTime()) { + if old, e := lstatIn(base, rel); e == nil && old.Mode().IsRegular() && old.Size() == info.Size() && old.ModTime().Equal(info.ModTime()) { return nil } } @@ -241,7 +241,7 @@ func treeHash(root string) string { // hashTree writes each entry's name, mode, size and content or symlink target. func hashTree(h io.Writer, root string) { - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(root, p) check(e) @@ -254,7 +254,7 @@ func hashTree(h io.Writer, root string) { check(e) fmt.Fprintf(h, "%q", target) case info.Mode().IsRegular(): - f, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + f, e := openGuest(p) check(e) defer f.Close() _, e = io.CopyN(h, f, info.Size()) @@ -315,5 +315,5 @@ func (b *engine) discard(dir string) { trash, e := os.MkdirTemp(private(filepath.Join(b.storeDir(), "build")), "trash-") check(e) check(os.Rename(dir, filepath.Join(trash, "entry"))) - go os.RemoveAll(trash) + go removeTree(trash) } diff --git a/dockivad/buildexport.go b/dockivad/buildexport.go index be1003b..bec04de 100644 --- a/dockivad/buildexport.go +++ b/dockivad/buildexport.go @@ -52,7 +52,7 @@ func (w bytesWriter) Write(p []byte) (int, error) { func exportTar(c *grpcCall, root string) int64 { counted := &counter{} tw := tar.NewWriter(io.MultiWriter(bytesWriter{c}, counted)) - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, e error) error { + check(walkTree(root, func(p string, d fs.DirEntry, e error) error { if e != nil || p == root { return e } @@ -78,7 +78,7 @@ func exportTree(c *grpcCall, root string) int64 { listed := make(chan error, 1) go func() { listed <- safeRun(func() { - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, e error) error { + check(walkTree(root, func(p string, d fs.DirEntry, e error) error { if e != nil || p == root { return e } @@ -101,7 +101,7 @@ func exportTree(c *grpcCall, root string) int64 { case !mode.IsDir(): // devices, pipes and sockets stay behind return nil } - files = append(files, p) // read by the receive loop only after the listing (listed) + files = append(files, rel) // read by the receive loop only after the listing (listed) return c.send(pbuf{}.num(1, packetStat).msg(2, st)) })) check(c.send(pbuf{}.num(1, packetStat))) // the end of the listing @@ -132,14 +132,14 @@ func exportTree(c *grpcCall, root string) int64 { waitListing() // the files' numbers are only final once listed id := p.num(3) reject(id < uint64(len(files)), "The client asked for an unknown file") - total += sendFile(c, uint32(id), files[id]) + total += sendFile(c, uint32(id), root, files[id]) } } } -func sendFile(c *grpcCall, id uint32, p string) int64 { - f, e := os.Open(p) - check(e) +func sendFile(c *grpcCall, id uint32, root, rel string) int64 { + var f *os.File + check(through(root, rel, func(p string) (e error) { f, e = openGuest(p); return })) // restricted.go defer f.Close() buf := make([]byte, exportChunk) var n int64 diff --git a/dockivad/buildkit.go b/dockivad/buildkit.go index 179e561..8aeece4 100644 --- a/dockivad/buildkit.go +++ b/dockivad/buildkit.go @@ -25,7 +25,6 @@ import ( "net" "net/http" "net/url" - "os" "slices" "sort" "strconv" @@ -627,7 +626,7 @@ func (b *engine) bkControlSolve(ctx context.Context, req pbFields) pbuf { bkSolves.Unlock() st.mu.Lock() if st.llb.work != "" { - go os.RemoveAll(st.llb.work) + go removeTree(st.llb.work) } st.mu.Unlock() }) diff --git a/dockivad/buildkit_build.go b/dockivad/buildkit_build.go index 82de255..03fbaf2 100644 --- a/dockivad/buildkit_build.go +++ b/dockivad/buildkit_build.go @@ -36,7 +36,7 @@ func (b *engine) bkBuild(ctx context.Context, st *bkSolve, opts map[string]strin } work, e := os.MkdirTemp(private(filepath.Join(b.storeDir(), "build")), "b-") check(e) - defer func() { go os.RemoveAll(work) }() + defer func() { go removeTree(work) }() bd := &builder{b: b, ctx: ctx, work: work, context: filepath.Join(work, "context"), network: opts["force-network-mode"], args: map[string]string{}, used: map[string]bool{}, global: platformArgs(platform), platform: platform, nocache: nocache, named: map[string]namedContext{}} bd.secret = func(id string) (value []byte, e error) { // RUN --mount=type=secret: the session's Secrets service diff --git a/dockivad/buildmount.go b/dockivad/buildmount.go index dccf091..c296f84 100644 --- a/dockivad/buildmount.go +++ b/dockivad/buildmount.go @@ -301,7 +301,7 @@ func (bd *builder) mountSources(mounts []runMount) (binds []any, env []string, t scratch := func() string { d, e := os.MkdirTemp(bd.work, "mount-") check(e) - undo = append(undo, func() { os.RemoveAll(d) }) // at once: secrets are among them + undo = append(undo, func() { removeTree(d) }) // at once: secrets are among them return d } bind := func(source, target string, readonly bool) { diff --git a/dockivad/image.go b/dockivad/image.go index 2ee21fc..111289c 100644 --- a/dockivad/image.go +++ b/dockivad/image.go @@ -401,7 +401,10 @@ func layerDir(root, dir string, create bool) (string, bool) { if !create { return "", false } - check(mkdirGuest(p)) + func() { + defer unlock(filepath.Dir(p), 0o700)() + check(mkdirGuest(p)) + }() continue } check(e) @@ -416,7 +419,8 @@ func layerDir(root, dir string, create bool) (string, bool) { // cloneTree copies an unpacked root for a container: one APFS clone of the whole // tree (clonefileat), sharing blocks until written; a plain copy elsewhere. A clone // turns setuid and setgid off on files and directories (Debian's /var/mail is -// 2775), so they are set again. +// 2775), so they are set again. The copy has the modes of src, also where they keep +// the engine from reading it. func cloneTree(src, dst string) { s, e := syscall.BytePtrFromString(src) check(e) @@ -424,7 +428,18 @@ func cloneTree(src, dst string) { check(e) cwd := -2 // AT_FDCWD const sysClonefileat, cloneNofollow = 462, 1 - if _, _, errno := syscall.Syscall6(sysClonefileat, uintptr(cwd), uintptr(unsafe.Pointer(s)), uintptr(cwd), uintptr(unsafe.Pointer(d)), cloneNofollow, 0); errno == 0 { + clone := func() syscall.Errno { + _, _, errno := syscall.Syscall6(sysClonefileat, uintptr(cwd), uintptr(unsafe.Pointer(s)), uintptr(cwd), uintptr(unsafe.Pointer(d)), cloneNofollow, 0) + return errno + } + errno := clone() + var held []locked + defer func() { lockTree(dst, held); lockTree(src, held) }() + if errno == syscall.EACCES { // a directory or file the guest left unreadable (restricted.go): nothing is copied + unlockTree(src, &held) + errno = clone() + } + if errno == 0 { // ponytail: one lstat per entry (~100 ms for 25k files); a list of an image's // setuid files, kept at unpack, would spare it. check(filepath.WalkDir(src, func(p string, entry fs.DirEntry, err error) error { @@ -582,7 +597,7 @@ func commitImage(cache, name, layout string) string { } stage, e := os.MkdirTemp(cache, ".incoming-") check(e) - defer os.RemoveAll(stage) + defer removeTree(stage) dest := filepath.Join(stage, "layout") check(os.MkdirAll(filepath.Join(dest, "blobs/sha256"), 0700)) for _, d := range im.blobs { @@ -640,14 +655,20 @@ func (b *engine) commitPulled(name, layout string) string { // migrateNames moves images cached under a docker.io/ name (docker tag did, before // commitImage shortened names) to the familiar name imagePath looks up. One whose -// familiar name is taken was unreachable anyway, and goes. +// familiar name is taken was unreachable anyway, and goes. So does a cache directory +// without its image.json: docker rmi removed that first, then failed on the image's +// files where a directory lacked write permission, and listing images failed on it. func migrateNames(cache string) { entries, e := os.ReadDir(cache) check(e) for _, entry := range entries { _ = safeRun(func() { dir := filepath.Join(cache, entry.Name()) - if len(entry.Name()) != 64 || !exists(filepath.Join(dir, "image.json")) { + if len(entry.Name()) != 64 || !entry.IsDir() { + return + } + if !exists(filepath.Join(dir, "image.json")) { // left by a removal that failed half way + removeImageDir(filepath.Dir(cache), filepath.Join(filepath.Dir(cache), "containers"), dir) return } meta := readObject(filepath.Join(dir, "image.json")) @@ -657,8 +678,7 @@ func migrateNames(cache string) { } target := filepath.Join(cache, key(shortName(name))) if exists(target) { - retainLowers(filepath.Dir(cache), filepath.Join(filepath.Dir(cache), "containers"), dir) - check(os.RemoveAll(dir)) + removeImageDir(filepath.Dir(cache), filepath.Join(filepath.Dir(cache), "containers"), dir) return } meta["name"] = shortName(name) @@ -676,7 +696,7 @@ func ensureUnpacked(target string) { im := loadImage(noLinks(target, "layout")) stage, e := os.MkdirTemp(target, ".rootfs-") check(e) - defer os.RemoveAll(stage) + defer removeTree(stage) size := im.unpack(stage) if e := os.Rename(stage, filepath.Join(target, "rootfs")); e != nil && !exists(filepath.Join(target, "rootfs")) { check(e) diff --git a/dockivad/layer.go b/dockivad/layer.go index 1ecdac6..1c56010 100644 --- a/dockivad/layer.go +++ b/dockivad/layer.go @@ -40,18 +40,18 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change check(out.WriteHeader(h)) changed = true } - link := func(p string) string { target, e := os.Readlink(p); check(e); return target } + link := func(tree, rel string) string { target, e := readlinkIn(tree, rel); check(e); return target } if base != "" { // Whiteouts first. A directory root lacks, or holds as another type, is not // descended: its whiteout or replacement removes it whole. - check(filepath.WalkDir(base, func(p string, d fs.DirEntry, err error) error { + check(walkTree(base, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(base, p) check(e) if rel == "." { return nil } - st, e := os.Lstat(filepath.Join(root, rel)) + st, e := lstatIn(root, rel) gone := os.IsNotExist(e) if gone { dir, name := filepath.Split(rel) @@ -68,7 +68,7 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change // fresh is a directory base lacks (or had as another type): all beneath it is new, // and base is not looked up there (its path may run through a file or symlink). fresh := "" - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(root, p) check(e) @@ -79,11 +79,10 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change return nil // the root itself; sockets, FIFOs and devices } if base != "" && !strings.HasPrefix(rel, fresh+"/") { - was := filepath.Join(base, rel) - old, e := os.Lstat(was) + old, e := lstatIn(base, rel) if e == nil && old.Mode() == info.Mode() && (kind == fs.ModeDir || - kind == fs.ModeSymlink && link(was) == link(p) || + kind == fs.ModeSymlink && link(base, rel) == link(root, rel) || kind == 0 && old.Size() == info.Size() && old.ModTime().Equal(info.ModTime())) { return nil } @@ -99,13 +98,13 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change case fs.ModeDir: h.Name, h.Typeflag = rel+"/", tar.TypeDir case fs.ModeSymlink: - h.Typeflag, h.Linkname = tar.TypeSymlink, link(p) + h.Typeflag, h.Linkname = tar.TypeSymlink, link(root, rel) default: h.Size = info.Size() } add(h) if kind == 0 { - in, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + in, e := openGuest(p) check(e) defer in.Close() _, e = io.CopyN(out, in, h.Size) @@ -137,7 +136,7 @@ func imageDocs(layout string) (manifest, config obj) { // treeSize is the total size of the regular files under root (docker images SIZE). func treeSize(root string) int64 { var n int64 - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) if d.Type().IsRegular() { info, e := d.Info() diff --git a/dockivad/main.go b/dockivad/main.go index 53addec..80a661f 100644 --- a/dockivad/main.go +++ b/dockivad/main.go @@ -26,6 +26,28 @@ func cleanupUnpublished(state string) { } } +// staleWork moves aside what an engine that has gone left in dir, the store or the +// home before it: its builds' and loads' work (build, load) and its commits' (.commit-*), +// and what an earlier sweep did not finish. It returns what to remove. Builds left +// their work behind whenever a stage held a directory without write permission (Go's +// module cache; restricted.go), and nothing removed it. Called before the engine +// serves its API: nothing is building. +func staleWork(dir string) []string { + entries, _ := os.ReadDir(dir) + gone := []string{} + for _, entry := range entries { + name := entry.Name() + if !entry.IsDir() || name != "build" && name != "load" && !strings.HasPrefix(name, ".commit-") && !strings.HasPrefix(name, ".removing-") { + continue + } + aside := filepath.Join(dir, ".removing-"+token()[:16]) + if os.Rename(filepath.Join(dir, name), aside) == nil { + gone = append(gone, aside) + } + } + return gone +} + func run() { syscall.Umask(0077) if len(os.Args) > 1 && os.Args[1] == "_supervise" { @@ -95,7 +117,19 @@ func run() { moveToStore(b.Home, store) } cleanupUnpublished(b.State) + cleanupUnpublished(b.Cache) migrateNames(b.Cache) + stale := staleWork(store) + if store != b.Home { + stale = append(stale, staleWork(b.Home)...) // from before the store + } + go func() { // gigabytes, perhaps: not in the way of the start + for _, p := range stale { + if e := removeTree(p); e != nil { + fmt.Fprintln(os.Stderr, "dockivad: an earlier engine's work stays until the next start:", e) + } + } + }() } if *importName != "" { reject(*layout != "", "Import requires --import-layout") diff --git a/dockivad/overlay.go b/dockivad/overlay.go index 4618e8e..c218cd9 100644 --- a/dockivad/overlay.go +++ b/dockivad/overlay.go @@ -158,9 +158,12 @@ func (r layers) upperDir(rel string, create bool) string { } else if !create { panic(apiError{404, "No such directory: /" + sofar}) } - if e := os.Mkdir(next, 0o700); e != nil && !errors.Is(e, fs.ErrExist) { - check(e) - } + func() { + defer unlock(p, 0o700)() + if e := os.Mkdir(next, 0o700); e != nil && !errors.Is(e, fs.ErrExist) { + check(e) + } + }() if from != nil { check(setMode(next, from.Mode()&permBits)) _ = os.Chtimes(next, from.ModTime(), from.ModTime()) @@ -194,6 +197,7 @@ func (r layers) settle(rel string, directory bool) { if !exists(wh) { return } + defer unlock(filepath.Dir(wh), 0o700)() if directory { f, e := os.OpenFile(filepath.Join(r.upper, dir, name, opaqueMarker), os.O_WRONLY|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) check(e) @@ -210,12 +214,15 @@ func (r layers) remove(rel string) { reject(name != "", "Cannot remove the container root") up, low := r.lookup(rel) if low != "" { - f, e := os.OpenFile(filepath.Join(r.upperDir(dir, true), whiteoutPrefix+name), os.O_WRONLY|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) + parent := r.upperDir(dir, true) + defer unlock(parent, 0o700)() + f, e := os.OpenFile(filepath.Join(parent, whiteoutPrefix+name), os.O_WRONLY|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) check(e) f.Close() } if up != "" { - check(os.RemoveAll(up)) + defer unlock(filepath.Dir(up), 0o700)() + check(removeTree(up)) } } @@ -233,6 +240,7 @@ func (r layers) copyUp(rel string) string { check(e) reject(!st.IsDir(), "Cannot copy a directory up: /"+rel) dest := r.writable(rel) + defer unlock(filepath.Dir(dest), 0o700)() cloneTree(host, dest) if st.Mode().IsRegular() { check(setMode(dest, st.Mode()&permBits)) @@ -248,10 +256,32 @@ type guestEntry struct { // readDir lists directory rel as the guest sees it, sorted by name. func (r layers) readDir(rel string) []guestEntry { + out, lock := r.openDir(rel) + lock() + return out +} + +// openDir is readDir of a directory that stays readable (restricted.go) until lock +// is called: for a walk into it. +func (r layers) openDir(rel string) (out []guestEntry, lock func()) { up, low := r.lookup(rel) reject(up != "" || low != "", "No such directory: /"+rel) out, seen := []guestEntry{}, map[string]bool{} + locks := []func(){} + lock = func() { + for _, l := range locks { + l() + } + locks = nil + } + defer func() { + if v := recover(); v != nil { + lock() + panic(v) + } + }() add := func(dir string, upper bool) { + locks = append(locks, unlock(dir, 0o500)) entries, e := os.ReadDir(dir) check(e) for _, entry := range entries { @@ -280,7 +310,7 @@ func (r layers) readDir(rel string) []guestEntry { add(low, false) } sort.Slice(out, func(i, j int) bool { return out[i].name < out[j].name }) - return out + return out, lock } // walk visits rel and everything below it as the guest sees it, in lexical order @@ -304,7 +334,9 @@ func (r layers) walkFrom(rel, host string, info os.FileInfo, fn func(rel, host s } return e } - for _, entry := range r.readDir(rel) { + entries, lock := r.openDir(rel) + defer lock() + for _, entry := range entries { if e := r.walkFrom(path.Join(rel, entry.name), entry.host, entry.info, fn); e != nil { return e } @@ -321,6 +353,7 @@ func (r layers) writeFile(rel, content string) { reject(s.Mode().IsRegular() || s.Mode()&os.ModeSymlink != 0, "Cannot replace "+rel) } p := r.writable(rel) + defer unlock(filepath.Dir(p), 0o700)() if _, e := os.Lstat(p); e == nil { check(os.Remove(p)) } @@ -343,9 +376,12 @@ func (r layers) mountPoint(guest string, directory bool) { if !ok { if last && !directory { p := r.writable(sofar) - f, e := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, 0o644) - check(e) - f.Close() + func() { + defer unlock(filepath.Dir(p), 0o700)() + f, e := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, 0o644) + check(e) + f.Close() + }() r.settle(sofar, false) } else { r.upperDir(sofar, true) @@ -428,7 +464,7 @@ func (b *engine) commitContainer(name string, q url.Values) obj { } work, e := os.MkdirTemp(b.storeDir(), ".commit-") check(e) - defer os.RemoveAll(work) + defer removeTree(work) merged := filepath.Join(work, "rootfs") cloneTree(r.lower, merged) applyUpper(r.upper, merged) @@ -446,9 +482,16 @@ func (b *engine) commitContainer(name string, q url.Values) obj { // applyUpper lays an upper layer over a copy of its lower one: whiteouts remove, // opaque directories empty, the rest replaces (APFS clones). The engine's own files -// (engineFiles) stay out. +// (engineFiles) stay out. A directory the guest left without owner permissions +// (restricted.go) is writable while it is filled, and gets its mode after. func applyUpper(upper, merged string) { - check(filepath.WalkDir(upper, func(p string, d fs.DirEntry, err error) error { + type late struct { + path string + mode fs.FileMode + } + dirs := []late{} + defer unlock(merged, 0o700)() + check(walkTree(upper, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(upper, p) check(e) @@ -461,12 +504,12 @@ func applyUpper(upper, merged string) { } case strings.HasPrefix(name, whiteoutPrefix+whiteoutPrefix): // the opaque marker (seen at its directory), a copy-up in flight case strings.HasPrefix(name, whiteoutPrefix): - check(os.RemoveAll(filepath.Join(filepath.Dir(dest), strings.TrimPrefix(name, whiteoutPrefix)))) + check(removeTree(filepath.Join(filepath.Dir(dest), strings.TrimPrefix(name, whiteoutPrefix)))) default: info, e := d.Info() check(e) if old, e := os.Lstat(dest); e == nil && !(old.IsDir() && d.IsDir()) || e == nil && exists(filepath.Join(p, opaqueMarker)) { - check(os.RemoveAll(dest)) + check(removeTree(dest)) } if !d.IsDir() { cloneTree(p, dest) @@ -478,8 +521,15 @@ func applyUpper(upper, merged string) { if e := os.Mkdir(dest, 0o700); e != nil && !errors.Is(e, fs.ErrExist) { check(e) } - check(setMode(dest, info.Mode()&permBits)) + mode := info.Mode() & permBits + check(setMode(dest, mode|0o700)) + if mode&0o700 != 0o700 { + dirs = append(dirs, late{dest, mode}) + } } return nil })) + for i := len(dirs) - 1; i >= 0; i-- { // children first + check(setMode(dirs[i].path, dirs[i].mode)) + } } diff --git a/dockivad/restricted.go b/dockivad/restricted.go new file mode 100644 index 0000000..0a4b70c --- /dev/null +++ b/dockivad/restricted.go @@ -0,0 +1,164 @@ +package main + +// Guest trees hold what the engine's own user may not read or change: guest root's +// chmod 555 of a directory (Go's module cache is made that way) or chmod 000 of a +// directory or file is the host mode too, and the engine is that host user, not root. +// To remove, copy or archive such a tree it gives the directory or file the owner +// bits it lacks for as long as it needs them, and its mode back after: what is stored +// and exported keeps the modes the guest set. A running container sees the wider mode +// for that moment, as it does while the runtime does the same for guest root +// (native/fs.h, fs_dac). +// +// ponytail: a path given to docker cp or a mount that itself runs through a 000 +// directory is still "not found" (layers.lookup); unlock on the way if it matters. + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "syscall" +) + +// unlock gives directory or file p the owner permission bits of need it lacks, and +// returns what puts its mode back: defer unlock(dir, 0o700)() around making or +// removing entries of dir, 0o500 around reading it. +func unlock(p string, need fs.FileMode) func() { + st, e := os.Lstat(p) + if e != nil || st.Mode()&fs.ModeSymlink != 0 { + return func() {} + } + return unlockAs(p, st, need) +} + +func unlockAs(p string, st fs.FileInfo, need fs.FileMode) func() { + mode := st.Mode() & permBits + if mode&need == need || setMode(p, mode|need) != nil { + return func() {} + } + return func() { _ = setMode(p, mode) } +} + +// walkTree is filepath.WalkDir for a guest tree. A directory without owner read and +// search permission is readable from before fn sees it (with its own mode in d) until +// the walk has left it, also when fn panics. +func walkTree(root string, fn fs.WalkDirFunc) error { + type held struct { + prefix string + lock func() + } + var open []held + leave := func(p string) { // lock again the directories p is not in + for len(open) > 0 && !strings.HasPrefix(p, open[len(open)-1].prefix) { + open[len(open)-1].lock() + open = open[:len(open)-1] + } + } + defer leave("") + return filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + leave(p) + if err != nil || !d.IsDir() { + return fn(p, d, err) + } + info, e := d.Info() + if e != nil { + return fn(p, d, e) + } + if info.Mode().Perm()&0o500 != 0o500 { + open = append(open, held{p + string(filepath.Separator), unlockAs(p, info, 0o500)}) + } + return fn(p, fs.FileInfoToDirEntry(info), nil) + }) +} + +// openGuest opens a guest's regular file to read it; one without read permission +// (Fedora's /etc/shadow is 000) has it for the open. +func openGuest(p string) (*os.File, error) { + f, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + if !errors.Is(e, fs.ErrPermission) { + return f, e + } + if st, e := os.Lstat(p); e != nil || !st.Mode().IsRegular() { + return nil, &fs.PathError{Op: "open", Path: p, Err: fs.ErrPermission} + } + defer unlock(p, 0o400)() + return os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) +} + +// through runs fn on rel of guest tree root; if a directory on the way has no search +// permission (fn answers EACCES), once more with them searchable. For a tree looked +// up beside the one being walked (a layer's base). +func through(root, rel string, fn func(p string) error) error { + p := filepath.Join(root, rel) + e := fn(p) + if !errors.Is(e, fs.ErrPermission) { + return e + } + dir := root + for _, part := range strings.Split(filepath.Dir(rel), string(filepath.Separator)) { + dir = filepath.Join(dir, part) + defer unlock(dir, 0o100)() + } + return fn(p) +} + +func lstatIn(root, rel string) (st os.FileInfo, e error) { + e = through(root, rel, func(p string) error { st, e = os.Lstat(p); return e }) + return +} + +func readlinkIn(root, rel string) (target string, e error) { + e = through(root, rel, func(p string) error { target, e = os.Readlink(p); return e }) + return +} + +// locked is an entry unlockTree made readable, with the mode it had. +type locked struct { + rel string + mode fs.FileMode +} + +// unlockTree makes every directory and regular file of a guest tree readable, for +// clonefile, which copies a tree whole or not at all; lockTree puts the modes back, +// in the tree or a copy of it. +func unlockTree(root string, held *[]locked) { + check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(err) + need := fs.FileMode(0o400) + if d.IsDir() { + need = 0o500 + } else if !d.Type().IsRegular() { + return nil + } + info, e := d.Info() + check(e) + if mode := info.Mode() & permBits; mode&need != need { + rel, e := filepath.Rel(root, p) + check(e) + check(setMode(p, mode|need)) // before WalkDir reads it + *held = append(*held, locked{rel, mode}) + } + return nil + })) +} + +func lockTree(root string, held []locked) { + for i := len(held) - 1; i >= 0; i-- { + _ = setMode(filepath.Join(root, held[i].rel), held[i].mode) + } +} + +// removeImageDir removes cached image directory dir, as docker rmi does. Its unpacked +// files stay for the containers running over them (retainLowers); the directory +// leaves the cache in one rename, so the image is listed whole or not at all, and what +// is left of a removal that fails goes at the next start (cleanupUnpublished). +func removeImageDir(store, state, dir string) { + retainLowers(store, state, dir) + gone := filepath.Join(filepath.Dir(dir), ".removing-"+token()[:16]) + check(os.Rename(dir, gone)) + if e := removeTree(gone); e != nil { + fmt.Fprintln(os.Stderr, "dockivad: files of a removed image stay until the next start:", e) + } +} diff --git a/dockivad/restricted_test.go b/dockivad/restricted_test.go new file mode 100644 index 0000000..ac1d1d8 --- /dev/null +++ b/dockivad/restricted_test.go @@ -0,0 +1,273 @@ +package main + +import ( + "archive/tar" + "bytes" + "fmt" + "io" + "io/fs" + "net/http/httptest" + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +// Guest trees with directories the engine's user may not change or read: 0555 (Go's +// module cache; guest root's chmod) and 000, each holding files. + +// restrictedTree makes, in dir (made if missing): +// +// m/a/f m and a 0555 +// z/k/f z 000, k 0555 +// s a 000 file +func restrictedTree(t *testing.T, dir string) { + t.Helper() + for _, d := range []string{"m/a", "z/k"} { + check(os.MkdirAll(filepath.Join(dir, d), 0o755)) + check(os.WriteFile(filepath.Join(dir, d, "f"), []byte(d), 0o644)) + } + check(os.WriteFile(filepath.Join(dir, "s"), []byte("secret"), 0o600)) + for _, rel := range []string{"m/a", "m", "z/k"} { // children first + check(os.Chmod(filepath.Join(dir, rel), 0o555)) + } + for _, rel := range []string{"z", "s"} { + check(os.Chmod(filepath.Join(dir, rel), 0)) + } +} + +// unlockLater lets the test's temporary directory go: t.TempDir's own removal stops +// at such directories. +func unlockLater(t *testing.T, dir string) { + t.Cleanup(func() { + _ = filepath.WalkDir(dir, func(p string, d fs.DirEntry, e error) error { + if e == nil && d.IsDir() { + _ = os.Chmod(p, 0o700) + } + return nil + }) + }) +} + +// treeModes lists a tree as "rel mode [content]" lines, reading into directories +// without permission (their modes are put back). +func treeModes(t *testing.T, root string) string { + t.Helper() + lines := []string{} + var visit func(rel string) + visit = func(rel string) { + p := filepath.Join(root, rel) + st, e := os.Lstat(p) + check(e) + line := fmt.Sprintf("%s %v", rel, st.Mode()) + switch { + case st.Mode().IsRegular(): + check(os.Chmod(p, 0o600)) + data, e := os.ReadFile(p) + check(e) + check(os.Chmod(p, st.Mode().Perm())) + line += " " + string(data) + case st.IsDir(): + check(os.Chmod(p, 0o700)) + entries, e := os.ReadDir(p) + check(e) + for _, entry := range entries { + visit(filepath.Join(rel, entry.Name())) + } + check(os.Chmod(p, st.Mode().Perm())) + } + lines = append(lines, line) + } + visit(".") + sort.Strings(lines) + return strings.Join(lines, "\n") +} + +const restrictedModes = `. drwxr-xr-x +m dr-xr-xr-x +m/a dr-xr-xr-x +m/a/f -rw-r--r-- m/a +s ---------- secret +z d--------- +z/k dr-xr-xr-x +z/k/f -rw-r--r-- z/k` + +// docker rmi of an image holding such directories removes all of it, and a cache +// directory an earlier engine left half removed (no image.json) breaks nothing. +func TestRemoveImageRestrictedDirs(t *testing.T) { + b := testEngine(t) + unlockLater(t, b.Home) + h := b.socketHandler() + dir := b.imagePath("dockiva/shell:demo") + restrictedTree(t, filepath.Join(dir, "rootfs", "data")) + half := filepath.Join(b.Cache, strings.Repeat("0", 64)) + restrictedTree(t, filepath.Join(half, "rootfs")) + if r := request(t, h, "GET", "/images/json", "", ""); r.Code != 200 || !strings.Contains(r.Body.String(), "dockiva/shell:demo") { + t.Fatalf("images beside a half-removed one: %d %s", r.Code, r.Body.String()) + } + if r := request(t, h, "DELETE", "/images/dockiva/shell:demo", "", ""); r.Code != 200 || !strings.Contains(r.Body.String(), "Deleted") { + t.Fatalf("rmi: %d %s", r.Code, r.Body.String()) + } + if r := request(t, h, "GET", "/images/json", "", ""); r.Code != 200 || strings.TrimSpace(r.Body.String()) != "[]" { + t.Fatalf("images after rmi: %d %s", r.Code, r.Body.String()) + } + cleanupUnpublished(b.Cache) + migrateNames(b.Cache) // the next start: the half-removed image goes + if entries, _ := os.ReadDir(b.Cache); len(entries) != 0 { + t.Fatalf("left in the image cache: %v", entries) + } +} + +// The work earlier engines left (builds whose stages held such directories, an +// interrupted commit) goes at a start; images, containers and volumes stay. +func TestStaleWorkRestrictedDirs(t *testing.T) { + dir := t.TempDir() + unlockLater(t, dir) + for _, d := range []string{"build/b-1/root", "build/trash-2/entry/rootfs", "load/l-3", ".commit-4/rootfs", ".removing-5"} { + restrictedTree(t, filepath.Join(dir, d)) + } + for _, keep := range []string{"images", "containers", "build-cache", "volumes"} { + check(os.MkdirAll(filepath.Join(dir, keep, "x"), 0o700)) + } + check(os.WriteFile(filepath.Join(dir, "load.json"), nil, 0o600)) // a file of these names is not the engine's work + stale := staleWork(dir) + for _, p := range stale { + if e := removeTree(p); e != nil { + t.Fatal(e) + } + } + names := []string{} + entries, _ := os.ReadDir(dir) + for _, entry := range entries { + names = append(names, entry.Name()) + } + if got := strings.Join(names, " "); len(stale) != 4 || got != "build-cache containers images load.json volumes" { + t.Fatalf("%d removed, left: %s", len(stale), got) + } +} + +// docker commit's copy: the image's files with the container's laid over them. Both +// hold such directories; the copy has their modes and files, the sources keep theirs, +// and the layer written from it lists them. +func TestCommitCopyRestrictedDirs(t *testing.T) { + dir := t.TempDir() + unlockLater(t, dir) + lower, upper, merged, blobs := filepath.Join(dir, "lower"), filepath.Join(dir, "upper"), filepath.Join(dir, "work", "rootfs"), filepath.Join(dir, "blobs") + for _, d := range []string{lower, filepath.Join(upper, "m/a"), filepath.Join(upper, "root"), filepath.Dir(merged), blobs} { + check(os.MkdirAll(d, 0o755)) + } + restrictedTree(t, lower) + // The container: a file added to, and one removed from, the image's 0555 and 000 + // directories, and directories of its own. + check(os.WriteFile(filepath.Join(upper, "m/a/new"), []byte("new"), 0o644)) + check(os.WriteFile(filepath.Join(upper, "m/a", whiteoutPrefix+"f"), nil, 0o600)) + check(os.MkdirAll(filepath.Join(upper, "z"), 0o755)) + check(os.WriteFile(filepath.Join(upper, "z", whiteoutPrefix+"k"), nil, 0o600)) + for _, rel := range []string{"m/a", "m"} { + check(os.Chmod(filepath.Join(upper, rel), 0o555)) + } + check(os.Chmod(filepath.Join(upper, "z"), 0)) + restrictedTree(t, filepath.Join(upper, "root")) + before := treeModes(t, lower) + "\n\n" + treeModes(t, upper) + + cloneTree(lower, merged) + applyUpper(upper, merged) + + own := strings.ReplaceAll(strings.Replace(restrictedModes, ". drwxr-xr-x\n", "", 1), "\n", "\nroot/") + want := ". drwxr-xr-x\nm dr-xr-xr-x\nm/a dr-xr-xr-x\nm/a/new -rw-r--r-- new\nroot drwxr-xr-x\nroot/" + own + "\ns ---------- secret\nz d---------" + if got := treeModes(t, merged); got != want { + t.Fatalf("merged:\n%s\nwant:\n%s", got, want) + } + if after := treeModes(t, lower) + "\n\n" + treeModes(t, upper); after != before { + t.Fatalf("the sources changed:\n%s\nwere:\n%s", after, before) + } + digest, size, changed := writeDiffLayer(merged, lower, blobs) + _, headers := layerHeaders(t, blobs, digest, size) + got := []string{} + for _, h := range headers { + got = append(got, fmt.Sprintf("%s %o", h.Name, h.Mode)) + } + sort.Strings(got) + wantLayer := "m/a/.wh.f 0, m/a/new 644, root/ 755, root/m/ 555, root/m/a/ 555, root/m/a/f 644, root/s 0, root/z/ 0, root/z/k/ 555, root/z/k/f 644, z/.wh.k 0" + if !changed || strings.Join(got, ", ") != wantLayer { + t.Fatalf("layer:\n%s\nwant:\n%s", strings.Join(got, ", "), wantLayer) + } + if size := treeSize(merged); size != int64(len("new")+2*len("secret")+2*len("m/a")) { + t.Fatalf("size: %d", size) + } + if e := removeTree(filepath.Dir(merged)); e != nil { + t.Fatal(e) + } +} + +// docker cp and docker export read such directories and files, with their modes, +// and docker cp into a 0555 directory works as it does for Docker's root. +func TestArchiveRestrictedDirs(t *testing.T) { + b := testEngine(t) + unlockLater(t, b.Home) + b.create("box", decode([]byte(`{"Image":"dockiva/shell:demo"}`), maxJSON)) + upper := filepath.Join(b.State, "box", "rootfs") + restrictedTree(t, filepath.Join(upper, "data")) + before := treeModes(t, filepath.Join(upper, "data")) + entries := func(r *httptest.ResponseRecorder, prefix string) string { + t.Helper() + lines := []string{} + for in := tar.NewReader(r.Body); ; { + h, e := in.Next() + if e == io.EOF { + break + } + if e != nil { + t.Fatalf("archive cut short after %v: %v", lines, e) + } + if name, ok := strings.CutPrefix(h.Name, prefix); ok { + data, e := io.ReadAll(in) + check(e) + lines = append(lines, strings.TrimSpace(fmt.Sprintf("%s %o %s", name, h.Mode, data))) + } + } + return strings.Join(lines, ", ") + } + want := "/ 755, /m/ 555, /m/a/ 555, /m/a/f 644 m/a, /s 0 secret, /z/ 0, /z/k/ 555, /z/k/f 644 z/k" + r := httptest.NewRecorder() + b.getArchive(r, "box", "/data", false) + if got := entries(r, "data"); got != want { + t.Fatalf("docker cp:\n%s\nwant:\n%s", got, want) + } + r = httptest.NewRecorder() + b.export(r, "box") + if got := entries(r, "data"); got != want { + t.Fatalf("docker export:\n%s", got) + } + if after := treeModes(t, filepath.Join(upper, "data")); after != before { + t.Fatalf("the container's files changed:\n%s\nwere:\n%s", after, before) + } + + // Into the container: a file beside m/a/f, a directory replacing it, and a file in + // a directory of the image the container has no copy of yet. + image := filepath.Join(b.imagePath("dockiva/shell:demo"), "rootfs") + check(os.Mkdir(filepath.Join(image, "ro"), 0o555)) + var in bytes.Buffer + w := tar.NewWriter(&in) + for _, h := range []*tar.Header{{Name: "data/m/a/new", Typeflag: tar.TypeReg, Mode: 0o644, Size: 2}, {Name: "data/m/a/f/", Typeflag: tar.TypeDir, Mode: 0o555}, + {Name: "data/m/a/f/in", Typeflag: tar.TypeReg, Mode: 0o600, Size: 2}, {Name: "ro/x", Typeflag: tar.TypeReg, Mode: 0o644, Size: 2}} { + check(w.WriteHeader(h)) + if h.Size > 0 { + _, _ = w.Write([]byte("hi")) + } + } + check(w.Close()) + b.putArchive("box", "/", &in, false) + want = strings.Replace(restrictedModes, "m/a/f -rw-r--r-- m/a", "m/a/f dr-xr-xr-x\nm/a/f/in -rw------- hi\nm/a/new -rw-r--r-- hi", 1) + if got := treeModes(t, filepath.Join(upper, "data")); got != want { + t.Fatalf("after docker cp into it:\n%s\nwant:\n%s", got, want) + } + if got := treeModes(t, filepath.Join(upper, "ro")); got != ". dr-xr-xr-x\nx -rw-r--r-- hi" { + t.Fatalf("copied-up directory:\n%s", got) + } + b.remove("box") + if exists(filepath.Join(b.State, "box")) { + t.Fatal("the container's files stay") + } +} diff --git a/dockivad/system.go b/dockivad/system.go index b8eff94..e1ac8ef 100644 --- a/dockivad/system.go +++ b/dockivad/system.go @@ -213,8 +213,7 @@ func (b *engine) pruneImages(danglingOnly bool, keep func(time.Time, obj) bool) if !keep(c.created, labels) { continue } - retainLowers(b.storeDir(), b.State, c.dir) - check(os.RemoveAll(c.dir)) + removeImageDir(b.storeDir(), b.State, c.dir) removed++ if !strings.HasPrefix(c.name, ":") { deleted = append(deleted, obj{"Untagged": c.name}) diff --git a/dockivad/volumes.go b/dockivad/volumes.go index d29ea4a..4015798 100644 --- a/dockivad/volumes.go +++ b/dockivad/volumes.go @@ -110,7 +110,7 @@ func (b *engine) removeVolume(name string) { if users := b.volumeUsers(name); len(users) > 0 { panic(apiError{409, "Volume " + name + " is in use by " + strings.Join(users, ", ")}) } - check(os.RemoveAll(filepath.Join(volumesDir(b.Home), name))) + check(removeTree(filepath.Join(volumesDir(b.Home), name))) } // removeAnonymousVolumes deletes the anonymous volumes a removed container used @@ -550,7 +550,7 @@ func copyTree(root layers, from, to string) { check(os.Mkdir(dest, 0o700)) check(setMode(dest, info.Mode()&permBits|0o700)) case info.Mode().IsRegular(): - in, e := os.Open(p) + in, e := openGuest(p) check(e) defer in.Close() out, e := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, info.Mode().Perm()) diff --git a/dockivad/x86.go b/dockivad/x86.go index 6223ae2..87882d2 100644 --- a/dockivad/x86.go +++ b/dockivad/x86.go @@ -120,7 +120,7 @@ func removeFexState(root string, made []string) { return } if path.Base(rel) == "fex-emu" { - check(os.RemoveAll(host)) + check(removeTree(host)) } else { _ = os.Remove(host) // a directory still holding files stays }