chinux/theProx
0
0
Fork 0
Code Issues 40 Pull requests Projects Releases Packages Wiki Activity Actions

[P2][SEC] Geplante Befehle prüfen die Berechtigung des Erstellers nicht zur Ausführungszeit — Rechteentzug wirkungslos #229

New issue
Open
opened 2026-07-27 10:44:23 +00:00 by chinux · 1 comment
chinux commented 2026-07-27 10:44:23 +00:00
Owner
Copy link

Befund

routers/command_router.py:163 — run_scheduled_command() führt einen geplanten Befehl aus, ohne die Berechtigung des Erstellers erneut zu prüfen:

async def run_scheduled_command(job: ScheduledJob) -> tuple[str, str]:
    params = dict(job.params or {})
    async with AsyncSessionLocal() as db:
        body = ExecuteIn(...)
        log = await execute_command_payload(db, body, user_email=job.created_by, ...)

execute_command_payload() enthält selbst keine Scope-Prüfung — die liegt ausschließlich im Request-Pfad (execute, Zeile 232: check_node_scope + check_vm_scope). Der Scheduler-Pfad umgeht sie vollständig.

Auswirkung

Ein Nutzer mit operator-Rolle plant einen Befehl auf Node X. Danach:

  • wird ihm der Zugriff auf Node X entzogen → der Job läuft weiter
  • wird sein Konto deaktiviert oder gelöscht → der Job läuft weiter
  • wird seine Rolle auf viewer reduziert → der Job läuft weiter

Der Job führt beliebigen Shell-Code als root auf dem PVE-Host aus (agent.run_script) bzw. per qm.agent.exec in einer VM. Es gibt keinen Pfad, über den ein Rechteentzug diese Ausführung stoppt — außer den Job manuell zu finden und zu löschen.

Dasselbe Muster bei security.port_scan über run_security_bulk_scan() im Scheduler.

Bewertung

Kein Rechteausweitungsfehler im engeren Sinn — der Nutzer hatte die Berechtigung, als er den Job anlegte. Aber Rechteentzug ist wirkungslos, und das ist bei einem Werkzeug, das root-Befehle auf Kundensystemen ausführt, ein Problem, das man vor dem Produktivbetrieb geschlossen haben will. Bei Personalwechsel im MSP ist es der realistische Fall.

Prompt für Claude Code

Schließe die Autorisierungslücke im Scheduler-Pfad von theProx: geplante
Befehle müssen die Berechtigung ihres Erstellers zur AUSFÜHRUNGSZEIT prüfen.

--- 1. PRÜFUNG IN DEN GEMEINSAMEN PFAD ---
execute_command_payload() in routers/command_router.py bekommt einen
verbindlichen Autorisierungsschritt. Signatur erweitern:

    async def execute_command_payload(db, body, *, actor: CurrentUser|None,
                                      user_email=None, job_id=None)

  - actor gesetzt  -> check_node_scope / check_vm_scope wie im Request-Pfad
  - actor None     -> nur erlaubt, wenn explizit als Systemlauf markiert
                      (eigener Parameter system=True), und DANN mit
                      Audit-Eintrag "warum ohne Actor"

Das Ziel ist, dass es keinen Aufrufweg mehr gibt, der die Prüfung überspringen
kann, ohne das ausdrücklich zu sagen. Der Request-Pfad (execute, Zeile ~232)
kann seine doppelte Prüfung behalten — schadet nicht.

--- 2. SCHEDULER-PFAD ---
run_scheduled_command(): Ersteller anhand job.created_by (E-Mail) laden und
einen CurrentUser rekonstruieren, inklusive Tenant-Berechtigungen
(_load_tenant_perms aus auth/rbac.py wiederverwenden, NICHT nachbauen).

Fälle, die zum Abbruch führen — Job wird NICHT ausgeführt:
  - Nutzer existiert nicht mehr
  - Nutzer ist deaktiviert (Feld in models/user.py prüfen)
  - Rolle reicht nicht mehr (operator nötig)
  - Node/VM nicht mehr im Scope

Bei Abbruch:
  - Job auf enabled=False setzen (nicht löschen — nachvollziehbar bleiben)
  - last_status="error", last_output mit klarem Grund
  - AuditLog action="scheduled_job.disabled_authz",
    details={job, creator, reason}
  - Notification an die Admins (Typ "schedule_err" existiert bereits)

Ein Job, der wegen fehlender Berechtigung stillschweigend nicht läuft, ist
schlimmer als einer, der abbricht: er erzeugt falsche Sicherheit. Der Grund
MUSS im Task-/Job-Log stehen.

