From a549f01ef51ca2bb593bb1c0c55824d2d73e8a16 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Thu, 29 Oct 2026 17:58:42 +0100 Subject: [PATCH] Security review: fix traversal, identity, injection and takeover paths 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 --- CHANGELOG.md | 67 ++++++++++++++++++ agent/rollcall-agent.py | 7 +- agent/rollcall-deploy.py | 73 +++++++++++++------- cmd/rollcall/admincert.go | 20 +++++- cmd/rollcall/ca.go | 71 ++++++++++++------- cmd/rollcall/ca_test.go | 32 ++++++++- cmd/rollcall/config.go | 6 +- cmd/rollcall/deploy.go | 44 +++++++++++- cmd/rollcall/dnsprov.go | 33 +++++---- cmd/rollcall/doctor.go | 2 +- cmd/rollcall/ipam.go | 53 +++++++++++++-- cmd/rollcall/ipmi.go | 10 ++- cmd/rollcall/netapi.go | 24 ++++++- cmd/rollcall/network.go | 8 ++- cmd/rollcall/power.go | 31 ++++++++- cmd/rollcall/rackctl.go | 34 ++++++++-- cmd/rollcall/rackd.go | 26 ++++++- cmd/rollcall/rackupdate.go | 48 ++++++++++++- cmd/rollcall/security_test.go | 109 ++++++++++++++++++++++++++++++ cmd/rollcall/server.go | 93 ++++++++++++++++++++----- cmd/rollcall/stateless.go | 16 ++++- cmd/rollcall/web/app.js | 12 ++-- deploy/bootstrap.sh | 12 ++-- deploy/install-rack.sh | 6 +- deploy/install.sh | 9 ++- deploy/publish-image.sh | 10 +-- deploy/rollcall-dnsmasq-reload.sh | 48 ++++++++++--- deploy/rollcall-rack-update.sh | 32 ++++++--- 28 files changed, 782 insertions(+), 154 deletions(-) create mode 100644 cmd/rollcall/security_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index dc9b32c..3ebabee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,74 @@ publish a tag that has no section here. `_rollcall.` TXT marker and update prerequisites, so foreign records are never overwritten; drift is detected by querying). `rollcall dns`, `rollcall dns sync [-dry]`, a doctor check. +### Security +A review of the whole code base; the fixes below. Upgrading: rerun `deploy/install.sh` on the +region and `deploy/install-rack.sh` on rack hosts (new dnsmasq layout, signed rack updates). +- **Region file read (critical).** A rack controller could read any file on the region (CA key, + config) through `/api/v1/rack/binary?arch=../..`. `arch` is now a known architecture. +- **Path traversal.** In `/api/v1/render/...` (admin) as well. +- **Machine-reported MACs.** A MAC could be a path, letting a PXE-booting host make the rack write + outside the TFTP directory. Only real hardware addresses are accepted. Rack controllers write only + `01-xx-..` file names. +- **Delegated intermediates.** An ACME intermediate could mint admin or rack certificates (name + constraints do not cover the CN), and IP addresses when only a domain was permitted. + - Admin and rack identities must now be signed by the root itself. + - Intermediates carry a ServerAuth-only EKU, and the unconstrained name type is closed. +- **Rack certificates.** These were ServerAuth certificates for any address the rack reported, so + one rack could pose as the region. + - They are now client-only with no names. + - Racks accept only the region's own certificate. + - `rack join` checks the returned CA against the fingerprint in the code. + - An unbound join code cannot take over an existing rack's name. + - Job results are accepted only from the rack that took the job. +- **Rack self-update.** The root unit ran a binary the unprivileged rack user staged. + - The region now signs each binary with its CA key. + - The root unit verifies the signature against the CA from `rack join`, in a root-owned directory, + without following links. +- **dnsmasq as root.** dnsmasq (root) read configuration the rollcall user writes, and the reload + script copied files through links in rollcall's directory. + - It now reads a checked copy in `/var/lib/rollcall-dnsmasq`. + - The copy may hold only the options Rollcall generates: nothing that runs a program or includes + a file. +- **Machine record takeover.** Any host on the provisioning network could overwrite a deployed + machine's record (and BMC) by reporting its UUID. + - Known machines are re-commissioned only when `ready`, `recommission` or failed. + - A MAC another machine owns keeps its boot file. + - The rack controller of a ready machine is not changed by whoever reports it. +- **Injection into dnsmasq, cloud-init, PXE menus, udev rules and DNS updates.** Subnet domain and + gateway, host names, NIC names, reservation names and machine-reported identity strings are now + validated or reduced to one line. Machine IDs are letters and digits only. +- **Web UI.** + - Escaping is fixed for machine-reported inventory fields (stored XSS). + - A Content-Security-Policy, `frame-ancestors 'none'`, nosniff and no-referrer are set. + - Cross-origin state-changing requests are refused (`http.CrossOriginProtection`), so a page + elsewhere cannot use a browser's admin certificate. + - `?token=` is no longer logged. +- **Deploy agent.** + - The image URL no longer goes into a shell command; the checksum is taken over the bytes written. + - Seed files cannot leave the target root. + - The BMC password is not kept in the fallback inventory file. +- **Redfish.** Requests stay on the BMC and under `/redfish/`: paths the BMC returns cannot send the + credentials elsewhere, and `scheme://` addresses are reduced to scheme and host. +- **Defaults and limits.** + - `admin_http` defaults to `loopback`. + - Request bodies are capped (commissioning 4 MB, all 16 MB), with read and idle timeouts. + - `bootstrap.sh` and `publish-image.sh` keep the database password and API tokens off command + lines. + ### Fixed +- IPAM: a reconcile running during an allocation could delete the record just reserved in NetBox. +- IPAM and DNS sync: one failing record no longer stops the rest of the pass, deletes included. + Reservations are stored in one spelling. +- The job watchdog could fail a job that had just started. +- Release without `-disk` wiped the smallest disk, not the one the OS was deployed to. Automatic + disk choice skips SAN LUNs (iSCSI, FC, SRP). +- RFC 2136 lookups follow the request context. The marker is not removed when the database cannot + say whether other records need it. +- Smaller fixes: + - `seq` in templates is capped. + - Layout selector `ib:-1` no longer panics. + - Redfish detection no longer runs on a machine that cannot be read. - dnsmasq busy loop in proxy mode (a dnsmasq bug with tagged proxy ranges, found with gdb): proxy ranges are untagged, PXE services limited by interface; `dhcp-boot`/`dhcp-match` only for served subnets, no `dhcp-host` lines for proxied ones. diff --git a/agent/rollcall-agent.py b/agent/rollcall-agent.py index 6bf1a02..3d0c91f 100644 --- a/agent/rollcall-agent.py +++ b/agent/rollcall-agent.py @@ -499,8 +499,11 @@ def main(): log(f"server accepted: machine {resp.get('id')} state={resp.get('state')}") else: log("could not deliver inventory to server") - with open("/root/inventory.json", "w") as f: - json.dump(inv, f, indent=1) + keep = json.loads(json.dumps(inv)) + if (keep.get("bmc") or {}).get("password"): + keep["bmc"]["password"] = "" + with open(os.open("/root/inventory.json", os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600), "w") as f: + json.dump(keep, f, indent=1) if opts.get("nopoweroff") or not resp: log("staying up (nopoweroff or delivery failure)") return 0 if resp else 1 diff --git a/agent/rollcall-deploy.py b/agent/rollcall-deploy.py index 4bebc01..11b2353 100644 --- a/agent/rollcall-deploy.py +++ b/agent/rollcall-deploy.py @@ -12,6 +12,7 @@ Everything the agent needs (image URL, checksum, disk, seed files) comes from GET /api/v1/jobs/self; it never decides policy itself. """ import glob +import hashlib import json import os import re @@ -156,7 +157,9 @@ def find_disk(spec): log(f"{want} not found; serial {serial} is {d['path']}") return d raise RuntimeError(f"target disk {want} not present") - cands = [d for d in disks if not d.get("rm") and d.get("tran") != "usb" and int(d["size"]) >= need] + # never a USB stick or a SAN LUN by itself (the region picks the same way) + remote = {"usb", "iscsi", "fc", "fcoe", "srp", "iser"} + cands = [d for d in disks if not d.get("rm") and d.get("tran") not in remote and int(d["size"]) >= need] if not cands: raise RuntimeError(f"no disk >= {need} bytes") cands.sort(key=lambda d: (int(d["size"]), d["name"])) @@ -182,31 +185,47 @@ def release_disk(path): # --------------------------------------------------------------------------- deploy def write_image(spec, disk, prog): url, want, size = spec["image_url"], spec["sha256"], int(spec.get("size") or 0) - sumf, pvf = "/run/rollcall.sum", "/run/rollcall.pv" - for f in (sumf, pvf): - if os.path.exists(f): - os.remove(f) - pipeline = (f"set -o pipefail; curl -fsSL --retry 5 --retry-connrefused '{url}' " - f"| tee >(sha256sum > {sumf}) " - f"| pv -n -f -i 2 -s {size} 2>{pvf} " - f"| zstd -dc -T0 " - f"| dd of='{disk}' bs=4M iflag=fullblock oflag=direct conv=fsync status=none") + if not re.match(r"^https?://", url): + raise RuntimeError(f"image url {url!r} is not http(s)") + pvf = "/run/rollcall.pv" + if os.path.exists(pvf): + os.remove(pvf) + # values go in as arguments, never into the shell text; the checksum is taken + # here, over exactly the bytes handed to the writer + down = subprocess.Popen( + ["bash", "-c", 'set -o pipefail; pv -n -f -i 2 -s "$1" 2>"$2" | zstd -dc -T0 ' + '| dd of="$3" bs=4M iflag=fullblock oflag=direct conv=fsync status=none', "write", str(size), pvf, disk], + stdin=subprocess.PIPE, stderr=subprocess.PIPE) + curl = subprocess.Popen(["curl", "-fsSL", "--retry", "5", "--retry-connrefused", "--", url], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) log(f"writing {url} -> {disk}") - proc = subprocess.Popen(["bash", "-c", pipeline], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - pct = 0 - while proc.poll() is None: - time.sleep(2) - try: - with open(pvf) as f: - lines = f.read().split() - if lines: - pct = int(lines[-1]) - except (OSError, ValueError): - pass - prog.set("writing", min(pct, 99), f"{pct}% of {size // (1 << 20)} MiB written to {disk}") - if proc.returncode != 0: - raise RuntimeError(f"image write failed rc={proc.returncode}: {proc.stderr.read().strip()[-400:]}") - got = readf(sumf).split()[0] if readf(sumf) else "" + h, pct, shown = hashlib.sha256(), 0, 0.0 + try: + while True: + b = curl.stdout.read(1 << 20) + if not b: + break + h.update(b) + down.stdin.write(b) + if time.time() - shown >= 2: + shown = time.time() + try: + with open(pvf) as f: + lines = f.read().split() + if lines: + pct = int(lines[-1]) + except (OSError, ValueError): + pass + prog.set("writing", min(pct, 99), f"{pct}% of {size // (1 << 20)} MiB written to {disk}") + down.stdin.close() + except BrokenPipeError: + pass + crc, drc = curl.wait(), down.wait() + if crc != 0: + raise RuntimeError(f"download failed rc={crc}: {curl.stderr.read().decode(errors='replace').strip()[-400:]}") + if drc != 0: + raise RuntimeError(f"image write failed rc={drc}: {down.stderr.read().decode(errors='replace').strip()[-400:]}") + got = h.hexdigest() if got != want: raise RuntimeError(f"sha256 mismatch: got {got} want {want}") run(["sync"]) @@ -244,7 +263,9 @@ def seed(root, files): if not os.path.isdir(f"{mnt}/etc"): raise RuntimeError(f"{root['path']} has no /etc; not a root filesystem") for f in files: - dst = os.path.join(mnt, f["path"].lstrip("/")) + dst = os.path.realpath(os.path.join(mnt, f["path"].lstrip("/"))) + if not dst.startswith(mnt + "/"): + raise RuntimeError(f"seed file {f['path']!r} is outside the root filesystem") os.makedirs(os.path.dirname(dst), exist_ok=True) with open(dst, "w") as fh: fh.write(f["content"]) diff --git a/cmd/rollcall/admincert.go b/cmd/rollcall/admincert.go index d3576f4..cc2aa3e 100644 --- a/cmd/rollcall/admincert.go +++ b/cmd/rollcall/admincert.go @@ -117,12 +117,28 @@ func (a *App) dropAdminCertCache() { a.adminCC.mu.Unlock() } +// rootIssued is the client certificate when the Rollcall root signed it +// itself. Admin and rack identities are never accepted through an +// intermediate: a delegated (ACME) intermediate could otherwise mint +// CN=admin:... - its name constraints cover SANs, not the CN. +func rootIssued(r *http.Request) *x509.Certificate { + if r.TLS == nil { + return nil + } + for _, ch := range r.TLS.VerifiedChains { + if len(ch) == 2 { + return ch[0] + } + } + return nil +} + // peerAdmin is the admin name of a verified client certificate, "" otherwise. func (a *App) peerAdmin(r *http.Request) string { - if r.TLS == nil || len(r.TLS.VerifiedChains) == 0 || len(r.TLS.PeerCertificates) == 0 { + c := rootIssued(r) + if c == nil { return "" } - c := r.TLS.PeerCertificates[0] name, ok := strings.CutPrefix(c.Subject.CommonName, "admin:") if !ok || name == "" { return "" diff --git a/cmd/rollcall/ca.go b/cmd/rollcall/ca.go index a2b8cf5..ee0b44f 100644 --- a/cmd/rollcall/ca.go +++ b/cmd/rollcall/ca.go @@ -209,7 +209,14 @@ func parsePermits(list string) (dns []string, nets []*net.IPNet, err error) { if net.ParseIP(p) != nil { return nil, nil, fmt.Errorf("%s: give an address range as CIDR", p) } - dns = append(dns, strings.TrimPrefix(strings.ToLower(p), "*.")) + d := strings.Trim(strings.TrimPrefix(strings.ToLower(p), "*."), ".") + if !validDomain(d) { + return nil, nil, fmt.Errorf("%s: give a domain (int.example.com) or a CIDR", p) + } + dns = append(dns, d) + } + if len(dns)+len(nets) == 0 { + return nil, nil, errors.New("give at least one permitted domain or CIDR") } return dns, nets, nil } @@ -233,7 +240,16 @@ func (ca *CA) intermediate(pub crypto.PublicKey, cn string, dns []string, nets [ tmpl := &x509.Certificate{SerialNumber: serial, Subject: pkix.Name{CommonName: cn, Organization: []string{"rollcall"}}, NotBefore: time.Now().Add(-backdate), NotAfter: time.Now().Add(validity), KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageDigitalSignature, BasicConstraintsValid: true, IsCA: true, MaxPathLenZero: true, - PermittedDNSDomainsCritical: len(dns)+len(nets) > 0, PermittedDNSDomains: dns, PermittedIPRanges: nets} + PermittedDNSDomainsCritical: true, PermittedDNSDomains: dns, PermittedIPRanges: nets, + // server certificates only: nothing it issues can be a client (admin, rack) identity + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}} + // a constraint on one name type leaves the other open: close it + if len(nets) == 0 { + tmpl.ExcludedIPRanges = []*net.IPNet{{IP: net.IPv4zero, Mask: net.CIDRMask(0, 32)}, {IP: net.IPv6zero, Mask: net.CIDRMask(0, 128)}} + } + if len(dns) == 0 { + tmpl.PermittedDNSDomains = []string{"invalid"} // RFC 6761: no name below it exists + } der, err := x509.CreateCertificate(rand.Reader, tmpl, ca.Cert, pub, ca.Key) if err != nil { return nil, nil, err @@ -357,6 +373,17 @@ func parseJoinCode(s string) (joinCode, error) { return c, nil } +// regionLeaf accepts the region's own certificate only: "rollcall region ...", +// signed by the root itself - not a rack's, not one from a delegated intermediate. +func regionLeaf(cs tls.ConnectionState) error { + for _, ch := range cs.VerifiedChains { + if len(ch) == 2 && strings.HasPrefix(ch[0].Subject.CommonName, "rollcall region ") { + return nil + } + } + return errors.New("the server's certificate is not the region's") +} + // pinnedTLS trusts a server whose chain carries the CA with this fingerprint. func pinnedTLS(fp [16]byte) *tls.Config { return &tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: true, //nolint:gosec // verified below against the pinned CA @@ -374,8 +401,11 @@ func pinnedTLS(fp [16]byte) *tls.Config { if c.IsCA && string(sum[:16]) == string(fp[:]) { pool := x509.NewCertPool() pool.AddCert(c) - _, err := leaf.Verify(x509.VerifyOptions{Roots: pool}) - return err + chains, err := leaf.Verify(x509.VerifyOptions{Roots: pool}) + if err != nil { + return err + } + return regionLeaf(tls.ConnectionState{VerifiedChains: chains}) } } return errors.New("the server's certificate is not from the authority this code is for") @@ -413,10 +443,10 @@ func (s *Store) RackCertSerial(ctx context.Context, name string) string { // peerRack is the rack controller a mutual-TLS request comes from, if any: // a verified client certificate whose serial is the one on record. func (a *App) peerRack(r *http.Request) string { - if r.TLS == nil || len(r.TLS.VerifiedChains) == 0 || len(r.TLS.PeerCertificates) == 0 { + c := rootIssued(r) + if c == nil { return "" } - c := r.TLS.PeerCertificates[0] name := strings.TrimPrefix(c.Subject.CommonName, "rack:") if name == c.Subject.CommonName { // only rack certificates return "" @@ -487,7 +517,7 @@ type enrollRequest struct { Rack RackController `json:"rack"` } -func (a *App) signRackCSR(csrPEM, name string, rc RackController) ([]byte, *x509.Certificate, error) { +func (a *App) signRackCSR(csrPEM, name string) ([]byte, *x509.Certificate, error) { blk, _ := pem.Decode([]byte(csrPEM)) if blk == nil { return nil, nil, errors.New("csr is not PEM") @@ -499,20 +529,8 @@ func (a *App) signRackCSR(csrPEM, name string, rc RackController) ([]byte, *x509 if err := csr.CheckSignature(); err != nil { return nil, nil, err } - hosts := []string{name} - if u := strings.TrimPrefix(strings.TrimPrefix(rc.URL, "http://"), "https://"); u != "" { - if h, _, err := net.SplitHostPort(u); err == nil { - hosts = append(hosts, h) - } else { - hosts = append(hosts, strings.Trim(u, "/[]")) - } - } - for _, i := range rc.Interfaces { - for _, ad := range i.Addrs { - hosts = append(hosts, stripPrefix(ad)) - } - } - return a.ca.sign(csr.PublicKey, "rack:"+name, hosts, x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth) + // racks serve no TLS: a client identity only, no names, so a rack cannot pose as the region + return a.ca.sign(csr.PublicKey, "rack:"+name, nil, x509.ExtKeyUsageClientAuth) } // handleRackEnroll: a rack controller presents a join code's secret and a CSR @@ -546,7 +564,13 @@ func (a *App) handleRackEnroll(w http.ResponseWriter, r *http.Request) { fail(w, 400, fmt.Errorf("bad rack controller name %q", name)) return } - certPEM, cert, err := a.signRackCSR(req.CSR, name, req.Rack) + // a code made for one name may re-enroll that rack; an unbound code may not + // take over a name in use (its machines, BMC jobs and certificate) + if _, exists := a.racksSnapshot()[name]; exists && bound == "" { + fail(w, 409, fmt.Errorf("rack controller %s exists; make a code for it: rollcall controller join-code -name %s", name, name)) + return + } + certPEM, cert, err := a.signRackCSR(req.CSR, name) if err != nil { fail(w, 400, err) return @@ -578,8 +602,7 @@ func (a *App) handleRackRenew(w http.ResponseWriter, r *http.Request, rack strin fail(w, 400, err) return } - rc := a.racksSnapshot()[rack] - certPEM, cert, err := a.signRackCSR(req.CSR, rack, rc) + certPEM, cert, err := a.signRackCSR(req.CSR, rack) if err != nil { fail(w, 400, err) return diff --git a/cmd/rollcall/ca_test.go b/cmd/rollcall/ca_test.go index 375486b..d1b362a 100644 --- a/cmd/rollcall/ca_test.go +++ b/cmd/rollcall/ca_test.go @@ -91,7 +91,7 @@ func TestSignRackCSR(t *testing.T) { if err != nil { t.Fatal(err) } - _, cert, err := a.signRackCSR(csr, "hall-b", RackController{URL: "http://10.213.78.1", Interfaces: []RackIface{{Name: "eth1", Addrs: []string{"fd00:213:78::1/64"}}}}) + _, cert, err := a.signRackCSR(csr, "hall-b") if err != nil { t.Fatal(err) } @@ -100,9 +100,13 @@ func TestSignRackCSR(t *testing.T) { if _, err := cert.Verify(x509.VerifyOptions{Roots: pool, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}}); err != nil { t.Errorf("rack certificate is no client certificate: %v", err) } - if cert.Subject.CommonName != "rack:hall-b" || cert.VerifyHostname("10.213.78.1") != nil || cert.VerifyHostname("fd00:213:78::1") != nil { + // a rack is a client only: no names, no server use, so it cannot pose as the region + if cert.Subject.CommonName != "rack:hall-b" || len(cert.IPAddresses)+len(cert.DNSNames) > 0 { t.Errorf("subject %s, SANs %v %v", cert.Subject.CommonName, cert.IPAddresses, cert.DNSNames) } + if _, err := cert.Verify(x509.VerifyOptions{Roots: pool, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}}); err == nil { + t.Error("rack certificate verifies as a server certificate") + } } func TestIntermediate(t *testing.T) { @@ -146,6 +150,30 @@ func TestIntermediate(t *testing.T) { if err := verify("node001.int.example.com", "192.168.1.1"); err == nil { t.Error("leaf outside the IP constraint verified") } + // nothing it issues is a client identity (admin:, rack:) + if _, leaf, err := sub.sign(&ikey.PublicKey, "admin:mallory", nil, x509.ExtKeyUsageClientAuth); err == nil { + if _, err := leaf.Verify(x509.VerifyOptions{Roots: roots, Intermediates: inters, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}}); err == nil { + t.Error("the intermediate issued a client certificate that verifies") + } + } + // DNS only: no address may be certified (the region's IP, say) + dnsOnly, _, _ := parsePermits("int.example.com") + _, ic2, err := root.intermediate(&ikey.PublicKey, "dns-only", dnsOnly, nil, time.Hour) + if err != nil { + t.Fatal(err) + } + inters2 := x509.NewCertPool() + inters2.AddCert(ic2) + if _, leaf, err := (&CA{Cert: ic2, Key: ikey}).sign(&ikey.PublicKey, "x", []string{"10.0.0.26"}, x509.ExtKeyUsageServerAuth); err == nil { + if _, err := leaf.Verify(x509.VerifyOptions{Roots: roots, Intermediates: inters2}); err == nil { + t.Error("a DNS-only intermediate certified an IP address") + } + } + for _, bad := range []string{"*.", "", "foo bar"} { + if _, _, err := parsePermits(bad); err == nil { + t.Errorf("permit %q accepted", bad) + } + } // a second level below the intermediate is refused by path length 0 k2, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) _, c2, err := sub.intermediate(&k2.PublicKey, "deeper", dns, nil, time.Hour) diff --git a/cmd/rollcall/config.go b/cmd/rollcall/config.go index c869688..4dc1fff 100644 --- a/cmd/rollcall/config.go +++ b/cmd/rollcall/config.go @@ -45,7 +45,7 @@ type Config struct { CADir string `json:"ca_dir"` // ca.pem, ca-key.pem, region.pem, region-key.pem (default /etc/rollcall/ca) TLSListen string `json:"tls_listen"` // HTTPS listener, default ":443"; "off" disables TLSNames []string `json:"tls_names"` // extra names/addresses for the region certificate - AdminHTTP string `json:"admin_http"` // admin API over plain HTTP: allow (default) | loopback | deny + AdminHTTP string `json:"admin_http"` // admin API over plain HTTP: loopback (default) | allow | deny // a deploy/release job the machine has not started by then fails (default 30m; // servers can take 4-5 minutes to POST, longer when many boot together) @@ -118,8 +118,8 @@ func loadConfig(path string) (*Config, error) { case "off", "none": c.TLSListen = "" } - if c.AdminHTTP == "" { - c.AdminHTTP = "allow" + if c.AdminHTTP == "" { // the token must not cross the provisioning network in clear + c.AdminHTTP = "loopback" } return c, nil } diff --git a/cmd/rollcall/deploy.go b/cmd/rollcall/deploy.go index de8a6ad..5f30789 100644 --- a/cmd/rollcall/deploy.go +++ b/cmd/rollcall/deploy.go @@ -161,6 +161,12 @@ func (s *Store) FinishJob(ctx context.Context, id, state, errMsg string) error { return err } +// FailPendingJob fails a job that has not started; false when it started meanwhile. +func (s *Store) FailPendingJob(ctx context.Context, id, errMsg string) (bool, error) { + tag, err := s.pool.Exec(ctx, `UPDATE jobs SET state='failed', finished=now(), started=coalesce(started, now()), error=nullif($2,'') WHERE id=$1 AND state='pending'`, id, errMsg) + return err == nil && tag.RowsAffected() == 1, err +} + func (s *Store) SetDeployed(ctx context.Context, id, os, version string) error { _, err := s.pool.Exec(ctx, `UPDATE machines SET state='deployed', os=$2, os_version=$3, deployed_at=now(), booted_at=NULL, last_seen=now() WHERE id=$1`, id, os, version) return err @@ -176,6 +182,9 @@ func (s *Store) SetReleased(ctx context.Context, id string) error { } func (s *Store) SetNodeAddr(ctx context.Context, id, name, mgmt, ib string) error { + if name != "" && !validDomain(name) { + return fmt.Errorf("node name %q is not a host name (letters, digits, hyphens)", name) + } _, err := s.pool.Exec(ctx, `UPDATE machines SET node_name=$2, mgmt_ip=$3, ib_ip=$4 WHERE id=$1`, id, name, nilIfEmpty(mgmt), nilIfEmpty(ib)) return err } @@ -213,6 +222,9 @@ func inventoryDisks(m *Machine) []invDisk { // pickDisk: the smallest non-removable, non-USB disk that fits the image (boot/OS disks are // usually the small ones on servers; data disks stay untouched). Explicit choice wins. +// remoteTran: transports of disks that are not the machine's own (lsblk TRAN). +var remoteTran = map[string]bool{"usb": true, "iscsi": true, "fc": true, "fcoe": true, "srp": true, "iser": true} + func pickDisk(disks []invDisk, want string, need int64) (*invDisk, error) { if want != "" { for i := range disks { @@ -230,7 +242,7 @@ func pickDisk(disks []invDisk, want string, need int64) (*invDisk, error) { } var cands []invDisk for _, d := range disks { - if d.Removable || d.Tran == "usb" || d.Size < need { + if d.Removable || remoteTran[d.Tran] || d.Size < need { // never a USB stick or a SAN LUN by itself continue } cands = append(cands, d) @@ -468,6 +480,20 @@ func (a *App) handleDeploy(w http.ResponseWriter, r *http.Request) { fail(w, 400, fmt.Errorf("os is required")) return } + req.Hostname = strings.ToLower(req.Hostname) + if req.Hostname != "" && !validDomain(req.Hostname) { + fail(w, 400, fmt.Errorf("hostname %q is not a host name (letters, digits, hyphens)", req.Hostname)) + return + } + // refuse before anything is allocated or renamed + if m.Profile != nil { + fail(w, 409, fmt.Errorf("machine is a stateless node (profile %s); unassign it first", *m.Profile)) + return + } + if j, err := a.store.ActiveJob(r.Context(), m.ID); err == nil { + fail(w, 409, fmt.Errorf("job %s (%s, %s) is still active; cancel it first", j.ID, j.Kind, j.State)) + return + } meta, err := osResolve(a.cfg.OSDir, req.OS, req.Version) if err != nil { fail(w, 404, err) @@ -550,6 +576,17 @@ func (a *App) handleRelease(w http.ResponseWriter, r *http.Request) { if req.Wipe == "" { req.Wipe = "quick" } + if req.Disk == "" { // the disk the OS went to, not "the smallest one" + if jobs, err := a.store.Jobs(r.Context(), m.ID, 50); err == nil { + for _, j := range jobs { + var sp DeploySpec + if j.Kind == "deploy" && j.State == "done" && json.Unmarshal(j.Spec, &sp) == nil && sp.Disk != "" { + req.Disk = sp.Disk + break + } + } + } + } disk, err := pickDisk(inventoryDisks(m), req.Disk, 0) if err != nil { fail(w, 409, err) @@ -775,8 +812,9 @@ func (a *App) jobWatchdog() { continue } msg := fmt.Sprintf("the machine did not start the job within %s: check its power, that it PXE-boots on the provisioning network, and its console for a hung POST (events show the power command's result; job_start_timeout raises the limit)", limit) - _ = a.store.FinishJob(ctx, j.ID, "failed", msg) - a.afterJob(ctx, m, j, "failed", msg) + if ok, _ := a.store.FailPendingJob(ctx, j.ID, msg); ok { // not if it reported progress since the snapshot + a.afterJob(ctx, m, j, "failed", msg) + } } } cancel() diff --git a/cmd/rollcall/dnsprov.go b/cmd/rollcall/dnsprov.go index a5c0f20..efd80a9 100644 --- a/cmd/rollcall/dnsprov.go +++ b/cmd/rollcall/dnsprov.go @@ -74,6 +74,8 @@ type dnsReport struct { Error string `json:"error,omitempty"` } +func (r *dnsReport) fail(e string) { r.Error = joinErr(r.Error, e) } + type dnsProvider interface { Name() string // Owned: Rollcall's records in the zone, and the names+types someone else holds. @@ -190,14 +192,18 @@ func desiredDNS(local []IPAddr, subnets []Subnet, zone string) map[string]dnsRec if !dns.IsSubDomain(dns.Fqdn(z), fq) { continue } + ip, ok := parseAddr(x.IP) + if !ok { + continue + } t := "A" - if strings.Contains(x.IP, ":") { + if ip.Is6() { t = "AAAA" } r := want[fq+" "+t] r.Name, r.Type = fq, t - if !hasTag(r.Values, x.IP) { - r.Values = append(r.Values, x.IP) + if !hasTag(r.Values, ip.String()) { + r.Values = append(r.Values, ip.String()) } sort.Strings(r.Values) want[r.key()] = r @@ -256,8 +262,8 @@ func (a *App) reconcileDNSWith(ctx context.Context, want map[string]dnsRecord, r conflict, err := a.dnsp.Upsert(ctx, w, mine) switch { case err != nil: - rep.Error = fmt.Sprintf("%s: %v", k, err) - return + rep.fail(fmt.Sprintf("%s: %v", k, err)) + continue case conflict: rep.Conflicts = append(rep.Conflicts, k+" exists and is not Rollcall's") default: @@ -275,8 +281,9 @@ func (a *App) reconcileDNSWith(ctx context.Context, want map[string]dnsRecord, r rep.Deleted = append(rep.Deleted, k) if !rep.DryRun { if err := a.dnsp.Delete(ctx, owned[k]); err != nil { - rep.Error = fmt.Sprintf("delete %s: %v", k, err) - return + rep.fail(fmt.Sprintf("delete %s: %v", k, err)) + rep.Deleted = rep.Deleted[:len(rep.Deleted)-1] + continue } } } @@ -435,11 +442,11 @@ func (p *rfc2136Provider) Owned(ctx context.Context) (map[string]dnsRecord, map[ // changed or a marker removed behind our back shows up as a difference, and // the next Upsert either repairs it or finds someone else's record (conflict). for k, r := range owned { - live, err := p.lookup(r.Name, r.Type) + live, err := p.lookup(ctx, r.Name, r.Type) if err != nil { return nil, nil, err } - marker, err := p.lookup(dnsMarkerPrefix+r.Name, "TXT") + marker, err := p.lookup(ctx, dnsMarkerPrefix+r.Name, "TXT") if err != nil { return nil, nil, err } @@ -453,12 +460,12 @@ func (p *rfc2136Provider) Owned(ctx context.Context) (map[string]dnsRecord, map[ } // lookup asks the primary for one RRset; it returns the values, sorted, nil if none. -func (p *rfc2136Provider) lookup(name, typ string) ([]string, error) { +func (p *rfc2136Provider) lookup(ctx context.Context, name, typ string) ([]string, error) { m := new(dns.Msg) m.SetQuestion(name, dns.StringToType[typ]) m.SetTsig(p.keyName, p.alg, 300, time.Now().Unix()) c := &dns.Client{Net: "tcp", Timeout: 10 * time.Second, TsigSecret: map[string]string{p.keyName: p.secret}} - r, _, err := c.Exchange(m, p.server) + r, _, err := c.ExchangeContext(ctx, m, p.server) if err != nil { return nil, fmt.Errorf("query %s %s: %w", name, typ, err) } @@ -545,7 +552,9 @@ func (p *rfc2136Provider) Delete(ctx context.Context, r dnsRecord) error { m.Used([]dns.RR{p.marker(r.Name)}) m.RemoveRRset([]dns.RR{&dns.ANY{Hdr: dns.RR_Header{Name: r.Name, Rrtype: dns.StringToType[r.Type], Class: dns.ClassINET}}}) var others int - _ = p.store.pool.QueryRow(ctx, `SELECT count(*) FROM dns_records WHERE zone=$1 AND name=$2 AND type<>$3`, p.zone, r.Name, r.Type).Scan(&others) + if err := p.store.pool.QueryRow(ctx, `SELECT count(*) FROM dns_records WHERE zone=$1 AND name=$2 AND type<>$3`, p.zone, r.Name, r.Type).Scan(&others); err != nil { + return err // not knowing, the marker could go while an AAAA of ours still needs it + } if others == 0 { // the last of our types at this name: the marker goes too m.RemoveRRset([]dns.RR{&dns.ANY{Hdr: dns.RR_Header{Name: dnsMarkerPrefix + r.Name, Rrtype: dns.TypeTXT, Class: dns.ClassINET}}}) } diff --git a/cmd/rollcall/doctor.go b/cmd/rollcall/doctor.go index a6587e1..43d8f75 100644 --- a/cmd/rollcall/doctor.go +++ b/cmd/rollcall/doctor.go @@ -178,7 +178,7 @@ func doctorCmd(args []string) error { // dnsmasq if out, _ := exec.Command("systemctl", "is-active", "dnsmasq").Output(); strings.TrimSpace(string(out)) != "active" { d.fail("dnsmasq", "not running", "journalctl -u dnsmasq -n 20") - } else if _, err := os.Stat(filepath.Join(cfg.DnsmasqDir, "rollcall.conf.rejected")); err == nil { + } else if _, err := os.Stat("/var/lib/rollcall-dnsmasq/rollcall.conf.rejected"); err == nil { d.fail("dnsmasq", "the generated configuration was rejected; the last working one is in use", "journalctl -u rollcall-dnsmasq; a hand-written DHCP/TFTP file in /etc/dnsmasq.d usually conflicts") } else { diff --git a/cmd/rollcall/ipam.go b/cmd/rollcall/ipam.go index 3565bd4..2dbfdec 100644 --- a/cmd/rollcall/ipam.go +++ b/cmd/rollcall/ipam.go @@ -89,6 +89,22 @@ type ipamReport struct { Error string `json:"error,omitempty"` } +// fail notes one record's error; the reconcile goes on with the others. +func (r *ipamReport) fail(e string) { r.Error = joinErr(r.Error, e) } + +// joinErr keeps the first few errors of a pass readable. +func joinErr(all, e string) string { + switch n := strings.Count(all, "; ") + 1; { + case all == "": + return e + case n < 5: + return all + "; " + e + case n == 5: + return all + "; ..." + } + return all +} + type ipamState struct { mu sync.Mutex // one reconcile at a time kick chan struct{} @@ -119,6 +135,20 @@ func orDefault(s, d string) string { var dnsLabel = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`) +// validDomain: lower-case dot-separated DNS labels, no trailing dot. Names end +// up in dnsmasq lines, cloud-init, PXE menus and DNS updates, so nothing else. +func validDomain(s string) bool { + if s == "" || len(s) > 253 { + return false + } + for _, l := range strings.Split(s, ".") { + if !dnsLabel.MatchString(l) { + return false + } + } + return true +} + // extFor maps a Rollcall address to the record the authority should hold. func (a *App) extFor(x IPAddr, sn *Subnet) extAddr { bits := 32 @@ -168,6 +198,9 @@ func dnsNameIn(x IPAddr, sn *Subnet, zone string) string { return "" } if strings.HasSuffix(host, "."+zone) { + if !validDomain(host) { + return "" // a wildcard or anything that is not a plain name never reaches the DNS + } return host } if !dnsLabel.MatchString(host) { @@ -186,6 +219,12 @@ func dnsNameIn(x IPAddr, sn *Subnet, zone string) string { // claimIP records an address in the authority (when one is configured) and // then locally; false when it is taken in either. func (a *App) claimIP(ctx context.Context, x IPAddr) (bool, error) { + if a.ipam != nil && a.ipamSt != nil { + // not while a reconcile runs: it read the address table before this + // claim and would delete the record Reserve is about to make + a.ipamSt.mu.Lock() + defer a.ipamSt.mu.Unlock() + } if a.ipam != nil { sn := subnetByRef(a.subnetsSnapshot(), x.Subnet) taken, err := a.ipam.Reserve(ctx, a.extFor(x, sn)) @@ -296,8 +335,9 @@ func (a *App) reconcileAddrs(ctx context.Context, local []IPAddr, rep *ipamRepor rep.Updated = append(rep.Updated, x.IP) if !dry { if err := a.ipam.Update(ctx, want); err != nil { - rep.Error = fmt.Sprintf("update %s: %v", x.IP, err) - return + rep.fail(fmt.Sprintf("update %s: %v", x.IP, err)) + rep.Updated = rep.Updated[:len(rep.Updated)-1] + continue } } } @@ -309,8 +349,8 @@ func (a *App) reconcileAddrs(ctx context.Context, local []IPAddr, rep *ipamRepor } taken, err := a.ipam.Reserve(ctx, want) if err != nil { - rep.Error = fmt.Sprintf("create %s: %v", x.IP, err) - return + rep.fail(fmt.Sprintf("create %s: %v", x.IP, err)) + continue } if taken { rep.Conflicts = append(rep.Conflicts, x.IP+" exists there and is not Rollcall's") @@ -327,8 +367,9 @@ func (a *App) reconcileAddrs(ctx context.Context, local []IPAddr, rep *ipamRepor rep.Deleted = append(rep.Deleted, o.IP) if !dry { if err := a.ipam.Delete(ctx, o); err != nil { - rep.Error = fmt.Sprintf("delete %s: %v", o.IP, err) - return + rep.fail(fmt.Sprintf("delete %s: %v", o.IP, err)) + rep.Deleted = rep.Deleted[:len(rep.Deleted)-1] + continue } } } diff --git a/cmd/rollcall/ipmi.go b/cmd/rollcall/ipmi.go index 89f34e1..811ac84 100644 --- a/cmd/rollcall/ipmi.go +++ b/cmd/rollcall/ipmi.go @@ -174,9 +174,15 @@ func (a *App) handleBMC(w http.ResponseWriter, r *http.Request) { b["driver"] = "ipmi" nb, _ := json.Marshal(b) _ = a.store.SetBMC(r.Context(), m.ID, nb, nil) - m, _ = a.store.Get(r.Context(), m.ID) + if m, err = a.store.Get(r.Context(), m.ID); err != nil { + fail(w, 500, err) + return + } a.probeBMC(m) - m, _ = a.store.Get(r.Context(), m.ID) + if m, err = a.store.Get(r.Context(), m.ID); err != nil { + fail(w, 500, err) + return + } _ = json.Unmarshal(m.BMC, &b) writeJSON(w, 200, obj{"id": m.ID, "bmc": b}) return diff --git a/cmd/rollcall/netapi.go b/cmd/rollcall/netapi.go index 51cb9fb..9cae16a 100644 --- a/cmd/rollcall/netapi.go +++ b/cmd/rollcall/netapi.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "net/netip" "os" "strconv" "strings" @@ -134,8 +135,17 @@ func (a *App) handleSubnets(w http.ResponseWriter, r *http.Request) { return } } - if sn.Gateway != "" && !sn.contains(sn.Gateway) { - fail(w, 400, fmt.Errorf("gateway %s is outside %s", sn.Gateway, sn.CIDR)) + if sn.Gateway != "" { + gw, err := netip.ParseAddr(sn.Gateway) + if err != nil || !sn.contains(gw.String()) { + fail(w, 400, fmt.Errorf("gateway %q is not an address in %s", sn.Gateway, sn.CIDR)) + return + } + sn.Gateway = gw.String() + } + sn.Domain = strings.ToLower(strings.TrimSuffix(sn.Domain, ".")) + if sn.Domain != "" && !validDomain(sn.Domain) { + fail(w, 400, fmt.Errorf("domain %q is not a DNS name", sn.Domain)) return } switch sn.Purpose { @@ -265,6 +275,16 @@ func (a *App) handleIPs(w http.ResponseWriter, r *http.Request) { fail(w, 400, err) return } + ip, ok := parseAddr(x.IP) + if !ok || strings.Contains(x.IP, "/") { + fail(w, 400, fmt.Errorf("%q is not an address", x.IP)) + return + } + x.IP = ip.String() // one spelling: the table's key, NetBox's and DNS's + if strings.IndexFunc(x.Hostname, func(c rune) bool { return c < ' ' || c == 0x7f }) >= 0 || len(x.Hostname) > 200 { + fail(w, 400, errors.New("hostname: one line of text")) + return + } sn := subnetFor(a.subnetsSnapshot(), x.IP) if sn == nil { fail(w, 404, fmt.Errorf("%s is in no subnet", x.IP)) diff --git a/cmd/rollcall/network.go b/cmd/rollcall/network.go index 7e5247c..4a0d5ac 100644 --- a/cmd/rollcall/network.go +++ b/cmd/rollcall/network.go @@ -583,6 +583,8 @@ type invNIC struct { var speedRe = regexp.MustCompile(`(\d+)`) +var ifnameRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,14}$`) + func machineNICs(m *Machine) []invNIC { var inv obj _ = json.Unmarshal(m.Inventory, &inv) @@ -596,7 +598,7 @@ func machineNICs(m *Machine) []invNIC { for _, ip := range toList(n["ipv4"]) { nic.IPv4 = append(nic.IPv4, str(ip)) } - if nic.Name != "" { + if ifnameRe.MatchString(nic.Name) { // the machine reports it; it lands in udev rules and dnsmasq files out = append(out, nic) } } @@ -681,8 +683,8 @@ func selectNICs(nics []invNIC, boot *invNIC, sel string) []invNIC { } } sort.Slice(ib, func(i, j int) bool { return ib[i].Name < ib[j].Name }) - k, _ := strconv.Atoi(arg) - if k < len(ib) { + k, err := strconv.Atoi(arg) + if err == nil && k >= 0 && k < len(ib) { return []invNIC{ib[k]} } default: diff --git a/cmd/rollcall/power.go b/cmd/rollcall/power.go index efa5545..e329b13 100644 --- a/cmd/rollcall/power.go +++ b/cmd/rollcall/power.go @@ -154,8 +154,12 @@ func bmcHTTPClient(c *bmcCreds) *http.Client { // 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, "/") - if strings.Contains(a, "://") { - return a + if strings.Contains(a, "://") { // an operator's https://host:port: scheme and authority only + u, err := url.Parse(a) + if err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Host == "" || u.User != nil { + return "https://invalid.invalid" + } + return u.Scheme + "://" + u.Host } port := c.Port if port == 623 || port == 0 { @@ -175,12 +179,35 @@ func newRF(c *bmcCreds) *rfClient { return &rfClient{base: bmcBase(c, "https", 443), user: c.User, pass: c.Password, hc: bmcHTTPClient(c)} } +// resolve turns a path the BMC handed us (@odata.id, an action target, a task +// monitor) into one on this BMC. Vendors return relative paths, full URLs and +// trailing slashes; anything that would leave the BMC or the Redfish tree (and +// take the credentials with it) is refused. +func (r *rfClient) resolve(p string) (string, error) { + if strings.Contains(p, "://") { + u, err := url.Parse(p) + b, _ := url.Parse(r.base) + if err != nil || b == nil || !strings.EqualFold(u.Host, b.Host) || u.User != nil { + return "", fmt.Errorf("the BMC pointed at %q, which is not this BMC", p) + } + p = u.RequestURI() + } + if !strings.HasPrefix(p, "/redfish/") || strings.Contains(p, "/../") || strings.HasSuffix(p, "/..") { + return "", fmt.Errorf("the BMC pointed at %q, outside /redfish/", p) + } + return p, nil +} + func (r *rfClient) do(ctx context.Context, method, path string, body, out any) (int, error) { var rd io.Reader if body != nil { b, _ := json.Marshal(body) rd = bytes.NewReader(b) } + path, err := r.resolve(path) + if err != nil { + return 0, err + } req, err := http.NewRequestWithContext(ctx, method, r.base+path, rd) if err != nil { return 0, err diff --git a/cmd/rollcall/rackctl.go b/cmd/rollcall/rackctl.go index 1a9746e..ad027a1 100644 --- a/cmd/rollcall/rackctl.go +++ b/cmd/rollcall/rackctl.go @@ -23,6 +23,7 @@ import ( "net/http" "os" "path/filepath" + "regexp" "runtime" "sort" "strconv" @@ -76,6 +77,7 @@ type rackJob struct { Op string `json:"op"` Arg string `json:"arg"` done chan rackResult + rack string // who took it: only that rack may answer } type rackResult struct { @@ -137,8 +139,13 @@ func (s *Store) SetRackController(ctx context.Context, id, rack string) error { } func (s *Store) PutPXE(ctx context.Context, mac, machineID, rack, content string) error { - _, err := s.pool.Exec(ctx, `INSERT INTO pxe_files(mac, machine_id, rack, content) VALUES ($1,$2,$3,$4) - ON CONFLICT (mac) DO UPDATE SET machine_id=$2, rack=$3, content=$4, updated=now()`, mac, machineID, rack, content) + // a MAC another machine owns is not taken over: a box claiming a deployed + // machine's MAC would otherwise replace that machine's boot file + tag, err := s.pool.Exec(ctx, `INSERT INTO pxe_files(mac, machine_id, rack, content) VALUES ($1,$2,$3,$4) + ON CONFLICT (mac) DO UPDATE SET machine_id=$2, rack=$3, content=$4, updated=now() WHERE pxe_files.machine_id=$2`, mac, machineID, rack, content) + if err == nil && tag.RowsAffected() == 0 { + return fmt.Errorf("MAC %s belongs to another machine; delete that machine first", mac) + } return err } @@ -164,6 +171,8 @@ func (s *Store) PXEFiles(ctx context.Context, rack string) (map[string]string, e return out, rows.Err() } +var pxeNameRe = regexp.MustCompile(`^01(-[0-9a-f]{2}){6,20}$`) + func pxeFileName(mac string) string { return "01-" + strings.ReplaceAll(normMAC(mac), ":", "-") } // ---------------------------------------------------------------- hub: state generations and the command queue @@ -264,6 +273,7 @@ func (h *rackHub) take(ctx context.Context, rack string, wait time.Duration) []* if q := h.work[rack]; len(q) > 0 { h.work[rack] = nil for _, j := range q { + j.rack = rack h.flight[j.ID] = j } h.mu.Unlock() @@ -291,10 +301,14 @@ func (h *rackHub) take(ctx context.Context, rack string, wait time.Duration) []* } } -func (h *rackHub) finish(r rackResult) bool { +func (h *rackHub) finish(rack string, r rackResult) bool { h.mu.Lock() j, ok := h.flight[r.ID] - delete(h.flight, r.ID) + if ok && j.rack != rack { + ok = false + } else { + delete(h.flight, r.ID) + } h.mu.Unlock() if ok { j.done <- r @@ -479,6 +493,8 @@ func (a *App) rackState(ctx context.Context, name string) (*rackState, error) { // hostsFiles: static DHCP leases for the machines this rack serves, and names // for every machine: the addresses on the interface carrying the management // address get the plain name (A and AAAA), others name-, IB name-ib. +var nonHostChars = regexp.MustCompile(`[^a-z0-9-]+`) + func (a *App) hostsFiles(ctx context.Context, rack string, subnets []Subnet) (string, string, error) { ms, err := a.store.List(ctx) if err != nil { @@ -545,7 +561,9 @@ func (a *App) hostsFiles(ctx context.Context, rack string, subnets []Subnet) (st fmt.Fprintf(&dhcp, "%s,[%s],%s\n", m.MACs[0], x.IP, name) } case x.Iface != "": - line(x.IP, name+"-"+strings.NewReplacer(".", "-", ":", "-").Replace(x.Iface)) + if sfx := strings.Trim(nonHostChars.ReplaceAllString(strings.ToLower(x.Iface), "-"), "-"); sfx != "" { + line(x.IP, name+"-"+sfx) + } } } if ibip != "" { @@ -586,6 +604,10 @@ func applyRackState(st *rackState, tftp, dnsDir string) (changed []string, err e note("default", ch, e) } for name, body := range st.PXE { + if !pxeNameRe.MatchString(name) { + note(name, false, fmt.Errorf("not a pxelinux file name")) + continue + } ch, e := writeIfChanged(filepath.Join(dir, name), body, 0o644) note(name, ch, e) } @@ -837,7 +859,7 @@ func (a *App) handleRackResult(w http.ResponseWriter, r *http.Request, rack stri fail(w, 400, err) return } - writeJSON(w, 200, obj{"accepted": a.hub.finish(res)}) + writeJSON(w, 200, obj{"accepted": a.hub.finish(rack, res)}) } func (a *App) handleControllers(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/rollcall/rackd.go b/cmd/rollcall/rackd.go index c7491b6..19beb37 100644 --- a/cmd/rollcall/rackd.go +++ b/cmd/rollcall/rackd.go @@ -13,6 +13,7 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "crypto/sha256" "crypto/tls" "crypto/x509" "crypto/x509/pkix" @@ -137,6 +138,7 @@ func (r *rackd) clients() error { pair.Leaf, _ = x509.ParseCertificate(pair.Certificate[0]) r.cert = &pair r.tlsc.RootCAs = pool + r.tlsc.VerifyConnection = regionLeaf r.tlsc.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { r.certMu.Lock() defer r.certMu.Unlock() @@ -272,6 +274,28 @@ func rackJoin(args []string) error { if err := json.Unmarshal(data, &out); err != nil { return err } + // what is written here is trusted for good: the CA must be the one the code + // names, and the certificate ours from it + caCert, err := parseCertPEM([]byte(out.CA)) + if err != nil { + return fmt.Errorf("the region sent no CA: %w", err) + } + if fp := sha256.Sum256(caCert.Raw); string(fp[:16]) != string(code.Fingerprint[:]) || !caCert.IsCA { + return errors.New("the region sent a CA that is not the one in the join code; not trusting it") + } + pair, err := tls.X509KeyPair([]byte(out.Cert), key) + if err != nil { + return fmt.Errorf("the certificate does not match our key: %w", err) + } + leaf, err := x509.ParseCertificate(pair.Certificate[0]) + if err != nil { + return err + } + pool := x509.NewCertPool() + pool.AddCert(caCert) + if _, err := leaf.Verify(x509.VerifyOptions{Roots: pool, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}}); err != nil { + return fmt.Errorf("the certificate is not from the code's CA: %w", err) + } cfg.Name = out.Name if err := os.MkdirAll(*dir, 0o750); err != nil { return err @@ -358,7 +382,7 @@ func runRack(args []string) error { mux.HandleFunc("GET /healthz", func(w http.ResponseWriter, _ *http.Request) { writeJSON(w, 200, obj{"rack": cfg.Name, "gen": r.gen, "version": version}) }) - srv := &http.Server{Addr: cfg.Listen, Handler: logging(mux), ReadHeaderTimeout: 10 * time.Second} + srv := &http.Server{Addr: cfg.Listen, Handler: logging(mux), ReadHeaderTimeout: 10 * time.Second, ReadTimeout: 2 * time.Minute, IdleTimeout: 2 * time.Minute} log.Printf("rack controller %s: region %s, serving %s on %s (tftp %s, cache %s)", cfg.Name, cfg.Region, cfg.URL, cfg.Listen, cfg.TFTPRoot, cfg.CacheDir) return srv.ListenAndServe() } diff --git a/cmd/rollcall/rackupdate.go b/cmd/rollcall/rackupdate.go index c89a3ea..f339616 100644 --- a/cmd/rollcall/rackupdate.go +++ b/cmd/rollcall/rackupdate.go @@ -7,10 +7,18 @@ package main // expected version, and leaves a "ready" marker. A root path unit // (rollcall-rack-update.path) installs it, restarts the rack and puts the // previous binary back if the rack does not register again within a minute. +// +// The rack process is not trusted with root: it faces the provisioning +// networks. The region signs each binary with its CA key, and the root unit +// installs nothing that does not verify against the CA certificate written at +// join (/etc/rollcall/rack/ca.pem, root's). import ( "context" + "crypto/ecdsa" + "crypto/rand" "crypto/sha256" + "encoding/base64" "encoding/hex" "fmt" "io" @@ -29,6 +37,7 @@ type rackUpdate struct { Version string `json:"version"` Arch string `json:"arch"` SHA256 string `json:"sha256"` + Sig string `json:"sig"` // base64 DER ECDSA over the binary's SHA-256, by the region CA } // ---------------------------------------------------------------- region @@ -37,14 +46,36 @@ type binCache struct { mu sync.Mutex key map[string]string // path -> "size/mtime" sums map[string]string // path -> sha256 + sigs map[string]string // sha256 -> signature (kept: a new one each time would change the state) } -var rackBins = &binCache{key: map[string]string{}, sums: map[string]string{}} +var rackBins = &binCache{key: map[string]string{}, sums: map[string]string{}, sigs: map[string]string{}} + +// binarySig signs a binary's SHA-256 with the region CA key. +func (a *App) binarySig(sum string) string { + digest, err := hex.DecodeString(sum) + if err != nil || a.ca == nil { + return "" + } + rackBins.mu.Lock() + defer rackBins.mu.Unlock() + if s, ok := rackBins.sigs[sum]; ok { + return s + } + der, err := ecdsa.SignASN1(rand.Reader, a.ca.Key, digest) + if err != nil { + return "" + } + rackBins.sigs[sum] = base64.StdEncoding.EncodeToString(der) + return rackBins.sigs[sum] +} // rackBinary: the binary a rack of this architecture should run: the region's // own for the same architecture, else rack_binaries/rollcall-linux-. func (a *App) rackBinary(arch string) (path, sum string, ok bool) { - if arch == "" { + switch arch { // a file name part: never a path (the rack chooses it) + case "amd64", "arm64", "386", "arm", "riscv64", "ppc64le", "s390x", "loong64": + default: return "", "", false } if arch == runtime.GOARCH && runtime.GOOS == "linux" { @@ -87,7 +118,7 @@ func (a *App) rackUpdateFor(rc RackController) *rackUpdate { return nil } if _, sum, ok := a.rackBinary(rc.Arch); ok { - return &rackUpdate{Version: version, Arch: rc.Arch, SHA256: sum} + return &rackUpdate{Version: version, Arch: rc.Arch, SHA256: sum, Sig: a.binarySig(sum)} } return nil } @@ -135,6 +166,10 @@ func (r *rackd) maybeUpdate(up *rackUpdate) { if up == nil || up.Version == "" || up.Version == version || !r.autoUpdate() || up.Arch != runtime.GOARCH { return } + if _, err := hex.DecodeString(up.SHA256); err != nil || len(up.SHA256) != 64 || up.Sig == "" { + log.Printf("update to %s: the region sent no valid checksum and signature", up.Version) + return + } r.upd.mu.Lock() if t, ok := r.upd.tried[up.Version]; ok && time.Since(t) < time.Hour { r.upd.mu.Unlock() @@ -205,6 +240,13 @@ func (r *rackd) stageUpdate(up *rackUpdate) error { if err := os.WriteFile(filepath.Join(dir, "rollcall.new.sha256"), []byte(up.SHA256+" rollcall.new\n"), 0o644); err != nil { return err } + sig, err := base64.StdEncoding.DecodeString(up.Sig) + if err != nil { + return fmt.Errorf("signature: %w", err) + } + if err := os.WriteFile(filepath.Join(dir, "rollcall.new.sig"), sig, 0o644); err != nil { + return err + } ready := filepath.Join(dir, "ready") if err := os.WriteFile(ready+".tmp", []byte(up.Version+"\n"), 0o644); err != nil { return err diff --git a/cmd/rollcall/security_test.go b/cmd/rollcall/security_test.go new file mode 100644 index 0000000..24e2012 --- /dev/null +++ b/cmd/rollcall/security_test.go @@ -0,0 +1,109 @@ +package main + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" +) + +// Regressions from the security review. + +func TestRackBinaryArchIsNoPath(t *testing.T) { + dir := t.TempDir() + _ = os.WriteFile(filepath.Join(dir, "secret"), []byte("key"), 0o600) + a := &App{cfg: &Config{RackBinaries: filepath.Join(dir, "bins")}} + for _, arch := range []string{"x/../../secret", "../secret", "amd64/../../secret", ""} { + if p, _, ok := a.rackBinary(arch); ok { + t.Errorf("arch %q served %s", arch, p) + } + } +} + +func TestNormMAC(t *testing.T) { + for in, want := range map[string]string{ + "52:54:00:AB:cd:01": "52:54:00:ab:cd:01", + "52-54-00-ab-cd-01": "52:54:00:ab:cd:01", + "../../etc/x": "", + "52:54:00:ab:cd:01\nx": "", + "": "", + } { + if got := normMAC(in); got != want { + t.Errorf("normMAC(%q) = %q, want %q", in, got, want) + } + } + if n := pxeFileName("../../../etc"); pxeNameRe.MatchString(n) { + t.Errorf("%q passes as a pxelinux file name", n) + } + if !pxeNameRe.MatchString(pxeFileName("52:54:00:ab:cd:01")) { + t.Error("a real MAC does not make a pxelinux file name") + } +} + +func TestValidDomain(t *testing.T) { + for _, ok := range []string{"node01", "node01.int.example.com", "a-b.c"} { + if !validDomain(ok) { + t.Errorf("%q refused", ok) + } + } + for _, bad := range []string{"", "*.int.example", "a b", "x\ndhcp-script=/x", "-a", "a..b", "Node01"} { + if validDomain(bad) { + t.Errorf("%q accepted", bad) + } + } + // a reservation name that is not a plain name never reaches DNS + if n := dnsNameIn(IPAddr{Hostname: "victim.int.example. 300 IN A 6.6.6.6 ;.int.example"}, nil, "int.example"); n != "" { + t.Errorf("dns name %q", n) + } +} + +func TestRedfishResolveStaysOnBMC(t *testing.T) { + r := &rfClient{base: "https://10.0.0.11:8443"} + for _, ok := range []string{"/redfish/v1/Systems/1", "/redfish/v1/Systems/1/", "https://10.0.0.11:8443/redfish/v1/TaskService/Tasks/5"} { + if _, err := r.resolve(ok); err != nil { + t.Errorf("%s: %v", ok, err) + } + } + for _, bad := range []string{"@evil.example/x", "https://evil.example/redfish/v1", "/api/x", "/redfish/../etc", "//evil.example/redfish/v1"} { + if p, err := r.resolve(bad); err == nil { + t.Errorf("%s resolved to %s", bad, p) + } + } + if b := bmcBase(&bmcCreds{Address: "https://u:p@bmc:8443/x/y"}, "https", 443); b != "https://invalid.invalid" { + t.Errorf("userinfo address gave %s", b) + } + if b := bmcBase(&bmcCreds{Address: "https://bmc:8443/x"}, "https", 443); b != "https://bmc:8443" { + t.Errorf("base %s", b) + } +} + +func TestIdentitiesOnlyFromTheRoot(t *testing.T) { + root, _ := newCA("test root") + ikey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + dns, nets, _ := parsePermits("int.example.com") + _, icert, err := root.intermediate(&ikey.PublicKey, "acme", dns, nets, time.Hour) + if err != nil { + t.Fatal(err) + } + k, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + _, direct, _ := root.sign(&k.PublicKey, "admin:alice", nil, x509.ExtKeyUsageClientAuth) + _, viaInter, _ := (&CA{Cert: icert, Key: ikey}).sign(&k.PublicKey, "admin:alice", nil, x509.ExtKeyUsageClientAuth) + r := httptest.NewRequest("GET", "/", nil) + r.TLS = &tls.ConnectionState{VerifiedChains: [][]*x509.Certificate{{direct, root.Cert}}} + if rootIssued(r) == nil { + t.Error("a root-issued certificate was not accepted") + } + r.TLS = &tls.ConnectionState{VerifiedChains: [][]*x509.Certificate{{viaInter, icert, root.Cert}}} + if rootIssued(r) != nil { + t.Error("a certificate from an intermediate passed as an identity") + } + if err := regionLeaf(tls.ConnectionState{VerifiedChains: [][]*x509.Certificate{{direct, root.Cert}}}); err == nil { + t.Error("an admin certificate passed as the region's") + } +} diff --git a/cmd/rollcall/server.go b/cmd/rollcall/server.go index 321aa5d..67af819 100644 --- a/cmd/rollcall/server.go +++ b/cmd/rollcall/server.go @@ -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() } diff --git a/cmd/rollcall/stateless.go b/cmd/rollcall/stateless.go index 6375f1a..8e33551 100644 --- a/cmd/rollcall/stateless.go +++ b/cmd/rollcall/stateless.go @@ -124,6 +124,9 @@ type nodeAssignment struct { } func (s *Store) Assign(ctx context.Context, id string, n nodeAssignment) error { + if !validDomain(n.Name) { + return fmt.Errorf("node name %q is not a host name (letters, digits, hyphens)", n.Name) + } tag, err := s.pool.Exec(ctx, `UPDATE machines SET profile=$2, node_name=$3, mgmt_ip=$4, ib_ip=$5, node_token_hash=$6, node_token=$7, state='stateless', booted_at=NULL WHERE id=$1`, id, n.Profile, n.Name, nilIfEmpty(n.MgmtIP), nilIfEmpty(n.IBIP), n.TokenHash, n.Token) @@ -379,6 +382,7 @@ var tmplFuncs = template.FuncMap{ "sub": func(a, b int) int { return a - b }, "pct": func(v, p int) int { return v * p / 100 }, "seq": func(n int) []int { + n = max(0, min(n, 4096)) // match rules run on every commissioning: no unbounded loops s := make([]int, n) for i := range s { s[i] = i @@ -743,6 +747,11 @@ func (a *App) handleAssign(w http.ResponseWriter, r *http.Request) { fail(w, 400, fmt.Errorf("profile is required")) return } + req.Name = strings.ToLower(req.Name) + if req.Name != "" && !validDomain(req.Name) { + fail(w, 400, fmt.Errorf("name %q is not a host name (letters, digits, hyphens)", req.Name)) + return + } p, err := a.store.Profile(r.Context(), req.Profile) if err != nil { fail(w, 404, fmt.Errorf("profile %s: %w", req.Profile, err)) @@ -893,7 +902,12 @@ func (a *App) handleRender(w http.ResponseWriter, r *http.Request) { if stage == "" { stage = "system" } - data, err := a.renderFile(r.Context(), r.PathValue("profile"), stage, path.Clean(r.PathValue("path"))) + rel := path.Clean("/" + r.PathValue("path"))[1:] // rooted first, so ".." cannot climb out of the overlay + if (stage != "system" && stage != "runtime") || rel == "" { + fail(w, 400, errors.New("stage is system or runtime, path a file in the overlay")) + return + } + data, err := a.renderFile(r.Context(), r.PathValue("profile"), stage, rel) if err != nil { fail(w, 500, err) return diff --git a/cmd/rollcall/web/app.js b/cmd/rollcall/web/app.js index de6c20d..f989d4b 100644 --- a/cmd/rollcall/web/app.js +++ b/cmd/rollcall/web/app.js @@ -281,7 +281,7 @@ function pageMachines() { ${esc(nameOf(m))}
${esc(m.mgmt_ip || m.id)} ${stateCell(m)} - ${m.location && (m.location.rack || m.location.room || m.location.building) ? `${esc([m.location.building, m.location.room].filter(Boolean).join(' / '))}
${esc(m.location.rack || '')}${m.location.unit ? ` U${m.location.unit}` : ''}` : '—'} + ${m.location && (m.location.rack || m.location.room || m.location.building) ? `${esc([m.location.building, m.location.room].filter(Boolean).join(' / '))}
${esc(m.location.rack || '')}${m.location.unit ? ` U${esc(m.location.unit)}` : ''}` : '—'} ${m.profile ? esc(m.profile) : m.os ? `${esc(m.os)}
${esc(m.os_version || '')}` : '—'} ${tagPills(m)} ${esc(((m.manufacturer || '') + ' ' + (m.product || '')).trim().slice(0, 26))}
${esc((m.serial || '').slice(0, 20))} @@ -446,7 +446,7 @@ function table(cols, rows) { return rows.length ? `${cols.map( function jobsTable(jobs, withMachine) { return table([...(withMachine ? ['Machine'] : []), 'Job', 'Kind', 'State', 'Target', 'Progress', 'Started', 'Finished', 'Error'], jobs.map(j => { const sp = j.spec || {}, p = j.progress || {}; const m = S.machines.find(x => x.id === j.machine_id); - const prog = (j.state === 'running' || j.state === 'pending') ? `
${esc(p.phase || '')} ${Math.round(p.pct || 0)}% ${esc((p.msg || '').slice(0, 60))}` : (p.details && p.details.seconds ? `${p.details.seconds}s` : ''); + const prog = (j.state === 'running' || j.state === 'pending') ? `
${esc(p.phase || '')} ${Math.round(p.pct || 0)}% ${esc((p.msg || '').slice(0, 60))}` : (p.details && p.details.seconds ? `${esc(p.details.seconds)}s` : ''); return [...(withMachine ? [`${esc(m ? nameOf(m) : j.machine_id)}`] : []), `${esc(j.id)}`, esc(j.kind), pill(j.state), `${esc(sp.os || '')}${sp.version ? ' ' + esc(sp.version) + '' : ''}${sp.disk ? ' → ' + esc(sp.disk) + '' : ''}`, prog, `${ago(j.started || j.created)}`, `${esc(when(j.finished))}`, j.error ? `${esc(j.error)}` : '']; })); } @@ -471,15 +471,15 @@ function pageMachine(d, tab) { ['BMC', b.address ? `${esc(b.address)} ${esc(b.user || '')} · ${d.has_bmc_secret ? 'credentials stored' : 'no credentials'}` : 'none'], ['Current job', job ? `${esc(job.id)} ${esc(job.kind)} ${pill(job.state)}${active ? `
${esc(p.phase || '')} ${Math.round(p.pct || 0)}% ${esc(p.msg || '')}` : ''}${job.error ? `
${esc(job.error)}` : ''}` : ''], ]))} - ${card(`Storage · ${st.disk_count ?? 0} disks · ${human(st.total_bytes)}`, table(['Dev', 'Size', 'Bus', 'Kind', 'Model', 'Serial', 'Health'], (st.disks || []).map(x => { const sm = (st.smart || {})[x.name] || {}; return [esc(x.name), human(x.size), esc(x.tran), x.rota === false ? 'ssd' : 'hdd', esc(x.model), `${esc(x.serial)}`, sm.healthy === true ? 'ok' : sm.healthy === false ? 'FAIL' : '']; })))} + ${card(`Storage · ${esc(st.disk_count ?? 0)} disks · ${human(st.total_bytes)}`, table(['Dev', 'Size', 'Bus', 'Kind', 'Model', 'Serial', 'Health'], (st.disks || []).map(x => { const sm = (st.smart || {})[x.name] || {}; return [esc(x.name), human(x.size), esc(x.tran), x.rota === false ? 'ssd' : 'hdd', esc(x.model), `${esc(x.serial)}`, sm.healthy === true ? 'ok' : sm.healthy === false ? 'FAIL' : '']; })))} ${card(`Network · ${(inv.network || []).length} NICs`, table(['If', 'MAC', 'Speed', 'Link', 'Switch port'], (inv.network || []).map(n => { const l = ((inv.lldp || {})[n.name]); const e = l ? (Array.isArray(l) ? l[0] : l) : null; const sw = e ? `${Object.keys(e.chassis || {})[0] || ''} ${((e.port || {}).id || {}).value || ''}` : ''; return [esc(n.name), `${esc(n.mac)}`, esc(n.speed), n.link === true ? 'up' : n.link === false ? 'down' : '', esc(sw)]; })))} ${card('Recent events', table(['When', 'Event', 'Detail'], (d.events || []).slice(-8).reverse().map(e => [`${ago(e.ts)}`, esc(e.event), `${esc(e.detail.slice(0, 90))}`])), true)} `; } else if (tab === 'hardware') { body = `
${card('CPU', kv([['Model', esc(cpu['Model name'])], ['Sockets × cores × threads', `${esc(cpu['Socket(s)'])} × ${esc(cpu['Core(s) per socket'])} × ${esc(cpu['Thread(s) per core'])} = ${esc(cpu['CPU(s)'])}`], ['NUMA nodes', esc(cpu['NUMA node(s)'])], ['Max MHz', esc(cpu['CPU max MHz'])], ['L3', esc(cpu['L3 cache'])], ['Virtualization', esc(cpu['Virtualization'])], ['Features', esc((cpu.features || []).join(' '))]]))} - ${card(`Memory ${human(mem.total_bytes)} · ${mem.dimms_populated ?? '?'}/${mem.dimm_slots ?? '?'} DIMMs`, table(['Slot', 'Size', 'Type', 'Speed', 'Vendor', 'Part', 'Serial'], (mem.dimms || []).filter(x => x.populated).map(x => [esc(x.locator), esc(x.size), esc(x.type), esc(x.configured_speed || x.speed), esc(x.manufacturer), `${esc(x.part_number)}`, `${esc(x.serial)}`])))} - ${card(`Storage · ${st.disk_count ?? 0} disks · ${human(st.total_bytes)}`, table(['Dev', 'Size', 'Bus', 'Kind', 'Model', 'Serial', 'FW', 'Hours', 'Health'], (st.disks || []).map(x => { const sm = (st.smart || {})[x.name] || {}; return [esc(x.name), human(x.size), esc(x.tran), x.rota === false ? 'ssd' : 'hdd', esc(x.model), `${esc(x.serial)}`, esc(sm.firmware), esc(sm.power_on_hours), sm.healthy === true ? 'ok' : sm.healthy === false ? 'FAIL' : '']; })) + ((st.controllers || []).length ? '

Controllers

' + table(['PCI', 'Vendor', 'Device', 'Driver'], st.controllers.map(c => [`${esc(c.slot)}`, esc(c.vendor), esc(c.device), esc(c.driver)])) : ''), true)} + ${card(`Memory ${human(mem.total_bytes)} · ${esc(mem.dimms_populated ?? '?')}/${esc(mem.dimm_slots ?? '?')} DIMMs`, table(['Slot', 'Size', 'Type', 'Speed', 'Vendor', 'Part', 'Serial'], (mem.dimms || []).filter(x => x.populated).map(x => [esc(x.locator), esc(x.size), esc(x.type), esc(x.configured_speed || x.speed), esc(x.manufacturer), `${esc(x.part_number)}`, `${esc(x.serial)}`])))} + ${card(`Storage · ${esc(st.disk_count ?? 0)} disks · ${human(st.total_bytes)}`, table(['Dev', 'Size', 'Bus', 'Kind', 'Model', 'Serial', 'FW', 'Hours', 'Health'], (st.disks || []).map(x => { const sm = (st.smart || {})[x.name] || {}; return [esc(x.name), human(x.size), esc(x.tran), x.rota === false ? 'ssd' : 'hdd', esc(x.model), `${esc(x.serial)}`, esc(sm.firmware), esc(sm.power_on_hours), sm.healthy === true ? 'ok' : sm.healthy === false ? 'FAIL' : '']; })) + ((st.controllers || []).length ? '

Controllers

' + table(['PCI', 'Vendor', 'Device', 'Driver'], st.controllers.map(c => [`${esc(c.slot)}`, esc(c.vendor), esc(c.device), esc(c.driver)])) : ''), true)} ${(inv.gpus || []).length ? card('GPUs / accelerators', table(['PCI', 'Vendor', 'Device', 'Driver'], inv.gpus.map(g => [`${esc(g.slot)}`, esc(g.vendor), esc(g.device), esc(g.driver)])), true) : ''} ${card('System', kv([['Board', esc([sys['baseboard-manufacturer'], sys['baseboard-product-name'], sys['baseboard-serial-number']].filter(Boolean).join(' / '))], ['SKU', esc(sys['system-sku-number'])], ['Chassis', esc([sys['chassis-type'], sys['chassis-serial-number'], sys['chassis-asset-tag']].filter(Boolean).join(' / '))], ['TPM', esc((inv.tpm || []).join(', ') || 'none')], ['Kernel (commissioning)', esc((inv.kernel || '').split(' (')[0])], ['Agent run', esc(inv.duration_s ? inv.duration_s + ' s' : '')]]))} ${(inv.pci || []).length ? card(`PCI devices · ${inv.pci.length}`, table(['Slot', 'Class', 'Vendor', 'Device', 'Driver'], inv.pci.map(p => [`${esc(p.slot)}`, esc(p.class), esc(p.vendor), esc(p.device), esc(p.driver)])), true) : ''} @@ -498,7 +498,7 @@ function pageMachine(d, tab) {
${card('BMC', kv([['Driver', `${esc(b.driver || 'ipmi')}${b.driver_set ? ' (set by operator)' : ''}${b.redfish ? ` · Redfish ${esc(b.redfish.redfish)} ${esc(b.redfish.manufacturer || '')} ${esc(b.redfish.model || '')}` : ''}`], ['Settings', b.extra && Object.keys(b.extra).length ? `${esc(Object.entries(b.extra).map(([k, v]) => k + '=' + v).join(' '))}` : ''], ['Source', esc(b.source)], ['Address', `${esc(b.address)}${b.port && b.port !== 623 ? ':' + esc(b.port) : ''}`], ['MAC', `${esc(b.mac)}`], ['User', esc(b.user)], ['Password', d.has_bmc_secret ? 'stored encrypted on the server' : 'none'], ['Configured', b.configured ? 'yes' : 'no'], ['Verified out-of-band', b.verified_oob === true ? 'yes' : b.verified_oob === false ? 'no' : ''], ['Channel', esc(b.channel)], ['LAN', b.lan ? `${esc(JSON.stringify(b.lan))}` : ''], ['Firmware', esc((b.mc_info || {})['Firmware Revision'])], ['Manufacturer', esc((b.mc_info || {})['Manufacturer Name'])], ['Errors', (b.errors || []).length ? `${esc(b.errors.join('; '))}` : '']]))} - ${(b.users_before || []).length ? card('User table before commissioning', table(['ID', 'Name', 'Priv', 'IPMI'], b.users_before.map(u => [u.id, esc(u.name), esc(u.priv), u.ipmi_msg ? 'yes' : 'no']))) : ''} + ${(b.users_before || []).length ? card('User table before commissioning', table(['ID', 'Name', 'Priv', 'IPMI'], b.users_before.map(u => [esc(u.id), esc(u.name), esc(u.priv), u.ipmi_msg ? 'yes' : 'no']))) : ''} ${b.fru ? card('FRU', `
${esc(JSON.stringify(b.fru, null, 1))}
`) : ''}
`; } else if (tab === 'jobs') body = jobsTable(d.jobs || [], false); else if (tab === 'events') body = table(['Time', 'Event', 'Detail'], (d.events || []).slice().reverse().map(e => [esc(when(e.ts)), esc(e.event), `${esc(e.detail)}`])); diff --git a/deploy/bootstrap.sh b/deploy/bootstrap.sh index 5096449..1db5aef 100755 --- a/deploy/bootstrap.sh +++ b/deploy/bootstrap.sh @@ -115,9 +115,9 @@ if [ -f "$CONF" ]; then else PGPASS=$(head -c 32 /dev/urandom | base64 | tr -dc A-Za-z0-9 | head -c 28) if [ "$(runuser -u postgres -- psql -tAc "select 1 from pg_roles where rolname='rollcall'")" = 1 ]; then - runuser -u postgres -- psql -q -c "ALTER ROLE rollcall PASSWORD '$PGPASS'" + printf "ALTER ROLE rollcall PASSWORD '%s';\n" "$PGPASS" | runuser -u postgres -- psql -q # stdin: not in ps else - runuser -u postgres -- psql -q -c "CREATE ROLE rollcall LOGIN PASSWORD '$PGPASS'" + printf "CREATE ROLE rollcall LOGIN PASSWORD '%s';\n" "$PGPASS" | runuser -u postgres -- psql -q fi [ "$(runuser -u postgres -- psql -tAc "select 1 from pg_database where datname='rollcall'")" = 1 ] || runuser -u postgres -- psql -q -c "CREATE DATABASE rollcall OWNER rollcall" ok "role and database rollcall" @@ -241,7 +241,11 @@ if [ -s $IMG/vmlinuz ] && [ -s $IMG/initrd.img ] && [ -s $IMG/filesystem.squashf ok "present, kept" elif [ -n "$IMAGE_URL" ]; then tmp=$(mktemp -d -p /srv/rollcall) - auth=(); [ -n "${IMAGE_TOKEN:-}" ] && auth=(-H "Authorization: token $IMAGE_TOKEN") + auth=() + if [ -n "${IMAGE_TOKEN:-}" ]; then # the token goes through a file, not the command line + printf 'Authorization: token %s\n' "$IMAGE_TOKEN" > "$tmp/.auth" && chmod 600 "$tmp/.auth" + auth=(-H "@$tmp/.auth") + fi # a release carries the image as commissioning-* (its plain SHA256SUMS is the binaries'); # a region's /rollcall directory has the plain names pre=commissioning- @@ -251,7 +255,7 @@ elif [ -n "$IMAGE_URL" ]; then fi for f in vmlinuz initrd.img filesystem.squashfs; do curl -fSL --progress-bar "${auth[@]}" -o "$tmp/$f" "$IMAGE_URL/$pre$f"; done (cd "$tmp" && sha256sum -c --quiet SHA256SUMS) || die "checksum mismatch" - mv "$tmp"/vmlinuz "$tmp"/initrd.img "$tmp"/filesystem.squashfs "$tmp"/SHA256SUMS $IMG/ && rm -rf "$tmp" + mv "$tmp"/vmlinuz "$tmp"/initrd.img "$tmp"/filesystem.squashfs "$tmp"/SHA256SUMS $IMG/; rm -rf "$tmp" chmod 0644 $IMG/* ok "downloaded and verified from $IMAGE_URL" elif [ $BUILD = 1 ]; then diff --git a/deploy/install-rack.sh b/deploy/install-rack.sh index bf4a0a6..fa30380 100755 --- a/deploy/install-rack.sh +++ b/deploy/install-rack.sh @@ -15,7 +15,7 @@ set -eu cd "$(dirname "$0")" [ -r /etc/rollcall/rack.json ] || { echo "enroll first: ./rollcall rack join CODE -url http://ADDR (code from: rollcall controller join-code on the region)" >&2; exit 1; } export DEBIAN_FRONTEND=noninteractive -apt-get install -y -q dnsmasq ipmitool pxelinux syslinux-common syslinux-efi jq >/dev/null +apt-get install -y -q dnsmasq ipmitool pxelinux syslinux-common syslinux-efi jq openssl >/dev/null getent passwd rollcall >/dev/null || useradd --system --no-create-home --shell /usr/sbin/nologin --user-group rollcall [ -x ./rollcall ] && install -m 0755 rollcall /usr/local/bin/rollcall @@ -48,7 +48,8 @@ install -d -o rollcall -g rollcall "$DNS" "$DNS/conf.d" for f in hosts hosts.dhcp; do [ -e "$DNS/$f" ] || install -m 0644 -o rollcall -g rollcall /dev/null "$DNS/$f"; done # bind-dynamic binds address by address, so systemd-resolved's 127.0.0.53 stub does not get in the way; # until the region assigns a VLAN only loopback is served -printf '# rollcall rack controller: the configuration is generated from the region'"'"'s network model\nbind-dynamic\ninterface=lo\nconf-dir=%s/conf.d/,*.conf\n' "$DNS" > /etc/dnsmasq.d/rollcall.conf +printf '# rollcall rack controller: the configuration is generated from the region'"'"'s network model\nbind-dynamic\ninterface=lo\nconf-dir=/var/lib/rollcall-dnsmasq/,*.conf\n' > /etc/dnsmasq.d/rollcall.conf +install -d -m 0755 /var/lib/rollcall-dnsmasq install -m 0755 rollcall-dnsmasq-reload.sh /usr/local/bin/rollcall-dnsmasq-reload.sh install -m 0755 rollcall-rack-update.sh /usr/local/bin/rollcall-rack-update.sh install -d -m 0755 -o rollcall -g rollcall /var/lib/rollcall/update @@ -57,6 +58,7 @@ install -m 0644 rollcall-dnsmasq.path rollcall-dnsmasq.service rollcall-rack.ser systemctl daemon-reload systemctl enable dnsmasq.service rollcall-dnsmasq.path rollcall-rack-update.path >/dev/null 2>&1 systemctl start rollcall-rack-update.path +/usr/local/bin/rollcall-dnsmasq-reload.sh >/dev/null 2>&1 || true systemctl restart dnsmasq.service || { journalctl -u dnsmasq -n 5 --no-pager >&2; exit 1; } systemctl start rollcall-dnsmasq.path systemctl enable --now rollcall-rack.service diff --git a/deploy/install.sh b/deploy/install.sh index e76487e..32cffff 100755 --- a/deploy/install.sh +++ b/deploy/install.sh @@ -39,9 +39,12 @@ for f in hosts hosts.dhcp; do [ -e /var/lib/rollcall/dnsmasq/$f ] || install -m install -m 0755 rollcall-dnsmasq-reload.sh /usr/local/bin/rollcall-dnsmasq-reload.sh install -m 0644 rollcall-dnsmasq.path rollcall-dnsmasq.service /etc/systemd/system/ # the embedded rack controller writes conf.d/rollcall.conf once a VLAN with dhcp is assigned to it; -# a hand-written dnsmasq configuration for the same interface has to go then (see README, Network) -if [ -d /etc/dnsmasq.d ] && [ ! -e /etc/dnsmasq.d/rollcall.conf ]; then - printf '# rollcall: generated from the network model (rollcall subnet / vlan)\nconf-dir=/var/lib/rollcall/dnsmasq/conf.d/,*.conf\n' > /etc/dnsmasq.d/rollcall.conf +# a hand-written dnsmasq configuration for the same interface has to go then (see README, Network). +# dnsmasq (root) reads the checked copy rollcall-dnsmasq-reload.sh keeps in a directory of root's. +install -d -m 0755 /var/lib/rollcall-dnsmasq +if [ -d /etc/dnsmasq.d ]; then + printf '# rollcall: generated from the network model (rollcall subnet / vlan), checked by rollcall-dnsmasq-reload.sh\nconf-dir=/var/lib/rollcall-dnsmasq/,*.conf\n' > /etc/dnsmasq.d/rollcall.conf + /usr/local/bin/rollcall-dnsmasq-reload.sh >/dev/null 2>&1 || true fi legacy=$(grep -lE '^(dhcp-range|bind-interfaces|enable-tftp)' /etc/dnsmasq.conf /etc/dnsmasq.d/*.conf 2>/dev/null | grep -v '/rollcall.conf$' || true) if [ -n "$legacy" ]; then diff --git a/deploy/publish-image.sh b/deploy/publish-image.sh index e07fec1..2963a82 100755 --- a/deploy/publish-image.sh +++ b/deploy/publish-image.sh @@ -8,15 +8,17 @@ TAG=${1:?usage: publish-image.sh TAG [API]} API=${2:-https://gitea.mynodes.xyz/api/v1/repos/ilker/rollcall} IMG=${IMG:-/srv/rollcall/image} : "${GITEA_TOKEN:?set GITEA_TOKEN (write:repository)}" +HDR=$(mktemp); trap 'rm -f "$HDR"' EXIT +printf 'Authorization: token %s\n' "$GITEA_TOKEN" > "$HDR" # not on curl's command line (ps) cd "$IMG" sha256sum vmlinuz initrd.img filesystem.squashfs > SHA256SUMS -id=$(curl -fsS -H "Authorization: token $GITEA_TOKEN" "$API/releases/tags/$TAG" | python3 -c 'import json,sys;print(json.load(sys.stdin)["id"])') -existing=$(curl -fsS -H "Authorization: token $GITEA_TOKEN" "$API/releases/$id/assets" | python3 -c 'import json,sys;[print(a["id"],a["name"]) for a in json.load(sys.stdin)]') +id=$(curl -fsS -H "@$HDR" "$API/releases/tags/$TAG" | python3 -c 'import json,sys;print(json.load(sys.stdin)["id"])') +existing=$(curl -fsS -H "@$HDR" "$API/releases/$id/assets" | python3 -c 'import json,sys;[print(a["id"],a["name"]) for a in json.load(sys.stdin)]') for f in vmlinuz initrd.img filesystem.squashfs SHA256SUMS; do name=commissioning-$f old=$(echo "$existing" | awk -v n="$name" '$2==n{print $1}') - [ -n "$old" ] && curl -fsS -X DELETE -H "Authorization: token $GITEA_TOKEN" "$API/releases/$id/assets/$old" >/dev/null + [ -n "$old" ] && curl -fsS -X DELETE -H "@$HDR" "$API/releases/$id/assets/$old" >/dev/null echo "uploading $name ($(du -h "$f" | cut -f1))" - curl -fsS -H "Authorization: token $GITEA_TOKEN" -F "attachment=@$f" "$API/releases/$id/assets?name=$name" >/dev/null + curl -fsS -H "@$HDR" -F "attachment=@$f" "$API/releases/$id/assets?name=$name" >/dev/null done echo "done: bootstrap.sh --image-url ${API%/api/v1/repos/*}/$(echo "$API" | sed 's#.*/repos/##')/releases/download/$TAG" diff --git a/deploy/rollcall-dnsmasq-reload.sh b/deploy/rollcall-dnsmasq-reload.sh index 9624ef1..9c0ea1a 100644 --- a/deploy/rollcall-dnsmasq-reload.sh +++ b/deploy/rollcall-dnsmasq-reload.sh @@ -7,29 +7,61 @@ # some of those), the last configuration that worked is put back and dnsmasq # is started with it, so DHCP stays up. The rejected content is kept next to it # and not tried again until it changes. +# +# rollcall (an unprivileged user) writes $SRC; dnsmasq starts as root and reads +# only the copy in $LIVE, a directory of root's. The copy is made without +# following links and holds only the options rollcall generates: nothing that +# runs a program, includes another file or changes dnsmasq's user. set -u -D=/var/lib/rollcall/dnsmasq -CONF=$D/conf.d/rollcall.conf -GOOD=$D/rollcall.conf.accepted -BAD=$D/rollcall.conf.rejected +SRC=/var/lib/rollcall/dnsmasq/conf.d/rollcall.conf +LIVE=/var/lib/rollcall-dnsmasq +CONF=$LIVE/rollcall.conf +GOOD=$LIVE/rollcall.conf.accepted +BAD=$LIVE/rollcall.conf.rejected STAMP=/run/rollcall-dnsmasq.sum +ALLOWED='addn-hosts bind-dynamic bogus-priv dhcp-authoritative dhcp-boot dhcp-hostsfile dhcp-match dhcp-option dhcp-range domain domain-needed enable-ra enable-tftp expand-hosts interface local log-dhcp no-resolv pxe-service ra-param server tftp-root' +install -d -m 0755 -o root -g root "$LIVE" +umask 022 sum() { cat "$1" 2>/dev/null | sha256sum | cut -d' ' -f1; } +put() { cp "$1" "$CONF.tmp" && mv -f "$CONF.tmp" "$CONF"; } # within $LIVE: root's restore() { # put the last working configuration back (or none) - if [ -s "$GOOD" ]; then cp "$GOOD" "$CONF.tmp" && mv -f "$CONF.tmp" "$CONF"; else rm -f "$CONF"; fi + if [ -s "$GOOD" ]; then put "$GOOD"; else rm -f "$CONF"; fi sum "$CONF" > "$STAMP" } -new=$(sum "$CONF") -if [ "$new" = "$(cat "$STAMP" 2>/dev/null)" ]; then +# take rollcall's file: a regular file, not a link, at most 1 MB +cand=$LIVE/candidate.tmp +rm -f "$cand" +if [ -e "$SRC" ]; then + if ! dd if="$SRC" of="$cand" iflag=nofollow bs=1M count=1 status=none 2>/dev/null; then + echo "$SRC is not a regular file; ignored" >&2 + exit 1 + fi + printf '%s\n' $ALLOWED > "$LIVE/allowed.tmp" + bad=$(grep -vE '^[[:space:]]*(#|$)' "$cand" | sed -E 's/^[[:space:]]*([^=[:space:]]*).*/\1/' | sort -u | + grep -vxFf "$LIVE/allowed.tmp" | tr '\n' ' ') + rm -f "$LIVE/allowed.tmp" + if [ -n "$bad" ]; then + rm -f "$cand" + echo "generated dnsmasq configuration has options rollcall does not write ($bad); not used" >&2 + exit 1 + fi +fi + +new=$(sum "$cand") +if [ "$new" = "$(cat "$STAMP" 2>/dev/null)" ] && { [ -e "$CONF" ] || [ ! -e "$cand" ]; }; then + rm -f "$cand" systemctl kill -s HUP dnsmasq.service exit 0 fi -if [ -e "$CONF" ] && [ "$new" = "$(sum "$BAD")" ]; then +if [ -e "$cand" ] && [ "$new" = "$(sum "$BAD")" ]; then + rm -f "$cand" restore echo "generated dnsmasq configuration was rejected before and has not changed; keeping the last working one" >&2 exit 1 fi +if [ -e "$cand" ]; then mv -f "$cand" "$CONF"; else rm -f "$CONF"; fi if out=$(dnsmasq --test 2>&1) && systemctl restart dnsmasq.service; then echo "$new" > "$STAMP" if [ -e "$CONF" ]; then cp "$CONF" "$GOOD"; else rm -f "$GOOD"; fi diff --git a/deploy/rollcall-rack-update.sh b/deploy/rollcall-rack-update.sh index a211200..51125d3 100755 --- a/deploy/rollcall-rack-update.sh +++ b/deploy/rollcall-rack-update.sh @@ -1,27 +1,39 @@ #!/bin/sh # Installs a rollcall binary the rack controller staged (rollcall-rack-update.path). -# The rack checked the region's checksum and the binary's version; this checks -# them again, keeps the running binary as .prev, restarts the rack and puts the -# previous binary back if the rack does not register with the region in 60 s. +# +# The staging directory belongs to the rack's unprivileged user, so nothing in +# it is trusted: the binary is copied into a directory of root's and installed +# only if the region's signature over it verifies against the CA certificate +# written at join (root's, from rack.json ca_file). Then the running binary is +# kept as .prev, the rack restarted, and the previous binary put back if the +# rack does not register with the region in 60 s. set -u D=/var/lib/rollcall/update BIN=/usr/local/bin/rollcall [ -f "$D/ready" ] || exit 0 -want=$(cat "$D/ready"); rm -f "$D/ready" -cd "$D" || exit 1 -result() { echo "$(date -Is) $*" > "$D/last-result"; echo "$*"; } -sha256sum -c rollcall.new.sha256 >/dev/null 2>&1 || { result "$want: checksum mismatch, not installed"; exit 1; } -got=$(./rollcall.new version 2>/dev/null | awk '{print $2}') +want=$(head -c 64 "$D/ready" | tr -cd 'A-Za-z0-9._+-'); rm -f "$D/ready" +W=$(mktemp -d /run/rollcall-update.XXXXXX) || exit 1 +trap 'rm -rf "$W"' EXIT +# last-result is read by the rack (backoff); renamed in, so a link planted there is replaced, not followed +result() { echo "$(date -Is) $*" > "$W/result" && mv -fT "$W/result" "$D/last-result"; echo "$*"; } +CA=$(jq -r '.ca_file // "/etc/rollcall/rack/ca.pem"' /etc/rollcall/rack.json 2>/dev/null) +[ -n "$CA" ] && [ "$(stat -c %u "$CA" 2>/dev/null)" = 0 ] || { result "$want: no root-owned CA certificate ($CA), not installed"; exit 1; } +cp -- "$D/rollcall.new" "$W/bin" && cp -- "$D/rollcall.new.sig" "$W/sig" || { result "$want: staged files missing, not installed"; exit 1; } +openssl x509 -in "$CA" -pubkey -noout > "$W/pub" 2>/dev/null && + openssl dgst -sha256 -verify "$W/pub" -signature "$W/sig" "$W/bin" >/dev/null 2>&1 || + { result "$want: signature does not verify against $CA, not installed"; exit 1; } +chmod 0755 "$W/bin" +got=$("$W/bin" version 2>/dev/null | awk '{print $2}') [ "$got" = "$want" ] || { result "$want: binary reports '$got', not installed"; exit 1; } cur=$("$BIN" version 2>/dev/null | awk '{print $2}') cp -p "$BIN" "$BIN.prev" -install -m 0755 rollcall.new "$BIN.tmp" && mv -f "$BIN.tmp" "$BIN" +install -m 0755 "$W/bin" "$BIN.tmp" && mv -f "$BIN.tmp" "$BIN" +rm -f "$D/rollcall.new" "$D/rollcall.new.sha256" "$D/rollcall.new.sig" since=$(date +%s) systemctl restart rollcall-rack.service for _ in $(seq 1 30); do sleep 2 if journalctl -u rollcall-rack.service --since "@$since" -o cat 2>/dev/null | grep -q "^.*registered with"; then - rm -f rollcall.new rollcall.new.sha256 result "updated $cur -> $want" exit 0 fi