webapp: fix review findings (imaging types, NIC status, iPXE gates)

- imaging: coerce client payload and stored session fields to str/int,
  drop lists/dicts; tiles template guards pctype/current_stage; POST
  rejects non-object bodies and non-string serials.
- nic_status: normalise every check-file entry; load nic.conf as-is with
  a [nic] fallback; split interface= lists; keep the check's binding while
  fresh, resolve wildcard sockets from the dnsmasq pin (never True for an
  excluded NIC); mark vanished NICs down; include role bridges; stale or
  future-dated files downgrade to warnings and drop evidence.
- Overview renders the network pill in its own try and shows
  'Network: unknown' when status fails.
- ipxe_update: read every Authenticode signature (all WIN_CERTIFICATE
  entries plus nested SPC_NESTED_SIGNATURE) with openssl; shim swap and
  install need both Microsoft UEFI CA 2011 and 2023; install re-reads
  each file (snponly ipxe.efi only); staged.json schema version, older
  records shown stale and refused; uhci counts as native USB.
- backups describe their files; risky rollbacks need a confirm flag.
- mac_folders: guarded listdir, cached describe, configured TFTP root,
  warning for a shim without ipxe.efi.
- evidence pack: years 2000..2100, 400 days inclusive.
- a11y role=group on quick ranges; progress bar stays full during 'shim'.
- tests/test_review_fixes.py: 29 unittest cases incl. both real shims.

webapp/services/imaging_status.py

@@ -39,6 +39,110 @@ ACTIVE_WINDOW_HOURS = 6
 _SAFE_SERIAL = re.compile(r"[^A-Za-z0-9._-]")
 
 
+# Payload and stored-session field types. The pages compare and look these
+# up (`pctype in {...}`, `'%]' in current_stage`), so a list or dict from a
+# client or a hand-edited file must never reach them.
+INT_FIELDS = ("stage_index", "stage_total")
+# Written by this module only; a client cannot set them.
+SERVER_FIELDS = ("log_tail", "stage_history", "started_at", "last_updated", "previous_run_at")
+STR_MAX = 4096
+LOG_LINE_MAX = 2000
+_SAFE_KEY = re.compile(r"[A-Za-z0-9_]{1,64}")
+
+
+def _scalar_str(v, limit=STR_MAX):
+    """str for a scalar, None for anything else (dict, list, None)."""
+    if isinstance(v, str):
+        return v[:limit]
+    if isinstance(v, bool):
+        return "true" if v else "false"
+    if isinstance(v, (int, float)):
+        return str(v)
+    return None
+
+
+def _as_int(v):
+    if isinstance(v, bool):
+        return None
+    try:
+        return int(v)
+    except (TypeError, ValueError, OverflowError):
+        return None
+
+
+def _str_lines(v):
+    """A str or a list of scalars as a list of str lines; [] otherwise."""
+    if isinstance(v, str):
+        v = [v]
+    if not isinstance(v, list):
+        return []
+    out = []
+    for x in v:
+        x = _scalar_str(x, LOG_LINE_MAX)
+        if x is not None:
+            out.append(x)
+    return out
+
+
+def _clean_payload(payload: dict) -> dict:
+    """A client payload with every value in the type the pages expect:
+    stage_index/stage_total int, log_lines a list of str, everything else a
+    str. Non-scalar values and server-owned fields are dropped."""
+    out = {}
+    for key, value in payload.items():
+        if not isinstance(key, str) or not _SAFE_KEY.fullmatch(key) or key in SERVER_FIELDS:
+            continue
+        if key in INT_FIELDS:
+            n = _as_int(value)
+            if n is not None:
+                out[key] = n
+        elif key == "log_lines":
+            lines = _str_lines(value)
+            if lines:
+                out[key] = lines
+        else:
+            v = _scalar_str(value)
+            if v is not None:
+                out[key] = v
+    return out
+
+
+def sanitize_session(state) -> Optional[dict]:
+    """A stored session made safe to render: same rules as a payload, plus
+    log_tail (list of str) and stage_history (list of dicts). None when it is
+    not a JSON object or has no usable serial."""
+    if not isinstance(state, dict):
+        return None
+    out = {}
+    for key, value in state.items():
+        if not isinstance(key, str):
+            continue
+        if key in INT_FIELDS:
+            n = _as_int(value)
+            if n is not None:
+                out[key] = n
+        elif key == "log_tail":
+            out[key] = _str_lines(value)[-LOG_TAIL_MAX:]
+        elif key == "stage_history":
+            hist = []
+            for h in value if isinstance(value, list) else []:
+                if isinstance(h, dict):
+                    hist.append({"ts": _scalar_str(h.get("ts")) or "",
+                                 "stage_index": _as_int(h.get("stage_index")) or 0,
+                                 "current_stage": _scalar_str(h.get("current_stage")) or "",
+                                 "status": _scalar_str(h.get("status")) or ""})
+            out[key] = hist[-STAGE_HISTORY_MAX:]
+        elif key == "log_lines":
+            continue
+        else:
+            v = _scalar_str(value)
+            if v is not None:
+                out[key] = v
+    if not out.get("serial"):
+        return None
+    return out
+
+
 def _ensure_dir():
     os.makedirs(config.IMAGING_DIR, exist_ok=True)
 
