diff --git a/modules/only-office/README.md b/modules/only-office/README.md index 3129fc4b..8b1a5291 100644 --- a/modules/only-office/README.md +++ b/modules/only-office/README.md @@ -7,10 +7,24 @@ The `only-office` module of the mesh catalogue: runs the container image `onlyof Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416). - `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:box}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:logs}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:data}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:server-lib}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:lib}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:db}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:rabbitmq}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:redis}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:fonts}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). + +## Where its directories live + +Everything the container mounts sits in `${dir:box}` (`/var/lib/only-office/box`), root-owned and 0700 (issue 1843). +The image opens what is below each mount to every account on every start (`chmod -R 755`), so the barrier is a +parent the container never sees. The module root stays root-owned 0711: the converter, which runs on the host +as `only-office-converter`, passes through it to its two secrets and nothing else. + +`${dir:lib}` (`/var/lib/only-office/lib`) is the copy the container used before, no longer mounted, kept by the +mesh because it is not empty and closed to 0700. It keeps its id so mesh-host sets that mode: a new id at a path +the image owns would be used as found (issue 339). Its contents, and those of the old `logs`, `data`, `db`, +`rabbitmq`, `redis` and `fonts` beside it, are removed on the operator's word, and then `lib` leaves the manifest. diff --git a/modules/only-office/manifest_test.go b/modules/only-office/manifest_test.go index 88cbddfe..2e44316d 100644 --- a/modules/only-office/manifest_test.go +++ b/modules/only-office/manifest_test.go @@ -96,32 +96,201 @@ func TestSecureLinkSecretIsPinned(t *testing.T) { } // 1837/AC3 (unit: manifest check) -// Given the catalogue, when only-office's manifest is read, then the lib directory (mounted at -// /var/lib/onlyoffice, where the document service writes the converted files nginx serves) lets -// other users pass through it (mode 0711): the image makes it the document service's own, and nginx -// runs as another user, so a mode of 0700 makes nginx answer 403 "Permission denied" on Editor.bin. +// Given the catalogue, when only-office's manifest is read, then the directory mounted at +// /var/lib/onlyoffice (server-lib, where the document service writes the converted files nginx +// serves) lets other users pass through it (mode 0711): the image makes it the document service's +// own, and nginx runs as another user, so a mode of 0700 makes nginx answer 403 "Permission denied" +// on Editor.bin. Host accounts are kept out by the box above it (1843/AC1). func TestLibDirectoryLetsNginxThrough(t *testing.T) { + m := readLayout(t) + src := m.volumeSource(t, "/var/lib/onlyoffice") + if src != "${dir:server-lib}" { + t.Fatalf("/var/lib/onlyoffice is mounted from %q, want ${dir:server-lib}", src) + } + d := m.dir(t, "server-lib") + if d.Mode != "0711" { + t.Errorf("server-lib directory mode = %q, want 0711 so nginx can reach the document service's cache", d.Mode) + } +} + +// 1843/AC1 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then it declares a directory box with no +// place, no path and no owner (root, /var/lib/only-office/box) and mode 0700, and every host source of +// the server container's volumes is a directory whose path is ${dir:box}/, so no account but +// root can pass to anything the container writes: the image opens what is below each mount to every +// account on every start. +func TestContainerDirectoriesSitInARootOnlyBox(t *testing.T) { + m := readLayout(t) + box := m.dir(t, "box") + if box.Place != "" || box.Path != "" || box.Owner != "" { + t.Errorf("box = place %q, path %q, owner %q, want none of them (root, at /only-office/box)", box.Place, box.Path, box.Owner) + } + if box.Mode != "0700" { + t.Errorf("box mode = %q, want 0700", box.Mode) + } + c := m.container(t, "server") + if len(c.Volumes) == 0 { + t.Fatal("the server container mounts nothing") + } + for _, v := range c.Volumes { + src, target := splitVolume(v) + id, ok := strings.CutPrefix(src, "${dir:") + if !ok || !strings.HasSuffix(id, "}") { + t.Errorf("volume %q has a host source that is not a directory of the module", v) + continue + } + d := m.dir(t, strings.TrimSuffix(id, "}")) + if !strings.HasPrefix(d.Path, "${dir:box}/") || strings.Count(d.Path, "/") != 1 { + t.Errorf("%s (mounted at %s) has path %q, want ${dir:box}/", d.ID, target, d.Path) + } + if d.Owner != "" { + t.Errorf("%s is owned by %q, want root", d.ID, d.Owner) + } + } +} + +// 1843/AC2 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then the module root (state) is still +// root-owned, mode 0711, place ".", and the converter's secrets and environment still name +// ${dir:state}/jwt.secret and ${dir:state}/broker-converter.secret: the converter passes the root and +// nothing below it, and no account but root owns a directory on any path. +func TestConverterPassesOnlyTheRoot(t *testing.T) { + m := readLayout(t) + state := m.dir(t, "state") + if state.Place != "." || state.Owner != "" || state.Mode != "0711" { + t.Errorf("state = place %q, owner %q, mode %q, want place ., root, 0711", state.Place, state.Owner, state.Mode) + } + for _, r := range m.Resources { + if r.Type == "directory" && r.Owner != "" { + t.Errorf("directory %s is owned by %q, want root", r.ID, r.Owner) + } + } + for name, want := range map[string]string{"jwt": "${dir:state}/jwt.secret", "broker-converter": "${dir:state}/broker-converter.secret"} { + if got := m.OwnSecrets[name].Path; got != want { + t.Errorf("own-secrets.%s.path = %q, want %q", name, got, want) + } + } + var conv *layoutResource + for i := range m.Resources { + if m.Resources[i].ID == "converter" { + conv = &m.Resources[i] + } + } + if conv == nil { + t.Fatal("manifest declares no converter") + } + if conv.Env["ONLY_OFFICE_JWT"] != "${dir:state}/jwt.secret" || conv.Env["ONLY_OFFICE_BROKER"] != "${dir:state}/broker-converter.secret" { + t.Errorf("converter env = %v, want its secrets in ${dir:state}", conv.Env) + } +} + +// 1843/AC3 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then the directory lib is declared with no +// path (its old place, /var/lib/only-office/lib) and mode 0700, so the next apply closes the copy the +// image opened, which mesh-host keeps because it is not empty; no container volume names ${dir:lib}; +// and every data entry points at a directory that is mounted now. +func TestOldLibIsClosedAndUnmounted(t *testing.T) { + m := readLayout(t) + lib := m.dir(t, "lib") + if lib.Path != "" || lib.Place != "" { + t.Errorf("lib = path %q, place %q, want neither: it stays where the old copy is", lib.Path, lib.Place) + } + if lib.Mode != "0700" { + t.Errorf("lib mode = %q, want 0700", lib.Mode) + } + mounted := map[string]bool{} + for _, v := range m.container(t, "server").Volumes { + src, _ := splitVolume(v) + mounted[src] = true + } + if mounted["${dir:lib}"] { + t.Error("the server container still mounts ${dir:lib}") + } + for _, d := range m.Data.Own { + if !mounted[d.Path] { + t.Errorf("data entry %s points at %q, which the server container does not mount", d.ID, d.Path) + } + } +} + +type layoutResource struct { + ID string `json:"id"` + Type string `json:"type"` + Mode string `json:"mode"` + Place string `json:"place"` + Path string `json:"path"` + Owner string `json:"owner"` + Volumes []string `json:"volumes"` + Env map[string]string `json:"env"` +} + +type layout struct { + Resources []layoutResource `json:"resources"` + OwnSecrets map[string]struct { + Path string `json:"path"` + } `json:"own-secrets"` + Data struct { + Own []struct { + ID string `json:"id"` + Path string `json:"path"` + } `json:"own"` + } `json:"data"` +} + +func readLayout(t *testing.T) layout { + t.Helper() raw, err := os.ReadFile("module.json") if err != nil { t.Fatalf("reading module.json: %v", err) } - var m struct { - Resources []struct { - ID string `json:"id"` - Type string `json:"type"` - Mode string `json:"mode"` - } `json:"resources"` + if _, err := catalogue.ParseManifest(raw); err != nil { + t.Fatalf("parsing module.json: %v", err) } + var m layout if err := json.Unmarshal(raw, &m); err != nil { t.Fatalf("parsing module.json: %v", err) } + return m +} + +func (m layout) dir(t *testing.T, id string) layoutResource { + t.Helper() for _, r := range m.Resources { - if r.ID == "lib" && r.Type == "directory" { - if r.Mode != "0711" { - t.Errorf("lib directory mode = %q, want 0711 so nginx can reach the document service's cache", r.Mode) - } - return + if r.ID == id && r.Type == "directory" { + return r } } - t.Fatal("manifest declares no lib directory") + t.Fatalf("manifest declares no directory %s", id) + return layoutResource{} +} + +func (m layout) container(t *testing.T, id string) layoutResource { + t.Helper() + for _, r := range m.Resources { + if r.ID == id && r.Type == "container" { + return r + } + } + t.Fatalf("manifest declares no container %s", id) + return layoutResource{} +} + +func (m layout) volumeSource(t *testing.T, target string) string { + t.Helper() + for _, v := range m.container(t, "server").Volumes { + if src, to := splitVolume(v); to == target { + return src + } + } + t.Fatalf("the server container mounts nothing at %s", target) + return "" +} + +// splitVolume is a volume's host source and container target; the source's own ${dir:…} holds a colon. +func splitVolume(v string) (src, target string) { + i := strings.LastIndex(v, ":") + if i < 0 { + return v, "" + } + return v[:i], v[i+1:] } diff --git a/modules/only-office/module.json b/modules/only-office/module.json index e31fefc0..b4c1f5a1 100644 --- a/modules/only-office/module.json +++ b/modules/only-office/module.json @@ -104,7 +104,7 @@ }, { "id": "lib", - "path": "${dir:lib}", + "path": "${dir:server-lib}", "class": "rebuildable", "why": "the document server's working files" }, @@ -161,45 +161,64 @@ "mode": "0600", "template": "files/server.env" }, + { + "id": "box", + "type": "directory", + "mode": "0700", + "unknown": "program" + }, { "id": "logs", "type": "directory", + "path": "${dir:box}/logs", "mode": "0700", "unknown": "program" }, { "id": "data", "type": "directory", + "path": "${dir:box}/data", "mode": "0700", "unknown": "program" }, + { + "id": "server-lib", + "type": "directory", + "path": "${dir:box}/lib", + "mode": "0711", + "unknown": "program" + }, { "id": "lib", "type": "directory", - "mode": "0711", + "mode": "0700", "unknown": "program" }, { "id": "db", "type": "directory", + "path": "${dir:box}/db", "mode": "0700", "unknown": "program" }, { "id": "rabbitmq", "type": "directory", + "path": "${dir:box}/rabbitmq", "mode": "0700", "unknown": "program" }, { "id": "redis", "type": "directory", + "path": "${dir:box}/redis", "mode": "0700", "unknown": "program" }, { "id": "fonts", "type": "directory", + "path": "${dir:box}/fonts", "mode": "0700", "unknown": "program" }, @@ -223,7 +242,7 @@ "volumes": [ "${dir:logs}:/var/log/onlyoffice", "${dir:data}:/var/www/onlyoffice/Data", - "${dir:lib}:/var/lib/onlyoffice", + "${dir:server-lib}:/var/lib/onlyoffice", "${dir:db}:/var/lib/postgresql", "${dir:rabbitmq}:/var/lib/rabbitmq", "${dir:redis}:/var/lib/redis",