Bound the certificate check to 3 s a handshake and 7 s, and answer waits while printer is not given (issue 1510)

modules/brother/README.md

@@ -66,7 +66,7 @@ makes it (mode 0755) when it files the first scan.
 | `scanner.scan` | `source` (`glass`, default, or `feeder`), `duplex` (feeder only) and `apart` (glass only): asks the device process to scan, crop (glass only), keep the PDF in the bucket under its sha256 and say `scan.added`; then fetches it in parts, checks the sha256 and files it in `scan-folder`, in the background, so a scan is filed whether or not the caller still waits. A glass scan is waited for up to 180 s. Several glass pages in one PDF (a page turned over, a stapled document): `document` `start` scans page 1 into a new document and answers its id (nothing is filed yet), `document` with that id scans the next page, and `document` with the id and `finish` true closes it into one multi-page PDF, filed once with one `scan.added` (nothing is scanned). The pages are kept in the bucket under `pending/<id>/` (not in memory, so a restart of the device process loses none); a document that gets no page for 30 minutes is closed and filed so by the tool process (a restart of the tool process forgets that timer; `finish` still works). The pages in the bucket are not deleted, the bucket having no delete. A feeder scan is waited for 25 s (the mesh console gives up after 30 s); if it is not done, the answer is `started: true` with its `id`, and `scanner.scan {id}` reads the outcome (`state` running, done with the event's body and `path`, or failed with `why`). The device process ends a feeder job at 20 s a page (at least 3 min, at most 30 min). A scan whose PDF the tool could not fetch (over 100 MiB) is refused before it is kept or announced. Answers the event's body and `path` |
 | `scanner.document` | one part of a kept scan (`object`, `part` from 1): `part`, `parts`, `size`, `sha256` (of the whole), `data` (base64). Join the parts and check the sha256 |
 | `scanner.status` | whether the scanner answers, its state (Idle or Processing) and its document feeder's state; a scanner that does not answer is `answers: false` with why. |
-| `brother_health` | the module's health, asked by the node-engine every 60 s (ADR 0240): the certificates the printer serves, one for each key type (ECDSA and RSA, told apart by the cipher suites offered under TLS 1.2), read with no login and compared with the pinned `printer-certificate` list. Unhealthy when one is not pinned, `why` naming the new fingerprint, so the pin can be changed before the admin pages are needed. A printer that does not answer is healthy, with `why` saying nothing was compared (it may be switched off). The process's liveness is judged by the node-engine without a declaration |
+| `brother_health` | the module's health, asked by the node-engine every 60 s (ADR 0240): the certificates the printer serves, one for each key type (ECDSA and RSA, told apart by the cipher suites offered under TLS 1.2), read with no login and compared with the pinned `printer-certificate` list. Unhealthy when one is not pinned, `why` naming the new fingerprint, so the pin can be changed before the admin pages are needed. With `printer` or `printer-certificate` not given it answers `state: waits` with `waits: [{config: ...}]` (ADR 0283), healthy and not an error. Each handshake is bounded by 3 s and the whole check by 7 s. A printer that does not answer is healthy, with `why` saying nothing was compared (it may be switched off). The process's liveness is judged by the node-engine without a declaration |
 | `scanner.list` | the newest filed scans, newest first (`limit`, default 20). Read-only |
 
 ## The printer seat

modules/brother/cmd/brother/certcheck.go

@@ -34,6 +34,13 @@ var keyKinds = []struct {
 	}},
 }
 
