ilker/rollcall
1
0 Fork 0
Code Issues 6 Pull Requests Actions Packages Projects Releases 5 Wiki Activity

Security review: fix traversal, identity, injection and takeover paths
CI / agent (push) Successful in 3s
Details
CI / site (push) Successful in 2s
Details
CI / go (push) Successful in 34s
Details

Browse Source 
Findings from a full review (auth/API, CA and rack channel, config
generation, integrations, UI/agents/scripts), each verified in code:

- rack binary endpoint read any region file via arch=../.. (CA key)
- render endpoint traversal; machine-reported MACs used as file paths
- delegated intermediates could mint admin/rack identities and IPs;
  identities now require root-issued certs, intermediates are
  ServerAuth-only with both name types constrained
- rack certs were ServerAuth for rack-chosen SANs: now client-only;
  racks accept only the region's own cert; join checks the CA
  fingerprint; unbound join codes cannot take an existing name; job
  results bound to the rack that took them
- rack self-update: region signs binaries with the CA key, the root
  unit verifies against the join-time CA in a root-owned dir
- dnsmasq reads a checked, root-owned copy limited to generated options
- commissioning could overwrite deployed machines; MAC takeover of PXE
- injection into dnsmasq/cloud-init/PXE/udev/DNS via names and
  machine-reported strings; stored XSS in the UI; CSP and cross-origin
  protection; deploy agent shell injection and seed traversal
- Redfish requests confined to the BMC's /redfish/ tree
- admin_http defaults to loopback; body limits and timeouts; secrets
  off command lines

Also: IPAM claim/reconcile race, sync stopping on one error, watchdog
race, release wiping the wrong disk, SAN LUNs as auto targets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ilker ManapandClaude Opus 5.5 committed 2026-10-29 17:58:42 +01:00
1 parent 6e4be736ec
commit a549f01ef5
28 files changed
+782 -154
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

