approve_node never rotates an existing key; node update-check endpoint; real Update Node button (node-26#14, #15)
- approve_node: reuses node_keys/{id}.api_key when one exists (still
re-syncs dynsec with it, still never overwrites node_keys). Re-clicking
Approve can no longer lock a live node out. Reissue stays explicit.
- POST /nodes/{id}/request-update (admin): sets update_requested_at on the
node doc AND sends MQTT node_update best-effort, so offline/locked-out
nodes still get it.
- POST /nodes/{id}/update-check (node, inline auth: Bearer api_key for THAT
node or X-Pickup-Secret): host updater's HTTPS check-in. Returns only
update_requested_at; records updater_last_check_at / sha / applied result.
Unknown node -> 401 (no node_id oracle). C2 cannot name a ref, image or
command.
- Frontend: "Update Node Software" (admin only) calls request-update, honest
confirm text; node page shows updater check-in and last update state.
Tests: tests/test_node_provisioning.py (12).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
fb299008f8
commit
7fa50f24d2
@@ -17,11 +17,13 @@ Auth is the token headers checked inline below, not the app-wide
|
||||
Firebase/service-key dependency the rest of routers/nodes.py uses.
|
||||
"""
|
||||
import hashlib
|
||||
import re
|
||||
import secrets
|
||||
import time
|
||||
from typing import Optional
|
||||
from datetime import datetime, timezone
|
||||
from typing import Literal, Optional
|
||||
from fastapi import APIRouter, HTTPException, Header, Request
|
||||
from pydantic import BaseModel
|
||||
from pydantic import BaseModel, Field
|
||||
from app.config import settings
|
||||
from app.internal import firestore as fstore
|
||||
from app.internal.logger import logger
|
||||
@@ -216,3 +218,82 @@ async def get_node_credentials(node_id: str, x_pickup_secret: Optional[str] = He
|
||||
return CredentialsResponse(approval_status=approval_status)
|
||||
|
||||
return CredentialsResponse(approval_status=approval_status, api_key=key_doc["api_key"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Host updater check-in (node-26#15)
|
||||
#
|
||||
# A node's host-level updater (Client/scripts/drb-update.sh, a systemd timer
|
||||
# installed by install.sh) calls this over HTTPS on a timer. It is the
|
||||
# out-of-band channel: it works for a node that is offline from MQTT or
|
||||
# locked out of the broker entirely, which is exactly when an update or a
|
||||
# credential fix is needed most.
|
||||
#
|
||||
# Auth, checked inline for THIS node_id only (never a fleet credential):
|
||||
# Authorization: Bearer <api_key> -> must equal node_keys/{id}.api_key
|
||||
# X-Pickup-Secret: <secret> -> must hash to nodes/{id}.pickup_secret_hash
|
||||
# The pickup secret is accepted so a node whose api_key is missing/stale can
|
||||
# still be told to update — it is the same credential that already returns
|
||||
# the api_key itself, so it grants nothing new.
|
||||
#
|
||||
# The response is a single timestamp. The node compares it with the last
|
||||
# request it applied and, if newer, pulls the repo/registry/ref IT was
|
||||
# installed with. C2 cannot name a ref, an image or a command here, so a
|
||||
# compromised C2 or a stolen node credential can at most make a node
|
||||
# re-pull its own pinned software.
|
||||
# ---------------------------------------------------------------------------
|
||||
_SHA_RE = re.compile(r"^[0-9a-f]{7,40}$")
|
||||
|
||||
|
||||
class UpdateCheckRequest(BaseModel):
|
||||
sha: Optional[str] = Field(None, max_length=40)
|
||||
applied_request_at: Optional[str] = Field(None, max_length=64)
|
||||
result: Optional[Literal["ok", "failed"]] = None
|
||||
|
||||
|
||||
class UpdateCheckResponse(BaseModel):
|
||||
update_requested_at: Optional[str] = None
|
||||
|
||||
|
||||
async def _authenticate_node(
|
||||
node_id: str, authorization: Optional[str], x_pickup_secret: Optional[str],
|
||||
) -> dict:
|
||||
node = await fstore.doc_get("nodes", node_id)
|
||||
if not node:
|
||||
# 401, not 404: this route must not double as a node_id oracle.
|
||||
raise HTTPException(401, "Invalid node credentials")
|
||||
if authorization and authorization.lower().startswith("bearer "):
|
||||
presented = authorization[7:].strip()
|
||||
key_doc = await fstore.doc_get("node_keys", node_id)
|
||||
stored = (key_doc or {}).get("api_key") or ""
|
||||
if stored and presented and secrets.compare_digest(stored, presented):
|
||||
return node
|
||||
if x_pickup_secret and node.get("pickup_secret_hash"):
|
||||
if secrets.compare_digest(_hash_secret(x_pickup_secret), node["pickup_secret_hash"]):
|
||||
return node
|
||||
raise HTTPException(401, "Invalid node credentials")
|
||||
|
||||
|
||||
@router.post("/{node_id}/update-check", response_model=UpdateCheckResponse)
|
||||
async def node_update_check(
|
||||
node_id: str,
|
||||
body: UpdateCheckRequest,
|
||||
authorization: Optional[str] = Header(None),
|
||||
x_pickup_secret: Optional[str] = Header(None),
|
||||
):
|
||||
node = await _authenticate_node(node_id, authorization, x_pickup_secret)
|
||||
|
||||
now = datetime.now(timezone.utc).isoformat()
|
||||
updates: dict = {"updater_last_check_at": now}
|
||||
if body.sha and _SHA_RE.match(body.sha):
|
||||
updates["updater_sha"] = body.sha
|
||||
requested = node.get("update_requested_at")
|
||||
if body.applied_request_at and requested and body.applied_request_at == requested and body.result:
|
||||
updates["update_applied_at"] = now
|
||||
updates["update_result"] = body.result
|
||||
try:
|
||||
await fstore.doc_update("nodes", node_id, updates)
|
||||
except Exception as e: # bookkeeping only — never fail the node's check on it
|
||||
logger.warning(f"update-check: could not record check-in for {node_id!r}: {e}")
|
||||
|
||||
return UpdateCheckResponse(update_requested_at=requested)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import secrets
|
||||
from datetime import datetime, timezone
|
||||
from typing import Dict, List, Optional
|
||||
from fastapi import APIRouter, HTTPException, Depends, Query
|
||||
from pydantic import BaseModel
|
||||
@@ -43,7 +44,17 @@ async def approve_node(node_id: str, _: dict = Depends(require_admin_token)):
|
||||
if not node:
|
||||
raise HTTPException(404, f"Node '{node_id}' not found.")
|
||||
|
||||
api_key = secrets.token_hex(32)
|
||||
# node-26#14: approve must NEVER rotate a key that already exists. The old
|
||||
# behaviour minted a fresh key on every click and overwrote node_keys, so
|
||||
# re-clicking Approve on a live node (or approving after a re-run) silently
|
||||
# invalidated the key already on the device and locked it out of MQTT with
|
||||
# no remote recovery. Rotation is an explicit, separate action
|
||||
# (POST /nodes/{id}/reissue-key). An existing key is reused as-is; the
|
||||
# dynsec upsert below still runs, so a second approve doubles as a safe
|
||||
# "re-sync this node's broker login" without changing the credential.
|
||||
key_doc = await fstore.doc_get("node_keys", node_id)
|
||||
existing_key = (key_doc or {}).get("api_key")
|
||||
api_key = existing_key or secrets.token_hex(32)
|
||||
|
||||
# dynsec FIRST, Firestore second: if the broker rejects/never confirms
|
||||
# the new client, we must not tell Firestore (and the admin UI) the
|
||||
@@ -56,13 +67,40 @@ async def approve_node(node_id: str, _: dict = Depends(require_admin_token)):
|
||||
logger.error(f"Approve {node_id!r}: dynsec upsert failed, NOT writing Firestore: {e}")
|
||||
raise HTTPException(502, f"Could not provision MQTT credentials for node: {e}")
|
||||
|
||||
await fstore.doc_set("node_keys", node_id, {"node_id": node_id, "api_key": api_key}, merge=False)
|
||||
if not existing_key:
|
||||
await fstore.doc_set("node_keys", node_id, {"node_id": node_id, "api_key": api_key}, merge=False)
|
||||
await fstore.doc_update("nodes", node_id, {"approval_status": "approved"})
|
||||
# TODO(mqtt-cutover): drop this MQTT push once nodes pull their key via
|
||||
# GET /nodes/{id}/credentials (routers/enrollment.py) exclusively — see
|
||||
# MQTT-PUBLIC-AUTH-PLAN.md "Rollout order" step 6. Kept for node-26.
|
||||
mqtt_handler.publish_node_key(node_id, api_key)
|
||||
return {"ok": True}
|
||||
if existing_key:
|
||||
logger.info(f"Approve {node_id!r}: key already exists — reused, not rotated")
|
||||
return {"ok": True, "key_rotated": False, "already_had_key": bool(existing_key)}
|
||||
|
||||
|
||||
@router.post("/{node_id}/request-update")
|
||||
async def request_node_update(node_id: str, _: dict = Depends(require_admin_token)):
|
||||
"""Ask a node to run its host-level updater (node-26#15).
|
||||
|
||||
Two channels, so a node that is offline or locked out of MQTT still gets
|
||||
it: (1) ``update_requested_at`` on the node doc, which the node's host
|
||||
updater reads over HTTPS on its timer (POST /nodes/{id}/update-check in
|
||||
routers/enrollment.py); (2) the MQTT ``node_update`` command, best-effort,
|
||||
which makes an online node's updater run within seconds.
|
||||
|
||||
Deliberately carries NO payload the node acts on beyond "update now": no
|
||||
ref, no image name, no command. The node pulls only the repo/registry and
|
||||
ref it was installed with — C2 cannot steer what code a node runs.
|
||||
"""
|
||||
node = await fstore.doc_get("nodes", node_id)
|
||||
if not node:
|
||||
raise HTTPException(404, f"Node '{node_id}' not found.")
|
||||
requested_at = datetime.now(timezone.utc).isoformat()
|
||||
await fstore.doc_update("nodes", node_id, {"update_requested_at": requested_at})
|
||||
mqtt_sent = mqtt_handler.send_command(node_id, {"action": "node_update"})
|
||||
logger.info(f"Update requested for {node_id!r} at {requested_at} (mqtt_sent={mqtt_sent})")
|
||||
return {"ok": True, "update_requested_at": requested_at, "mqtt_sent": mqtt_sent}
|
||||
|
||||
|
||||
@router.delete("/{node_id}", status_code=204)
|
||||
|
||||
Reference in New Issue
Block a user