diff --git a/agent/rollcall-agent.py b/agent/rollcall-agent.py index 3d0c91f..fc2b084 100644 --- a/agent/rollcall-agent.py +++ b/agent/rollcall-agent.py @@ -416,7 +416,12 @@ def bmc_hint(server, macs): if not h or not h.get("address"): return None res = {"source": "hint", "address": h["address"], "port": h.get("port", 623), - "user": h.get("user"), "password": h.get("password"), "errors": []} + "user": h.get("user"), "errors": []} + if not h.get("password"): + # the region keeps the credentials and checks the BMC itself (rollcall >= 0.3) + res["configured"] = True + return res + res["password"] = h["password"] base = ["-I", "lanplus", "-H", h["address"], "-p", str(h.get("port", 623)), "-U", h.get("user", ""), "-P", h.get("password", "")] rc, mc = ipmi(base + ["mc", "info"], timeout=20) diff --git a/cmd/rollcall/ipmi.go b/cmd/rollcall/ipmi.go index 803b199..0f40c7d 100644 --- a/cmd/rollcall/ipmi.go +++ b/cmd/rollcall/ipmi.go @@ -114,6 +114,27 @@ func (s *Store) SetBMC(ctx context.Context, id string, bmc []byte, secret []byte // probeBMC: after commissioning, a BMC still on plain IPMI is asked whether it // speaks Redfish with the same account; if it does, the vendor's driver takes over. // An operator's explicit choice (driver_set) is left alone. +// verifyHintedBMC runs a status call against a BMC from bmc_hints (the agent no +// longer gets its password) and records whether it answered. +func (a *App) verifyHintedBMC(m *Machine) { + var b obj + if m.BMC == nil || json.Unmarshal(m.BMC, &b) != nil || str(b["source"]) != "hint" || b["driver_set"] == true { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + out, err := a.power(ctx, m, "status", "") + b["verified_oob"] = err == nil + if err != nil { + b["errors"] = []string{"hinted BMC did not answer: " + oneLine(err.Error())} + } else { + delete(b, "errors") + } + nb, _ := json.Marshal(b) + _ = a.store.SetBMC(ctx, m.ID, nb, nil) + _ = a.store.AddEvent(ctx, m.ID, "bmc hint", fmt.Sprintf("%s: %s", str(b["address"]), oneLine(orDefault(out, fmt.Sprint(err))))) +} + func (a *App) probeBMC(m *Machine) { if m.BMC == nil || m.bmcSecret == nil { return diff --git a/cmd/rollcall/security_test.go b/cmd/rollcall/security_test.go index 24e2012..6ae68d7 100644 --- a/cmd/rollcall/security_test.go +++ b/cmd/rollcall/security_test.go @@ -6,9 +6,11 @@ import ( "crypto/rand" "crypto/tls" "crypto/x509" + "encoding/json" "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" ) @@ -107,3 +109,18 @@ func TestIdentitiesOnlyFromTheRoot(t *testing.T) { t.Error("an admin certificate passed as the region's") } } + +// The hint API names the BMC but never hands out its password. +func TestBMCHintHasNoPassword(t *testing.T) { + a := &App{cfg: &Config{BMCHints: []BMCHint{{MAC: "52:54:00:AA:BB:01", Address: "10.0.0.11", User: "admin", Password: "s3cret"}}}} + rec := httptest.NewRecorder() + a.handleHint(rec, httptest.NewRequest("GET", "/api/v1/bmc-hint?macs=02:00:00:00:00:09,52-54-00-aa-bb-01", nil)) + var h map[string]any + _ = json.Unmarshal(rec.Body.Bytes(), &h) + if h["address"] != "10.0.0.11" || h["user"] != "admin" || h["password"] != nil || strings.Contains(rec.Body.String(), "s3cret") { + t.Errorf("hint %s", rec.Body.String()) + } + if a.hintFor([]string{"../x", ""}) != nil { + t.Error("garbage matched a hint") + } +} diff --git a/cmd/rollcall/server.go b/cmd/rollcall/server.go index 9736ef8..158592d 100644 --- a/cmd/rollcall/server.go +++ b/cmd/rollcall/server.go @@ -554,8 +554,28 @@ func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) { // Pull the BMC password out of the payload before anything is persisted. var secret []byte detail := "bmc=none" + if bmc, _ := inv["bmc"].(obj); bmc == nil || str(bmc["source"]) == "hint" || str(bmc["address"]) == "" { + // a configured out-of-band BMC: the region's credentials, whatever the agent sent + if h := a.hintFor(macs); h != nil { + port := h.Port + if port == 0 { + port = 623 + } + nb := obj{"source": "hint", "address": h.Address, "port": port, "user": h.User, "configured": true, "password": ""} + if bmc != nil { + nb["agent"] = obj{"source": bmc["source"], "errors": bmc["errors"]} + } + inv["bmc"] = nb + if h.Password != "" { + if secret, err = a.sealer.Seal(h.Password, m.ID); err != nil { + fail(w, 500, err) + return + } + } + } + } if bmc, ok := inv["bmc"].(obj); ok { - if pw := str(bmc["password"]); pw != "" { + if pw := str(bmc["password"]); pw != "" && str(bmc["source"]) != "hint" { secret, err = a.sealer.Seal(pw, m.ID) if err != nil { fail(w, 500, err) @@ -603,9 +623,15 @@ func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) { if isNew || oldErr != nil || old.State == "recommission" || old.RackController == "" { _ = a.store.SetRackController(r.Context(), m.ID, a.fromRack(r)) // a ready machine is not moved by whoever reports it } - if full, err := a.store.Get(r.Context(), m.ID); err == nil { - go a.probeBMC(full) - } + go func(id string) { // check a hinted BMC from where it is reachable, then look for Redfish + ctx := context.Background() + if full, err := a.store.Get(ctx, id); err == nil { + a.verifyHintedBMC(full) + } + if full, err := a.store.Get(ctx, id); err == nil { + a.probeBMC(full) + } + }(m.ID) if full, err := a.store.Get(r.Context(), m.ID); err == nil { m = full // carries profile / node token when this is a re-commission of a stateless node } @@ -623,22 +649,37 @@ func recommissionable(state string) bool { return state == "new" || state == "ready" || state == "recommission" || strings.HasSuffix(state, "-failed") } -func (a *App) handleHint(w http.ResponseWriter, r *http.Request) { +// hintFor is the configured out-of-band BMC for one of these MACs. +func (a *App) hintFor(macs []string) *BMCHint { want := map[string]bool{} - for _, m := range strings.Split(r.URL.Query().Get("macs"), ",") { - want[normMAC(m)] = true - } - for _, h := range a.cfg.BMCHints { - if want[normMAC(h.MAC)] { - port := h.Port - if port == 0 { - port = 623 - } - writeJSON(w, 200, obj{"address": h.Address, "port": port, "user": h.User, "password": h.Password}) - return + for _, m := range macs { + if n := normMAC(m); n != "" { + want[n] = true } } - writeJSON(w, 200, obj{}) + for i, h := range a.cfg.BMCHints { + if want[normMAC(h.MAC)] { + return &a.cfg.BMCHints[i] + } + } + return nil +} + +// handleHint tells a commissioning agent that a BMC is configured for it: the +// address, never the password (the agent token is public, MACs are visible on +// the wire). The region attaches the credentials itself when the machine +// reports in (handleCommission) and checks them from the rack controller. +func (a *App) handleHint(w http.ResponseWriter, r *http.Request) { + h := a.hintFor(strings.Split(r.URL.Query().Get("macs"), ",")) + if h == nil { + writeJSON(w, 200, obj{}) + return + } + port := h.Port + if port == 0 { + port = 623 + } + writeJSON(w, 200, obj{"address": h.Address, "port": port, "user": h.User, "credentials": "on the region"}) } func (a *App) handleList(w http.ResponseWriter, r *http.Request) {