Merge pull request 'Issue 339 follow-ups: only step-ca's root may hold lines, every line end refused, the runtime's data and any .ssh refused' (#170) from fix/339-review-follow-ups into main

This commit was merged in pull request #170.
This commit is contained in:
2026-10-08 23:55:34 +00:00
15 changed files with 701 additions and 99 deletions
+9 -2
View File
@@ -17,9 +17,16 @@ import (
//
// 04-ISSUES/038 was the first of them (the port). These are the rest.
// A root certificate, in the shape a certificate authority serves one.
// A root certificate, as a certificate authority serves one: a real, self-signed one made for these tests (its key
// thrown away), because the one setting that may hold lines must parse as a certificate (novox/hq issue 339).
const servedRoot = `-----BEGIN CERTIFICATE-----
MIIBeDCCAR2gAwIBAgIQfake000000000000000000000000
MIIBPjCB8aADAgECAhRZG3p93hUUB5bz00uxhYo/nJnTQjAFBgMrZXAwFDESMBAG
A1UEAwwJdGVzdCByb290MCAXDTI2MTAwODIyMjE0NloYDzIxMjYwOTE0MjIyMTQ2
WjAUMRIwEAYDVQQDDAl0ZXN0IHJvb3QwKjAFBgMrZXADIQA0pt/ld+W0MXwBhPfO
cuAt56kIW6Qcn+4vqWpuvHTiqaNTMFEwHQYDVR0OBBYEFIjgaLk4OVGIOeZQiqnN
p9vhciFjMB8GA1UdIwQYMBaAFIjgaLk4OVGIOeZQiqnNp9vhciFjMA8GA1UdEwEB
/wQFMAMBAf8wBQYDK2VwA0EAl9uWeSM2XAV8u0reyV3BLRxNVik+4FCRO1QKPs2k
IlB1rK9oAOYManH+VFuBMI/JJ31ajSti81q4E0CSw8r5DQ==
-----END CERTIFICATE-----
`
+1
View File
@@ -913,6 +913,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
// such field, and the reason is for a reader of the manifest.
delete(copied, SecretsInEnvironment)
delete(copied, NamesOnPurpose)
delete(copied, TrustedField)
// **An operator's value, from the assignment** (novox/hq ADR 0112, ADR 0155): what a
// definition may not carry because it is true of one installation only. Filled from
// the same layers a mergeable file takes, and refused when no layer set it.
+14 -4
View File
@@ -54,18 +54,28 @@ var ownerShape = regexp.MustCompile(`^[0-9]+:[0-9]+$`)
// before anything is kept or composed, and the node-engine refuses them again where it applies.
//
// systemTrees are refused at and below: the machine's system, the kernel's, the boot loader's, root's home,
// what lives only while the machine runs, and the node-engine's and the mesh's own state. systemRoots are
// what lives only while the machine runs, the spool (cron's tables are there), the container runtimes' data
// (every container's filesystem), /opt, and the node-engine's and the mesh's own state. Any home's .ssh is
// refused as well, wherever the home is. systemRoots are
// refused at, and wherever a path holds one (an ancestor of /var/lib holds it): each is the parent of every
// module's or every person's directories, and owning it is owning all of them.
var (
systemTrees = []string{"/etc", "/usr", "/boot", "/root", "/run", "/var/run", "/var/lock", "/proc", "/sys",
"/dev", "/bin", "/sbin", "/lib", "/lib32", "/lib64", "/var/lib/mesh", "/var/lib/mesh-host"}
systemRoots = []string{"/", "/var", "/var/lib", "/var/cache", "/var/log", "/var/tmp", "/var/spool", "/home",
"/mnt", "/media", "/srv", "/opt", "/tmp", "/storage", "/data", "/services"}
"/dev", "/bin", "/sbin", "/lib", "/lib32", "/lib64", "/var/lib/mesh", "/var/lib/mesh-host", "/var/spool",
"/var/lib/docker", "/var/lib/containers", "/var/lib/containerd", "/opt"}
systemRoots = []string{"/", "/var", "/var/lib", "/var/cache", "/var/log", "/var/tmp", "/home",
"/mnt", "/media", "/srv", "/tmp", "/storage", "/data", "/services"}
)
// systemPath says why a clean absolute path is the machine's own and never a placement's or an access's, or "".
func systemPath(path string) string {
// Any home's keys, wherever the home is: a .ssh directory is its account's, and the keys and the list of who
// may log in as it are in there.
for _, part := range strings.Split(path, "/") {
if part == ".ssh" {
return path + " is an account's .ssh, which holds its keys and who may log in as it"
}
}
for _, tree := range systemTrees {
if path == tree || strings.HasPrefix(path, tree+"/") {
return path + " is in " + tree + ", the machine's own or the mesh's state"
+49 -52
View File
@@ -1,10 +1,14 @@
package catalogue
import (
"bytes"
"crypto/x509"
"encoding/json"
"encoding/pem"
"fmt"
"regexp"
"sort"
"strconv"
"strings"
)
@@ -86,6 +90,7 @@ func ApplySettings(resource map[string]any, layers []Layer) (map[string]any, err
out["content"] = string(rendered) + "\n"
delete(out, "merge")
delete(out, "protected")
delete(out, TrustedField)
return out, nil
}
@@ -209,18 +214,25 @@ func deepCopy(in map[string]any) map[string]any {
return out
}
// settingsHoldOneLine refuses a line break, a carriage return or a NUL in any string of any setting, for every
// settingsHoldOneLine refuses a line break, a carriage return, any other line end (lineEnds) or a NUL in any
// string of any setting, for every
// module, at any depth, keys as well as values, in an object or a list (novox/hq issue 339). A value is
// substituted into env and configuration files the node-engine writes as root — an app's env file, a logind
// drop-in — and a line break there is a line of the caller's own: a directive, an assignment, a section. A NUL
// ends a string early wherever C reads it. Judged where a layer is kept and again where it is composed, so a
// layer that holds one, however it got into the store, is said with its key. What must hold lines is a file
// of the module's own, never a setting.
// of the module's own, never a setting — with one exception, the certificate authority's root as certificates
// (certificates), because that is how step-ca serves it.
func settingsHoldOneLine(module string, layers []Layer) error {
for _, layer := range layers {
for _, key := range sortedKeysAny(layer.Values) {
if module == rootModule && key == rootSetting {
if text, ok := layer.Values[key].(string); ok && certificates(text) {
continue
}
}
if at := lineBreakIn(layer.Values[key], key); at != "" {
return fmt.Errorf("%s: the setting %s in %q holds a line break, a carriage return or a NUL, which a "+
return fmt.Errorf("%s: the setting %s in %q holds a line break, a carriage return, another line end or a NUL, which a "+
"file it is written into would read as a line of its own; a setting is one line (novox/hq "+
"issue 339)", module, at, layer.From)
}
@@ -229,66 +241,51 @@ func settingsHoldOneLine(module string, layers []Layer) error {
return nil
}
// pemBlocks says whether a value is PEM blocks and nothing else: the one value with lines a setting may hold,
// because a provider serves its certificate authority's root to its consumers as one (step-ca to the route
// proxy). Each block is a BEGIN line, base64 lines of exactly 64 characters but the last, and the END line of
// the same label; the last line is a whole number of base64 groups, padded at its end alone. So no line of it
// is a path, an option, a section or an assignment of a name a program reads — a line break with anything
// else around it is refused.
func pemBlocks(v string) bool {
lines := strings.Split(strings.TrimSuffix(v, "\n"), "\n")
if len(lines) < 3 {
return false
}
for i := 0; i < len(lines); {
label, ok := strings.CutPrefix(lines[i], "-----BEGIN ")
if !ok || !strings.HasSuffix(label, "-----") {
return false
}
label = strings.TrimSuffix(label, "-----")
if label == "" || strings.Trim(label, "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 ") != "" {
return false
}
i++
body := 0
for i < len(lines) && !strings.HasPrefix(lines[i], "-----END ") {
body++
i++
}
if body == 0 || i == len(lines) || lines[i] != "-----END "+label+"-----" {
return false
}
for j, line := range lines[i-body : i] {
if !base64Line(line, j == body-1) {
return false
}
}
i++
}
return true
}
// lineEnds are every character a reader may take as the end of a line: \n and \r, vertical tab and form feed,
// NEL, and the Unicode line and paragraph separators; and NUL, which ends a string early wherever C reads it.
const lineEnds = "\n\r\v\f\x00\u0085\u2028\u2029"
// base64Line is one line of a PEM body: 64 characters of the base64 alphabet, or for the last line at most 64,
// a whole number of groups, with at most two '=' at its end.
func base64Line(line string, last bool) bool {
if line == "" || len(line) > 64 || (!last && len(line) != 64) || len(line)%4 != 0 {
// rootSetting is the one setting that may hold lines: the certificate authority's root, which step-ca serves to
// its consumers as the setting `root` and which they write into a bundle file (the co-located path, issue 038).
const (
rootModule = "step-ca"
rootSetting = "root"
)
// certificates says whether a value is one or more PEM CERTIFICATE blocks and nothing else: each decodes with
// encoding/pem, carries no headers, and parses with x509.ParseCertificate; nothing but a final line break
// surrounds them, and every line ends with \n alone.
func certificates(v string) bool {
if strings.ContainsAny(v, "\r\v\f\x00\u0085\u2028\u2029") {
return false
}
data := strings.TrimRight(line, "=")
if len(line)-len(data) > 2 || (!last && data != line) {
return false
rest := []byte(v)
found := 0
for len(rest) > 0 {
if !bytes.HasPrefix(rest, []byte("-----BEGIN ")) {
return false
}
block, after := pem.Decode(rest)
if block == nil || block.Type != "CERTIFICATE" || len(block.Headers) > 0 {
return false
}
if _, err := x509.ParseCertificate(block.Bytes); err != nil {
return false
}
found++
rest = after
}
return strings.Trim(data, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/") == ""
return found > 0
}
// lineBreakIn is where the first string under v holding \n, \r or NUL is, or "".
func lineBreakIn(v any, at string) string {
if strings.ContainsAny(at, "\n\r\x00") {
return strings.NewReplacer("\n", `\n`, "\r", `\r`, "\x00", `\0`).Replace(at)
if strings.ContainsAny(at, lineEnds) {
return strconv.Quote(at)
}
switch t := v.(type) {
case string:
if strings.ContainsAny(t, "\n\r\x00") && !pemBlocks(t) {
if strings.ContainsAny(t, lineEnds) {
return at
}
case map[string]any:
+112
View File
@@ -0,0 +1,112 @@
package catalogue
import (
"fmt"
"sort"
)
// Which settings are the controller's terminal's alone (novox/hq issue 339).
//
// A setting is the operator's word on how a module is configured, and the `settings` verb writes it for any
// caller allowed to call verbs — agents among them. Most settings change only the module itself. Some change
// what root or another module trusts, and those are said at the terminal alone, never through a verb:
//
// 1. `places` and `accesses`: where the node-engine creates and, as root, owns a module's directories, and
// which of the machine's paths are mounted into its container.
// 2. **Every key a provider serves**, and every setting a served value asks for. A setting overrides a served
// key (Settle), and what is served is what every consumer of the provision connects to and believes: a
// database's port, an object store's scheme, a registry's port, an identity provider's issuer and token
// path. Through a verb, any caller could point every consumer at a listener of its own and collect the
// credentials they present.
// 3. **Every setting a file asks for, unless the file says `"trusted": false`.** A file root or a consumer trusts —
// a logind drop-in, an env file that says which uid a container runs as, a script run as root — must not
// change through a verb, and the safe reading of a file that says nothing is that it is one of them (fail
// closed). `"trusted": false` is the opt-out, for a file nothing trusts: a person's own notifier settings.
// `module check` lists the files that say nothing, so an author can opt one out where that is true.
//
// Derived from the manifest, never listed by hand, so a provider or a trusted file added tomorrow is covered.
// TrustedField is the key a file resource carries to say whether the settings it asks for are trusted: true, or
// absent, makes each a terminal key; false says, out loud, that none changes what root or a consumer trusts. Said in the
// catalogue, never on the machine: the composer takes it out before the node-engine, which parses strictly.
const TrustedField = "trusted"
// TerminalKeys are the settings keys of a module that are set at the controller's terminal alone: places and
// accesses, every key its provisions serve and every setting a served value asks for, and every setting a file
// asks for unless it says `"trusted": false`. Places and accesses first, then the rest sorted.
func TerminalKeys(m Manifest) []string {
keys := map[string]bool{}
for _, served := range m.Serves {
for key, value := range served {
keys[key] = true
if s, ok := value.(string); ok {
for _, asked := range settingsUsed(s) {
keys[asked] = true
}
}
}
}
for _, r := range m.Resources {
if fmt.Sprint(r["type"]) != "file" {
continue
}
if trusted, said := r[TrustedField].(bool); said && !trusted {
continue
}
if content, ok := r["content"].(string); ok {
for _, asked := range settingsUsed(content) {
keys[asked] = true
}
}
}
delete(keys, PlacesSetting)
delete(keys, AccessesSetting)
rest := make([]string, 0, len(keys))
for k := range keys {
rest = append(rest, k)
}
sort.Strings(rest)
return append([]string{PlacesSetting, AccessesSetting}, rest...)
}
// UnsaidTrust is every file of a module that asks for a setting and does not say whether it is trusted, by id:
// each counts as trusted, and is listed so an author can opt out a file nothing trusts.
func UnsaidTrust(m Manifest) []string {
var out []string
for _, r := range m.Resources {
if fmt.Sprint(r["type"]) != "file" {
continue
}
content, _ := r["content"].(string)
if len(settingsUsed(content)) == 0 {
continue
}
if _, said := r[TrustedField]; !said {
out = append(out, fmt.Sprint(r["id"]))
}
}
sort.Strings(out)
return out
}
// TrustProblems are the ways a manifest states `trusted` wrongly: anything but true or false, or on anything but
// a file. Refused at registration and by `module check`.
func TrustProblems(m Manifest) []string {
var out []string
for _, r := range m.Resources {
v, said := r[TrustedField]
if !said {
continue
}
if fmt.Sprint(r["type"]) != "file" {
out = append(out, fmt.Sprintf("%s: %v is a %v and says %q; only a file says whether the settings it "+
"asks for are trusted (novox/hq issue 339)", m.Module, r["id"], r["type"], TrustedField))
continue
}
if _, ok := v.(bool); !ok {
out = append(out, fmt.Sprintf("%s: %v says %q as %v; it is true or false (novox/hq issue 339)",
m.Module, r["id"], TrustedField, v))
}
}
return out
}
+114 -20
View File
@@ -55,29 +55,123 @@ func TestASettingHoldsOneLine(t *testing.T) {
}
}
// PEM blocks alone may hold lines: a provider serves its authority's root as a setting. Anything around them, or
// a line in them that is not base64, is refused.
func TestAPEMBlockIsTheOneSettingWithLines(t *testing.T) {
m := Manifest{Module: "route-proxy"}
full := strings.Repeat("MIIB", 16)
pem := "-----BEGIN CERTIFICATE-----\n" + full + "\n" + full + "\nAbCd+/==\n-----END CERTIFICATE-----\n"
for _, ok := range []string{pem, pem + pem, strings.TrimSuffix(pem, "\n"),
"-----BEGIN CERTIFICATE-----\nMIIBeDCCAR2gAwIBAgIQfake000000000000000000000000\n-----END CERTIFICATE-----\n"} {
if err := JudgeSettings(m, []Layer{{From: "anchor", Values: map[string]any{"root": ok}}}, false); err != nil {
t.Errorf("a PEM block: %v", err)
// Lines are allowed in one setting only: step-ca's `root`, as certificates that encoding/pem decodes and
// x509.ParseCertificate parses (novox/hq issue 339). Any other module or key, another label, or a block that is
// not a certificate is refused like any other line break.
func TestOnlyTheAuthoritysRootMayHoldLines(t *testing.T) {
ca := Manifest{Module: "step-ca"}
judge := func(m Manifest, key, v string) error {
return JudgeSettings(m, []Layer{{From: "anchor", Values: map[string]any{key: v}}}, false)
}
for _, ok := range []string{servedRoot, servedRoot + servedRoot, strings.TrimSuffix(servedRoot, "\n")} {
if err := judge(ca, "root", ok); err != nil {
t.Errorf("the authority's root: %v", err)
}
}
for _, bad := range []string{
pem + "PATH=/tmp\n",
"PATH=\n" + pem,
"-----BEGIN CERTIFICATE-----\n" + full + "\nPATH=\n-----END CERTIFICATE-----\n",
"-----BEGIN CERTIFICATE-----\nshort\n" + full + "\n-----END CERTIFICATE-----\n",
"-----BEGIN CERTIFICATE-----\n" + full + "\n-----END KEY-----\n",
"-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n",
"-----BEGIN CERTIFICATE-----\r\n" + full + "\r\n-----END CERTIFICATE-----\r\n",
body := strings.TrimSuffix(strings.TrimPrefix(servedRoot, "-----BEGIN CERTIFICATE-----\n"), "-----END CERTIFICATE-----\n")
for name, bad := range map[string]string{
"a line after it": servedRoot + "PATH=/tmp\n",
"a line before it": "PATH=\n" + servedRoot,
"another label": "-----BEGIN PRIVATE KEY-----\n" + body + "-----END PRIVATE KEY-----\n",
"headers": "-----BEGIN CERTIFICATE-----\nProc-Type: 4,ENCRYPTED\n\n" + body + "-----END CERTIFICATE-----\n",
"base64 that is no cert": "-----BEGIN CERTIFICATE-----\nMIIBeDCCAR2gAwIBAgIQfake000000000000000000000000\n-----END CERTIFICATE-----\n",
"carriage returns": strings.ReplaceAll(servedRoot, "\n", "\r\n"),
} {
if err := JudgeSettings(m, []Layer{{From: "anchor", Values: map[string]any{"root": bad}}}, false); err == nil {
t.Errorf("taken: %q", bad)
if err := judge(ca, "root", bad); err == nil {
t.Errorf("%s was taken", name)
}
}
if err := judge(ca, "other", servedRoot); err == nil {
t.Error("a certificate in another key of the authority was taken")
}
if err := judge(Manifest{Module: "mailu"}, "root", servedRoot); err == nil {
t.Error("a certificate in another module's setting was taken")
}
if err := JudgeSettings(ca, []Layer{{From: "anchor", Values: map[string]any{"root": []any{servedRoot}}}}, false); err == nil {
t.Error("a certificate below the top of the setting was taken")
}
}
// Every character a reader takes as the end of a line is refused, not only \n and \r: vertical tab, form feed,
// NEL and the Unicode line and paragraph separators (novox/hq issue 339).
func TestEveryLineEndIsRefused(t *testing.T) {
m := Manifest{Module: "mailu"}
for _, v := range []string{"a\vb", "a\fb", "a\u0085b", "a\u2028b", "a\u2029b"} {
if err := JudgeSettings(m, []Layer{{From: "home", Values: map[string]any{"v": v}}}, false); err == nil ||
!strings.Contains(err.Error(), "line break") {
t.Errorf("%q: %v", v, err)
}
}
}
// The review's additions: the spool, the container runtimes' data, /opt, and any home's .ssh.
func TestTheRuntimesDataAndAnyHomesSSHAreTheMachinesOwn(t *testing.T) {
m := Manifest{Module: "notes", Resources: []map[string]any{{"id": "data", "type": "directory"}},
Accesses: []Access{{ID: "media"}}}
for _, path := range []string{"/var/spool", "/var/spool/cron", "/var/lib/docker", "/var/lib/docker/volumes",
"/var/lib/containers/storage", "/var/lib/containerd", "/var/lib/containerd/io.containerd.snapshotter.v1", "/opt", "/opt/app", "/home/alice/.ssh", "/home/alice/.ssh/keys",
"/srv/backup/.ssh", "/root/.ssh"} {
layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}}
if _, err := Places(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") {
t.Errorf("a place at %s: %v", path, err)
}
layers = []Layer{{From: "laptop", Values: map[string]any{AccessesSetting: map[string]any{"media": path}}}}
if _, err := AccessPlaces(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") {
t.Errorf("an access at %s: %v", path, err)
}
}
for _, path := range []string{"/var/lib/dockerish", "/home/alice/ssh", "/home/alice/.sshd-notes", "/storage/media"} {
layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}}
if _, err := Places(m, layers); err != nil {
t.Errorf("a place at %s: %v", path, err)
}
}
}
// What a provider serves its consumers is the terminal's (novox/hq issue 339): any key under its `serves`, and any
// setting a served value asks for, is set at the terminal alone — a verb that could change a port could point every
// consumer at a listener of the caller's own. So is any setting a file marked `trusted` asks for.
func TestTerminalKeysAreDerived(t *testing.T) {
postgres := Manifest{Module: "postgres", Serves: map[string]map[string]any{"postgres-database": {"port": 5432.0}}}
keycloak := Manifest{Module: "keycloak", Serves: map[string]map[string]any{"oidc-client": {
"issuer": "${setting:issuer}", "token-path": "/protocol/openid-connect/token"}}}
power := Manifest{Module: "power", Resources: []map[string]any{
{"id": "logind", "type": "file", "path": "/etc/systemd/logind.conf.d/power.conf", "trusted": true,
"content": "HandleLidSwitch=${setting:handle-lid-switch}\n"},
{"id": "note", "type": "file", "path": "/var/lib/power/note", "trusted": false, "content": "${setting:greeting}\n"},
// Unmarked counts as trusted (fail closed): only `"trusted": false` lets a verb change what a file asks for.
{"id": "unmarked", "type": "file", "path": "/etc/power/unmarked", "content": "${setting:unmarked}\n"}}}
for _, c := range []struct {
m Manifest
want string
}{
{postgres, "places,accesses,port"},
{keycloak, "places,accesses,issuer,token-path"},
{power, "places,accesses,handle-lid-switch,unmarked"},
{Manifest{Module: "plain"}, "places,accesses"},
} {
if got := strings.Join(TerminalKeys(c.m), ","); got != c.want {
t.Errorf("%s: %s; want %s", c.m.Module, got, c.want)
}
}
}
// A file that asks for a setting says whether what it asks is trusted (novox/hq issue 339): `trusted` is a
// boolean, on a file alone, and a file asking for a setting without it is named.
func TestAFileSaysWhetherItsSettingsAreTrusted(t *testing.T) {
m := Manifest{Module: "power", Resources: []map[string]any{
{"id": "said", "type": "file", "path": "/etc/a", "trusted": true, "content": "${setting:a}"},
{"id": "unsaid", "type": "file", "path": "/etc/b", "content": "${setting:b}"},
{"id": "no-setting", "type": "file", "path": "/etc/c", "content": "plain"}}}
if got := strings.Join(UnsaidTrust(m), ","); got != "unsaid" {
t.Errorf("unsaid: %s; want unsaid", got)
}
for _, bad := range []map[string]any{
{"id": "x", "type": "file", "path": "/etc/x", "trusted": "yes", "content": "${setting:a}"},
{"id": "y", "type": "directory", "trusted": true},
} {
if problems := TrustProblems(Manifest{Module: "power", Resources: []map[string]any{bad}}); len(problems) == 0 {
t.Errorf("%v was taken", bad)
}
}
}
+8
View File
@@ -446,6 +446,14 @@ const KindUnit = "unit"
// starting ReasonRelogin when only a new login is missing.
const KindAccount = "account"
// KindDirectory is a directory of a module that the node-engine uses as found (novox/hq issue 339): there before
// the mesh, with another owner or mode than declared, and left so until a person hands it over at the machine
// (`mesh-host hand-over`). Stated unhealthy with a reason that starts ReasonUsedAsFound.
const KindDirectory = "directory"
// ReasonUsedAsFound starts the reason of a directory used as found.
const ReasonUsedAsFound = "used as found:"
// ReasonRelogin starts the reason of an account whose running session began before it was put in a group
// (ADR 0252): the build did what it should, and a person has one step left (novox/hq ADR 0254).
const ReasonRelogin = "relogin needed"