+// certDial bounds one handshake and certTotal the whole check (two handshakes), so the node-engine's look, which has
+// ten seconds, is always answered (issue 1510).
+var (
+	certDial  = 3 * time.Second
+	certTotal = 7 * time.Second
+)
+
 // Served is a certificate the printer served to a handshake of one key type.
 type Served struct {
 	Key         string `json:"key"`
@@ -45,9 +52,30 @@ type Served struct {
 // CertHealth is brother_health's answer: healthy, why when it is not (or what could not be compared), and what the
 // printer served.
 type CertHealth struct {
-	Healthy bool     `json:"healthy"`
-	Why     string   `json:"why"`
-	Served  []Served `json:"served"`
+	Healthy bool   `json:"healthy"`
+	State   string `json:"state"` // healthy, unhealthy or waits (ADR 0283)
+	Why     string `json:"why"`
+	// Waits names the config values the operator has yet to give, when State is waits.
+	Waits  []map[string]string `json:"waits,omitempty"`
+	Served []Served            `json:"served"`
+}
+
+// CertWaits is the answer while printer or printer-certificate is not given: the module waits for the operator, which
+// is not a fault (ADR 0283), and names the setting.
+func CertWaits(s Settings) (CertHealth, bool) {
+	var waits []map[string]string
+	var names []string
+	for _, c := range []struct{ name, value string }{{"printer", s.Printer}, {"printer-certificate", s.PrinterCertificate}} {
+		if c.value == "" {
+			waits = append(waits, map[string]string{"config": c.name})
+			names = append(names, c.name)
+		}
+	}
+	if len(waits) == 0 {
+		return CertHealth{}, false
+	}
+	return CertHealth{Healthy: true, State: "waits", Waits: waits, Served: []Served{},
+		Why: "waits for the operator to give the config value " + strings.Join(names, " and ")}, true
 }
 
 // serveAddr is the host with the HTTPS port when it names none.
@@ -60,7 +88,9 @@ func serveAddr(host string) string {
 
 // served is the leaf's fingerprint the printer gives a handshake that offers only these suites, or why it gave none.
 func served(ctx context.Context, addr string, suites []uint16) (string, error) {
-	d := tls.Dialer{NetDialer: &net.Dialer{Timeout: 5 * time.Second}, Config: &tls.Config{
+	ctx, cancel := context.WithTimeout(ctx, certDial)
+	defer cancel()
+	d := tls.Dialer{NetDialer: &net.Dialer{Timeout: certDial}, Config: &tls.Config{
 		MinVersion: tls.VersionTLS12, MaxVersion: tls.VersionTLS12, CipherSuites: suites,
 		// Nothing is sent over this connection, and the certificate is only read, to be compared with the pins.
 		InsecureSkipVerify: true,
@@ -79,9 +109,11 @@ func served(ctx context.Context, addr string, suites []uint16) (string, error) {
 
 // CheckCertificates compares what the printer serves with the pins. The login is never used.
 func CheckCertificates(ctx context.Context, host, pins string) CertHealth {
+	ctx, cancel := context.WithTimeout(ctx, certTotal)
+	defer cancel()
 	want := pinList(pins)
 	addr := serveAddr(host)
-	h := CertHealth{Healthy: true}
+	h := CertHealth{Healthy: true, State: "healthy"}
 	var changed, unreachable []string
 	for _, k := range keyKinds {
 		s := Served{Key: k.Name}
@@ -99,7 +131,7 @@ func CheckCertificates(ctx context.Context, host, pins string) CertHealth {
 	}
 	switch {
 	case len(changed) > 0:
-		h.Healthy = false
+		h.Healthy, h.State = false, "unhealthy"
 		h.Why = fmt.Sprintf("the printer serves a certificate that is not pinned: %s; the pinned printer-certificate is %s. If the printer was reset or given new certificates, set printer-certificate to the new fingerprint(s) (with the old ones kept if they still serve)",
 			strings.Join(changed, "; "), strings.Join(want, ", "))
 	case len(unreachable) == len(keyKinds):

modules/brother/cmd/brother/certcheck_test.go

@@ -103,7 +103,48 @@ func TestBrotherHealthIsAToolThatReadsTheSettings(t *testing.T) {
 	if h := got.(CertHealth); h.Healthy {
 		t.Errorf("a wrong pin is healthy: %+v", h)
 	}
-	if _, err := tool(t, newRig(t, `{}`, "pw").tl, "brother_health").Run(map[string]any{}); err == nil {
-		t.Error("an unset printer is healthy")
+}
+
+// 1510: with the printer not given the answer is waits, naming the setting (ADR 0283), not an error; and so is
+// the pin.
+func TestHealthWaitsForTheSettingsTheOperatorHasYetToGive(t *testing.T) {
+	got, err := tool(t, newRig(t, `{}`, "pw").tl, "brother_health").Run(map[string]any{})
+	h, ok := got.(CertHealth)
+	if err != nil || !ok || !h.Healthy || h.State != "waits" || len(h.Waits) != 2 || h.Waits[0]["config"] != "printer" || h.Waits[1]["config"] != "printer-certificate" {
+		t.Errorf("answer = %+v, %v", got, err)
+	}
+	got, _ = tool(t, newRig(t, `{"printer":"192.0.2.10"}`, "pw").tl, "brother_health").Run(map[string]any{})
+	if h := got.(CertHealth); h.State != "waits" || len(h.Waits) != 1 || h.Waits[0]["config"] != "printer-certificate" {
+		t.Errorf("no pin: %+v", h)
+	}
+}
+
+// 1510: each handshake is bounded by 3 s and the whole check by 7 s, and a printer that accepts and never answers
+// ends the check at the bound with the module healthy (nothing compared).
+func TestTheCheckIsBoundedAndAHungPrinterEndsItAtTheBound(t *testing.T) {
+	if certDial != 3*time.Second || certTotal != 7*time.Second {
+		t.Fatalf("bounds = %s and %s, want 3 s and 7 s", certDial, certTotal)
+	}
+	ln, _ := net.Listen("tcp", "127.0.0.1:0")
+	defer ln.Close()
+	go func() {
+		for {
+			c, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			defer c.Close() // held open, never answered
+		}
+	}()
+	oldD, oldT := certDial, certTotal
+	certDial, certTotal = 200*time.Millisecond, 500*time.Millisecond
+	t.Cleanup(func() { certDial, certTotal = oldD, oldT })
+	start := time.Now()
+	h := CheckCertificates(context.Background(), ln.Addr().String(), strings.Repeat("AB:", 31)+"AB")
+	if took := time.Since(start); took > 700*time.Millisecond {
+		t.Errorf("the check took %s", took)
+	}
+	if !h.Healthy || !strings.Contains(h.Why, "none was compared") {
+		t.Errorf("%+v", h)
 	}
 }

modules/brother/cmd/brother/main.go

@@ -339,10 +339,10 @@ func tools(settingsFile string, now func() time.Time) []stdio.Tool {
 						}
 					}()
 				}
-				if err := s.Settings.MissingPrinter(); err != nil {
-					return nil, err
+				if w, waits := CertWaits(s.Settings); waits {
+					return w, nil
 				}
-				ctx, cancel := context.WithTimeout(context.Background(), 9*time.Second)
+				ctx, cancel := context.WithTimeout(context.Background(), certTotal+time.Second)
 				defer cancel()
 				return CheckCertificates(ctx, s.Settings.Printer, s.Settings.PrinterCertificate), nil
 			},