From a058cad7357332cad55c9fd5232405727d6a29ce Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 15:50:56 +0200 Subject: [PATCH] mounts: the SMB login is typed as one line at the desk's prompt (issue 379) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The secret smb-credentials was an INI file alone, which the desk's one-line prompt (secret ask, hq ADR 0277) cannot type: on the desktop the Pi's source said "line 1 is not key=value inside a [section]". A login is now also one line, `[recalbox] username=root password=…`, password last and to the end of the line so it may hold spaces and `=`; the file form still reads, and a file may mix both. A line the module cannot read is refused with the form it expected, never with what was typed, and a login without a username is not answered. The source's "login is not given" says the desk verb and the line to type. --- modules/mounts/README.md | 24 +++- modules/mounts/cmd/mounts/apply.go | 3 +- modules/mounts/cmd/mounts/config.go | 145 ++++++++++++++++++++---- modules/mounts/cmd/mounts/write_test.go | 85 ++++++++++++++ 4 files changed, 229 insertions(+), 28 deletions(-) diff --git a/modules/mounts/README.md b/modules/mounts/README.md index b9c3cc9f..20aff3ee 100644 --- a/modules/mounts/README.md +++ b/modules/mounts/README.md @@ -73,8 +73,22 @@ A mount point is an absolute, plainly written path, not a directory of the syste ## The secret `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. The operator gives it with -`secret accept mounts smb-credentials --from `; the file holds one section per SMB source: +never makes one in its place. It holds one login per SMB source, named by the source's name in brackets. + +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: + +``` +[games] 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 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: ``` [games] @@ -83,8 +97,10 @@ password=… domain=… (optional) ``` -A source whose section 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. +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. ## The processes and the bus diff --git a/modules/mounts/cmd/mounts/apply.go b/modules/mounts/cmd/mounts/apply.go index 3376f26a..54e8016d 100644 --- a/modules/mounts/cmd/mounts/apply.go +++ b/modules/mounts/cmd/mounts/apply.go @@ -303,7 +303,8 @@ func (a *Applier) Look(ctx context.Context) Status { c, ok := creds[s.Name] if !ok || 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: put a [%s] section in the module's secret with `secret accept mounts smb-credentials --from `", s.Name)} + 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)} continue } file := filepath.Join(cfg.State, "smb-"+s.Name+".credentials") diff --git a/modules/mounts/cmd/mounts/config.go b/modules/mounts/cmd/mounts/config.go index b0a9deb9..bb41c280 100644 --- a/modules/mounts/cmd/mounts/config.go +++ b/modules/mounts/cmd/mounts/config.go @@ -270,54 +270,98 @@ type SMBCredential struct { Domain string } -// ParseSMBCredentials reads the secret `smb-credentials`: one section per source, +// 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=…" + +// 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) // -// Its values are never answered by any tool; a problem names the section and the key, never the value. +// 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, "[") && strings.HasSuffix(line, "]") { - section = strings.TrimSpace(line[1 : len(line)-1]) - if _, dup := out[section]; dup { - problems = append(problems, fmt.Sprintf("the section [%s] is given twice; the first is kept", section)) - section = "" + 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 } - out[section] = SMBCredential{} + 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 } - k, v, ok := strings.Cut(line, "=") - if !ok || section == "" { - problems = append(problems, fmt.Sprintf("line %d is not key=value inside a [section]", i+1)) + // `[` 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 } - c := out[section] - switch strings.ToLower(strings.TrimSpace(k)) { - case "username", "user": - c.Username = strings.TrimSpace(v) - case "password", "pass": - c.Password = strings.TrimRight(v, "\r") - case "domain": - c.Domain = strings.TrimSpace(v) - default: - problems = append(problems, fmt.Sprintf("[%s] has a key %q this module does not read", section, strings.TrimSpace(k))) + if _, dup := out[name]; dup { + problems = append(problems, fmt.Sprintf("the section [%s] is given twice; the first is kept", name)) + continue } - out[section] = c + 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", name)) + 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)) @@ -327,6 +371,61 @@ func ParseSMBCredentials(text string) (map[string]SMBCredential, []string) { return out, problems } +// 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 + } + return true +} + +// 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):] + } +} + // CredentialsFile is a source's credentials in mount.cifs's format. func (c SMBCredential) CredentialsFile() string { s := "username=" + c.Username + "\npassword=" + c.Password + "\n" diff --git a/modules/mounts/cmd/mounts/write_test.go b/modules/mounts/cmd/mounts/write_test.go index 1446ff6f..9ae35456 100644 --- a/modules/mounts/cmd/mounts/write_test.go +++ b/modules/mounts/cmd/mounts/write_test.go @@ -587,3 +587,88 @@ func (f *fakeSystem) setUnit(unit string, state map[string]string) { defer f.mu.Unlock() 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) + } + // 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) + } +} + +// 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", + } + 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 + } + 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) + } + } +}