@@ -83,9 +187,15 @@ def update_session(payload: dict) -> dict:
     present overwrites the existing field. log_tail is appended to (capped).
     Returns the resulting full state.
     """
+    if not isinstance(payload, dict):
+        raise ValueError("payload must be a JSON object")
+    if not isinstance(payload.get("serial") or "", str):
+        raise ValueError("serial must be a string")
+    payload = _clean_payload(payload)
     serial = (payload.get("serial") or "").strip()
     if not serial:
         raise ValueError("payload missing 'serial'")
+    payload["serial"] = serial
     # "inferred" is the server's own label for log-only bays; a client that
     # sends it would hide its row from the floor's failure-first order.
     if payload.get("source") not in (None, "client", "blancco"):
@@ -101,8 +211,8 @@ def update_session(payload: dict) -> dict:
     if os.path.isfile(path):
         try:
             with open(path, "r") as f:
-                state = json.load(f)
-        except (json.JSONDecodeError, OSError):
+                state = sanitize_session(json.load(f)) or {}
+        except (ValueError, OSError):
             state = {}
 
     # Reimage detection: if the new payload's stage_index <= 2 (WinPE startnet
@@ -216,9 +326,11 @@ def list_sessions(include_inferred: bool = True) -> list[dict]:
         path = os.path.join(config.IMAGING_DIR, name)
         try:
             with open(path, "r") as f:
-                out.append(json.load(f))
-        except (json.JSONDecodeError, OSError):
+                state = sanitize_session(json.load(f))
+        except (ValueError, OSError):
             continue
+        if state is not None:  # a broken or hand-edited file must not 500 the pages
+            out.append(state)
     if include_inferred:
         from services import imaging_log_tail
         out = imaging_log_tail.merge_with_client_sessions(out)
@@ -292,8 +404,8 @@ def get_session(serial: str) -> Optional[dict]:
         return None
     try:
         with open(path, "r") as f:
-            return json.load(f)
-    except (json.JSONDecodeError, OSError):
+            return sanitize_session(json.load(f))
+    except (ValueError, OSError):
         return None
 
 

webapp/services/ipxe_update.py

@@ -24,8 +24,10 @@ https://github.com/ipxe/ipxe/releases/latest/download/ipxeboot.tar.gz, with
 the signed x86 files in ipxeboot/x86_64-sb/. GitHub lists a sha256 digest per
 release asset, which is checked before anything is unpacked, and every EFI
 file's Authenticode chain is read with sbverify before it can be installed.
-The chain check is by certificate name (sbverify --list); the cryptographic
-trust comes from the GitHub digest. Do not swap in `sbverify --cert`: it
+The chain check is by certificate name (sbverify --list, plus every
+signature in the PE security directory, nested ones included, read with
+openssl for the shim's Microsoft CAs); the cryptographic trust comes from
+the GitHub digest. Do not swap in `sbverify --cert`: it
 prints "Signature verification OK" even for a missing or unrelated cert.
 
 Why it matters: the Broadcom build, and the shim in the iPXE tarball, are
@@ -85,6 +87,12 @@ NEED_LABEL = {"microsoft": "a Microsoft UEFI CA", "ipxe": "the iPXE Secure Boot
 MS_CA_2011 = "Microsoft Corporation UEFI CA 2011"
 MS_CA_2023 = "Microsoft UEFI CA 2023"
 IPXE_CA = "iPXE Secure Boot CA"
+MS_CAS = (MS_CA_2011, MS_CA_2023)
+
+# Bumped when the staged.json layout or the checks behind it change. A
+# record written by an older version was verified by weaker checks and is
+# shown as stale (download it again) and never installed.
+STAGED_SCHEMA = 2
 
 
 def _dir():
@@ -115,6 +123,111 @@ def signers(path):
     return pairs
 
 
+# Authenticode nested signature (SPC_NESTED_SIGNATURE, 1.3.6.1.4.1.311.2.4.1),
+# DER-encoded OID. A second signature can sit in a second WIN_CERTIFICATE
+# entry or nested inside the first one's unsigned attributes; sbverify may
+# list only the outer one, so both places are read here.
+NESTED_SIG_OID = bytes.fromhex("060a2b060104018237020401")
+WIN_CERT_TYPE_PKCS_SIGNED_DATA = 0x0002
+
+
+def _der_tlv(buf, i):
+    """(content start, content end) of the DER element at buf[i]; raises
+    ValueError when it runs past the buffer."""
+    if i + 2 > len(buf):
+        raise ValueError("short DER")
+    n = buf[i + 1]
+    if n < 0x80:
+        start, length = i + 2, n
+    else:
+        k = n & 0x7F
+        if not 1 <= k <= 4 or i + 2 + k > len(buf):
+            raise ValueError("bad DER length")
+        start, length = i + 2 + k, int.from_bytes(buf[i + 2:i + 2 + k], "big")
+    if start + length > len(buf):
+        raise ValueError("DER runs past the buffer")
+    return start, start + length
+
+
+def _pe_signatures(data):
+    """The PKCS#7 blobs in a PE file's security directory, [] if none."""
+    try:
+        pe = struct.unpack_from("<I", data, 0x3C)[0]
+        if data[pe:pe + 4] != b"PE\0\0":
+            return []
+        magic = struct.unpack_from("<H", data, pe + 24)[0]
+        dirs = pe + 24 + (96 if magic == 0x10B else 112)
+        off, size = struct.unpack_from("<II", data, dirs + 4 * 8)
+    except struct.error:
+        return []
+    out, i, end = [], off, min(off + size, len(data))
+    while off and i + 8 <= end:
+        length, _rev, ctype = struct.unpack_from("<IHH", data, i)
+        if length < 8:
+            break
+        if ctype == WIN_CERT_TYPE_PKCS_SIGNED_DATA:
+            blob = data[i + 8:min(i + length, end)]
+            try:
+                out.append(blob[:_der_tlv(blob, 0)[1]])  # drop the 8-byte padding
+            except ValueError:
+                pass
+        i += (length + 7) & ~7
+    return out
+
+
+def _nested_signatures(blob, depth=0):
+    """ContentInfo blobs nested in `blob` under SPC_NESTED_SIGNATURE, deep
+    nests included."""
+    out = []
+    pos = blob.find(NESTED_SIG_OID)
+    while pos >= 0 and depth < 4:
+        try:
+            i = pos + len(NESTED_SIG_OID)
+            if blob[i] == 0x31:  # SET OF ContentInfo
+                start, end = _der_tlv(blob, i)
+                j = start
+                while j < end:
+                    _cs, ce = _der_tlv(blob, j)
+                    inner = blob[j:ce]
+                    if blob[j] == 0x30:
+                        out.append(inner)
+                        out += _nested_signatures(inner, depth + 1)
+                    j = ce
+        except (ValueError, IndexError):
+            pass
+        pos = blob.find(NESTED_SIG_OID, pos + 1)
+    return out
+
+
+def _cert_issuers(blob):
+    """Issuer CNs of every certificate in one PKCS#7 blob (openssl)."""
+    r = subprocess.run(["openssl", "pkcs7", "-inform", "DER", "-print_certs", "-noout"],
+                       input=blob, capture_output=True, timeout=30)
+    text = r.stdout.decode("utf-8", errors="replace")
+    return {m.strip() for m in re.findall(r"^issuer=.*?CN\s*=\s*([^,/\n]+)", text, re.M)}
+
+
+def microsoft_cas(path):
+    """Which of the Microsoft UEFI CAs (2011, 2023) sign this file, looking
+    at every signature: each WIN_CERTIFICATE entry and any nested inside one.
+    Raises FileNotFoundError when openssl is not installed."""
+    try:
+        with open(path, "rb") as f:
+            data = f.read()
+    except OSError:
+        return []
+    issuers = set()
+    for blob in _pe_signatures(data):
+        for b in [blob] + _nested_signatures(blob):
+            issuers |= _cert_issuers(b)
+    return [ca for ca in MS_CAS if ca in issuers]
+
+
+def dual_signed(info):
+    """True when a described file carries both Microsoft UEFI CAs."""
+    return bool(info) and all(ca in (info.get("ms_cas") or []) for ca in MS_CAS)
+
+
 def signature_kind(pairs):
     """microsoft-2023 | microsoft-2011 | microsoft | ipxe | other | unsigned.
     A file signed under both Microsoft CAs counts as 2023."""
