diff --git a/cmd/rollcall/firmware.go b/cmd/rollcall/firmware.go index 81c9db0..ee25ad9 100644 --- a/cmd/rollcall/firmware.go +++ b/cmd/rollcall/firmware.go @@ -469,6 +469,23 @@ func (a *App) runFirmware(jobID, machineID string, resume bool) { _ = json.Unmarshal(j.Spec, &sp) var pr fwProgress _ = json.Unmarshal(j.Progress, &pr) + // every call reads the record again: the BMC certificate pin may change on the way + via := func(op, arg string) (string, error) { + if mm, err := a.store.Get(ctx, machineID); err == nil { + m = mm + } + out, err := a.firmwareVia(ctx, m, op, arg) + if err != nil && sp.Component == "bmc" && strings.Contains(err.Error(), "certificate changed") { + // the BMC we are updating came back with a new certificate: expected, pin it anew + _ = a.setBMCExtra(ctx, machineID, "tls_fingerprint", "") + _ = a.store.AddEvent(ctx, machineID, "bmc certificate", "changed during the BMC update; pinned again") + if mm, gerr := a.store.Get(ctx, machineID); gerr == nil { + m = mm + } + out, err = a.firmwareVia(ctx, m, op, arg) + } + return out, err + } finish := func(state, msg string) { _ = a.store.FinishJob(ctx, j.ID, state, map[bool]string{true: "", false: msg}[state == "done"]) ev := "firmware " + state @@ -486,7 +503,7 @@ func (a *App) runFirmware(jobID, machineID string, resume bool) { return } arg, _ := json.Marshal(fwStartArg{URL: a.firmwareURL(m, p), File: p.File, Component: p.Component, Method: sp.Method, Apply: sp.Apply}) - out, err := a.firmwareVia(ctx, m, "fw-start", string(arg)) + out, err := via("fw-start", string(arg)) if err != nil { finish("failed", err.Error()) return @@ -500,7 +517,7 @@ func (a *App) runFirmware(jobID, machineID string, resume bool) { if pr.Task != "" && pr.Phase == "flashing" { var lastOK time.Time = time.Now() for { - out, err := a.firmwareVia(ctx, m, "fw-poll", pr.Task) + out, err := via("fw-poll", pr.Task) if err != nil { if time.Since(lastOK) > 25*time.Minute { finish("failed", "the BMC has not answered for 25 minutes: "+err.Error()) @@ -517,7 +534,7 @@ func (a *App) runFirmware(jobID, machineID string, resume bool) { pr.Pct, pr.Msg = pl.Pct, pl.Msg a.setFWProgress(ctx, j, pr) if pl.State == "failed" { - _, _ = a.firmwareVia(ctx, m, "fw-finish", "") + _, _ = via("fw-finish", "") finish("failed", orDefault(pl.Msg, "the BMC reported the update failed")) return } @@ -531,7 +548,7 @@ func (a *App) runFirmware(jobID, machineID string, resume bool) { time.Sleep(15 * time.Second) } } - _, _ = a.firmwareVia(ctx, m, "fw-finish", "") // e.g. Lenovo's busy flag + _, _ = via("fw-finish", "") // e.g. Lenovo's busy flag // a component that applies at the next reset (BIOS, most NICs/RAID) waits for it if sp.Reboot && pr.Phase != "verifying" { pr.Phase, pr.Msg = "rebooting", "power cycle so the update applies" @@ -545,7 +562,15 @@ func (a *App) runFirmware(jobID, machineID string, resume bool) { a.setFWProgress(ctx, j, pr) deadline := time.Now().Add(20 * time.Minute) // servers take minutes to POST, BMCs to come back for { + if mm, gerr := a.store.Get(ctx, machineID); gerr == nil { + m = mm + } inv, err := a.readFirmware(ctx, m) + if err != nil && sp.Component == "bmc" && strings.Contains(err.Error(), "certificate changed") { + _ = a.setBMCExtra(ctx, machineID, "tls_fingerprint", "") + time.Sleep(5 * time.Second) + continue + } if err == nil { for _, c := range inv { if c.Component == sp.Component && fwSameVersion(c.Version, sp.Version) { diff --git a/cmd/rollcall/ipmi.go b/cmd/rollcall/ipmi.go index 0f40c7d..2a25010 100644 --- a/cmd/rollcall/ipmi.go +++ b/cmd/rollcall/ipmi.go @@ -69,9 +69,66 @@ func (a *App) power(ctx context.Context, m *Machine, op, arg string) (string, er if err != nil { return "", fmt.Errorf("%w: %v", errNoBMC, err) } + if c, err = a.pinBMC(ctx, m, c); err != nil { + return "", err + } return a.powerVia(ctx, a.rackOf(m), c, op, arg) } +// pinBMC records the certificate of an HTTPS BMC (Redfish, Proxmox) before +// any credentials go to it: trust on first use. From then on every connection +// must show the same certificate (bmcHTTPClient). An address change or a BMC +// firmware update through Rollcall clears the pin; so does tls_fingerprint=. +func (a *App) pinBMC(ctx context.Context, m *Machine, c *bmcCreds) (*bmcCreds, error) { + d, _ := driverByName(c.Driver) + if (d.Family != "redfish" && d.Family != "api") || c.Extra["tls_fingerprint"] != "" || c.Extra["verify_tls"] == "true" { + return c, nil + } + fp, err := a.powerVia(ctx, a.rackOf(m), c, "tls-fingerprint", "") + if err != nil { + if strings.Contains(err.Error(), "unknown power operation") { // a rack controller not updated yet + return c, nil + } + return nil, fmt.Errorf("reading the BMC's certificate: %w", err) + } + fp = strings.TrimSpace(fp) + if err := a.setBMCExtra(ctx, m.ID, "tls_fingerprint", fp); err != nil { + return nil, err + } + _ = a.store.AddEvent(ctx, m.ID, "bmc certificate pinned", fp) + c2 := *c + c2.Extra = map[string]string{} + for k, v := range c.Extra { + c2.Extra[k] = v + } + c2.Extra["tls_fingerprint"] = fp + return &c2, nil +} + +// setBMCExtra sets (or with "" removes) one setting on a machine's BMC record. +func (a *App) setBMCExtra(ctx context.Context, id, k, v string) error { + m, err := a.store.Get(ctx, id) + if err != nil || m.BMC == nil { + return err + } + var b obj + if err := json.Unmarshal(m.BMC, &b); err != nil { + return err + } + ex, _ := b["extra"].(map[string]any) + if ex == nil { + ex = map[string]any{} + } + if v == "" { + delete(ex, k) + } else { + ex[k] = v + } + b["extra"] = ex + nb, _ := json.Marshal(b) + return a.store.SetBMC(ctx, id, nb, nil) +} + func (a *App) powerVia(ctx context.Context, rack string, c *bmcCreds, op, arg string) (string, error) { if rack == a.cfg.RackName || rack == "" { return powerRun(ctx, a.cfg.IPMITool, c, op, arg) @@ -90,6 +147,9 @@ func (a *App) firmwareVia(ctx context.Context, m *Machine, op, arg string) (stri if err != nil { return "", fmt.Errorf("%w: %v", errNoBMC, err) } + if c, err = a.pinBMC(ctx, m, c); err != nil { + return "", err + } rack := a.rackOf(m) if rack == a.cfg.RackName || rack == "" { return powerRun(ctx, a.cfg.IPMITool, c, op, arg) @@ -152,6 +212,12 @@ func (a *App) probeBMC(m *Machine) { } probe := *c probe.Driver, probe.Port = "redfish", 0 + pinned, err := a.pinBMC(context.Background(), m, &probe) + if err != nil { + _ = a.store.AddEvent(context.Background(), m.ID, "bmc probe", "staying on ipmi: "+oneLine(err.Error())) + return + } + probe = *pinned ctx, cancel := context.WithTimeout(context.Background(), 90e9) defer cancel() out, err := a.powerVia(ctx, a.rackOf(m), &probe, "probe", "") @@ -235,7 +301,12 @@ func (a *App) handleBMC(w http.ResponseWriter, r *http.Request) { b["driver"] = *req.Driver } if req.Address != nil { - b["address"] = strings.TrimSpace(*req.Address) + if a := strings.TrimSpace(*req.Address); a != str(b["address"]) { + if ex, _ := b["extra"].(map[string]any); ex != nil { + delete(ex, "tls_fingerprint") // another BMC: its certificate is pinned anew + } + b["address"] = a + } } if req.Port != nil { b["port"] = *req.Port diff --git a/cmd/rollcall/power.go b/cmd/rollcall/power.go index e911ef9..e5427b2 100644 --- a/cmd/rollcall/power.go +++ b/cmd/rollcall/power.go @@ -12,7 +12,11 @@ package main import ( "bytes" "context" + "crypto/sha256" + "crypto/subtle" "crypto/tls" + "crypto/x509" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -57,6 +61,11 @@ var errManualPower = errors.New("this machine has manual power control: do it by // powerRun executes one operation with a driver. func powerRun(ctx context.Context, ipmitool string, c *bmcCreds, op, arg string) (string, error) { + if op == "tls-fingerprint" { + ctx, cancel := context.WithTimeout(ctx, 20*time.Second) + defer cancel() + return bmcTLSFingerprint(ctx, c) + } if t, ok := fwTimeouts[op]; ok { // firmware: Redfish UpdateService (redfish_fw.go) ctx, cancel := context.WithTimeout(ctx, t) defer cancel() @@ -136,13 +145,69 @@ func ipmiExec(ctx context.Context, tool string, c *bmcCreds, args ...string) (st // ---------------------------------------------------------------- BMC HTTP (Redfish, Proxmox) +// bmcHTTPClient: BMCs ship self-signed certificates, so a BMC is trusted by +// the fingerprint of the certificate it showed first (extra tls_fingerprint, +// pinned by the region before credentials are ever sent; see pinBMC). +// verify_tls=true checks it against the system's CAs instead. func bmcHTTPClient(c *bmcCreds) *http.Client { - // BMCs ship self-signed certificates; checking them is opt-in (extra verify_tls=true) - tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: c.Extra["verify_tls"] != "true"}, //nolint:gosec - DialContext: (&net.Dialer{Timeout: 10 * time.Second}).DialContext} + tc := &tls.Config{InsecureSkipVerify: c.Extra["verify_tls"] != "true"} //nolint:gosec // pinned below + if pin := strings.ToLower(c.Extra["tls_fingerprint"]); pin != "" && c.Extra["verify_tls"] != "true" { + tc.VerifyConnection = func(cs tls.ConnectionState) error { + if len(cs.PeerCertificates) == 0 { + return errors.New("the BMC sent no certificate") + } + got := certSHA256(cs.PeerCertificates[0]) + if subtle.ConstantTimeCompare([]byte(got), []byte(pin)) != 1 { + return fmt.Errorf("the BMC's certificate changed (pinned %s, now %s): not sending credentials; if the BMC was replaced or got new firmware, clear tls_fingerprint to trust the new one", short(pin), short(got)) + } + return nil + } + } + tr := &http.Transport{TLSClientConfig: tc, DialContext: (&net.Dialer{Timeout: 10 * time.Second}).DialContext} return &http.Client{Timeout: 45 * time.Second, Transport: tr} } +func certSHA256(c *x509.Certificate) string { + s := sha256.Sum256(c.Raw) + return hex.EncodeToString(s[:]) +} + +func short(fp string) string { + if len(fp) > 16 { + return fp[:16] + "…" + } + return fp +} + +// bmcTLSFingerprint connects to the BMC's HTTPS port and returns its +// certificate's SHA-256 (op tls-fingerprint): a handshake only, no request, +// no credentials. +func bmcTLSFingerprint(ctx context.Context, c *bmcCreds) (string, error) { + port := 443 + if d, _ := driverByName(c.Driver); d.Family == "api" { + port = 8006 + } + u, err := url.Parse(bmcBase(c, "https", port)) + if err != nil { + return "", err + } + host := u.Host + if u.Port() == "" { + host = net.JoinHostPort(u.Hostname(), "443") + } + d := &tls.Dialer{NetDialer: &net.Dialer{Timeout: 10 * time.Second}, Config: &tls.Config{InsecureSkipVerify: true}} //nolint:gosec // reading the certificate is the point + conn, err := d.DialContext(ctx, "tcp", host) + if err != nil { + return "", err + } + defer conn.Close() + cs := conn.(*tls.Conn).ConnectionState() + if len(cs.PeerCertificates) == 0 { + return "", errors.New("the BMC sent no certificate") + } + return certSHA256(cs.PeerCertificates[0]), nil +} + // bmcBase builds https://host[:port] from the record; IPMI's port is not Redfish's. func bmcBase(c *bmcCreds, scheme string, defPort int) string { a := strings.TrimRight(c.Address, "/") diff --git a/cmd/rollcall/security_test.go b/cmd/rollcall/security_test.go index 6c12c5e..40e09e4 100644 --- a/cmd/rollcall/security_test.go +++ b/cmd/rollcall/security_test.go @@ -1,12 +1,14 @@ package main import ( + "context" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "crypto/tls" "crypto/x509" "encoding/json" + "net/http" "net/http/httptest" "os" "path/filepath" @@ -142,3 +144,33 @@ func TestClientIP(t *testing.T) { t.Errorf("through a rack: %s", ip) } } + +// An HTTPS BMC is trusted by the certificate it showed first; another one +// gets no credentials. +func TestBMCCertificatePin(t *testing.T) { + seen := 0 + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if _, _, ok := r.BasicAuth(); ok { + seen++ + } + _, _ = w.Write([]byte(`{"PowerState":"On","Members":[{"@odata.id":"/redfish/v1/Systems/1"}]}`)) + })) + defer srv.Close() + c := creds(srv.URL, "redfish") + fp, err := powerRun(context.Background(), "", c, "tls-fingerprint", "") + if err != nil || fp != certSHA256(srv.Certificate()) { + t.Fatalf("fingerprint %q %v", fp, err) + } + if seen != 0 { + t.Error("credentials were sent while reading the certificate") + } + c.Extra["tls_fingerprint"] = fp + if _, err := powerRun(context.Background(), "", c, "status", ""); err != nil { + t.Fatalf("pinned and matching: %v", err) + } + c.Extra["tls_fingerprint"] = strings.Repeat("ab", 32) + seen = 0 + if _, err := powerRun(context.Background(), "", c, "status", ""); err == nil || !strings.Contains(err.Error(), "certificate changed") || seen != 0 { + t.Errorf("another certificate: %v (credentials sent %d times)", err, seen) + } +} diff --git a/lab/pve-vbmc/pve_redfish.py b/lab/pve-vbmc/pve_redfish.py index f8ba453..7efcc9f 100644 --- a/lab/pve-vbmc/pve_redfish.py +++ b/lab/pve-vbmc/pve_redfish.py @@ -316,9 +316,15 @@ def make_handler(pve, prof, user, password, fw): return H -def self_signed(addr): - d = tempfile.mkdtemp() - crt, key = os.path.join(d, "c.pem"), os.path.join(d, "k.pem") +def self_signed(addr, vmid): + """A self-signed certificate, kept across restarts like a real BMC's (rm the files for a new one).""" + d = "/var/lib/pve-vbmc" + crt, key = os.path.join(d, f"tls-{vmid}.crt"), os.path.join(d, f"tls-{vmid}.key") + if os.path.exists(crt) and os.path.exists(key): + return crt, key + if not os.access(d, os.W_OK): + d = tempfile.mkdtemp() + crt, key = os.path.join(d, "c.pem"), os.path.join(d, "k.pem") subprocess.run(["openssl", "req", "-x509", "-newkey", "ec", "-pkeyopt", "ec_paramgen_curve:prime256v1", "-nodes", "-days", "3650", "-subj", "/CN=pve-redfish", "-addext", f"subjectAltName=IP:{addr}", "-keyout", key, "-out", crt], check=True, capture_output=True) @@ -348,7 +354,7 @@ def main(): ip = ipaddress.ip_address(addr) cls = Server6 if ip.version == 6 else ThreadingHTTPServer srv = cls((addr, int(entry.get("redfish_port", 443))), handler) - crt, key = self_signed(addr) + crt, key = self_signed(addr, args.vmid) ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) ctx.load_cert_chain(crt, key) srv.socket = ctx.wrap_socket(srv.socket, server_side=True)