diff --git a/modules/brother/cmd/brother/admin_test.go b/modules/brother/cmd/brother/admin_test.go index f346d8cd..c78a61e1 100644 --- a/modules/brother/cmd/brother/admin_test.go +++ b/modules/brother/cmd/brother/admin_test.go @@ -3,6 +3,7 @@ package main import ( "context" "crypto/tls" + "net" "net/http" "net/http/httptest" "net/url" @@ -22,6 +23,7 @@ type fakePrinter struct { field string // the password field's name the login form gives password string mu sync.Mutex + conns int // connections opened to it requests []string // method and path of every request posted []url.Values // every form body posted session string // the cookie value of the one logged-in session @@ -99,7 +101,15 @@ func newFakePrinter(t *testing.T, field, password string) *fakePrinter { } w.Write([]byte(body)) }) - f.Server = httptest.NewTLSServer(mux) + f.Server = httptest.NewUnstartedServer(mux) + f.Config.ConnState = func(_ net.Conn, st http.ConnState) { + if st == http.StateNew { + f.mu.Lock() + f.conns++ + f.mu.Unlock() + } + } + f.StartTLS() t.Cleanup(f.Close) return f } diff --git a/modules/brother/cmd/brother/main.go b/modules/brother/cmd/brother/main.go index 11866ab7..2f9ca11d 100644 --- a/modules/brother/cmd/brother/main.go +++ b/modules/brother/cmd/brother/main.go @@ -29,9 +29,41 @@ func tools(settingsFile string, say Emitter, now func() time.Time) []stdio.Tool if err != nil { return nil, err } - return &Service{Settings: s, Node: os.Getenv("MESH_NODE"), Now: now, Emit: say}, nil + return &Service{Settings: s, Node: os.Getenv("MESH_NODE"), Now: now, Emit: say, + PasswordFile: os.Getenv("BROTHER_ADMIN_PASSWORD")}, nil } return []stdio.Tool{ + { + Name: "printer.settings", + Description: "Read one page of the printer's admin site, read-only: log in with the secret admin-password (the " + + "operator's; the module trusts the printer's pinned certificate alone), get the page, log out. Answers the " + + "page's title, its links (href and text) and its forms with their fields (name, type, value, label); a " + + "password field's value is never given. It sends nothing but the login and the logout, and writes no " + + "setting of the printer.", + Input: map[string]any{ + "type": "object", + "properties": map[string]any{ + "page": map[string]any{"type": "string", "description": "a page of the admin site: a path starting with / and ending in .html (default /general/status.html)"}, + }, + }, + Run: func(args map[string]any) (any, error) { + page := "" + if v, ok := args["page"]; ok { + str, isStr := v.(string) + if !isStr { + return nil, fmt.Errorf("page is a string") + } + page = str + } + s, err := service() + if err != nil { + return nil, err + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + return s.PrinterSettings(ctx, page) + }, + }, { Name: "scanner.scan", Description: "Scan the glass of the configured network scanner, cut the document out of the platen's white, " + diff --git a/modules/brother/cmd/brother/manifest_test.go b/modules/brother/cmd/brother/manifest_test.go index 557ee790..06d3f6c4 100644 --- a/modules/brother/cmd/brother/manifest_test.go +++ b/modules/brother/cmd/brother/manifest_test.go @@ -136,21 +136,26 @@ func TestTheConfigReachesTheCodeThroughItsSettingsFile(t *testing.T) { // 1453/AC6: the manifest declares the scanner seat, held once for the mesh, and claims it, with the verbs // ADR 0467 clause 2 names; the module serves each verb as the tool .. -func TestTheManifestDeclaresAndClaimsTheScannerSeat(t *testing.T) { +func TestTheManifestDeclaresAndClaimsTheScannerAndPrinterSeats(t *testing.T) { m := readManifest(t) - want := seat{Name: "scanner", Scope: "mesh", Serves: []string{"scan", "status", "list"}} - for what, got := range map[string][]seat{"seats": m.Seats, "claims": m.Claims} { - found := false - for _, s := range got { - if s.Name == "scanner" { - found = true - if !reflect.DeepEqual(s, want) { - t.Errorf("%s: scanner = %+v, want %+v", what, s, want) + for _, want := range []seat{ + {Name: "scanner", Scope: "mesh", Serves: []string{"scan", "status", "list"}}, + // restart is round 2 of issue 1453, and is not declared yet. + {Name: "printer", Scope: "mesh", Serves: []string{"settings"}}, + } { + for what, got := range map[string][]seat{"seats": m.Seats, "claims": m.Claims} { + found := false + for _, s := range got { + if s.Name == want.Name { + found = true + if !reflect.DeepEqual(s, want) { + t.Errorf("%s: %s = %+v, want %+v", what, want.Name, s, want) + } } } - } - if !found { - t.Errorf("%s does not hold the scanner seat", what) + if !found { + t.Errorf("%s does not hold the %s seat", what, want.Name) + } } } } diff --git a/modules/brother/cmd/brother/printer.go b/modules/brother/cmd/brother/printer.go new file mode 100644 index 00000000..b3330e58 --- /dev/null +++ b/modules/brother/cmd/brother/printer.go @@ -0,0 +1,82 @@ +package main + +import ( + "context" + "fmt" + "regexp" + "strings" + "time" +) + +// pagePattern is a page of the admin site a caller may ask for: an absolute path under the site ending in +// .html, with no query, no dot-dot and no second slash at its start (which would name another host). +var pagePattern = regexp.MustCompile(`^/[A-Za-z0-9_./-]*\.html$`) + +// DefaultPage is the page printer.settings reads when none is asked for. +const DefaultPage = "/general/status.html" + +// admin is a session on the printer's admin page, once the settings and the secret say it may be opened. The +// printer is contacted by nothing before this has answered without an error (1453/AC4). +func (s *Service) admin() (*Admin, error) { + if err := s.Settings.MissingPrinter(); err != nil { + return nil, err + } + pw, err := AdminSecret(s.PasswordFile) + if err != nil { + return nil, err + } + return NewAdmin(s.Settings.Printer, s.Settings.PrinterCertificate, pw) +} + +// PrinterSettings logs in, reads one page of the admin site and logs out. It is read-only: the login (and the +// logout) are the only things it sends, and a password field's value is never in the answer (ADR 0467 clause 5). +func (s *Service) PrinterSettings(ctx context.Context, page string) (map[string]any, error) { + if page == "" { + page = DefaultPage + } + if !pagePattern.MatchString(page) || strings.Contains(page, "..") || strings.Contains(page, "//") { + return nil, fmt.Errorf("page %q is not a page of the admin site: it starts with / and ends in .html", page) + } + a, err := s.admin() + if err != nil { + return nil, err + } + if err := a.Login(ctx); err != nil { + return nil, err + } + body, at, err := a.Get(ctx, page) + if err != nil { + // Log out with a context of its own: the caller's may be what ended this. + lctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + if home, hat, herr := a.Get(lctx, DefaultPage); herr == nil { + a.Logout(lctx, home, hat) + } + return nil, fmt.Errorf("page %s cannot be read: %w", page, err) + } + lctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + logout := a.Logout(lctx, body, at) + p := ParsePage(body) + return map[string]any{ + "page": page, + "title": p.Title, + "links": nonNil(p.Links), + "forms": nonNilForms(p.Forms), + "logout": logout, + }, nil +} + +func nonNil(l []Link) []Link { + if l == nil { + return []Link{} + } + return l +} + +func nonNilForms(f []Form) []Form { + if f == nil { + return []Form{} + } + return f +} diff --git a/modules/brother/cmd/brother/printer_test.go b/modules/brother/cmd/brother/printer_test.go new file mode 100644 index 00000000..35e3ff9c --- /dev/null +++ b/modules/brother/cmd/brother/printer_test.go @@ -0,0 +1,160 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// printerTools is the tool set over a settings file for the fake printer, with the secret in a file the way the +// mesh writes it (a trailing newline included). +func printerTools(t *testing.T, f *fakePrinter, password string) (settings string, run func(args map[string]any) (any, error)) { + t.Helper() + secret := filepath.Join(t.TempDir(), "admin-password.secret") + if password != "" { + if err := os.WriteFile(secret, []byte(password+"\n"), 0o600); err != nil { + t.Fatal(err) + } + } + t.Setenv("BROTHER_ADMIN_PASSWORD", secret) + js := `{"printer":"` + f.host() + `","printer-certificate":"` + f.print() + `"}` + tl := tools(settingsFile(t, js), nil, time.Now) + return js, tool(t, tl, "printer.settings").Run +} + +// 1453/AC3: printer.settings logs in by the field name the form gives, sends the password from the secret +// file, follows the session cookie, and returns the page's title, links and form fields; a password field's +// value is never returned, and the session is logged out. The page is SYNTHETIC (testdata/admin/ +// synthetic-settings.html): the real pages arrive in round 2. +func TestPrinterSettingsReadsAPageBehindTheLogin(t *testing.T) { + f := newFakePrinter(t, "Qw9z3", "p4ss word") + _, run := printerTools(t, f, "p4ss word") + got, err := run(map[string]any{"page": "/general/other.html"}) + if err != nil { + t.Fatal(err) + } + if len(f.posted) != 1 || f.posted[0].Get("Qw9z3") != "p4ss word" { + t.Errorf("the login posted %v", f.posted) + } + raw, _ := json.Marshal(got) + out := string(raw) + var a struct { + Title string + Links []Link + Forms []Form + Logout string + } + json.Unmarshal(raw, &a) + if a.Title != "Synthetic settings" { + t.Errorf("title = %q", a.Title) + } + if len(a.Links) != 2 || a.Links[1] != (Link{Href: "/general/status.html", Text: "Status"}) { + t.Errorf("links = %v", a.Links) + } + if len(a.Forms) != 1 || a.Forms[0].Action != "/general/other.html" || a.Forms[0].Method != "post" { + t.Fatalf("forms = %+v", a.Forms) + } + want := map[string]Field{ + "lock": {Name: "lock", Type: "checkbox", Value: "on", Label: "Scan lock"}, + "node": {Name: "node", Type: "text", Value: "BRN-SYNTH", Label: "Name"}, + "pw": {Name: "pw", Type: "password", Value: "", Label: "Secret"}, + "note": {Name: "note", Type: "textarea", Value: "hello & bye"}, + "token": {Name: "token", Type: "hidden", Value: "t1"}, + } + for _, fl := range a.Forms[0].Fields { + if fl.Name == "mode" { + if fl.Type != "select" || fl.Value != "m" || fl.Label != "Mode" || strings.Join(fl.Options, ",") != "a,m" { + t.Errorf("mode = %+v", fl) + } + continue + } + w, ok := want[fl.Name] + if !ok || fl.Name != w.Name || fl.Type != w.Type || fl.Value != w.Value || fl.Label != w.Label { + t.Errorf("field %+v, want %+v", fl, w) + } + delete(want, fl.Name) + } + if len(want) != 0 { + t.Errorf("fields missing: %v", want) + } + if strings.Contains(out, "shouldnotshow") { + t.Errorf("a password field's value was returned: %s", out) + } + if a.Logout != "logged out" || f.loggedInNow() != 0 { + t.Errorf("logout = %q, sessions open %d", a.Logout, f.loggedInNow()) + } + // Read-only: the only form posted is the login. + if len(f.posted) != 1 { + t.Errorf("forms posted: %v", f.posted) + } +} + +// 1453/AC3: with no page asked for, the status page is read. +func TestPrinterSettingsDefaultsToTheStatusPage(t *testing.T) { + f := newFakePrinter(t, "B1891", "pw") + _, run := printerTools(t, f, "pw") + got, err := run(nil) + if err != nil { + t.Fatal(err) + } + if got.(map[string]any)["title"] != "Synthetic status" { + t.Errorf("answer = %v", got) + } +} + +// 1453/AC4: a wrong password is said as a failed login. +func TestPrinterSettingsSaysAFailedLogin(t *testing.T) { + f := newFakePrinter(t, "B1891", "right") + _, run := printerTools(t, f, "wrong") + if _, err := run(nil); err == nil || !strings.Contains(err.Error(), "login failed") { + t.Fatalf("error = %v", err) + } +} + +// 1453/AC4: a value or the secret not set is said by name, and the printer is not contacted at all. +func TestPrinterSettingsSaysWhatIsUnsetAndContactsNothing(t *testing.T) { + f := newFakePrinter(t, "B1891", "pw") + secretOK := filepath.Join(t.TempDir(), "s") + os.WriteFile(secretOK, []byte("pw\n"), 0o600) + for _, c := range []struct{ name, settings, secret, want string }{ + {"no printer", `{"printer-certificate":"` + f.print() + `"}`, secretOK, "printer is not set"}, + {"no certificate", `{"printer":"` + f.host() + `"}`, secretOK, "printer-certificate is not set"}, + {"neither", `{}`, secretOK, "printer and printer-certificate"}, + {"no secret file", `{"printer":"` + f.host() + `","printer-certificate":"` + f.print() + `"}`, filepath.Join(t.TempDir(), "none"), "admin-password is not given"}, + {"empty secret", `{"printer":"` + f.host() + `","printer-certificate":"` + f.print() + `"}`, func() string { + p := filepath.Join(t.TempDir(), "e") + os.WriteFile(p, []byte("\n"), 0o600) + return p + }(), "admin-password is not given"}, + } { + t.Setenv("BROTHER_ADMIN_PASSWORD", c.secret) + tl := tools(settingsFile(t, c.settings), nil, time.Now) + _, err := tool(t, tl, "printer.settings").Run(nil) + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: error = %v, want it to say %q", c.name, err, c.want) + } + } + if f.conns != 0 || len(f.requests) != 0 { + t.Errorf("the printer was contacted: %d connections, %v", f.conns, f.requests) + } +} + +// 1453: a page that is not a page of the admin site is refused before anything is sent. +func TestPrinterSettingsRefusesAPageOutsideTheSite(t *testing.T) { + f := newFakePrinter(t, "B1891", "pw") + _, run := printerTools(t, f, "pw") + for _, p := range []string{"general/status.html", "/general/status", "//evil.example/x.html", "/a/../b.html", "/a.html?x=1", "/a b.html"} { + if _, err := run(map[string]any{"page": p}); err == nil || !strings.Contains(err.Error(), "not a page") { + t.Errorf("page %q: error = %v", p, err) + } + } + if _, err := run(map[string]any{"page": 5}); err == nil { + t.Errorf("a page that is no string was accepted") + } + if f.conns != 0 { + t.Errorf("the printer was contacted %d times", f.conns) + } +} diff --git a/modules/brother/cmd/brother/scan.go b/modules/brother/cmd/brother/scan.go index d625fa03..f29d26ed 100644 --- a/modules/brother/cmd/brother/scan.go +++ b/modules/brother/cmd/brother/scan.go @@ -23,6 +23,8 @@ type Service struct { Node string Now func() time.Time Emit Emitter + // PasswordFile is the file the module's own secret admin-password is written to. + PasswordFile string } func mm(pixels, dpi int) float64 { diff --git a/modules/brother/module.json b/modules/brother/module.json index 0f8e3a5d..91f4eb00 100644 --- a/modules/brother/module.json +++ b/modules/brother/module.json @@ -77,6 +77,13 @@ "status", "list" ] + }, + { + "name": "printer", + "scope": "mesh", + "serves": [ + "settings" + ] } ], "claims": [ @@ -88,9 +95,17 @@ "status", "list" ] + }, + { + "name": "printer", + "scope": "mesh", + "serves": [ + "settings" + ] } ], "tools": [ + "printer.settings", "scanner.scan", "scanner.status", "scanner.list"