@@ -221,29 +334,74 @@ def drivers_of(path):
         return ""
     if b"iPXE" not in data or b"$Version: " in data:
         return ""  # not iPXE, or a shim
-    return "native" if re.search(rb"\0xhci\0|\0ehci\0", data) else "firmware"
+    return "native" if re.search(rb"\0(?:xhci|ehci|uhci)\0", data) else "firmware"
+
+
+_DESCRIBE_CACHE = {}
 
 
 def describe(path):
+    """Size, hash, version, signers and drivers of one boot file, or None.
+    Cached per (path, size, mtime): reading and sbverify-ing every file on
+    each page load is slow. Returns a copy the caller may change."""
+    try:
+        st = os.stat(path)
+    except OSError:
+        return None
     if not os.path.isfile(path):
         return None
-    st = os.stat(path)
+    key = (os.path.abspath(path), st.st_size, st.st_mtime_ns)
+    hit = _DESCRIBE_CACHE.get(key)
+    if hit is None:
+        hit = _describe(path, st)
+        if len(_DESCRIBE_CACHE) > 256:
+            _DESCRIBE_CACHE.clear()
+        _DESCRIBE_CACHE[key] = hit
+    return dict(hit)
+
+
+def _describe(path, st):
     efi = path.endswith(".efi")
-    pairs, error = [], ""
+    pairs, error, ms_cas = [], "", []
     if efi:
         try:
             pairs = signers(path)
         except (OSError, subprocess.SubprocessError) as ex:
             error = f"sbverify failed: {ex}"