--- 3. SICHERHEITSSCAN-PFAD ---
scheduler.py:_run_job -> security.port_scan ruft run_security_bulk_scan()
ohne Actor-Prüfung. Dieselbe Behandlung: created_by auflösen, Scope prüfen,
bei Verlust deaktivieren. Prüfen, ob weitere job_types denselben Weg gehen
(JOB_ACTION_MAP durchsehen) und alle gleich behandeln.

--- 4. AUFRÄUMEN BESTEHENDER JOBS ---
Kein Migrationsschritt, aber ein einmaliger Prüf-Endpunkt
GET /api/command/schedule/authz-check (require_superadmin), der alle
enabled Jobs durchgeht und meldet, welche mit den heutigen Berechtigungen
NICHT mehr laufen dürften — ohne sie zu ändern. Damit lässt sich vor dem
Produktivbetrieb sehen, ob Altlasten existieren.

--- 5. TESTS ---
tests/test_scheduled_authz.py (Muster: tests/test_tenant_scope.py, gleiche
echte-DB-Harness):
  - Job läuft, solange der Ersteller berechtigt ist
  - Node aus dem Scope entfernt -> Job wird deaktiviert, Grund im last_output
  - Ersteller gelöscht -> Job wird deaktiviert
  - Ersteller auf viewer reduziert -> Job wird deaktiviert
  - Audit-Eintrag wird geschrieben
  - authz-check meldet betroffene Jobs, ändert nichts
  - system=True-Pfad funktioniert und erzeugt einen Audit-Eintrag

Definition of Done

  • Kein Ausführungspfad ohne Autorisierungsprüfung oder ausdrückliche Systemmarkierung
  • Rechteentzug stoppt geplante Jobs
  • Abbruchgrund im Job-Log und im Audit-Log
  • Jobs werden deaktiviert, nicht gelöscht
  • Prüf-Endpunkt für Altlasten
  • Tests gegen die echte DB-Harness
