xss fixes from audit:
- F1 case_detail.html: scheme-check source_ref href (block javascript: URLs) - F2 admin.html / F3 admin_federation.html: replace inline onsubmit confirm() with data-attr + global handler in base.html (no more label/domain interpolation into onsubmit attribute string) - federation.register_peer: validate hostname + fingerprint regex at ingest - federation_explore.html: window.PSYC_EXPLORE via | tojson - federation_network.js: DOMAIN_RE guard on peer-supplied domain before building cross-origin fetch URL (also closes open-redirect via 'open their explorer' button) - app.py: nosniff + Referrer-Policy: no-referrer + X-Frame-Options: DENY - sw.js: psyc-v11 cache bump CSP deferred — needs inline scripts moved to external files first. Tests: +2 cases, 245/245 green.
This commit is contained in:
@@ -35,6 +35,19 @@ app = FastAPI(title="psyc Operations Cockpit", version="0.1.0")
|
|||||||
app.add_middleware(SessionMiddleware, secret_key=adminauth.session_secret(), max_age=3600)
|
app.add_middleware(SessionMiddleware, secret_key=adminauth.session_secret(), max_age=3600)
|
||||||
app.mount("/static", StaticFiles(directory=str(HERE / "static")), name="static")
|
app.mount("/static", StaticFiles(directory=str(HERE / "static")), name="static")
|
||||||
|
|
||||||
|
|
||||||
|
@app.middleware("http")
|
||||||
|
async def _security_headers(request: Request, call_next):
|
||||||
|
"""Defense-in-depth headers. CSP is intentionally NOT set yet — the
|
||||||
|
cockpit currently uses inline scripts in base.html / journey.html /
|
||||||
|
federation_explore.html which would need nonces or extraction first."""
|
||||||
|
resp = await call_next(request)
|
||||||
|
resp.headers.setdefault("X-Content-Type-Options", "nosniff")
|
||||||
|
resp.headers.setdefault("Referrer-Policy", "no-referrer")
|
||||||
|
resp.headers.setdefault("X-Frame-Options", "DENY")
|
||||||
|
return resp
|
||||||
|
|
||||||
|
|
||||||
pulse_routes.register(app, TEMPLATES)
|
pulse_routes.register(app, TEMPLATES)
|
||||||
federation_routes.register(app, TEMPLATES)
|
federation_routes.register(app, TEMPLATES)
|
||||||
|
|
||||||
|
|||||||
@@ -673,11 +673,21 @@
|
|||||||
// them from any cockpit page. The fetch is best-effort — if the peer is
|
// them from any cockpit page. The fetch is best-effort — if the peer is
|
||||||
// unreachable, blocked by CSP, or older, we render whatever we got.
|
// unreachable, blocked by CSP, or older, we render whatever we got.
|
||||||
|
|
||||||
|
// Reject anything that isn't a bare hostname (+ optional port). Stops a
|
||||||
|
// hostile peer-supplied `domain` value from steering a click to an
|
||||||
|
// attacker-controlled URL via path/query injection or a different scheme.
|
||||||
|
const DOMAIN_RE = /^[A-Za-z0-9._\-]+(:\d{1,5})?$/;
|
||||||
|
|
||||||
async function fetchPeerSelfView(domain, expectedFp) {
|
async function fetchPeerSelfView(domain, expectedFp) {
|
||||||
const sec = detailEl.querySelector(`.fn-remote-sec[data-remote-fp="${expectedFp}"]`);
|
const sec = detailEl.querySelector(`.fn-remote-sec[data-remote-fp="${expectedFp}"]`);
|
||||||
if (!sec) return;
|
if (!sec) return;
|
||||||
const statusEl = sec.querySelector(".fn-remote-status");
|
const statusEl = sec.querySelector(".fn-remote-status");
|
||||||
const bodyEl = sec.querySelector(".fn-remote-body");
|
const bodyEl = sec.querySelector(".fn-remote-body");
|
||||||
|
if (!DOMAIN_RE.test(domain || "")) {
|
||||||
|
statusEl.textContent = "blocked";
|
||||||
|
bodyEl.innerHTML = `<span class="muted">peer domain failed hostname validation — not fetching.</span>`;
|
||||||
|
return;
|
||||||
|
}
|
||||||
const base = `https://${domain}`;
|
const base = `https://${domain}`;
|
||||||
let data = null;
|
let data = null;
|
||||||
let kind = "";
|
let kind = "";
|
||||||
@@ -789,7 +799,13 @@
|
|||||||
if (!sec) return;
|
if (!sec) return;
|
||||||
const statusEl = sec.querySelector(".fn-topo-status");
|
const statusEl = sec.querySelector(".fn-topo-status");
|
||||||
const bodyEl = sec.querySelector(".fn-topology-body");
|
const bodyEl = sec.querySelector(".fn-topology-body");
|
||||||
// Empty domain → same-origin fetch for SELF. Otherwise cross-origin.
|
// Empty domain → same-origin fetch for SELF. Otherwise cross-origin —
|
||||||
|
// guard against a poisoned peer-supplied domain steering elsewhere.
|
||||||
|
if (domain && !DOMAIN_RE.test(domain)) {
|
||||||
|
statusEl.textContent = "blocked";
|
||||||
|
bodyEl.innerHTML = `<span class="muted">peer domain failed hostname validation — not fetching topology.</span>`;
|
||||||
|
return;
|
||||||
|
}
|
||||||
const base = domain ? `https://${domain}` : "";
|
const base = domain ? `https://${domain}` : "";
|
||||||
let data = null;
|
let data = null;
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
// This makes the cockpit installable as a PWA and survives flaky connections,
|
// This makes the cockpit installable as a PWA and survives flaky connections,
|
||||||
// without serving stale operational data behind the operator's back.
|
// without serving stale operational data behind the operator's back.
|
||||||
|
|
||||||
const CACHE_VERSION = "psyc-v10";
|
const CACHE_VERSION = "psyc-v11";
|
||||||
const STATIC_ASSETS = [
|
const STATIC_ASSETS = [
|
||||||
"/static/cockpit.css",
|
"/static/cockpit.css",
|
||||||
"/static/psyc-tokens.css",
|
"/static/psyc-tokens.css",
|
||||||
|
|||||||
@@ -37,7 +37,7 @@
|
|||||||
<td class="lg-ts">{{ ((m.last_used or '')[:16] | replace('T', ' ')) or '—' }}</td>
|
<td class="lg-ts">{{ ((m.last_used or '')[:16] | replace('T', ' ')) or '—' }}</td>
|
||||||
<td>
|
<td>
|
||||||
<form method="post" action="/admin/members/{{ m.id }}/revoke" class="queue-action"
|
<form method="post" action="/admin/members/{{ m.id }}/revoke" class="queue-action"
|
||||||
onsubmit="return confirm('Revoke {{ m.label }}? Their codes stop working immediately.');">
|
data-confirm-revoke="member" data-confirm-name="{{ m.label }}">
|
||||||
<button type="submit" class="btn btn-reject">revoke</button>
|
<button type="submit" class="btn btn-reject">revoke</button>
|
||||||
</form>
|
</form>
|
||||||
</td>
|
</td>
|
||||||
|
|||||||
@@ -71,7 +71,7 @@
|
|||||||
<button type="submit" class="btn btn-reject" {% if p.status == 'blocked' %}disabled{% endif %}>block</button>
|
<button type="submit" class="btn btn-reject" {% if p.status == 'blocked' %}disabled{% endif %}>block</button>
|
||||||
</form>
|
</form>
|
||||||
<form method="post" action="/admin/federation/peers/{{ p.domain }}/remove" class="queue-action"
|
<form method="post" action="/admin/federation/peers/{{ p.domain }}/remove" class="queue-action"
|
||||||
onsubmit="return confirm('Remove {{ p.domain }}? Their signals will no longer count toward quorum.');">
|
data-confirm-revoke="peer" data-confirm-name="{{ p.domain }}">
|
||||||
<button type="submit" class="btn">remove</button>
|
<button type="submit" class="btn">remove</button>
|
||||||
</form>
|
</form>
|
||||||
</td>
|
</td>
|
||||||
|
|||||||
@@ -39,6 +39,21 @@
|
|||||||
btn.setAttribute("aria-expanded", "false");
|
btn.setAttribute("aria-expanded", "false");
|
||||||
}));
|
}));
|
||||||
});
|
});
|
||||||
|
// data-driven confirms (used by /admin and /admin/federation revoke/remove
|
||||||
|
// buttons; replaces inline onsubmit which was XSS-vulnerable when the
|
||||||
|
// confirm prompt interpolated a member label or peer domain).
|
||||||
|
document.addEventListener("DOMContentLoaded", () => {
|
||||||
|
document.querySelectorAll("form[data-confirm-revoke]").forEach(form => {
|
||||||
|
form.addEventListener("submit", ev => {
|
||||||
|
const kind = form.getAttribute("data-confirm-revoke") || "item";
|
||||||
|
const name = form.getAttribute("data-confirm-name") || "";
|
||||||
|
const msg = kind === "peer"
|
||||||
|
? `Remove ${name}? Their signals will no longer count toward quorum.`
|
||||||
|
: `Revoke ${name}? Their codes stop working immediately.`;
|
||||||
|
if (!confirm(msg)) ev.preventDefault();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
</script>
|
</script>
|
||||||
</head>
|
</head>
|
||||||
<body class="{% block body_class %}{% endblock %}">
|
<body class="{% block body_class %}{% endblock %}">
|
||||||
|
|||||||
@@ -42,7 +42,7 @@
|
|||||||
<h2>Source</h2>
|
<h2>Source</h2>
|
||||||
<dl>
|
<dl>
|
||||||
<dt>Type</dt><dd>{{ case.source_type }}</dd>
|
<dt>Type</dt><dd>{{ case.source_type }}</dd>
|
||||||
<dt>Reference</dt><dd>{% if case.source_ref %}<a href="{{ case.source_ref }}" target="_blank" rel="noopener">{{ case.source_ref }}</a>{% else %}—{% endif %}</dd>
|
<dt>Reference</dt><dd>{% if case.source_ref %}{% if case.source_ref.startswith(('http://', 'https://')) %}<a href="{{ case.source_ref }}" target="_blank" rel="noopener">{{ case.source_ref }}</a>{% else %}<code>{{ case.source_ref }}</code>{% endif %}{% else %}—{% endif %}</dd>
|
||||||
<dt>Observed</dt><dd class="muted">{{ case.observed_at.strftime('%Y-%m-%d %H:%M UTC') }}</dd>
|
<dt>Observed</dt><dd class="muted">{{ case.observed_at.strftime('%Y-%m-%d %H:%M UTC') }}</dd>
|
||||||
<dt>Ingested</dt><dd class="muted">{{ case.ingested_at.strftime('%Y-%m-%d %H:%M UTC') }}</dd>
|
<dt>Ingested</dt><dd class="muted">{{ case.ingested_at.strftime('%Y-%m-%d %H:%M UTC') }}</dd>
|
||||||
</dl>
|
</dl>
|
||||||
|
|||||||
@@ -134,10 +134,13 @@
|
|||||||
<script>
|
<script>
|
||||||
// The walk-to-peer param ("?peer=domain") tells the JS to focus that peer
|
// The walk-to-peer param ("?peer=domain") tells the JS to focus that peer
|
||||||
// in the graph as soon as the data lands.
|
// in the graph as soon as the data lands.
|
||||||
|
// Use tojson so values are honest JS literals — does not depend on the
|
||||||
|
// subtle HTML-entity-vs-script-context parser rules. Empty values become
|
||||||
|
// null/"" cleanly. Reach the values via PSYC_EXPLORE.* in the script.
|
||||||
window.PSYC_EXPLORE = {
|
window.PSYC_EXPLORE = {
|
||||||
selfFingerprint: "{{ fingerprint }}",
|
selfFingerprint: {{ fingerprint | tojson }},
|
||||||
selfDomain: "{{ domain }}",
|
selfDomain: {{ (domain or "") | tojson }},
|
||||||
focusPeer: "{{ peer }}"
|
focusPeer: {{ (peer or "") | tojson }}
|
||||||
};
|
};
|
||||||
</script>
|
</script>
|
||||||
<script src="/static/federation_explore.js" defer></script>
|
<script src="/static/federation_explore.js" defer></script>
|
||||||
|
|||||||
@@ -16,9 +16,15 @@ import base64
|
|||||||
import hashlib
|
import hashlib
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
from typing import Any, Dict, List, Optional, Tuple
|
from typing import Any, Dict, List, Optional, Tuple
|
||||||
|
|
||||||
|
# Hostname-with-optional-port pattern for peer domains. Reject anything else at
|
||||||
|
# registration so a hostile domain string can't reach a render context where
|
||||||
|
# it could break out of an HTML attr or JS string.
|
||||||
|
_DOMAIN_RE = re.compile(r"^[A-Za-z0-9._\-]+(:\d{1,5})?$")
|
||||||
|
|
||||||
from cryptography.hazmat.primitives import serialization
|
from cryptography.hazmat.primitives import serialization
|
||||||
from cryptography.hazmat.primitives.asymmetric import ed25519
|
from cryptography.hazmat.primitives.asymmetric import ed25519
|
||||||
from pydantic import BaseModel, Field
|
from pydantic import BaseModel, Field
|
||||||
@@ -420,7 +426,17 @@ def _row_to_peer(row: Dict[str, Any]) -> Peer:
|
|||||||
|
|
||||||
|
|
||||||
def register_peer(domain: str, fingerprint: str, pubkey_pem: str, status: str = "unknown") -> None:
|
def register_peer(domain: str, fingerprint: str, pubkey_pem: str, status: str = "unknown") -> None:
|
||||||
"""Insert or update a peer in the registry. Idempotent on `domain`."""
|
"""Insert or update a peer in the registry. Idempotent on `domain`.
|
||||||
|
|
||||||
|
Rejects malformed domain strings — only hostname chars + optional :port.
|
||||||
|
Closes a stored-XSS hole where a hostile `domain` would have been rendered
|
||||||
|
into the admin federation page's confirm() prompt.
|
||||||
|
"""
|
||||||
|
domain = (domain or "").strip()
|
||||||
|
if not _DOMAIN_RE.match(domain):
|
||||||
|
raise ValueError(f"invalid domain: {domain!r}")
|
||||||
|
if fingerprint and not re.fullmatch(r"[0-9a-fA-F]{32}", fingerprint):
|
||||||
|
raise ValueError(f"invalid fingerprint: {fingerprint!r}")
|
||||||
now = datetime.now(timezone.utc).isoformat()
|
now = datetime.now(timezone.utc).isoformat()
|
||||||
existing = db.get_peer(domain)
|
existing = db.get_peer(domain)
|
||||||
discovered_at = existing["discovered_at"] if existing else now
|
discovered_at = existing["discovered_at"] if existing else now
|
||||||
|
|||||||
@@ -230,3 +230,33 @@ def test_peer_registry_crud(fresh_db, fed_dir):
|
|||||||
|
|
||||||
federation.remove_peer("peer.example")
|
federation.remove_peer("peer.example")
|
||||||
assert federation.list_peers() == []
|
assert federation.list_peers() == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_register_peer_rejects_malformed_domain(fresh_db, fed_dir):
|
||||||
|
"""XSS guard: domain must look like a hostname (+ optional :port)."""
|
||||||
|
import pytest
|
||||||
|
bad = [
|
||||||
|
"evil.com'); alert(1); //",
|
||||||
|
"evil.com<script>",
|
||||||
|
"evil.com onclick=alert(1)",
|
||||||
|
"",
|
||||||
|
"evil com", # space
|
||||||
|
"/etc/passwd",
|
||||||
|
"evil.com/?phish=1",
|
||||||
|
]
|
||||||
|
for d in bad:
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
federation.register_peer(d, "ff" * 16, "PEM")
|
||||||
|
# And good ones still pass:
|
||||||
|
for d in ["peer.example.com", "peer.example.com:8443", "peer-1.example", "127.0.0.1:8767"]:
|
||||||
|
federation.register_peer(d, "ff" * 16, "PEM")
|
||||||
|
federation.remove_peer(d)
|
||||||
|
|
||||||
|
|
||||||
|
def test_register_peer_rejects_malformed_fingerprint(fresh_db, fed_dir):
|
||||||
|
"""Defense-in-depth: fingerprint must be 32 hex chars."""
|
||||||
|
import pytest
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
federation.register_peer("peer.example", "not-hex", "PEM")
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
federation.register_peer("peer.example", "ff" * 8, "PEM") # too short
|
||||||
|
|||||||
Reference in New Issue
Block a user