-    return {
+    kind = (signature_kind(pairs) if not error else "error") if efi else "bios"
+    if kind.startswith("microsoft"):
+        try:
+            ms_cas = microsoft_cas(path)
+        except (OSError, subprocess.SubprocessError) as ex:
+            error = f"openssl failed: {ex}"
+            kind = "error"
+    info = {
         "size": st.st_size, "modified": st.st_mtime,
         "sha256": uc.file_hash(path), "version": version_of(path),
         "sbat": sbat_generations(path) if efi else "",
-        "signers": pairs, "kind": (signature_kind(pairs) if not error else "error") if efi else "bios",
+        "signers": pairs, "kind": kind,
         "trusts": vendor_cert_cn(path) if efi else None,
         "drivers": drivers_of(path) if efi else "",
-        "sig_error": error,
+        "sig_error": error, "ms_cas": ms_cas,
     }
+    info["dual"] = dual_signed(info)
+    return info
+
+
+def risks(name, info):
+    """Why a boot file should not go live, [] when none: an ipxe.efi that is
+    not the snponly (firmware drivers) build, or a shim that does not carry
+    both Microsoft UEFI CAs."""
+    if not info:
+        return []
+    if name == "ipxe.efi" and info.get("drivers") != "firmware":
+        if info.get("drivers") == "native":
+            return ["ipxe.efi drives USB itself (not snponly): USB keyboards stop working at the iPXE menu"]
+        return ["ipxe.efi is not recognised as an iPXE snponly build"]
+    if name in SHIMS and not info.get("dual"):
+        have = " and ".join(info.get("ms_cas") or []) or info.get("kind") or "no Microsoft CA"
+        return [f"{name} is not dual-signed (2011 + 2023): it carries {have}"]
+    return []
 
 
 def _installed_log():
@@ -289,14 +447,25 @@ def mac_folders():
         folder = os.path.join(config.TFTP_ROOT, d)
         if not (MAC_DIR_RE.match(d) and os.path.isdir(folder)):
             continue
-        files = []
-        for f in sorted(os.listdir(folder)):
+        files, warnings = [], []
+        try:
+            entries = sorted(os.listdir(folder))
+        except OSError as ex:
+            entries = []
+            warnings.append(f"cannot read the folder: {ex.strerror or ex}")
+        for f in entries:
             info = describe(os.path.join(folder, f))
             if info:
                 files.append({"name": f, "info": info})
         by = {f["name"]: f["info"] for f in files}
-        _check_chain(next((by[n] for n in SHIMS if n in by), None), [by.get("ipxe.efi")])
-        out.append({"mac": d.replace("-", ":"), "dir": d, "files": files})
+        shim = next((by[n] for n in SHIMS if n in by), None)
+        if shim is None:
+            shim = next((f["info"] for f in files if (f["info"].get("version") or "").startswith("shim ")), None)
+        _check_chain(shim, [by.get("ipxe.efi")])
+        if shim is not None and "ipxe.efi" not in by:
+            warnings.append("This folder has a shim but no ipxe.efi. The shim loads ipxe.efi from this folder "
+                            "(no fallback to the root), so this client stops after the shim.")
+        out.append({"mac": d.replace("-", ":"), "dir": d, "files": files, "warnings": warnings})
     return out
 
 
@@ -530,10 +699,21 @@ def stage(info=None):
     if shim.get("ok") and ipxe.get("ok") and shim.get("trusts"):
         if shim["trusts"] not in {i for _, i in ipxe["signers"]}:
             ipxe.update(ok=False, error=f"not signed under {shim['trusts']}, which this shim trusts")
-    if ipxe.get("ok") and ipxe.get("drivers") == "native":
-        # The full build would kill USB keyboards on every client.
-        ipxe.update(ok=False, error="this build drives USB itself (not snponly); refusing it as ipxe.efi")
-    record = {"tag": info["tag"], "published": info.get("published"), "folder": folder,
+    if ipxe.get("ok") and ipxe.get("drivers") != "firmware":
+        # The full build would kill USB keyboards on every client; anything
+        # not recognised as snponly is refused too.
+        ipxe.update(ok=False, error=("this build drives USB itself (not snponly); refusing it as ipxe.efi"
+                                     if ipxe.get("drivers") == "native" else
+                                     "not recognised as an iPXE snponly build; refusing it as ipxe.efi"))
+    for n in SHIMS:
+        f = files.get(n) or {}
+        if f.get("ok") and not f.get("dual"):
+            # Kept on disk and shown, but never installed: a 2011-only shim
+            # stops booting once machines drop or revoke the 2011 CA.
+            have = " and ".join(f.get("ms_cas") or []) or f.get("kind")
+            f.update(ok=False, error=f"not dual-signed (needs {MS_CA_2011} and {MS_CA_2023}; has {have}). "
+                                     + (f.get("note") or ""))
+    record = {"schema": STAGED_SCHEMA, "tag": info["tag"], "published": info.get("published"), "folder": folder,
               "tarball_sha256": info["sha256"], "files": files, "staged": uc.now()}
     uc.write_json(os.path.join(folder, "staged.json"), record)
     return record
@@ -541,10 +721,11 @@ def stage(info=None):
 
 def _use_dual_signed_shim(folder, files):
     """Swap the tarball's shim for the ipxe/shim release build when that one
-    is signed under the Microsoft UEFI CA 2023 and trusts the same iPXE CA.
-    Any problem leaves the tarball shim in place, with the reason noted."""
+    carries both Microsoft UEFI CAs (2011 and 2023, read from every
+    signature, nested ones included) and trusts the same iPXE CA. Any
+    problem leaves the tarball shim in place, with the reason noted."""
     tar_shim = files.get("ipxe-shim.efi") or {}
-    if not tar_shim.get("ok") or tar_shim.get("kind") == "microsoft-2023":
+    if not tar_shim.get("ok") or tar_shim.get("dual"):
         return
     try:
         rel = json.loads(uc.fetch_text(SHIM_API, timeout=20))
@@ -559,8 +740,12 @@ def _use_dual_signed_shim(folder, files):
                 os.remove(dest)
             uc.download(asset["browser_download_url"], dest, expect=sha, timeout=300)
         d = describe(dest)
-        if d["kind"] != "microsoft-2023":
-            raise RuntimeError(f"{SHIM_ASSET} is {d['kind']}, not signed under {MS_CA_2023}")
+        if d["sig_error"]:
+            raise RuntimeError(f"{SHIM_ASSET}: {d['sig_error']}")
+        if not d["dual"]:
+            have = " and ".join(d["ms_cas"]) or d["kind"]
+            raise RuntimeError(f"{SHIM_ASSET} is not signed under both {MS_CA_2011} and {MS_CA_2023} "
+                               f"(found {have})")
         if d.get("trusts") != tar_shim.get("trusts"):
             raise RuntimeError(f"{SHIM_ASSET} trusts {d.get('trusts')}, the tarball's iPXE needs "
                                f"{tar_shim.get('trusts')}")
@@ -574,7 +759,18 @@ def _use_dual_signed_shim(folder, files):
         if n in files:
             shutil.copy2(dest, os.path.join(folder, n))
             files[n] = dict(d, ok=True, error="", source=f"github.com/ipxe/shim {rel.get('tag_name')} "
-                            f"{SHIM_ASSET} (dual-signed, 2011 + 2023)")
+                            f"{SHIM_ASSET} (dual-signed: {' + '.join(d['ms_cas'])})")
+
+
+def _record_stale(rec):
+    try:
+        schema = int(rec.get("schema") or 0)
+    except (TypeError, ValueError):
+        schema = 0
+    if schema < STAGED_SCHEMA:
+        return True
+    return any(isinstance(f, dict) and f.get("signers") is not None and "drivers" not in f
+               for f in rec["files"].values())
 
 
 def staged(tag=None):
@@ -590,6 +786,12 @@ def staged(tag=None):
         if tag is None or rec.get("tag") == tag:
             # Where it is now, not the path recorded at staging time.
             rec["folder"] = os.path.join(root, t)
+            if _record_stale(rec):
+                # Verified by older, weaker checks: show it, never install it.
+                rec["stale"] = True
+                for f in rec["files"].values():
+                    if isinstance(f, dict):
+                        f.update(ok=False, error="verified by an older version of this page; download it again")
             out.append(rec)
     out.sort(key=lambda r: r.get("staged") or "", reverse=True)
     return out
@@ -751,6 +953,8 @@ def install(tag, names=DEFAULT_INSTALL):
     rec = next(iter(staged(tag)), None)
     if not rec:
         raise ValueError(f"{tag} is not staged")
+    if rec.get("stale"):
+        raise ValueError(f"{tag} was verified by an older version of this page; download it again")
     names = [n for n in MANAGED if n in names]
     if not names:
         raise ValueError("nothing selected")
@@ -763,13 +967,25 @@ def install(tag, names=DEFAULT_INSTALL):
 
 
 def _install(tag, rec, names):
+    if _record_stale(rec):
+        raise ValueError(f"{tag} was verified by an older version of this page; download it again")
     for n in names:
         f = rec["files"].get(n) or {}
         if not f.get("ok"):
             raise ValueError(f"{n}: {f.get('error') or 'not staged'}")
         path = os.path.join(rec["folder"], n)
-        if not os.path.isfile(path) or uc.file_hash(path) != f["sha256"]:
+        if not os.path.isfile(path) or uc.file_hash(path) != f.get("sha256"):
             raise ValueError(f"{n}: staged copy changed since it was verified; download again")
+        # Re-read the file itself: the record says what was true at staging.
+        need = MANAGED[n][1]
+        if need:
+            now = _describe(path, os.stat(path))  # uncached: this gates going live
+            if now.get("sig_error") or not str(now.get("kind") or "").startswith(need):
+                raise ValueError(f"{n}: expected a signature under {NEED_LABEL[need]}, found "
+                                 f"{now.get('sig_error') or now.get('kind')}")
+            bad = risks(n, now)
+            if bad:
+                raise ValueError(f"{n}: {bad[0]}; refusing to install it")
     if not any(n in names for n in SHIMS):
         # iPXE alone must still be one the installed shim will start.
         trusts = vendor_cert_cn(os.path.join(config.TFTP_ROOT, "ipxe-shim.efi"))
@@ -799,14 +1015,19 @@ def backups():
         meta = uc.read_json(os.path.join(folder, "backup.json"), {})
         meta = meta if isinstance(meta, dict) else {}
         files = [f for f in MANAGED if os.path.isfile(os.path.join(folder, f))]
+        infos = {f: describe(os.path.join(folder, f)) for f in files}
         out.append({"id": b, "reason": meta.get("reason", ""), "at": meta.get("at", ""),
                     "files": files, "absent": meta.get("absent", []),
-                    "versions": {f: version_of(os.path.join(folder, f)) for f in files}})
+                    "info": infos,
+                    "versions": {f: (infos[f] or {}).get("version", "") for f in files},
+                    "warnings": [w for f in files for w in risks(f, infos[f])]})
     return out
 
 
-def rollback(backup_id):
-    """Put the files of one backup back, after backing up what is there now."""
+def rollback(backup_id, confirm=False):
+    """Put the files of one backup back, after backing up what is there now.
+    A backup holding a native-USB ipxe.efi or a shim that is not dual-signed
+    is only restored with confirm=True."""
     bid = os.path.basename(str(backup_id or ""))
     folder = os.path.join(_dir(), "backups", bid)
     if not bid or bid.startswith(".") or not os.path.isdir(folder):
@@ -814,6 +1035,10 @@ def rollback(backup_id):
     files = [f for f in MANAGED if os.path.isfile(os.path.join(folder, f))]
     if not files:
         raise ValueError("that backup holds no boot files")
+    warnings = [w for f in files
+                for w in risks(f, _describe(os.path.join(folder, f), os.stat(os.path.join(folder, f))))]
+    if warnings and not confirm:
+        raise ValueError("; ".join(warnings) + ". Tick the confirmation to restore it anyway")
     with _Lock():
         _backup(files, f"before rolling back to {bid}")
         _place_all([(os.path.join(folder, f), f, uc.file_hash(os.path.join(folder, f)))

webapp/services/nic_status.py

@@ -21,6 +21,8 @@ from datetime import datetime, timezone
 import config
 
 STALE_SECONDS = 20 * 60  # the watch timer runs every 5 min
+# A checked_at this far in the future is clock skew or a bad file: stale.
+FUTURE_SKEW_SECONDS = 300
 SYS_NET = "/sys/class/net"
 # cdc_ncm (the generic USB NIC driver, e.g. a WisdPI RTL8157 5G adapter on a
 # kernel whose r8152 does not know the chip) keeps the link speed as a 32-bit
@@ -37,26 +39,73 @@ def _read(path):
         return ""
 
 
-def _roles():
+def _norm_mac(v):
+    return str(v or "").strip().lower().replace("-", ":")
+
+
+def _nic_conf():
+    """{key: value} from nic.conf, loaded like pxe-nic-check does: as-is
+    (the playbook writes a [nic] header), else with a [nic] header added."""
+    try:
+        with open(config.NIC_CONF) as f:
+            text = f.read()
+    except (OSError, UnicodeDecodeError):
+        return {}
+    cp = configparser.ConfigParser(inline_comment_prefixes=("#", ";"))
+    try:
+        try:
+            cp.read_string(text)
+        except configparser.MissingSectionHeaderError:
+            cp = configparser.ConfigParser(inline_comment_prefixes=("#", ";"))
+            cp.read_string("[nic]\n" + text)
+        out = {}
+        for sect in cp.sections():
+            for k, v in cp.items(sect):
+                if v and v.strip():
+                    out[k] = v.strip()
+        return out
+    except configparser.Error:
+        return {}
+
+
+def _dnsmasq_rules():
+    """dnsmasq's NIC pin: {interface, except, no_dhcp, listen} sets, from
+    DNSMASQ_CONF (and what it includes) plus DNSMASQ_NIC_CONF. None when
+    neither could be read (the pin is then unknown)."""
+    from services import ipxe_update  # its include-aware dnsmasq.conf reader
+    seen = set()
+    lines = ipxe_update._conf_lines(config.DNSMASQ_CONF, seen=seen)
+    lines += ipxe_update._conf_lines(config.DNSMASQ_NIC_CONF, seen=seen)
+    if not lines:
+        return None
+    rules = {"interface": set(), "except": set(), "no_dhcp": set(), "listen": set()}
+    keymap = {"interface": "interface", "except-interface": "except",
+              "no-dhcp-interface": "no_dhcp", "listen-address": "listen"}
+    for _src, line in lines:
+        key, _, val = line.partition("=")
+        key = key.strip()
+        if key in keymap:
+            for item in val.split(","):
+                if item.strip():
+                    rules[keymap[key]].add(item.strip())
+    return rules
+
+
+def _roles(rules=None):
     """{iface or mac: role} from nic.conf, else from dnsmasq's NIC pin."""
     roles = {}
-    cp = configparser.ConfigParser()
-    try:
-        cp.read_string("[nic]\n" + open(config.NIC_CONF).read())
-        sec = cp["nic"]
-        for role in ("imaging", "ge"):
-            for key in (f"{role}_iface", f"{role}_mac"):
-                if sec.get(key):
-                    roles[sec.get(key).strip().lower()] = role
-    except (OSError, configparser.Error):
-        pass
+    conf = _nic_conf()
+    for role in ("imaging", "ge"):
+        if conf.get(f"{role}_iface"):
+            roles[conf[f"{role}_iface"].lower()] = role
+        if conf.get(f"{role}_mac"):
+            roles[_norm_mac(conf[f"{role}_mac"])] = role
     if not roles:
-        for line in _read(config.DNSMASQ_NIC_CONF).splitlines():
-            key, _, val = line.split("#", 1)[0].strip().partition("=")
-            if key == "interface" and val:
-                roles[val.strip()] = "imaging"
-            elif key == "except-interface" and val:
-                roles.setdefault(val.strip(), "ge")
+        rules = _dnsmasq_rules() if rules is None else rules
+        for name in sorted((rules or {}).get("interface", ())):
+            roles[name] = "imaging"
+        for name in sorted((rules or {}).get("except", ())):
+            roles.setdefault(name, "ge")
     return roles
 
 
@@ -68,39 +117,93 @@ def _addrs():
     except (OSError, subprocess.SubprocessError, ValueError):
         return {}
     res = {}
-    for link in data:
-        res[link.get("ifname", "")] = [
-            f"{a['local']}/{a['prefixlen']}" for a in link.get("addr_info", [])
-            if a.get("family") == "inet" or (a.get("family") == "inet6" and a.get("scope") == "global")]
+    for link in data if isinstance(data, list) else []:
+        if not isinstance(link, dict):
+            continue
+        res[str(link.get("ifname", ""))] = [
+            f"{a.get('local')}/{a.get('prefixlen')}" for a in link.get("addr_info", []) or []
+            if isinstance(a, dict) and a.get("local")
+            and (a.get("family") == "inet" or (a.get("family") == "inet6" and a.get("scope") == "global"))]
     return res
 
 
-def _dhcp_bound():
-    """Interfaces dnsmasq's DHCP socket (UDP 67) is bound to; None if unknown."""
+def _dhcp_sockets():
+    """[(addr, dev)] of the UDP 67 sockets; dev is None for an unbound one.
+    None if ss cannot be run."""
     try:
-        out = subprocess.run(["ss", "-H", "-uln", "sport", "= :67"], capture_output=True, text=True,
-                             timeout=5).stdout
+        r = subprocess.run(["ss", "-H", "-uln", "sport", "= :67"], capture_output=True, text=True, timeout=5)
     except (OSError, subprocess.SubprocessError):
         return None
-    bound = set(re.findall(r"%([\w.-]+):67\b", out))
-    if re.search(r"(^|\s)(0\.0\.0\.0|\*|\[::\]):67\b", out):
-        bound.add("*")  # bound on every interface
-    return bound
+    if r.returncode != 0:
+        return None
+    out = []
+    for line in r.stdout.splitlines():
+        fields = line.split()
+        if len(fields) < 4:
+            continue
+        host = fields[3].rsplit(":", 1)[0]
+        host, _, dev = host.partition("%")
+        out.append((host.strip("[]"), dev or None))
+    return out
+
+
+def _bound(name, ips, socks, rules):
+    """Does dnsmasq answer DHCP on `name`? True/False, or None if unknown.
+    Mirrors pxe-nic-check: a wildcard socket serves only what dnsmasq's
+    interface pin allows, so it never marks an excluded NIC as served."""
+    if socks is None:
+        return None
+    if not socks:
+        return False
+    for addr, dev in socks:
+        if dev == name or (dev is None and addr in ips):
+            return True
+    if not any(dev is None and addr in ("0.0.0.0", "*", "", "::") for addr, dev in socks):
+        return False
+    if rules is None:
+        return None  # wildcard socket and the pin is unknown
+    if name in rules["no_dhcp"] or name in rules["except"]:
+        return False
+    if name in rules["interface"] or any(ip in rules["listen"] for ip in ips):
+        return True
+    if not rules["interface"] and not rules["listen"]:
+        return bool(ips)
+    return False
 
 
 def _kernel_view():
-    """Interfaces as the kernel sees them (physical NICs only)."""
-    roles, addrs, bound = _roles(), _addrs(), _dhcp_bound()
-    out = []
+    """Interfaces as the kernel sees them: physical NICs, plus bridges that
+    carry a role (the imaging NIC is often a bridge over the port).
+    Returns (interfaces, ok) where ok is False if /sys could not be read."""
+    rules = _dnsmasq_rules()
+    roles, addrs, socks = _roles(rules), _addrs(), _dhcp_sockets()
     try:
         names = sorted(os.listdir(SYS_NET))
     except OSError:
-        names = []
+        return [], False
+    macs = {n: _read(os.path.join(SYS_NET, n, "address")).lower() for n in names}
+    bridges, port_of = {}, {}
+    for name in names:
+        brif = os.path.join(SYS_NET, name, "brif")
+        if os.path.isdir(os.path.join(SYS_NET, name, "bridge")):
+            try:
+                ports = sorted(os.listdir(brif))
+            except OSError:
+                ports = []
+            role = roles.get(name) or roles.get(macs.get(name, ""))
+            role = role or next((roles.get(p) or roles.get(macs.get(p, "")) for p in ports
+                                 if roles.get(p) or roles.get(macs.get(p, ""))), None)
+            if role:
+                bridges[name] = role
+                for p in ports:
+                    port_of[p] = name
+    out = []
     for name in names:
         base = os.path.join(SYS_NET, name)
-        if name == "lo" or not os.path.exists(os.path.join(base, "device")):
-            continue  # skip loopback, bridges, veths, tunnels
-        mac = _read(os.path.join(base, "address")).lower()
+        physical = os.path.exists(os.path.join(base, "device"))
+        if name == "lo" or not (physical or name in bridges):
+            continue  # skip loopback, plain bridges, veths, tunnels
+        mac = macs.get(name, "")
         oper = _read(os.path.join(base, "operstate"))
         try:
             speed = int(_read(os.path.join(base, "speed")))
@@ -108,21 +211,30 @@ def _kernel_view():
         except ValueError:
             speed = None
         notes = []
-        driver = os.path.basename(os.path.realpath(os.path.join(base, "device", "driver")))
+        driver = os.path.basename(os.path.realpath(os.path.join(base, "device", "driver"))) if physical else ""
         if driver == "cdc_ncm" and speed in CDC_NCM_WRAPPED:
             notes.append(f"Link {CDC_NCM_WRAPPED[speed] // 1000} Gb/s (the cdc_ncm driver reports "
                          f"{speed} Mb/s: its 32-bit speed counter wraps above 4.29 Gb/s)")
             speed = CDC_NCM_WRAPPED[speed]
+        if name in bridges:
+            role = bridges[name]
+            notes.append("bridge")
+        elif name in port_of:
+            role = "other"  # the bridge carries the role and the address
+            notes.append(f"port of bridge {port_of[name]}")
+        else:
+            role = roles.get(name) or roles.get(mac) or "other"
+        up = oper == "up" or (name in bridges and oper == "unknown")
+        ips = [a.split("/")[0] for a in addrs.get(name, [])]
         out.append({
-            "iface": name, "mac": mac,
-            "role": roles.get(name) or roles.get(mac) or "other",
-            "link": "up" if oper == "up" else "down",
-            "speed_mbps": speed if oper == "up" else None,
+            "iface": name, "mac": mac, "role": role,
+            "link": "up" if up else "down",
+            "speed_mbps": speed if up else None,
             "addrs": addrs.get(name, []),
             "verdict": None, "evidence": [], "notes": notes,
-            "dnsmasq_bound": (None if bound is None else (name in bound or "*" in bound)),
+            "dnsmasq_bound": None if name in port_of else _bound(name, ips, socks, rules),
         })
-    return out
+    return out, True
 
 
 def _age_seconds(stamp):
@@ -131,21 +243,73 @@ def _age_seconds(stamp):
         if t.tzinfo is None:
             t = t.replace(tzinfo=timezone.utc)
         return (datetime.now(timezone.utc) - t).total_seconds()
-    except ValueError:
+    except (ValueError, TypeError, OverflowError):
         return None
 
 
-def _problems(ifaces, guard):
+def _str(v):
+    return v.strip() if isinstance(v, str) else ""
+
+
+def _str_list(v):
+    if isinstance(v, str):
+        v = [v]
+    if not isinstance(v, (list, tuple)):
+        return []
+    return [x if isinstance(x, str) else str(x) for x in v
+            if isinstance(x, (str, int, float)) and not isinstance(x, bool)]
+
+
+def _speed(v):
+    if isinstance(v, bool):
+        return None
+    try:
+        n = int(v)
+    except (TypeError, ValueError, OverflowError):
+        return None
+    return n if 0 < n < 10_000_000 else None
+
+
+def _norm_iface(i):
+    """One interface entry of the check file with every field the page uses
+    in the type it expects; None for something that is not an entry."""
+    if not isinstance(i, dict):
+        return None
+    bound = i.get("dnsmasq_bound")
+    return {
+        "iface": _str(i.get("iface")) or "?",
+        "mac": _norm_mac(_str(i.get("mac"))),
+        "role": _str(i.get("role")) or "other",
+        "link": "up" if _str(i.get("link")) == "up" else "down",
+        "speed_mbps": _speed(i.get("speed_mbps")),
+        "addrs": _str_list(i.get("addrs")),
+        "verdict": _str(i.get("verdict")) or None,
+        "evidence": _str_list(i.get("evidence")),
+        "notes": _str_list(i.get("notes")),
+        "dnsmasq_bound": bound if isinstance(bound, bool) else None,
+    }
+
+
+def _when(stamp):
+    """checked_at for a sentence: 'YYYY-MM-DD HH:MM'."""
+    return str(stamp or "?").replace("T", " ")[:16]
+
+
+def _problems(ifaces, guard, stale=False, checked_at=None):
     """[(level, text)]: 'bad' is a DHCP-on-the-wrong-network risk."""
     out = []
     if (guard or {}).get("state") == "blocked":
-        out.append(("bad", "DHCP/PXE is blocked: " + (guard.get("reason") or "the imaging NIC is not isolated")))
+        reason = guard.get("reason") or "the imaging NIC is not isolated"
+        if stale:
+            out.append(("warn", f"The last network check ({_when(checked_at)}) said DHCP/PXE is blocked: {reason}"))
+        else:
+            out.append(("bad", "DHCP/PXE is blocked: " + reason))
     imaging = [i for i in ifaces if i["role"] == "imaging"]
     for i in ifaces:
         if i["role"] == "imaging" and i.get("verdict") in ("ge", "foreign-dhcp"):
             what = "the GE network" if i["verdict"] == "ge" else "a network with another DHCP server"
             out.append(("bad", f"Imaging NIC {i['iface']} is on {what}. Do not serve DHCP/PXE here."))
-        if i["role"] != "imaging" and i.get("dnsmasq_bound"):
+        if i["role"] != "imaging" and i.get("dnsmasq_bound") is True:
             out.append(("bad", f"dnsmasq is answering DHCP on {i['iface']} ({i['role']} NIC), not only on the imaging NIC."))
     if not imaging:
         out.append(("warn", "No imaging NIC configured (/etc/pxe-server/nic.conf or dnsmasq's interface= pin)."))
@@ -162,7 +326,8 @@ def _problems(ifaces, guard):
 
 def status():
     """{interfaces, source, checked_at, stale, guard, problems, state}.
-    state: 'ok' | 'warn' | 'bad'."""
+    state: 'ok' | 'warn' | 'bad'. Every interface entry is normalised (see
+    _norm_iface), whatever the check file holds."""
     data, source = None, "kernel"
     try:
         with open(config.NIC_STATUS_FILE) as f:
@@ -172,27 +337,45 @@ def status():
     stale = True
     if isinstance(data, dict) and isinstance(data.get("interfaces"), list):
         age = _age_seconds(data.get("checked_at"))
-        stale = age is None or age > STALE_SECONDS
+        stale = age is None or age > STALE_SECONDS or age < -FUTURE_SKEW_SECONDS
         source = "check"
-    live = {i["iface"]: i for i in _kernel_view()}
+    kernel, kernel_ok = _kernel_view()
+    live = {i["iface"]: i for i in kernel}
+    checked_at = None
     if source == "check":
+        checked_at = data.get("checked_at") if isinstance(data.get("checked_at"), str) else None
         ifaces = []
-        for i in data["interfaces"]:
-            i = dict(i)
-            k = live.get(i.get("iface"))
+        for raw in data["interfaces"]:
+            i = _norm_iface(raw)
+            if i is None:
+                continue
+            k = live.get(i["iface"])
             if k:  # link and addresses are cheap to read now; keep the check's verdict
-                i.update(link=k["link"], speed_mbps=k["speed_mbps"], addrs=k["addrs"], notes=k["notes"])
-                if k["dnsmasq_bound"] is not None:
+                i.update(link=k["link"], speed_mbps=k["speed_mbps"], addrs=k["addrs"],
+                         notes=k["notes"] + [n for n in i["notes"] if n not in k["notes"]])
+                # The check knows the dnsmasq config and process; the
+                # kernel fallback is only used once the check is stale.
+                if stale or i["dnsmasq_bound"] is None:
                     i["dnsmasq_bound"] = k["dnsmasq_bound"]
+            elif kernel_ok:
+                i.update(link="down", speed_mbps=None, addrs=[], dnsmasq_bound=False,
+                         notes=i["notes"] + ["not present (unplugged?)"])
             if stale:
                 i["verdict"] = None
+                i["evidence"] = []
             ifaces.append(i)
-        guard = data.get("dhcp_guard") or {}
+        g = data.get("dhcp_guard")
+        guard = {"state": _str(g.get("state")), "reason": _str(g.get("reason"))} if isinstance(g, dict) else {}
     else:
-        ifaces, guard = list(live.values()), {}
+        ifaces, guard = kernel, {}
     ifaces.sort(key=lambda i: {"imaging": 0, "ge": 1}.get(i.get("role"), 2))
-    problems = _problems(ifaces, guard)
+    problems = _problems(ifaces, guard, stale and source == "check", checked_at)
     state = "bad" if any(l == "bad" for l, _ in problems) else "warn" if problems else "ok"
     return {"interfaces": ifaces, "source": source, "stale": stale and source == "check",
-            "checked_at": (data or {}).get("checked_at") if source == "check" else None,
-            "guard": guard, "problems": problems, "state": state}
+            "checked_at": checked_at, "guard": guard, "problems": problems, "state": state}
+
+
+def unknown(error=""):
+    """What the Overview shows when status() itself failed."""
+    return {"interfaces": [], "source": "error", "stale": False, "checked_at": None, "guard": {},
+            "problems": [], "state": "unknown", "error": str(error)[:200]}