cmd/rollcall/server.go
+77 -16
View File
@@ -151,9 +151,14 @@ func logging(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
start := time.Now()
rw := &statusWriter{ResponseWriter: w, code: 200}
r.Body = http.MaxBytesReader(rw, r.Body, 16<<20) // handlers set tighter limits; nothing takes more
next.ServeHTTP(rw, r)
if (!strings.HasPrefix(r.URL.Path, "/rollcall/") && !strings.HasPrefix(r.URL.Path, "/images/") && !strings.HasPrefix(r.URL.Path, "/os/")) || rw.code >= 400 {
log.Printf("%s %s %s %d %s", r.RemoteAddr, r.Method, r.URL.RequestURI(), rw.code, time.Since(start).Round(time.Millisecond))
uri := r.URL.RequestURI()
if r.URL.Query().Has("token") { // /?token=... signs the UI in; it does not go to the journal
uri = r.URL.Path + "?token=REDACTED"
}
log.Printf("%s %s %s %d %s", r.RemoteAddr, r.Method, uri, rw.code, time.Since(start).Round(time.Millisecond))
}
})
}
@@ -279,8 +284,25 @@ func (a *App) routes() http.Handler {
mux.Handle("GET /os/", http.StripPrefix("/os/", http.FileServer(http.Dir(a.cfg.OSDir))))
// embedded web UI
sub, _ := fs.Sub(webFS, "web")
mux.Handle("GET /", a.uiOverHTTPS(http.FileServer(http.FS(sub))))
return logging(mux)
mux.Handle("GET /", a.uiOverHTTPS(uiHeaders(http.FileServer(http.FS(sub)))))
// Browsers send admin client certificates on their own, so a page elsewhere
// could post to the API with the admin's identity: cross-origin requests
// that change state are refused (Sec-Fetch-Site / Origin). The CLI, agents
// and racks send neither header and are not affected.
return logging(http.NewCrossOriginProtection().Handler(mux))
}
// uiHeaders: the UI loads only its own scripts and cannot be framed, so
// markup that slips through (machine-reported strings) does not run.
func uiHeaders(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
h := w.Header()
h.Set("Content-Security-Policy", "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'none'; form-action 'self'")
h.Set("X-Content-Type-Options", "nosniff")
h.Set("X-Frame-Options", "DENY")
h.Set("Referrer-Policy", "no-referrer")
next.ServeHTTP(w, r)
})
}
// ---------------------------------------------------------------- helpers over inventory JSON
@@ -327,14 +349,23 @@ func listLen(v any) int {
return 0
}
func normMAC(s string) string { return strings.ToLower(strings.ReplaceAll(s, "-", ":")) }
// normMAC is the canonical form of a hardware address, "" when s is not one.
// MACs name files (pxelinux.cfg/01-...) and dnsmasq lines, so nothing else passes.
func normMAC(s string) string {
hw, err := net.ParseMAC(strings.TrimSpace(s))
if err != nil {
return ""
}
return hw.String()
}
var nonAlnum = regexp.MustCompile(`[^a-z0-9]`)
func machineID(uuid, serial string, macs []string) string {
u := strings.ToLower(uuid)
if u != "" && u != "00000000-0000-0000-0000-000000000000" && !strings.Contains(u, "not ") {
u = strings.ReplaceAll(u, "-", "")
if u != "" && u != "00000000-0000-0000-0000-000000000000" && !strings.Contains(u, "not ") &&
nonAlnum.ReplaceAllString(u, "") != "" {
u = nonAlnum.ReplaceAllString(u, "") // the ID names files and URLs: letters and digits only
if len(u) > 12 {
u = u[:12]
}
@@ -464,7 +495,7 @@ func (a *App) writePXE(m *Machine) error {
// ---------------------------------------------------------------- handlers
func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) {
body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, 32<<20))
body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, 4<<20))
if err != nil {
fail(w, 400, err)
return
@@ -491,13 +522,26 @@ func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) {
}
}
}
// identity strings come from the machine: one line each, they go into PXE menus and logs
line := func(v any) *string {
s := strings.Map(func(c rune) rune {
if c < ' ' || c == 0x7f {
return ' '
}
return c
}, str(v))
if s = strings.TrimSpace(s); len(s) > 128 {
s = s[:128]
}
return strPtr(s)
}
m := &Machine{
ID: machineID(str(ident["uuid"]), str(ident["serial"]), macs),
UUID: strPtr(ident["uuid"]),
Serial: strPtr(ident["serial"]),
Hostname: strPtr(inv["hostname"]),
Manufacturer: strPtr(dig(inv, "system", "system-manufacturer")),
Product: strPtr(dig(inv, "system", "system-product-name")),
UUID: line(ident["uuid"]),
Serial: line(ident["serial"]),
Hostname: line(inv["hostname"]),
Manufacturer: line(dig(inv, "system", "system-manufacturer")),
Product: line(dig(inv, "system", "system-product-name")),
MACs: macs,
}
// Pull the BMC password out of the payload before anything is persisted.
@@ -517,9 +561,20 @@ func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) {
}
m.Inventory, _ = json.Marshal(inv)
// Anyone on the provisioning network can post here, so a known machine is
// only re-commissioned when that is expected: an operator asked
// (recommission), or it has nothing to lose (ready, a failed job). A report
// for a deployed or running machine is refused, not merged.
old, oldErr := a.store.Get(r.Context(), m.ID)
if oldErr == nil && !recommissionable(old.State) {
_ = a.store.AddEvent(r.Context(), m.ID, "commission refused", fmt.Sprintf("state %s; from %s", old.State, r.RemoteAddr))
log.Printf("commission refused for %s (state %s) from %s", m.ID, old.State, r.RemoteAddr)
fail(w, 409, fmt.Errorf("machine %s is %s; recommission it first", m.ID, old.State))
return
}
// a BMC an operator configured (driver_set) survives commissioning; what the agent
// found is kept next to it for reference
if old, err := a.store.Get(r.Context(), m.ID); err == nil && old.BMC != nil {
if oldErr == nil && old.BMC != nil {
var ob obj
if json.Unmarshal(old.BMC, &ob) == nil && ob["driver_set"] == true {
var found obj
@@ -538,7 +593,9 @@ func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) {
return
}
_ = a.store.AddEvent(r.Context(), m.ID, "commissioned", detail)
_ = a.store.SetRackController(r.Context(), m.ID, a.fromRack(r))
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)
}
@@ -555,6 +612,10 @@ func (a *App) handleCommission(w http.ResponseWriter, r *http.Request) {
writeJSON(w, 200, map[string]any{"id": m.ID, "state": "ready", "new": isNew})
}
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) {
want := map[string]bool{}
for _, m := range strings.Split(r.URL.Query().Get("macs"), ",") {
@@ -746,7 +807,7 @@ func serve(cfgPath string) error {
}
pool := x509.NewCertPool()
pool.AddCert(app.ca.Cert)
tsrv := &http.Server{Addr: cfg.TLSListen, Handler: handler, ReadHeaderTimeout: 10 * time.Second,
tsrv := &http.Server{Addr: cfg.TLSListen, Handler: handler, ReadHeaderTimeout: 10 * time.Second, ReadTimeout: 2 * time.Minute, IdleTimeout: 2 * time.Minute,
TLSConfig: &tls.Config{MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{pair}, ClientCAs: pool, ClientAuth: tls.VerifyClientCertIfGiven}}
go func() {
log.Printf("rollcall HTTPS on %s (CA %s)", cfg.TLSListen, app.ca.Fingerprint())
@@ -755,7 +816,7 @@ func serve(cfgPath string) error {
}
}()
}
srv := &http.Server{Addr: cfg.Listen, Handler: handler, ReadHeaderTimeout: 10 * time.Second}
srv := &http.Server{Addr: cfg.Listen, Handler: handler, ReadHeaderTimeout: 10 * time.Second, ReadTimeout: 2 * time.Minute, IdleTimeout: 2 * time.Minute}
log.Printf("rollcall listening on %s (image=%s tftp=%s agent_token=%v)", cfg.Listen, cfg.ImageDir, cfg.TFTPRoot, cfg.AgentToken != "")
return srv.ListenAndServe()
}