## Befund `routers/command_router.py:163` — `run_scheduled_command()` führt einen geplanten Befehl aus, ohne die Berechtigung des Erstellers erneut zu prüfen: ```python async def run_scheduled_command(job: ScheduledJob) -> tuple[str, str]: params = dict(job.params or {}) async with AsyncSessionLocal() as db: body = ExecuteIn(...) log = await execute_command_payload(db, body, user_email=job.created_by, ...) ``` `execute_command_payload()` enthält selbst **keine** Scope-Prüfung — die liegt ausschließlich im Request-Pfad (`execute`, Zeile 232: `check_node_scope` + `check_vm_scope`). Der Scheduler-Pfad umgeht sie vollständig. ## Auswirkung Ein Nutzer mit `operator`-Rolle plant einen Befehl auf Node X. Danach: - wird ihm der Zugriff auf Node X entzogen → **der Job läuft weiter** - wird sein Konto deaktiviert oder gelöscht → **der Job läuft weiter** - wird seine Rolle auf `viewer` reduziert → **der Job läuft weiter** Der Job führt beliebigen Shell-Code als root auf dem PVE-Host aus (`agent.run_script`) bzw. per `qm.agent.exec` in einer VM. Es gibt keinen Pfad, über den ein Rechteentzug diese Ausführung stoppt — außer den Job manuell zu finden und zu löschen. Dasselbe Muster bei `security.port_scan` über `run_security_bulk_scan()` im Scheduler. ## Bewertung Kein Rechteausweitungsfehler im engeren Sinn — der Nutzer hatte die Berechtigung, als er den Job anlegte. Aber Rechteentzug ist wirkungslos, und das ist bei einem Werkzeug, das root-Befehle auf Kundensystemen ausführt, ein Problem, das man vor dem Produktivbetrieb geschlossen haben will. Bei Personalwechsel im MSP ist es der realistische Fall. --- ## Prompt für Claude Code ``` Schließe die Autorisierungslücke im Scheduler-Pfad von theProx: geplante Befehle müssen die Berechtigung ihres Erstellers zur AUSFÜHRUNGSZEIT prüfen. --- 1. PRÜFUNG IN DEN GEMEINSAMEN PFAD --- execute_command_payload() in routers/command_router.py bekommt einen verbindlichen Autorisierungsschritt. Signatur erweitern: async def execute_command_payload(db, body, *, actor: CurrentUser|None, user_email=None, job_id=None) - actor gesetzt -> check_node_scope / check_vm_scope wie im Request-Pfad - actor None -> nur erlaubt, wenn explizit als Systemlauf markiert (eigener Parameter system=True), und DANN mit Audit-Eintrag "warum ohne Actor" Das Ziel ist, dass es keinen Aufrufweg mehr gibt, der die Prüfung überspringen kann, ohne das ausdrücklich zu sagen. Der Request-Pfad (execute, Zeile ~232) kann seine doppelte Prüfung behalten — schadet nicht. --- 2. SCHEDULER-PFAD --- run_scheduled_command(): Ersteller anhand job.created_by (E-Mail) laden und einen CurrentUser rekonstruieren, inklusive Tenant-Berechtigungen (_load_tenant_perms aus auth/rbac.py wiederverwenden, NICHT nachbauen). Fälle, die zum Abbruch führen — Job wird NICHT ausgeführt: - Nutzer existiert nicht mehr - Nutzer ist deaktiviert (Feld in models/user.py prüfen) - Rolle reicht nicht mehr (operator nötig) - Node/VM nicht mehr im Scope Bei Abbruch: - Job auf enabled=False setzen (nicht löschen — nachvollziehbar bleiben) - last_status="error", last_output mit klarem Grund - AuditLog action="scheduled_job.disabled_authz", details={job, creator, reason} - Notification an die Admins (Typ "schedule_err" existiert bereits) Ein Job, der wegen fehlender Berechtigung stillschweigend nicht läuft, ist schlimmer als einer, der abbricht: er erzeugt falsche Sicherheit. Der Grund MUSS im Task-/Job-Log stehen. --- 3. SICHERHEITSSCAN-PFAD --- scheduler.py:_run_job -> security.port_scan ruft run_security_bulk_scan() ohne Actor-Prüfung. Dieselbe Behandlung: created_by auflösen, Scope prüfen, bei Verlust deaktivieren. Prüfen, ob weitere job_types denselben Weg gehen (JOB_ACTION_MAP durchsehen) und alle gleich behandeln. --- 4. AUFRÄUMEN BESTEHENDER JOBS --- Kein Migrationsschritt, aber ein einmaliger Prüf-Endpunkt GET /api/command/schedule/authz-check (require_superadmin), der alle enabled Jobs durchgeht und meldet, welche mit den heutigen Berechtigungen NICHT mehr laufen dürften — ohne sie zu ändern. Damit lässt sich vor dem Produktivbetrieb sehen, ob Altlasten existieren. --- 5. TESTS --- tests/test_scheduled_authz.py (Muster: tests/test_tenant_scope.py, gleiche echte-DB-Harness): - Job läuft, solange der Ersteller berechtigt ist - Node aus dem Scope entfernt -> Job wird deaktiviert, Grund im last_output - Ersteller gelöscht -> Job wird deaktiviert - Ersteller auf viewer reduziert -> Job wird deaktiviert - Audit-Eintrag wird geschrieben - authz-check meldet betroffene Jobs, ändert nichts - system=True-Pfad funktioniert und erzeugt einen Audit-Eintrag ``` ## Definition of Done - [ ] Kein Ausführungspfad ohne Autorisierungsprüfung oder ausdrückliche Systemmarkierung - [ ] Rechteentzug stoppt geplante Jobs - [ ] Abbruchgrund im Job-Log und im Audit-Log - [ ] Jobs werden deaktiviert, nicht gelöscht - [ ] Prüf-Endpunkt für Altlasten - [ ] Tests gegen die echte DB-Harness
chinux commented 2026-07-27 10:44:24 +00:00
Author
Owner
Copy link

Aus dem Code-Review vom 2026-07-27 (Commit 4152c4d). Gesamtblock: #226, #227, #228, #229, #230, #231.

Abarbeitung niedrig → hoch: #231 → #230 → #229 → #228 → #227 → #226.

Aus dem Code-Review vom 2026-07-27 (Commit 4152c4d). Gesamtblock: #226, #227, #228, #229, #230, #231. Abarbeitung niedrig → hoch: #231 → #230 → #229 → #228 → #227 → #226.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
fix/vm-agent-update-error
fix/bulk-update-autoclose
security/tenant-scope-108
refactor/dashboard-page-split
fix/rust-agent-parity-features
feat/rust-agent-parity
archive/python-node-agent
Labels
Clear labels   
agent
Node-/VM-Agent

  
bug
Funktionaler Defekt

  
frontend
Svelte/TS Frontend

  
refactor
Umbau ohne Verhaltensaenderung

  
tech-debt
Altlast / Aufraeumen

No labels
agent
bug
frontend
refactor
tech-debt
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
chinux/theProx#229
No description provided.