diff --git a/.gitignore b/.gitignore index 26b5cfc0..0c8e31a7 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,4 @@ modules/mesh-watcher/mesh-watcher modules/dnsmasq/dnsmasq modules/mesh-delivery/mesh-delivery modules/mesh-issues/cmd/mesh-issues/mesh-issues +modules/mounts/cmd/mounts/mounts diff --git a/modules/mounts/README.md b/modules/mounts/README.md index 20aff3ee..65cac223 100644 --- a/modules/mounts/README.md +++ b/modules/mounts/README.md @@ -11,15 +11,16 @@ provision `nfs-share`, which `nfs-server` provides on the machine that shares. ## What it owns - The packages `nfs-utils` and `cifs-utils`. -- `/etc/mesh-mounts/mounts.conf`, placed by the mesh from the assignment: the settings `shares` and - `sources`, the binding of `nfs-share` (`from`, the sharing machine, and `at`, where it is reached on - the private network), the operator account, and where the module's secret is. +- `/etc/mesh-mounts/mounts.conf`, placed by the mesh from the assignment: the settings `shares`, + and `sources`, the binding of `nfs-share` (`from`, the sharing machine, and `at`, where it is + reached on the private network), the operator account, and where the module's secrets are placed. - One `.mount` and one `.automount` unit per share or source, in `/etc/systemd/system`, named as `systemd-escape --path` names the mount point and opened by the marker `# Written by the mesh (module mounts, novox/hq ADR 0263).` A unit file without that marker is not the module's: it is never written over, stopped or removed, and the share whose mount point it holds is said as not set up. -- The SMB logins, one file per source in the module's state directory, readable by root only. +- The SMB logins, one credentials file per source in the module's state directory, readable by root only, + written from the source's username and password. - `/var/lib/mesh-mounts/from-bus` (the bundle's: what the sharing machine exports) and `/run/mesh-mounts` (the processes': what was applied and what was found). @@ -58,53 +59,49 @@ read-write share is mounted hard, so a write is never dropped while the server i ## Settings -Both are preferences whose default, `none`, mounts nothing (ADR 0262 allows no empty default). +Both are preferences whose default, `none`, names nothing (ADR 0262 allows no empty default). - `shares`: space-separated `name=mountpoint`, each optionally `:ro`. The name is the share's on the sharing machine; its path there comes from that machine over the bus, never from this setting. For example `library=/srv/library downloads=/srv/downloads:ro`. - `sources`: space-separated occasional sources, each optionally `:ro`: - `name=smb:///@` or `name=disk:@`. - For example `games=smb://192.0.2.76/Share@/mnt/games`. + `name=smb://[@]/@` or `name=disk:@`. + For example `games=smb://root@192.0.2.76/Share@/mnt/games`; the host is a name or an IPv4 address. An SMB + source's username is in its entry: it is not a secret (novox/hq ADR 0283). It is percent-decoded, so a device that needs a domain takes it as + `WORKGROUP%5Cuser` (for `WORKGROUP\user`), and an `@` in it is written `%40`; it holds no space, comma or + `=`. A password never goes in the entry: `user:password@` is refused, saying so and never what was + written. An entry that cannot be read whole is said as malformed by its position alone ("entry 2 of sources + is malformed: …"), never with its name or anything else it holds. A mount point is an absolute, plainly written path, not a directory of the system's own (`/etc`, `/usr`, `/run`, …). An entry that cannot be read is said, and only that entry is left out. -## The secret +Both are trusted settings: an agent proposes them (`settings propose mounts … --node `) and the +operator approves the change on their phone; it is set on that approval and pushed after (ADR 0277). -`smb-credentials` is the module's own secret, issued by an outside party (the device's owner), so the mesh -never makes one in its place. It holds one login per SMB source, named by the source's name in brackets. +## The secrets -The operator gives it at the desk with `secret ask mounts smb-credentials` (ADR 0277): a one-line -prompt opens that shows nothing of what is typed, and the login is typed as one line, the keys -space-separated after the name: +Each SMB source's password is its own secret, `smb-password-`: a member of the module's secret +family `smb-password-*`, issued by an outside party (the device's owner). The mesh never makes a value for +a member (ADR 0283), so one not given is no file, and the module reads that as not given. -``` -[games] username=… password=… -``` +The operator gives it at the desk: an agent asks for it with the controller's `secret-ask` verb (or the +operator runs `secret ask mounts smb-password-`), a prompt opens on that machine's desk that +shows nothing of what is typed and names the module, the secret and who asked, and the password is typed +as one line (ADR 0277). It may also be given from a file at the controller's terminal with +`secret accept mounts smb-password- --from `. -`password=` is last and takes everything after it to the end of the line, so a password may hold spaces -and `=`; `domain=`, when the device wants one, comes before it. The line is read trimmed, so a password -does not end with a space; after `password=` it is kept as typed, so it may begin with one. Several -sources are several such lines, which the prompt cannot take at once: a file can, given with -`secret accept mounts smb-credentials --from `, holding -one line per source, or one section per source with one key a line: +The file is read as one line: one trailing line break is dropped and everything else is kept as typed, so +a password may hold spaces and `=`. An empty file is not given. A file of more than one line is refused, +saying so and never what it holds; give it again. No tool answers a password: the options, fstab lines and +mountinfo the tools read are redacted. -``` -[games] -username=… -password=… -domain=… (optional) -``` - -Both forms give the same login, and a file may mix them. A line the module cannot read is said with the -form it expected, never with what was typed. A source whose login is missing is not mounted, and is said -with that remedy. No tool answers a password: the options, fstab lines and mountinfo the tools read are -redacted. +A source whose username or password is not given is not mounted. It is said `waiting`, naming the setting +or the secret it waits for, and the module's health answers *waits* (below). ## The processes and the bus -- `apply` (root, judged by its unit) writes and removes the unit pairs and the logins, makes a missing +- `apply` (root, judged by its unit) writes and removes the unit pairs and the credentials files, makes a missing mount point, reloads the service manager when anything changed and arms every automount, every 30 s. - `watch` (root, judged by `mounts_health`) tries each source without touching its mount point — TCP 2049 for a node's share, 445 for an SMB source, the device for a disk — and says each one's state. @@ -144,15 +141,25 @@ And what it never does: The states: `armed` (the automount waits), `mounted`, `absent` (an occasional source that is not there — a state, not a fault), `unreachable` (should be there and is not), `failed` (tried and refused, or not -set up; `why` says which). +set up; `why` says which), `waiting` (an SMB source whose username or password the operator has not +given; `waits` names the setting or the secret). ## Health +The answer is mesh-sdk's (`go/health`): healthy, unhealthy, or waits (novox/hq ADR 0283). + Unhealthy when a share that should always be there has been unreachable or failed for five minutes ("The share library of is not reachable from this machine since 19:21."), when an automount is not -armed, when something the settings name cannot be set up (a missing login, a unit file that is not the -module's), or when `watch` has not looked for two minutes. An occasional source that is absent is -healthy, and said as not present since when. +armed, when something the settings name cannot be set up (a unit file that is not the module's, a +password of more than one line), or when `watch` has not looked for two minutes. An occasional source that +is absent is healthy, and said as not present since when. + +A source waiting for its username or password is not a fault. With nothing else found, the answer is +*waits*, with one wait per thing not given: the setting `sources` ("the username of the source games, in +its entry of sources") or the secret `smb-password-games` ("the password of the source games"). The node-engine states the +resource `waiting`, the controller says it to the operator as needing them, naming the act, and the +first-node gate does not fail a build for it. Beside a fault, the waits are said and the answer is +unhealthy. ## Taking over a hand-written line diff --git a/modules/mounts/cmd/mounts/apply.go b/modules/mounts/cmd/mounts/apply.go index 54e8016d..97cb80da 100644 --- a/modules/mounts/cmd/mounts/apply.go +++ b/modules/mounts/cmd/mounts/apply.go @@ -19,7 +19,9 @@ package main import ( "context" "encoding/json" + "errors" "fmt" + "io/fs" "net" "os" "os/user" @@ -28,6 +30,8 @@ import ( "strconv" "strings" "time" + + "git.novox.be/novox/mesh-sdk/go/health" ) // LookEvery is how often the process looks. @@ -48,6 +52,9 @@ const ( StateAbsent = "absent" // an occasional source that is not there: a state, not a fault StateUnreachable = "unreachable" // should be there and is not StateFailed = "failed" // tried and refused, or not set up; why says which + // StateWaiting is a source that cannot be set up until the operator gives its username or password + // (novox/hq ADR 0283): not a fault, said with what it waits for. + StateWaiting = "waiting" ) // ExportsAnswer is what the bundle left from the sharing machine's `node-nfs-server.exports`. @@ -74,6 +81,9 @@ type ShareStatus struct { Why string `json:"why,omitempty"` MountUnit string `json:"mount_unit,omitempty"` AutoUnit string `json:"automount_unit,omitempty"` + // Waits is what a waiting source waits for: its username in the setting sources, or its secret + // smb-password-. + Waits []health.Wait `json:"waits,omitempty"` } // Status is what one look found. @@ -272,23 +282,6 @@ func (a *Applier) Look(ctx context.Context) Status { } } - var creds map[string]SMBCredential - needCreds := false - for _, s := range cfg.Sources { - needCreds = needCreds || s.Kind == "smb" - } - if needCreds { - secret, err := a.M.read(cfg.Secret) - if cfg.Secret == "" || err != nil { - st.Problems = append(st.Problems, "the module's secret smb-credentials is not on this machine") - } else { - var p []string - creds, p = ParseSMBCredentials(secret) - for _, x := range p { - st.Problems = append(st.Problems, "smb-credentials: "+x) - } - } - } uid, gid := -1, -1 if cfg.Account != "" && a.Lookup != nil { if u, g, err := a.Lookup(cfg.Account); err == nil { @@ -300,11 +293,22 @@ func (a *Applier) Look(ctx context.Context) Status { case "disk": pairs = append(pairs, DiskPair(s)) case "smb": - c, ok := creds[s.Name] - if !ok || cfg.State == "" { + c, waits, why := a.login(cfg, s) + switch { + case why != "": + notSet[s.Name] = ShareStatus{Name: s.Name, Kind: "smb", Occasional: true, Mountpoint: s.Mountpoint, State: StateFailed, Why: why} + continue + case len(waits) > 0: + var gives []string + for _, w := range waits { + gives = append(gives, w.What+" ("+w.Names()+")") + } + notSet[s.Name] = ShareStatus{Name: s.Name, Kind: "smb", Occasional: true, Mountpoint: s.Mountpoint, State: StateWaiting, + Why: "it waits for the operator to give " + strings.Join(gives, " and "), Waits: waits} + continue + case cfg.State == "": notSet[s.Name] = ShareStatus{Name: s.Name, Kind: "smb", Occasional: true, Mountpoint: s.Mountpoint, State: StateFailed, - Why: fmt.Sprintf("its login is not given: at the desk, `secret ask mounts smb-credentials` and type one line `[%s] username=… password=…`; "+ - "or put a [%s] section in a file and `secret accept mounts smb-credentials --from `", s.Name, s.Name)} + Why: "the module's state directory is not named in its configuration, so no credentials file can be written"} continue } file := filepath.Join(cfg.State, "smb-"+s.Name+".credentials") @@ -701,3 +705,50 @@ func orInterval(d, otherwise time.Duration) time.Duration { } return otherwise } + +// login is an SMB source's login, or what it waits for, or why it cannot be read. The password is read as +// one line from the source's own secret; it is never said, and neither is anything the file holds. +func (a *Applier) login(cfg Config, s Source) (SMBCredential, []health.Wait, string) { + var waits []health.Wait + source, user := s.Name, s.User + part := "the source " + source + if user == "" { + waits = append(waits, health.Wait{Part: part, Setting: "sources", + What: "the username of the source " + source + ", in its entry of sources"}) + } + // Never a relative path: without the directory the mesh places the passwords in, nothing is read. + if !filepath.IsAbs(cfg.Passwords) { + return SMBCredential{}, nil, "the module's configuration names no directory for its passwords, so " + + PasswordSecret(source) + " cannot be read" + } + var password string + raw, err := a.M.read(PasswordFile(cfg.Passwords, source)) + switch { + case errors.Is(err, fs.ErrNotExist): + waits = append(waits, health.Wait{Part: part, Secret: PasswordSecret(source), What: "the password of the source " + source}) + case err != nil: + return SMBCredential{}, nil, "its password " + PasswordSecret(source) + " could not be read: " + bare(err) + default: + pw, given, perr := ReadPassword(raw) + if perr != nil { + return SMBCredential{}, nil, "its password " + PasswordSecret(source) + " " + perr.Error() + ": give it again" + } + if !given { + waits = append(waits, health.Wait{Part: part, Secret: PasswordSecret(source), What: "the password of the source " + source}) + } + password = pw + } + if len(waits) > 0 { + return SMBCredential{}, waits, "" + } + return SMBCredential{Username: user, Password: password}, nil, "" +} + +// bare is an error without the path it names: the reason alone. +func bare(err error) string { + var pe *fs.PathError + if errors.As(err, &pe) { + return pe.Err.Error() + } + return err.Error() +} diff --git a/modules/mounts/cmd/mounts/config.go b/modules/mounts/cmd/mounts/config.go index bb41c280..b6922e4d 100644 --- a/modules/mounts/cmd/mounts/config.go +++ b/modules/mounts/cmd/mounts/config.go @@ -6,14 +6,16 @@ package main // The mesh renders ConfigFile from the assignment: the settings `shares` and `sources`, and the binding // of the provision `nfs-share` (`from`, the machine that shares, and `at`, where it is reached). The // long-running process reads it as root and writes the mount units; the bundle reads it as the operator -// account to read what the sharing machine exports from the bus. Nothing in it is secret: the SMB credentials are the -// module's own secret, in a file only root reads. +// account to read what the sharing machine exports from the bus. Nothing in it is secret: an SMB source's username is in its +// entry of sources, and its password the module's own secret smb-password-, in a file only root reads. import ( "fmt" + "net/url" "path/filepath" "regexp" "strings" + "unicode" ) // Where the module keeps its things. ConfigFile is placed by the mesh; the rest by the module's code. @@ -41,11 +43,12 @@ type Config struct { // reached on the private network (the binding's `at`). Server string `json:"server,omitempty"` At string `json:"at,omitempty"` - // Secret is the path of the module's own secret `smb-credentials`; State is the module's state - // directory, where the per-source credentials files are written. - Secret string `json:"secret,omitempty"` - State string `json:"state,omitempty"` - Account string `json:"account,omitempty"` + // Passwords is the directory where the mesh places each SMB source's password, + // `smb-password-.secret` (ADR 0283); State is the module's state directory, where the + // per-source credentials files are written. + Passwords string `json:"passwords,omitempty"` + State string `json:"state,omitempty"` + Account string `json:"account,omitempty"` // Node is this machine's name: a share this machine serves itself is never mounted onto it. Node string `json:"node,omitempty"` // Problems are what in the file could not be read; each is said, and its share is left out. @@ -65,10 +68,13 @@ type Wanted struct { // Source is one occasional source: a device that is not a node and is not always there. type Source struct { - Name string `json:"name"` - Kind string `json:"kind"` // smb or disk - Host string `json:"host,omitempty"` - Share string `json:"share,omitempty"` + Name string `json:"name"` + Kind string `json:"kind"` // smb or disk + Host string `json:"host,omitempty"` + Share string `json:"share,omitempty"` + // User is an SMB source's username, from its entry (`smb://@/`), percent-decoded; + // empty when the entry names none, and the source then waits for it (ADR 0283). Not a secret. + User string `json:"user,omitempty"` UUID string `json:"uuid,omitempty"` Mountpoint string `json:"mountpoint"` ReadOnly bool `json:"read_only"` @@ -157,49 +163,48 @@ func ParseSources(v string) ([]Source, []string) { var out []Source var problems []string seen := map[string]bool{} - for _, e := range entries(v) { + // **Nothing an entry holds is said back until the whole entry has read**: an entry may carry a password + // where the user goes, in any scheme, and a password with a space or a comma splits it in two, whose + // tail may itself look like name=…. A malformed entry is named by its position alone. + for i, e := range entries(v) { name, rest, ok := strings.Cut(e, "=") - if !ok { - problems = append(problems, fmt.Sprintf("the source %q is not name=@", e)) - continue - } - if !nameRe.MatchString(name) { - problems = append(problems, fmt.Sprintf("the source name %q is not lower-case letters, digits and dashes", name)) + if !ok || !nameRe.MatchString(name) { + problems = append(problems, malformed(i, "it is not name=@, its name lower-case letters, digits and dashes")) continue } at := strings.LastIndex(rest, "@") if at < 0 { - problems = append(problems, fmt.Sprintf("the source %s names no mount point (…@)", name)) + problems = append(problems, malformed(i, "it names no mount point (…@)")) continue } where, pointRO := rest[:at], rest[at+1:] point, ro := readOnly(pointRO) - if err := CheckMountpoint(point); err != nil { - problems = append(problems, fmt.Sprintf("the source %s: %v", name, err)) + if CheckMountpoint(point) != nil { + problems = append(problems, malformed(i, "its mount point is not an absolute, plainly written path outside the system's own directories")) continue } s := Source{Name: name, Mountpoint: point, ReadOnly: ro} switch { case strings.HasPrefix(where, "smb://"): - host, share, ok := strings.Cut(strings.TrimPrefix(where, "smb://"), "/") - if !ok || !hostRe.MatchString(host) || !shareRe.MatchString(share) { - problems = append(problems, fmt.Sprintf("the source %s: %q is not smb:///", name, where)) + user, host, share, problem := parseSMB(strings.TrimPrefix(where, "smb://")) + if problem != "" { + problems = append(problems, malformed(i, problem)) continue } - s.Kind, s.Host, s.Share = "smb", host, share + s.Kind, s.User, s.Host, s.Share = "smb", user, host, share case strings.HasPrefix(where, "disk:"): uuid := strings.TrimPrefix(where, "disk:") if !uuidRe.MatchString(uuid) { - problems = append(problems, fmt.Sprintf("the source %s: %q is not disk:", name, where)) + problems = append(problems, malformed(i, "it is not disk:")) continue } s.Kind, s.UUID = "disk", uuid default: - problems = append(problems, fmt.Sprintf("the source %s: %q is neither smb:/// nor disk:", name, where)) + problems = append(problems, malformed(i, "it is neither smb://[@]/ nor disk:")) continue } if seen[name] || seen["@"+point] { - problems = append(problems, fmt.Sprintf("the source %s or its mount point %s is named twice", name, point)) + problems = append(problems, fmt.Sprintf("the source %s or its mount point is named twice", name)) continue } seen[name], seen["@"+point] = true, true @@ -208,6 +213,11 @@ func ParseSources(v string) ([]Source, []string) { return out, problems } +// malformed is a problem with the i-th entry of sources, by its position alone: nothing it holds is said. +func malformed(i int, why string) string { + return fmt.Sprintf("entry %d of sources is malformed: %s", i+1, why) +} + // ParseConfig reads the rendered file: `key=value` lines. func ParseConfig(text string) Config { kv := map[string]string{} @@ -229,7 +239,10 @@ func ParseConfig(text string) Config { c.Sources, p = ParseSources(kv["sources"]) c.Problems = append(c.Problems, p...) c.Unreadable = append(c.Unreadable, p...) - c.Server, c.At, c.Secret, c.State, c.Account = kv["server"], kv["at"], kv["secret"], kv["state"], kv["account"] + c.Server, c.At, c.Passwords, c.State, c.Account = kv["server"], kv["at"], kv["passwords"], kv["state"], kv["account"] + if c.Passwords == "" { + c.Passwords = c.State + } c.Node = kv["node"] if c.At != "" && !hostRe.MatchString(c.At) { c.Problems = append(c.Problems, fmt.Sprintf("the sharing machine's address %q is not a name or an address", c.At)) @@ -242,7 +255,7 @@ func ParseConfig(text string) Config { kept := c.Sources[:0] for _, s := range c.Sources { if other, dup := points[s.Mountpoint]; dup { - c.Problems = append(c.Problems, fmt.Sprintf("the source %s mounts at %s, where the share %s already does", s.Name, s.Mountpoint, other)) + c.Problems = append(c.Problems, fmt.Sprintf("the source %s mounts where the share %s already does", s.Name, other)) continue } kept = append(kept, s) @@ -263,174 +276,69 @@ func (c Config) ServerNode() string { return first } -// SMBCredential is one source's login, from the module's own secret. +// SMBCredential is one source's login: the username from its entry of `sources`, the password from the +// source's own secret `smb-password-` (novox/hq ADR 0283). type SMBCredential struct { Username string Password string - Domain string } -// SMBOneLineForm is the form of a login the operator types at the desk's one-line prompt (`secret ask`, -// novox/hq ADR 0277), said in every refusal so the operator knows what to type next. -const SMBOneLineForm = "[] username=… password=…" +// PasswordSecret is the name of a source's password: a member of the module's secret family +// `smb-password-*`, given by that full name (`secret ask mounts smb-password-`). +func PasswordSecret(source string) string { return "smb-password-" + source } -// ParseSMBCredentials reads the secret `smb-credentials`: one login per SMB source, named by its section. -// A login is one line, as typed at the desk's prompt (ADR 0277), with the source's name in brackets and -// the keys after it, space-separated: -// -// [name] username=… password=… -// -// `password=` is last and takes everything after it to the end of the line, so a password may hold spaces -// and `=`; `domain=`, when given, comes before it. Or a login is a section of a file, as `secret accept -// --from ` takes it, one key a line: -// -// [name] -// username=… -// password=… -// domain=… (optional) -// -// The two forms give the same login, and a file may mix them. A line is read trimmed, so a password does -// not end with a space; after `password=` it is kept as typed, so it may begin with one. The values are -// never answered by any tool; a problem names the line or the section, never a value nor a key the -// operator typed. -func ParseSMBCredentials(text string) (map[string]SMBCredential, []string) { - out := map[string]SMBCredential{} - var problems []string - // section is the open section of the file form, whose key=value lines follow; "" is none. A one-line - // login closes it, so a key on a later line cannot land in a login given whole. - section := "" - for i, raw := range strings.Split(text, "\n") { - line := strings.TrimSpace(raw) - if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") { - continue - } - if !strings.HasPrefix(line, "[") { - if section == "" { - problems = append(problems, fmt.Sprintf("line %d is outside a [section]: a login is one line as %s, or a [section] with username= and password= lines", i+1, SMBOneLineForm)) - continue - } - k, v, ok := strings.Cut(line, "=") - if !ok { - problems = append(problems, fmt.Sprintf("line %d in [%s] is not key=value", i+1, section)) - continue - } - c := out[section] - if !c.read(k, v) { - problems = append(problems, fmt.Sprintf("[%s] has a key this module does not read; it reads username, password and domain", section)) - } - out[section] = c - continue - } - // `[` opens a section: the file form when nothing follows the `]` on the line, the one-line form when - // the keys do. The name is the first `]`'s, and a source's name holds no `]`, so nothing typed after it - // can open or close another section. - section = "" - name, rest, closed := strings.Cut(line[1:], "]") - name = strings.TrimSpace(name) - if !closed || !nameRe.MatchString(name) { - problems = append(problems, fmt.Sprintf("line %d opens no source: the name in brackets is lower-case letters, digits and dashes, as %s", i+1, SMBOneLineForm)) - continue - } - if _, dup := out[name]; dup { - problems = append(problems, fmt.Sprintf("the section [%s] is given twice; the first is kept", name)) - continue - } - if strings.TrimSpace(rest) == "" { - out[name] = SMBCredential{} - section = name - continue - } - // From here the line may be a password typed alone that happens to start like a name, so until it - // reads as a login nothing of it is said: not the name, not what follows it. - if rest[0] != ' ' && rest[0] != '\t' { - problems = append(problems, fmt.Sprintf("line %d: the name in brackets is not followed by a space; the one-line form is %s", i+1, SMBOneLineForm)) - continue - } - c, ok := readOneLineLogin(rest) - if !ok { - problems = append(problems, fmt.Sprintf("line %d: what follows the name in brackets is not key=value pairs; the one-line form is %s, password last", i+1, SMBOneLineForm)) - continue - } - if c.unknownKey { - problems = append(problems, fmt.Sprintf("[%s] has a key this module does not read; it reads username, password and domain", name)) - } - out[name] = c.SMBCredential - } - // A login that cannot be used is not answered at all: apply says the source as not set up, rather than - // writing a credentials file with a hole in it. - for name, c := range out { - if c.Username == "" { - problems = append(problems, fmt.Sprintf("[%s] gives no username; a login is %s", name, SMBOneLineForm)) - delete(out, name) - } - if strings.ContainsAny(c.Username+c.Password+c.Domain, "\n") { - problems = append(problems, fmt.Sprintf("[%s] holds a line break", name)) - delete(out, name) - } - } - return out, problems +// PasswordFile is where the mesh places a source's password, in the directory the config names. +func PasswordFile(dir, source string) string { + return filepath.Join(dir, PasswordSecret(source)+".secret") } -// read puts one key's value in the login and says whether the key is one this module reads. The key is -// matched lower-cased and trimmed; a username or domain is trimmed, a password is kept as given. -func (c *SMBCredential) read(k, v string) bool { - switch strings.ToLower(strings.TrimSpace(k)) { - case "username", "user": - c.Username = strings.TrimSpace(v) - case "password", "pass": - c.Password = v - case "domain": - c.Domain = strings.TrimSpace(v) - default: - return false +// userRe is a username mount.cifs takes on its own line: no space, no `=`, no line break, nothing that +// would end the line early. +var userRe = regexp.MustCompile(`^[^\s=,]{1,64}$`) + +// parseSMB reads what follows smb:// in a source's entry: `[@]/`. The user is +// percent-decoded, so `DOMAIN%5Cuser` or a user holding `@` can be written; a password in its place +// (`user:pass@`) is refused, and no problem says anything of what was written. +func parseSMB(body string) (user, host, share, problem string) { + const form = "is not smb://[@]/, the host a name or an IPv4 address" + if at := strings.LastIndex(body, "@"); at >= 0 { + raw := body[:at] + body = body[at+1:] + switch { + case strings.Contains(raw, ":"): + return "", "", "", "it carries a password in the URL; a password is given as the secret smb-password-, never in a setting" + case strings.Contains(raw, "@"): + return "", "", "", "its user holds an @; write it %40" + } + decoded, err := url.PathUnescape(raw) + if err != nil || !userRe.MatchString(decoded) || strings.IndexFunc(decoded, unicode.IsControl) >= 0 { + return "", "", "", "its user is not a username without spaces, commas, = or control characters (percent-encode a \\ or an @ in it)" + } + user = decoded } - return true + host, share, ok := strings.Cut(body, "/") + if !ok || !hostRe.MatchString(host) || !shareRe.MatchString(share) { + return "", "", "", form + } + return user, host, share, "" } -// oneLineLogin is a login read from the one-line form, and whether a key in it is one this module does -// not read (said by the caller; the key itself is never said). -type oneLineLogin struct { - SMBCredential - unknownKey bool -} - -// readOneLineLogin reads what follows `[name]` on a line: space-separated key=value pairs, where -// `password=` takes everything after it to the end of the line and so is last. It is not ok when a -// word is not key=value: that word is never said, as it may be the password typed without its key. -func readOneLineLogin(rest string) (oneLineLogin, bool) { - var c oneLineLogin - for { - rest = strings.TrimLeft(rest, " \t") - if rest == "" { - return c, true - } - word := rest - if sp := strings.IndexAny(rest, " \t"); sp >= 0 { - word = rest[:sp] - } - k, v, ok := strings.Cut(word, "=") - key := strings.ToLower(strings.TrimSpace(k)) - if !ok || key == "" { - return oneLineLogin{}, false - } - switch key { - case "password", "pass": - // To the end of the line: a password may hold spaces and `=`. - c.read(key, rest[len(k)+1:]) - return c, true - } - if !c.read(key, v) { - c.unknownKey = true - } - rest = rest[len(word):] +// ReadPassword reads a source's password secret as one line, as typed at the desk's hidden prompt (ADR +// 0277): one trailing line break is dropped, everything else is kept as typed, so a password may hold +// spaces and `=`. Empty is not given. A second line is refused: it is said, never what it holds. +func ReadPassword(text string) (password string, given bool, err error) { + text = strings.TrimSuffix(text, "\n") + text = strings.TrimSuffix(text, "\r") + if text == "" { + return "", false, nil } + if strings.ContainsAny(text, "\n\r") { + return "", true, fmt.Errorf("holds more than one line; a password is one line") + } + return text, true, nil } // CredentialsFile is a source's credentials in mount.cifs's format. func (c SMBCredential) CredentialsFile() string { - s := "username=" + c.Username + "\npassword=" + c.Password + "\n" - if c.Domain != "" { - s += "domain=" + c.Domain + "\n" - } - return s + return "username=" + c.Username + "\npassword=" + c.Password + "\n" } diff --git a/modules/mounts/cmd/mounts/failed_test.go b/modules/mounts/cmd/mounts/failed_test.go index 2b03ae2e..38431990 100644 --- a/modules/mounts/cmd/mounts/failed_test.go +++ b/modules/mounts/cmd/mounts/failed_test.go @@ -41,11 +41,11 @@ func TestTheFailureOfTheUnitTheModuleTakesOverIsCleared(t *testing.T) { func TestAMountThatFailedBecauseItsSourceWasAwayIsCleared(t *testing.T) { f := newSystem(t) - f.write(ConfigFile, "shares=library=/srv/library\nsources=games=smb://192.0.2.76/Share@/mnt/games\n"+ + f.write(ConfigFile, "shares=library=/srv/library\nsources=games=smb://root@192.0.2.76/Share@/mnt/games\n"+ "server=homeserver\nat=192.0.2.10\nnode=workstation\naccount=operator\n"+ - "secret=/var/lib/mesh/mounts/smb-credentials.secret\nstate=/var/lib/mesh/mounts\n") + "passwords=/var/lib/mesh/mounts\nstate=/var/lib/mesh/mounts\n") f.write(ExportsFile, exportsAnswer(t, library)) - f.write("/var/lib/mesh/mounts/smb-credentials.secret", "[games]\nusername=root\npassword="+testPassword+"\n") + f.write("/var/lib/mesh/mounts/smb-password-games.secret", testPassword+"\n") a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) a.Look(context.Background()) // writes and arms both pairs; neither source answers f.calls = nil diff --git a/modules/mounts/cmd/mounts/health.go b/modules/mounts/cmd/mounts/health.go index cdd5ce0b..e966a293 100644 --- a/modules/mounts/cmd/mounts/health.go +++ b/modules/mounts/cmd/mounts/health.go @@ -5,20 +5,23 @@ package main // Unhealthy when a share that should always be there has been unreachable or failed for five minutes, // when an automount is not armed, when something the settings name cannot be set up, or when the // process has not looked for two minutes. An occasional source that is absent is healthy: it is said as -// not present since when, and raises nothing. +// not present since when, and raises nothing. A source waiting for the operator's username or password is +// not a fault: when nothing else is found the answer is *waits*, naming each (novox/hq ADR 0283). import ( "fmt" "strings" "time" + + "git.novox.be/novox/mesh-sdk/go/health" ) -// Health is the answer the node-engine reads from a tool check. +// Health is the answer the node-engine reads from a tool check: mesh-sdk's answer (state, healthy, why, +// waits), with what was found and what was said beside it. type Health struct { - Healthy bool `json:"healthy"` - Why string `json:"why"` - Found []string `json:"found,omitempty"` - Said []string `json:"said,omitempty"` + health.Answer + Found []string `json:"found,omitempty"` + Said []string `json:"said,omitempty"` } // Grace is how long a share may be away before it is a fault; Stale how old the process's look may be. @@ -38,10 +41,11 @@ func clock(ts string) string { // Judge is the health of one status at a moment. func Judge(st Status, readErr error, now time.Time) Health { if readErr != nil { - return Health{Healthy: false, Why: "The module's process has left no account of this machine's mounts (" + readErr.Error() + ").", + return Health{Answer: health.Unhealthy("The module's process has left no account of this machine's mounts (" + readErr.Error() + ")."), Found: []string{"no status"}} } - var found, said []string + var found, said, waiting []string + var waits []health.Wait if looked, err := time.Parse(time.RFC3339, st.Looked); err != nil || now.Sub(looked) > Stale { found = append(found, fmt.Sprintf("The module's process last looked at %s, more than two minutes ago.", clock(st.Looked))) } @@ -58,6 +62,10 @@ func Judge(st Status, readErr error, now time.Time) Health { switch s.State { case StateMounted, StateArmed: said = append(said, fmt.Sprintf("%s at %s: %s since %s.", s.Name, s.Mountpoint, s.State, clock(s.Since))) + case StateWaiting: + waiting = append(waiting, fmt.Sprintf("The source %s at %s is not mounted since %s: %s.", s.Name, s.Mountpoint, clock(s.Since), + strings.TrimSuffix(s.Why, "."))) + waits = append(waits, s.Waits...) case StateAbsent: said = append(said, fmt.Sprintf("%s at %s: not present since %s.", s.Name, s.Mountpoint, clock(s.Since))) case StateUnreachable: @@ -73,7 +81,7 @@ func Judge(st Status, readErr error, now time.Time) Health { default: line := fmt.Sprintf("The %s %s at %s cannot be mounted since %s: %s.", map[bool]string{true: "source", false: "share"}[s.Occasional], s.Name, s.Mountpoint, clock(s.Since), strings.TrimSuffix(s.Why, ".")) - if long || strings.Contains(s.Why, "not armed") || strings.Contains(s.Why, "not the module's") || strings.Contains(s.Why, "not given") { + if long || strings.Contains(s.Why, "not armed") || strings.Contains(s.Why, "not the module's") { found = append(found, line) } else { said = append(said, line+" Raised after five minutes.") @@ -81,13 +89,16 @@ func Judge(st Status, readErr error, now time.Time) Health { } } if len(found) > 0 { - return Health{Healthy: false, Why: strings.Join(found, " "), Found: found, Said: said} + return Health{Answer: health.Unhealthy(strings.Join(append(found, waiting...), " "), waits...), Found: found, Said: append(said, waiting...)} + } + if len(waits) > 0 { + return Health{Answer: health.Waiting(strings.Join(waiting, " "), waits...), Said: append(said, waiting...)} } why := "Every share this machine is asked to mount is armed or mounted, and every occasional source is armed, mounted or not present." if len(st.Shares) == 0 { why = "This machine is asked to mount nothing." } - return Health{Healthy: true, Why: why, Said: said} + return Health{Answer: health.Healthy(why), Said: said} } // Health reads the process's last look and judges it. diff --git a/modules/mounts/cmd/mounts/manifest_test.go b/modules/mounts/cmd/mounts/manifest_test.go index 9676f26f..ea95543a 100644 --- a/modules/mounts/cmd/mounts/manifest_test.go +++ b/modules/mounts/cmd/mounts/manifest_test.go @@ -82,9 +82,17 @@ func TestTheManifestNamesWhatTheBundleServesAndWhatItNeeds(t *testing.T) { if len(m.State) != 1 || m.State[0]["name"] != SharesState || m.State[0]["per-machine"] != true { t.Errorf("state %v", m.State) } - sec := m.OwnSecrets["smb-credentials"] - if sec["issued-by"] != "outside" || sec["taken"] != "at-start" || !strings.HasPrefix(sec["path"], "${dir:state}/") { - t.Errorf("the SMB logins are an outside party's, given with secret accept: %v", sec) + // Each SMB source's password is its own one-line secret, a member of a family the mesh never makes a value + // for (novox/hq ADR 0283), placed where the process looks for it. + sec := m.OwnSecrets["smb-password-*"] + if len(m.OwnSecrets) != 1 || sec["issued-by"] != "outside" || sec["path"] != "${dir:state}/"+PasswordSecret("*")+".secret" { + t.Errorf("the SMB passwords are an outside party's, one secret per source: %v", m.OwnSecrets) + } + if got := PasswordFile("${dir:state}", "games"); got != strings.Replace(sec["path"], "*", "games", 1) { + t.Errorf("the process reads %s, the mesh places %s", got, sec["path"]) + } + if _, has := m.Settings["smb-users"]; has || len(m.Settings) != 2 { + t.Errorf("an SMB source's username is in its entry of sources, not a setting of its own: %v", m.Settings) } for _, key := range []string{"shares", "sources"} { s := m.Settings[key] @@ -134,7 +142,7 @@ func TestTheProcessesAreRootAndJudgedByTheirUnitAndTheHealthTool(t *testing.T) { if len(cfg.Problems) != 0 && !strings.Contains(strings.Join(cfg.Problems, " "), "${") { t.Errorf("the config's template reads: %v", cfg.Problems) } - for _, want := range []string{"server=${bound:nfs-share:from}", "at=${bound:nfs-share:at}", "secret=${dir:state}/smb-credentials.secret"} { + for _, want := range []string{"server=${bound:nfs-share:from}", "at=${bound:nfs-share:at}", "passwords=${dir:state}"} { if !strings.Contains(content, want) { t.Errorf("the config lacks %s", want) } @@ -142,7 +150,7 @@ func TestTheProcessesAreRootAndJudgedByTheirUnitAndTheHealthTool(t *testing.T) { if byID["bus-dir"]["path"] != BusDir || byID["bus-dir"]["owner"] != "${machine:account}" { t.Errorf("the bundle, as the operator account, writes the bus's answer: %v", byID["bus-dir"]) } - if strings.Contains(raw, "password") { + if strings.Contains(raw, "password=") { t.Error("the manifest holds no password") } } diff --git a/modules/mounts/cmd/mounts/verbs.go b/modules/mounts/cmd/mounts/verbs.go index aadf29bd..12d07c29 100644 --- a/modules/mounts/cmd/mounts/verbs.go +++ b/modules/mounts/cmd/mounts/verbs.go @@ -49,7 +49,7 @@ func Verbs(m *Machine) []stdio.Tool { return []stdio.Tool{ verb("list", "Every mount point on this machine — its fstab line, its mount and automount units, whether it is "+ "mounted now — and every share and occasional source the module is asked to mount, with its state (armed, "+ - "mounted, absent, unreachable, failed), since when and why. Credentials are never answered. Replaces cat "+ + "mounted, absent, unreachable, failed, waiting), since when and why; a waiting source names the setting or secret it waits for. Credentials are never answered. Replaces cat "+ "/etc/fstab, findmnt and systemctl list-units --type=mount. (r)", map[string]any{"network": map[string]any{"type": "boolean", "description": "only network filesystems (default false)"}}, func(a map[string]any) (any, error) { @@ -142,11 +142,13 @@ func Verbs(m *Machine) []stdio.Tool { }), { Name: "mounts_health", - Description: "Whether this machine's shares are healthy, as the module's health check (novox/hq ADR 0263): unhealthy " + - "when a share that should always be there has been unreachable or failed for five minutes, an automount is not " + - "armed, something the settings name cannot be set up (a missing login, a unit file that is not the module's), or " + - "the module's process has not looked for two minutes; with why, one sentence per finding. An occasional source " + - "that is absent is healthy, and said as not present since when. (r)", + Description: "Whether this machine's shares are healthy, as the module's health check (novox/hq ADR 0263, ADR 0283): " + + "unhealthy when a share that should always be there has been unreachable or failed for five minutes, an automount " + + "is not armed, something the settings name cannot be set up (a unit file that is not the module's), or the " + + "module's process has not looked for two minutes; with why, one sentence per finding. An occasional source that " + + "is absent is healthy, and said as not present since when. A source whose username (in its entry of the setting sources) or " + + "password (the secret smb-password-) is not given waits for the operator: with nothing else found the " + + "answer is waits, naming each, never unhealthy. (r)", Run: func(map[string]any) (any, error) { return m.Health(time.Now()), nil }, }, } diff --git a/modules/mounts/cmd/mounts/write_test.go b/modules/mounts/cmd/mounts/write_test.go index 9ae35456..ad9dd82c 100644 --- a/modules/mounts/cmd/mounts/write_test.go +++ b/modules/mounts/cmd/mounts/write_test.go @@ -10,6 +10,8 @@ import ( "sync" "testing" "time" + + "git.novox.be/novox/mesh-sdk/go/health" ) // The secret every test that has one uses; it must never appear in anything the module says. @@ -147,9 +149,9 @@ func (f *fakeSystem) everything(except ...string) string { } const config = "shares=library=/srv/library downloads=/srv/downloads:ro\n" + - "sources=games=smb://192.0.2.76/Share@/mnt/games usb=disk:1234-ABCD@/mnt/usb\n" + + "sources=games=smb://root@192.0.2.76/Share@/mnt/games usb=disk:1234-ABCD@/mnt/usb\n" + "server=homeserver\nat=192.0.2.10\nnode=workstation\naccount=operator\n" + - "secret=/var/lib/mesh/mounts/smb-credentials.secret\nstate=/var/lib/mesh/mounts\n" + "passwords=/var/lib/mesh/mounts\nstate=/var/lib/mesh/mounts\n" func exportsAnswer(t *testing.T, shares ...ServerShare) string { raw, err := json.Marshal(ExportsAnswer{Server: "homeserver", At: "192.0.2.10", Shares: shares}) @@ -264,7 +266,7 @@ func TestALookWritesArmsAndRemovesOnlyTheModulesOwnUnits(t *testing.T) { f := newSystem(t) f.write(ConfigFile, config) f.write(ExportsFile, exportsAnswer(t, library, downloads)) - f.write("/var/lib/mesh/mounts/smb-credentials.secret", "[games]\nusername=root\npassword="+testPassword+"\n") + f.write("/var/lib/mesh/mounts/smb-password-games.secret", testPassword+"\n") // The operator's own unit for the downloads' mount point: never written over. f.write(filepath.Join(UnitDir, "srv-downloads.mount"), "[Mount]\nWhat=elsewhere\n") // A pair of the module's whose mount point nothing names any more. @@ -318,7 +320,7 @@ func TestALookWritesArmsAndRemovesOnlyTheModulesOwnUnits(t *testing.T) { t.Errorf("games %+v", states["games"]) } // The password is in the secret and in the credentials file, and nowhere else. - if strings.Contains(f.everything("smb-credentials.secret", "smb-games.credentials"), testPassword) { + if strings.Contains(f.everything("smb-password-games.secret", "smb-games.credentials"), testPassword) { t.Error("the password leaked into a file the module wrote") } raw, _ := json.Marshal(f.machine().Health(now)) @@ -361,26 +363,153 @@ func TestASilentServerRemovesNoMount(t *testing.T) { } } -func TestAMissingLoginIsSaidWithItsRemedyAndTheSourceIsNotMounted(t *testing.T) { +func TestASourceWithoutItsLoginWaitsForTheOperatorAndIsNotMounted(t *testing.T) { + for name, c := range map[string]struct { + user, password string + writePassword bool + waitsFor []string + }{ + "no username": {"", testPassword, true, []string{"setting sources"}}, + "no password": {"root@", "", false, []string{"secret smb-password-games"}}, + "an empty password file": {"root@", "", true, []string{"secret smb-password-games"}}, + "a line break only": {"root@", "\n", true, []string{"secret smb-password-games"}}, + "neither": {"", "", false, []string{"setting sources", "secret smb-password-games"}}, + } { + f := newSystem(t) + f.write(ConfigFile, "sources=games=smb://"+c.user+"192.0.2.76/Share@/mnt/games\npasswords=/var/lib/mesh/mounts\nstate=/var/lib/mesh/mounts\n") + if c.writePassword { + f.write("/var/lib/mesh/mounts/smb-password-games.secret", c.password) + } + now := time.Now() + st := f.applier(now).Look(context.Background()) + if len(st.Shares) != 1 || st.Shares[0].State != StateWaiting || len(st.Shares[0].Waits) != len(c.waitsFor) { + t.Fatalf("%s: shares %+v", name, st.Shares) + } + for i, w := range st.Shares[0].Waits { + if got := strings.TrimPrefix(w.Names(), "the "); got != c.waitsFor[i] || w.Part != "the source games" || w.Check() != nil { + t.Errorf("%s: wait %d is %+v, want %s", name, i, w, c.waitsFor[i]) + } + } + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "mnt-games.mount")); !os.IsNotExist(err) { + t.Errorf("%s: a source without its login gets no units", name) + } + if _, err := os.Stat(filepath.Join(f.root, "/var/lib/mesh/mounts/smb-games.credentials")); !os.IsNotExist(err) { + t.Errorf("%s: a source without its login gets no credentials file", name) + } + h := f.machine().Health(now) + raw, _ := json.Marshal(h) + if h.State != health.StateWaits || h.Healthy || len(h.Waits) != len(c.waitsFor) || len(h.Found) != 0 { + t.Errorf("%s: health %s", name, raw) + } + // What the node-engine reads is a wait it can check: the SDK's reader keeps it a wait. + if read, err := health.Read(raw); err != nil || read.State != health.StateWaits { + t.Errorf("%s: the engine reads %+v %v", name, read, err) + } + if strings.Contains(string(raw), testPassword) || strings.Contains(f.everything("smb-password-games.secret"), testPassword) { + t.Errorf("%s: the password leaked", name) + } + } +} + +func TestAPasswordIsOneLineKeptAsTyped(t *testing.T) { + pw := " a b=c " + testPassword + "==" + for text, want := range map[string]string{pw: pw, pw + "\n": pw, pw + "\r\n": pw} { + got, given, err := ReadPassword(text) + if err != nil || !given || got != want { + t.Errorf("%q read as %q %v %v", strings.ReplaceAll(text, testPassword, ""), strings.ReplaceAll(got, testPassword, ""), given, err) + } + } + if _, given, err := ReadPassword(""); given || err != nil { + t.Error("an empty file is not given") + } + if _, given, err := ReadPassword(testPassword + "\nsecond\n"); !given || err == nil || strings.Contains(err.Error(), testPassword) { + t.Errorf("two lines are refused, saying nothing typed: %v", err) + } f := newSystem(t) - f.write(ConfigFile, "sources=games=smb://192.0.2.76/Share@/mnt/games\nsecret=/var/lib/mesh/mounts/smb-credentials.secret\nstate=/var/lib/mesh/mounts\n") - f.write("/var/lib/mesh/mounts/smb-credentials.secret", "[other]\nusername=x\npassword="+testPassword+"\n") + f.write(ConfigFile, "sources=games=smb://root@192.0.2.76/Share@/mnt/games\npasswords=/var/lib/mesh/mounts\nstate=/var/lib/mesh/mounts\n") + f.write("/var/lib/mesh/mounts/smb-password-games.secret", pw+"\n") + f.applier(time.Now()).Look(context.Background()) + creds := filepath.Join(f.root, "/var/lib/mesh/mounts/smb-games.credentials") + if info, err := os.Stat(creds); err != nil || info.Mode().Perm() != 0o600 { + t.Fatalf("the source's login is a file only root reads: %v", err) + } + if b, _ := os.ReadFile(creds); string(b) != "username=root\npassword="+pw+"\n" { + t.Errorf("credentials file %q", strings.ReplaceAll(string(b), testPassword, "")) + } + // Two lines: refused as a fault the operator mends by giving it again, the password never said. + f.write("/var/lib/mesh/mounts/smb-password-games.secret", testPassword+"\nsecond\n") now := time.Now() st := f.applier(now).Look(context.Background()) - if len(st.Shares) != 1 || st.Shares[0].State != StateFailed || - !strings.Contains(st.Shares[0].Why, "secret accept mounts smb-credentials --from ") { + if len(st.Shares) != 1 || st.Shares[0].State != StateFailed || !strings.Contains(st.Shares[0].Why, "more than one line") { t.Fatalf("shares %+v", st.Shares) } - if _, err := os.Stat(filepath.Join(f.root, UnitDir, "mnt-games.mount")); !os.IsNotExist(err) { - t.Error("a source without its login gets no units") + raw, _ := json.Marshal(f.machine().Health(now)) + if strings.Contains(string(raw), testPassword) || strings.Contains(f.everything("smb-password-games.secret", "smb-games.credentials"), testPassword) { + t.Error("the password leaked") } - h := f.machine().Health(now) - if h.Healthy || !strings.Contains(h.Why, "login is not given") || strings.Contains(h.Why, testPassword) { - t.Errorf("health %+v", h) +} + +func TestAnSMBSourcesUserIsInItsEntryAndAPasswordThereIsRefusedUnsaid(t *testing.T) { + sources, problems := ParseSources("a=smb://root@192.0.2.76/Share@/mnt/a b=smb://192.0.2.76/Share@/mnt/b " + + "c=smb://WORKGROUP%5Cme@nas/Media@/mnt/c d=smb://me%40example@nas/Media@/mnt/d:ro") + users := map[string]string{} + for _, s := range sources { + users[s.Name] = s.User + if s.Host == "" || s.Share == "" { + t.Errorf("%s: %+v", s.Name, s) + } } - creds, problems := ParseSMBCredentials("[a]\nusername=u\npassword=" + testPassword + "\nbogus=1\n[b]\npassword=p\n") - if creds["a"].Password != testPassword || len(problems) != 2 || strings.Contains(strings.Join(problems, " "), testPassword) { - t.Errorf("creds %v problems %q", len(creds), problems) + if len(problems) != 0 || users["a"] != "root" || users["b"] != "" || users["c"] != `WORKGROUP\me` || users["d"] != "me@example" { + t.Errorf("users %v problems %q", users, problems) + } + for what, entry := range map[string]string{ + "a password in the URL": "e=smb://root:" + testPassword + "@nas/Media@/mnt/e", + "a password with a slash": "e=smb://root:a/" + testPassword + "@nas/Media@/mnt/e", + "a user with a space, encoded": "e=smb://a%20" + testPassword + "@nas/Media@/mnt/e", + "a bad percent escape": "e=smb://%zz" + testPassword + "@nas/Media@/mnt/e", + "no mount point, with a password": "smb://root:" + testPassword + "@nas/Media", + "another scheme, upper case": "e=SMB://root:" + testPassword + "@nas/Media@/mnt/e", + "a cifs URL": "e=cifs://root:" + testPassword + "@nas/Media@/mnt/e", + "two @ before the host": "e=smb://me@example@nas/Media@/mnt/e" + testPassword, + "a control character in the user": "e=smb://me%07" + testPassword + "@nas/Media@/mnt/e", + } { + got, problems := ParseSources(entry) + said := strings.Join(problems, " ") + if len(got) != 0 || len(problems) != 1 || strings.Contains(said, testPassword) { + t.Errorf("%s: %+v %q", what, got, said) + } + } + if _, problems := ParseSources("e=smb://root:" + testPassword + "@nas/Media@/mnt/e"); !strings.Contains(problems[0], "smb-password-") { + t.Errorf("a password in the URL is said with where it goes: %q", problems) + } + // A password with a space or a comma splits the entry: neither half, nor a name that is not one, is said. + const secret = "SECRETPW" + for _, v := range []string{ + "e=smb://root:SEC RETPW@nas/Media@/mnt/e", + "e=smb://root:SEC,RETPW@nas/Media@/mnt/e", + "e=SMB://root:" + secret + "@nas/Media@/mnt/e", + "e=cifs://root:" + secret + "@nas/Media@/mnt/e", + "SECRET" + "PW=smb://nas/Media@/mnt/e", + // A password holding '=' and a space, or '=' and a comma: the tail reads like name=… and is still not said. + "e=smb://root:sec RETPW=x@nas/Media@/mnt/e", + "e=smb://root:sec,RETPW=x@nas/Media@/mnt/e", + "e=smb://root:SEC RETPW=smb://nas/Media@/mnt/e", + } { + _, problems := ParseSources(v) + said := strings.Join(problems, " ") + if len(problems) == 0 || strings.Contains(strings.ToUpper(said), "RETPW") || strings.Contains(said, "SEC") || + strings.Contains(said, "sec") && !strings.Contains(said, "secret smb-password") { + t.Errorf("%q says %q", strings.ReplaceAll(v, "RETPW", ""), said) + } + // And nothing of it reaches the status or the health answer. + f := newSystem(t) + f.write(ConfigFile, "sources="+v+"\npasswords=/var/lib/mesh/mounts\nstate=/var/lib/mesh/mounts\n") + now := time.Now() + f.applier(now).Look(context.Background()) + raw, _ := json.Marshal(f.machine().Health(now)) + if strings.Contains(strings.ToUpper(string(raw)), "RETPW") || strings.Contains(strings.ToUpper(f.everything("mounts.conf")), "RETPW") { + t.Errorf("%q leaked into the status or the health: %s", strings.ReplaceAll(v, "RETPW", ""), raw) + } } } @@ -537,7 +666,7 @@ func TestApplyActsAndWatchJudgesAndNeitherDoesTheOthersWork(t *testing.T) { f := newSystem(t) f.write(ConfigFile, config) f.write(ExportsFile, exportsAnswer(t, library, downloads)) - f.write("/var/lib/mesh/mounts/smb-credentials.secret", "[other]\nusername=x\npassword="+testPassword+"\n") + f.write("/var/lib/mesh/mounts/smb-password-other.secret", testPassword+"\n") f.write(filepath.Join(UnitDir, "srv-downloads.mount"), "[Mount]\nWhat=elsewhere\n") f.reachable["192.0.2.10:2049"] = true now := time.Now() @@ -573,10 +702,10 @@ func TestApplyActsAndWatchJudgesAndNeitherDoesTheOthersWork(t *testing.T) { for _, s := range st.Shares { states[s.Name] = s.State } - if states["library"] != StateArmed || states["downloads"] != StateFailed || states["games"] != StateFailed || states["usb"] != StateAbsent { + if states["library"] != StateArmed || states["downloads"] != StateFailed || states["games"] != StateWaiting || states["usb"] != StateAbsent { t.Errorf("watch says how each is: %v", states) } - if strings.Contains(f.everything("smb-credentials.secret"), testPassword) { + if strings.Contains(f.everything("smb-password-other.secret"), testPassword) { t.Error("the password leaked") } } @@ -588,87 +717,54 @@ func (f *fakeSystem) setUnit(unit string, state map[string]string) { f.units[unit] = state } -// The login as typed at the desk's one-line prompt (novox/hq ADR 0277) is the login the file form gives -// (issue 379): the password is last and runs to the end of the line, with its spaces and its `=`. -func TestAOneLineLoginIsTheLoginTheFileFormGives(t *testing.T) { - pw := "a b=c " + testPassword + "==" - file := "[recalbox]\nusername=root\npassword=" + pw + "\ndomain=WORKGROUP\n" - line := "[recalbox] username=root domain=WORKGROUP password=" + pw + "\n" - fromFile, fileProblems := ParseSMBCredentials(file) - fromLine, lineProblems := ParseSMBCredentials(line) - want := SMBCredential{Username: "root", Password: pw, Domain: "WORKGROUP"} - if len(fileProblems) != 0 || len(lineProblems) != 0 || fromFile["recalbox"] != want || fromLine["recalbox"] != want { - t.Fatalf("file %+v %q, line %+v %q", fromFile, fileProblems, fromLine, lineProblems) +// A wait beside a fault is said and excuses nothing: the answer is unhealthy, naming both (ADR 0283). +func TestAWaitBesideAFaultIsUnhealthy(t *testing.T) { + now := time.Date(2026, 10, 10, 19, 30, 0, 0, time.UTC) + at := now.Add(-time.Hour).Format(time.RFC3339) + wait := health.Wait{Part: "the source games", Secret: "smb-password-games", What: "the password of the source games"} + games := ShareStatus{Name: "games", Kind: "smb", Occasional: true, Mountpoint: "/mnt/games", State: StateWaiting, Since: at, + Why: "it waits for the operator to give the password of the source games (the secret smb-password-games)", Waits: []health.Wait{wait}} + library := ShareStatus{Name: "library", Kind: "nfs", Mountpoint: "/srv/library", State: StateUnreachable, Since: at, Why: "no answer"} + + h := Judge(Status{Looked: now.Format(time.RFC3339), Server: "homeserver", Shares: []ShareStatus{games, library}}, nil, now) + if h.State != health.StateUnhealthy || h.Healthy || len(h.Waits) != 1 || !strings.Contains(h.Why, "library") || !strings.Contains(h.Why, "games") { + t.Errorf("a wait beside a fault: %+v", h) } - // After `password=` it is kept as typed: a leading space is the password's, in both forms. - fromFile, _ = ParseSMBCredentials("[recalbox]\nusername=root\npassword= " + pw + "\n") - fromLine, _ = ParseSMBCredentials("[recalbox] username=root password= " + pw + "\n") - if fromFile["recalbox"].Password != " "+pw || fromLine["recalbox"] != fromFile["recalbox"] { - t.Errorf("a leading space: file %+v, line %+v", fromFile, fromLine) - } - fromLine, _ = ParseSMBCredentials(line) - if fromLine["recalbox"].CredentialsFile() != "username=root\npassword="+pw+"\ndomain=WORKGROUP\n" { - t.Errorf("credentials file %q", strings.ReplaceAll(fromLine["recalbox"].CredentialsFile(), pw, "")) - } - // Several sources are several lines, in either form, mixed; a `\r` from a terminal is not the password's. - mixed := "[recalbox] username=root password=" + pw + "\r\n[nas]\nusername=u\npassword=p\n[usb] user=x pass=y\n" - got, problems := ParseSMBCredentials(mixed) - if len(problems) != 0 || len(got) != 3 || got["recalbox"].Password != pw || got["nas"].Password != "p" || got["usb"] != (SMBCredential{Username: "x", Password: "y"}) { - t.Errorf("mixed %v %q", len(got), problems) - } - // A one-line login closes the file-form section open before it: a key on the next line lands in no - // login, neither the one given whole nor the one that was open. - got, problems = ParseSMBCredentials("[nas]\nusername=u\npassword=p\n[recalbox] username=root password=x\nusername=other\n") - if got["recalbox"].Username != "root" || got["nas"].Username != "u" || len(problems) != 1 || !strings.Contains(problems[0], "outside a [section]") { - t.Errorf("a stray line after a one-line login: %+v %q", got, problems) + h = Judge(Status{Looked: now.Format(time.RFC3339), Shares: []ShareStatus{games}}, nil, now) + if h.State != health.StateWaits || len(h.Waits) != 1 || len(h.Found) != 0 || !strings.Contains(h.Why, "waits for the operator") { + t.Errorf("a wait alone: %+v", h) } } -// A line the module cannot read is refused with the form it expected, and nothing typed is said back: -// not the password, not a word that may be the password typed without its key. -func TestAMalformedLoginIsRefusedWithTheExpectedFormAndSaysNothingTyped(t *testing.T) { - cases := map[string]string{ - "the password alone, as the prompt invites": testPassword + "\n", - "a line without a section": "username=root password=" + testPassword + "\n", - "a word that is not key=value": "[recalbox] root " + testPassword + "\n", - "a value without a key": "[recalbox] =" + testPassword + "\n", - "no space after the name": "[recalbox]username=root password=" + testPassword + "\n", - "a name that is not a source's": "[Recal box] username=root password=" + testPassword + "\n", - "a bracket never closed": "[recalbox username=root password=" + testPassword + "\n", - "a password alone that looks like a name": "[abc-1]" + testPassword + "\n", - "no username, line form": "[recalbox] password=" + testPassword + "\n", - "no username, file form": "[recalbox]\npassword=" + testPassword + "\n", - "a key this module does not read, file form": "[recalbox]\nusername=root\n" + testPassword + "=1\npassword=x\n", - "a key this module does not read, line form": "[recalbox] username=root " + testPassword + "=1 password=x\n", - "a section given twice, the second one-line": "[recalbox]\nusername=root\npassword=x\n[recalbox] username=u password=" + testPassword + "\n", +// A password that exists and cannot be read, and a configuration naming no directory for the passwords, are +// refused as faults: said without the value and without the path, and nothing is read from a relative path. +func TestAPasswordThatCannotBeReadIsAFaultSaidWithoutValueOrPath(t *testing.T) { + // A directory where the file should be: read fails, and not as "not there" (as root a mode would not stop it). + f := newSystem(t) + f.write(ConfigFile, "sources=games=smb://root@192.0.2.76/Share@/mnt/games\npasswords=/var/lib/mesh/mounts\nstate=/var/lib/mesh/mounts\n") + f.write("/var/lib/mesh/mounts/smb-password-games.secret/inside", testPassword) + st := f.applier(time.Now()).Look(context.Background()) + if len(st.Shares) != 1 || st.Shares[0].State != StateFailed || !strings.Contains(st.Shares[0].Why, "could not be read") || + strings.Contains(st.Shares[0].Why, "/var/lib") || strings.Contains(st.Shares[0].Why, testPassword) { + t.Fatalf("an unreadable password: %+v", st.Shares) } - for what, text := range cases { - got, problems := ParseSMBCredentials(text) - said := strings.Join(problems, " ") - if len(problems) == 0 { - t.Errorf("%s: not refused: %+v", what, got) - continue + + // No passwords= and no state=: the source is not set up, and no relative path is read. + for _, cfg := range []string{ + "sources=games=smb://root@192.0.2.76/Share@/mnt/games\n", + "sources=games=smb://root@192.0.2.76/Share@/mnt/games\npasswords=relative/dir\n", + } { + g := newSystem(t) + g.write(ConfigFile, cfg) + g.write("smb-password-games.secret", testPassword) + g.write("relative/dir/smb-password-games.secret", testPassword) + st := g.applier(time.Now()).Look(context.Background()) + if len(st.Shares) != 1 || st.Shares[0].State != StateFailed || !strings.Contains(st.Shares[0].Why, "names no directory for its passwords") || + strings.Contains(st.Shares[0].Why, testPassword) || strings.Contains(st.Shares[0].Why, "relative") { + t.Errorf("%q: %+v", cfg, st.Shares) } - if strings.Contains(said, testPassword) || strings.Contains(said, "root") || strings.Contains(said, "abc-1") { - t.Errorf("%s: a refusal says what was typed: %q", what, said) - } - if strings.Contains(what, "does not read") { - if !strings.Contains(said, "it reads username, password and domain") || got["recalbox"].Password != "x" { - t.Errorf("%s: %q %+v", what, said, got) - } - continue - } - if strings.Contains(what, "twice") { - if got["recalbox"].Username != "root" || got["recalbox"].Password != "x" { - t.Errorf("%s: the first is not kept: %+v", what, got) - } - continue - } - if !strings.Contains(said, SMBOneLineForm) { - t.Errorf("%s: the refusal does not say the expected form: %q", what, said) - } - if _, kept := got["recalbox"]; kept { - t.Errorf("%s: a refused login is answered: %+v", what, got) + if _, err := os.Stat(filepath.Join(g.root, "/var/lib/mesh/mounts/smb-games.credentials")); !os.IsNotExist(err) { + t.Errorf("%q: a credentials file was written", cfg) } } } diff --git a/modules/mounts/go.mod b/modules/mounts/go.mod index 9549ae83..76732a34 100644 --- a/modules/mounts/go.mod +++ b/modules/mounts/go.mod @@ -2,4 +2,4 @@ module mounts go 1.22 -require git.novox.be/novox/mesh-sdk/go v0.1.7 +require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261010191001-33917f91ac3c diff --git a/modules/mounts/go.sum b/modules/mounts/go.sum index 017ed9ad..e655124d 100644 --- a/modules/mounts/go.sum +++ b/modules/mounts/go.sum @@ -2,3 +2,7 @@ git.novox.be/novox/mesh-sdk/go v0.1.6 h1:9qzdYONYbJdWcu6sxQcq9v1LI0JxcfkiKYkMUzJ git.novox.be/novox/mesh-sdk/go v0.1.6/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= git.novox.be/novox/mesh-sdk/go v0.1.7 h1:C0sTQmtTiyYH7bnqZb7PusXnqA37gKuT7Nqjn9gG47w= git.novox.be/novox/mesh-sdk/go v0.1.7/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261010190644-622b6b4f556d h1:BR8uwv4BqyLaBwITuLylrd9DRUnu0D5DjJw0RuQSXbk= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261010190644-622b6b4f556d/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261010191001-33917f91ac3c h1:l5onJwoIeH8yE/PjVlEeoPyXBsHp2NQiWLllz2fwX7s= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261010191001-33917f91ac3c/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= diff --git a/modules/mounts/module.json b/modules/mounts/module.json index c11f0283..108133c9 100644 --- a/modules/mounts/module.json +++ b/modules/mounts/module.json @@ -47,13 +47,12 @@ "sources": { "kind": "preference", "default": "none", - "why": "a machine has no occasional source until its assignment names one: space-separated name=smb:///@ or name=disk:@, each optionally :ro (hq ADR 0263 rule 6)" + "why": "a machine has no occasional source until its assignment names one: space-separated name=smb://[@]/@ or name=disk:@, each optionally :ro (hq ADR 0263 rule 6); an SMB source's username is in its entry, its password the secret smb-password- (hq ADR 0283)" } }, "own-secrets": { - "smb-credentials": { - "path": "${dir:state}/smb-credentials.secret", - "taken": "at-start", + "smb-password-*": { + "path": "${dir:state}/smb-password-*.secret", "issued-by": "outside" } }, @@ -85,7 +84,7 @@ "type": "file", "path": "/etc/mesh-mounts/mounts.conf", "mode": "0644", - "content": "# Written by the mesh (module mounts, novox/hq ADR 0263) from this machine's assignment: the settings\n# shares and sources, and the binding of nfs-share. Replaced on every push; change the settings, not this.\n# The module's process reads it as root every 30 seconds and writes one .mount and .automount per entry.\nshares=${setting:shares}\nsources=${setting:sources}\nserver=${bound:nfs-share:from}\nat=${bound:nfs-share:at}\naccount=${machine:account}\nnode=${machine:name}\nsecret=${dir:state}/smb-credentials.secret\nstate=${dir:state}\n" + "content": "# Written by the mesh (module mounts, novox/hq ADR 0263) from this machine's assignment: the settings\n# shares and sources, and the binding of nfs-share. Replaced on every push; change the settings, not this.\n# The module's process reads it as root every 30 seconds and writes one .mount and .automount per entry.\nshares=${setting:shares}\nsources=${setting:sources}\nserver=${bound:nfs-share:from}\nat=${bound:nfs-share:at}\naccount=${machine:account}\nnode=${machine:name}\npasswords=${dir:state}\nstate=${dir:state}\n" }, { "id": "bus-dir-parent",